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

Re: r1414810 - dry_run_added_parent_p() - libsvn_client/merge.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 28 Nov 2012 18:44:56 +0000 (GMT)

> Author: cmpilato

> URL: http://svn.apache.org/viewvc?rev=1414810&view=rev
> Log:
> Teach the dry-run merge logic to account for additional flavors of
> ra_serf editor drive ordering violations (*sigh*).  [...]
>
> This change causes Subversion to check not just the most recently
> added directory, but all added directories (which were already being
> tracked elsewhere, anyway) before deeming "dir1/file" an obstructed
> merge addition.
>
> * subversion/libsvn_client/merge.c
>   (merge_cmd_baton_t): Remove the 'added_path' member.
>   (dry_run_added_parent_p): New helper function.
>   (merge_file_added): Now use dry_run_added_parent_p() instead of
>     merge_b->added_path to look for added parent paths.
>   (merge_dir_added): Now use dry_run_added_parent_p() instead of
>     merge_b->added_path to record added paths and to check for added
>     parent paths.
>   (do_merge): Don't initialize the now-removed 'added_path' merge
>     context baton member.

If the logic is that the list of added paths is not necessarily contain all leaf nodes, then doesn't it follow that dry_run_deleted_p() also needs to be updated to check whether any parent of the path being checked is in the list?

- Julian

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

> +/* 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). */
> +static svn_boolean_t
> +dry_run_added_parent_p(const merge_cmd_baton_t *merge_b,
> +                      const char *edit_relpath,
> +                      apr_pool_t *scratch_pool)
> +{
> +  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++)
> +    {
> +      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;
> +}
> +
> /* Return whether any WC path was put in conflict by the merge
>     operation corresponding to MERGE_B. */
> static APR_INLINE svn_boolean_t
> @@ -1857,8 +1883,8 @@ merge_file_added(svn_wc_notify_state_t *
>
>     if (obstr_state != svn_wc_notify_state_inapplicable)
>       {
> -        if (merge_b->dry_run && merge_b->added_path
> -            && svn_dirent_is_child(merge_b->added_path,
> mine_abspath, NULL))
> +        if (merge_b->dry_run
> +            && dry_run_added_parent_p(merge_b, mine_relpath,
> scratch_pool))
>           {
>             if (content_state)
>               *content_state = svn_wc_notify_state_changed;
> @@ -2337,13 +2363,15 @@ merge_dir_added(svn_wc_notify_state_t *s
>
>     if (obstr_state != svn_wc_notify_state_inapplicable)
>       {
> -        if (state && merge_b->dry_run &&
> merge_b->added_path
> -            && svn_dirent_is_child(merge_b->added_path,
> local_abspath, NULL))
> +        if (state && merge_b->dry_run
> +            && dry_run_added_parent_p(merge_b, local_relpath,
> scratch_pool))
>           {
>             *state = svn_wc_notify_state_changed;
>           }
>         else if (state)
> -          *state = obstr_state;
> +          {
> +            *state = obstr_state;
> +          }
>         return SVN_NO_ERROR;
>       }
>
> @@ -2358,9 +2386,10 @@ merge_dir_added(svn_wc_notify_state_t *s
>       /* Unversioned or schedule-delete */
>       if (merge_b->dry_run)
>         {
> -          merge_b->added_path = apr_pstrdup(merge_b->pool,
> local_abspath);
> -          apr_hash_set(merge_b->dry_run_added, merge_b->added_path,
> -                      APR_HASH_KEY_STRING, merge_b->added_path);
> +          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);
>         }
>       else
>         {
> @@ -2385,15 +2414,22 @@ merge_dir_added(svn_wc_notify_state_t *s
>           /* The dir is not known to Subversion, or is schedule-delete.
>             * We will make it schedule-add. */
>           if (!merge_b->dry_run)
> -            SVN_ERR(svn_wc_add4(merge_b->ctx->wc_ctx, local_abspath,
> -                                svn_depth_infinity,
> -                                copyfrom_url, copyfrom_rev,
> -                                merge_b->ctx->cancel_func,
> -                                merge_b->ctx->cancel_baton,
> -                                NULL, NULL, /* no notification func! */
> -                                scratch_pool));
> +            {
> +              SVN_ERR(svn_wc_add4(merge_b->ctx->wc_ctx, local_abspath,
> +                                  svn_depth_infinity,
> +                                  copyfrom_url, copyfrom_rev,
> +                                  merge_b->ctx->cancel_func,
> +                                  merge_b->ctx->cancel_baton,
> +                                  NULL, NULL, /* no notification func! */
> +                                  scratch_pool));
> +            }
>           else
> -            merge_b->added_path = apr_pstrdup(merge_b->pool,
> local_abspath);
> +            {
> +              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);
> +            }
>           if (state)
>             *state = svn_wc_notify_state_changed;
>         }
> @@ -2431,9 +2467,6 @@ merge_dir_added(svn_wc_notify_state_t *s
>         }
>       break;
>     case svn_node_file:
> -      if (merge_b->dry_run)
> -        merge_b->added_path = NULL;
> -
>       if (is_versioned && dry_run_deleted_p(merge_b, local_abspath))
>         {
>           /* ### TODO: Retain record of this dir being added to
> @@ -2455,8 +2488,6 @@ merge_dir_added(svn_wc_notify_state_t *s
>         }
>       break;
>     default:
> -      if (merge_b->dry_run)
> -        merge_b->added_path = NULL;
>       if (state)
>         *state = svn_wc_notify_state_unknown;
>       break;
> @@ -9163,7 +9194,6 @@ do_merge(apr_hash_t **modified_subtrees,
>           be reset for each merge source iteration. */
>       merge_cmd_baton.merge_source = *source;
>       merge_cmd_baton.implicit_src_gap = NULL;
> -      merge_cmd_baton.added_path = NULL;
>       merge_cmd_baton.add_necessitated_merge = FALSE;
>       merge_cmd_baton.dry_run_deletions =
>         dry_run ? apr_hash_make(iterpool) : NULL;
>
Received on 2012-11-28 19:45:34 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.