[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1414880 - /subversion/trunk/subversion/libsvn_client/merge.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 28 Nov 2012 20:39:48 +0000 (GMT)

> Author: cmpilato

> URL: http://svn.apache.org/viewvc?rev=1414880&view=rev
> Log:
> After consultation with pburba, restore the 'added_path' merge context
> baton member and related functionality removed in r1414810, using it
> as a preferred sourch of information about added parent directories,
> and only falling back to the more expensive consultation of the
> 'dry_run_added' hash when we have to.  Though, rather than a *precise*
> reintroduction of that baton member, call its restored form
> 'dry_run_last_added_dir' and position it alongside the
> 'dry_run_added'
> baton member.
>
> * subversion/libsvn_client/merge.c
>   (merge_cmd_baton_t): Add new 'dry_run_last_added_dir' member.
>   (dry_run_added_parent_p): Add 'local_abspath' parameter, and rename
>     'edit_relpath' to 'local_relpath'.  Now check the incoming
> paths
>     against the merge baton's 'dry_run_last_added_dir' as an
>     optimization before consulting the 'dry_run_added' hash.
>   (merge_file_added): Update call to dry_run_added_parent_p().
>   (merge_dir_added): Manage the state of the new
> 'dry_run_last_added_dir'
>     baton member.  Update call to dry_run_added_parent_p().
>   (do_merge): Reset the new 'dry_run_last_added_dir' context baton
> member, too.

> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> ==============================================================================

> @@ -286,9 +286,10 @@ typedef struct merge_cmd_baton_t {
>
> -  /* The list of paths for entries we've added, used only when in
> -    dry_run mode. */
> +  /* The list of paths for entries we've added and the most
> +    recently added directory.  (Used only when in dry_run mode.) */
>   apr_hash_t *dry_run_added;
> +  const char *dry_run_last_added_dir;

> @@ -487,32 +488,45 @@ dry_run_added_p(const merge_cmd_baton_t
>                         APR_HASH_KEY_STRING) != NULL);
> }
>
> -/* Return true iff we're in dry-run mode and a parent of EDIT_RELPATH
> -  would have been added by now if we weren't in dry-run mode.
> -  Used to avoid spurious notifications (e.g. conflicts) from a merge
> -  attempt into an existing target which would have been deleted if we
> -  weren't in dry_run mode (issue #2584).  Assumes that WCPATH is
> -  still versioned (e.g. has an associated entry). */
> +/* Return true iff we're in dry-run mode and a parent of
> +  LOCAL_RELPATH/LOCAL_ABSPATH would have been added by now if we
> +  weren't in dry-run mode.  Used to avoid spurious notifications
> +  (e.g. conflicts) from a merge attempt into an existing target which
> +  would have been deleted if we weren't in dry_run mode (issue
> +  #2584).  Assumes that WCPATH is still versioned (e.g. has an
> +  associated entry). */
> static svn_boolean_t
> dry_run_added_parent_p(const merge_cmd_baton_t *merge_b,
> -                      const char *edit_relpath,
> +                      const char *local_relpath,
> +                      const char *local_abspath,
>                         apr_pool_t *scratch_pool)
> {
> +  const char *abspath = local_abspath;
>   int i;
> -  const char *abspath;
>
>   if (!merge_b->dry_run)
>     return FALSE;
>
> -  abspath = svn_dirent_join(merge_b->target->abspath, edit_relpath,
> -                            scratch_pool);
> -  for (i = 0; i < (svn_path_component_count(edit_relpath) - 1); i++)
> +  /* See if LOCAL_ABSPATH is a child of the most recently added
> +    directory.  This is an optimization over searching through
> +    dry_run_added that plays to the strengths of the editor's drive
> +    ordering constraints.  In fact, we need the fallback approach
> +    below only because of ra_serf's insufficiencies in this area.  */
> +  if (merge_b->dry_run_last_added_dir
> +      && svn_dirent_is_child(merge_b->dry_run_last_added_dir,
> +                            local_abspath, NULL))
> +    return TRUE;
> +
> +  /* The fallback:  see if any of LOCAL_ABSPATH's parents have been
> +    added in this merge so far. */
> +  for (i = 0; i < (svn_path_component_count(local_relpath) - 1); i++)
>     {
>       abspath = svn_dirent_dirname(abspath, scratch_pool);
>       if (apr_hash_get(merge_b->dry_run_added, abspath,
>                         APR_HASH_KEY_STRING))
>         return TRUE;
>     }
> +
>   return FALSE;
> }
>

> @@ -2386,10 +2402,10 @@ merge_dir_added(svn_wc_notify_state_t *s
>       /* Unversioned or schedule-delete */
>       if (merge_b->dry_run)
>         {
> -          const char *added_path = apr_pstrdup(merge_b->pool,
> -                                              local_abspath);
> -          apr_hash_set(merge_b->dry_run_added, added_path,
> -                      APR_HASH_KEY_STRING, added_path);
> +          merge_b->dry_run_last_added_dir =
> +            apr_pstrdup(merge_b->pool, local_abspath);
> +          apr_hash_set(merge_b->dry_run_added,
> merge_b->dry_run_last_added_dir,
> +                      APR_HASH_KEY_STRING,
> merge_b->dry_run_last_added_dir);
>         }
>       else
>         {
> @@ -2425,10 +2441,8 @@ merge_dir_added(svn_wc_notify_state_t *s
>             }
>           else
>             {
> -              const char *added_path = apr_pstrdup(merge_b->pool,
> -                                                  local_abspath);
> -              apr_hash_set(merge_b->dry_run_added, added_path,
> -                          APR_HASH_KEY_STRING, added_path);
> +              merge_b->dry_run_last_added_dir =
> +                apr_pstrdup(merge_b->pool, local_abspath);

Oops -- no longer setting the hash here.  Maybe use a macro or function to encapsulate both parts of the "registration".

>             }
>           if (state)
>             *state = svn_wc_notify_state_changed;
> @@ -2467,6 +2481,9 @@ merge_dir_added(svn_wc_notify_state_t *s
>         }
>       break;
>     case svn_node_file:
> +      if (merge_b->dry_run)
> +        merge_b->dry_run_last_added_dir = NULL;

Clearing that path is just an optimization, right?  (Also below.)

>       if (is_versioned && dry_run_deleted_p(merge_b, local_abspath))
>         {
>           /* ### TODO: Retain record of this dir being added to
> @@ -2488,6 +2505,8 @@ merge_dir_added(svn_wc_notify_state_t *s
>         }
>       break;
>     default:
> +      if (merge_b->dry_run)
> +        merge_b->dry_run_last_added_dir = NULL;
>       if (state)
>         *state = svn_wc_notify_state_unknown;
>       break;
> @@ -9199,6 +9218,7 @@ do_merge(apr_hash_t **modified_subtrees,
>         dry_run ? apr_hash_make(iterpool) : NULL;
>       merge_cmd_baton.dry_run_added =
>         dry_run ? apr_hash_make(iterpool) : NULL;
> +      merge_cmd_baton.dry_run_last_added_dir = NULL;
>       merge_cmd_baton.conflicted_paths = NULL;
>       merge_cmd_baton.paths_with_new_mergeinfo = NULL;
>       merge_cmd_baton.paths_with_deleted_mergeinfo = NULL;
>
Received on 2012-11-28 21:40:25 CET

This is an archived mail posted to the Subversion Dev mailing list.