subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Burba <ptbu...@gmail.com>
Subject Re: svn commit: r1127134 - in /subversion/trunk/subversion: libsvn_client/add.c libsvn_wc/adm_ops.c tests/cmdline/tree_conflict_tests.py
Date Tue, 06 Sep 2011 22:04:32 GMT
On Tue, May 24, 2011 at 12:35 PM,  <stsp@apache.org> wrote:
> Author: stsp
> Date: Tue May 24 16:35:14 2011
> New Revision: 1127134
>
> URL: http://svn.apache.org/viewvc?rev=1127134&view=rev
> Log:
> As part of issue #3779, "actual-only nodes need regression tests",
> make 'svn add' detect tree conflict victims that do not exist on disk
> and prevent adding new nodes at that path with a meaningful error message.
>
> This implies that if users need to add a new node to resolve the conflict
> they need to mark the conflict as resolved first. I think this is safer
> than allowing accidental additions to take place. Since the node is not
> visible on disk the addition might be a mistake.
>
> * subversion/libsvn_wc/adm_ops.c
>  (check_can_add_node): Don't allow adding new items on top of nonexistent
>   conflicted nodes.
>
> * subversion/libsvn_client/add.c
>  (add): As previous.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
>  (actual_only_node_behaviour): Adjust test cases for 'add' and 'mkdir'.
>
> Modified:
>    subversion/trunk/subversion/libsvn_client/add.c
>    subversion/trunk/subversion/libsvn_wc/adm_ops.c
>    subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/add.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/add.c (original)
> +++ subversion/trunk/subversion/libsvn_client/add.c Tue May 24 16:35:14 2011
> @@ -537,10 +537,29 @@ add(void *baton, apr_pool_t *result_pool
>   else if (kind == svn_node_file)
>     err = add_file(b->local_abspath, b->ctx, scratch_pool);
>   else if (kind == svn_node_none)
> -    return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> -                             _("'%s' not found"),
> -                             svn_dirent_local_style(b->local_abspath,
> -                                                    scratch_pool));
> +    {
> +      svn_boolean_t tree_conflicted;
> +
> +      /* Provide a meaningful error message if the node does not exist
> +       * on disk but is a tree conflict victim. */
> +      err = svn_wc_conflicted_p3(NULL, NULL, &tree_conflicted,
> +                                 b->ctx->wc_ctx, b->local_abspath,
> +                                 scratch_pool);
> +      if (err)
> +        svn_error_clear(err);
> +      else if (tree_conflicted)
> +        return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> +                                 _("'%s' is an existing item in conflict;
"
> +                                   "please mark the conflict as resolved
"
> +                                   "before adding a new item here"),
> +                                 svn_dirent_local_style(b->local_abspath,
> +                                                        scratch_pool));
> +      return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> +                               _("'%s' not found"),
> +                               svn_dirent_local_style(b->local_abspath,
> +                                                      scratch_pool));
> +    }
>   else
>     return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
>                              _("Unsupported node kind for path '%s'"),
>
> Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue May 24 16:35:14 2011
> @@ -849,10 +849,12 @@ check_can_add_node(svn_node_kind_t *kind
>      adding the new node; if not, return an error. */
>   {
>     svn_wc__db_status_t status;
> +    svn_boolean_t conflicted;
>     svn_error_t *err
>       = svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
>                              NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> -                             NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> +                             NULL, NULL, NULL, NULL, NULL, NULL,
> +                             &conflicted,
>                              NULL, NULL, NULL, NULL, NULL, NULL,
>                              db, local_abspath,
>                              scratch_pool, scratch_pool);
> @@ -870,6 +872,16 @@ check_can_add_node(svn_node_kind_t *kind
>       {
>         is_wc_root = FALSE;
>         exists = TRUE;
> +
> +        /* Note that the node may be in conflict even if it does not
> +         * exist on disk (certain tree conflict scenarios). */
> +        if (conflicted)
> +          return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> +                                   _("'%s' is an existing item in conflict;
"
> +                                   "please mark the conflict as resolved
"
> +                                   "before adding a new item here"),
> +                                   svn_dirent_local_style(local_abspath,
> +                                                        
 scratch_pool));

Hi Stefan,

While reviewing some outstanding merge-related issues I noticed that
this change breaks the use-case of incoming replacements on local
deletes.  The delete portion of the replacement is handled and a
tree-conflict set by the time the add is done and the above error is
raised.

Here's a quick demonstration starting with a vanilla Greek tree:

### Make a branch:

  >svn copy ^^/A ^^/branch -m "Create a branch"

  Committed revision 2.

### Replace a directory on the "trunk":

  >svn del A\C
  D         A\C

  >svn mkdir A\C
  A         A\C

  >svn ci -m "Replace A/C"
  Replacing      A\C

  Committed revision 3.

### Delete the corresponding directory on the "branch":

  >svn del ^^/branch/C -m "Delete branch/C"

  Committed revision 4.

  >svn up -q

### Now try to merge the replacement onto the deletion; it fails:

  >svn merge ^^/A branch
  ..\..\..\subversion\svn\util.c:913: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\merge.c:11349: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\merge.c:11303: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\merge.c:11303: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\merge.c:11273: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\merge.c:9287: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\merge.c:8870: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\merge.c:5349: (apr_err=155015)
  ..\..\..\subversion\libsvn_repos\reporter.c:1430: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\ra.c:247: (apr_err=155015)
  ..\..\..\subversion\libsvn_repos\reporter.c:1269: (apr_err=155015)
  ..\..\..\subversion\libsvn_repos\reporter.c:1205: (apr_err=155015)
  ..\..\..\subversion\libsvn_repos\reporter.c:920: (apr_err=155015)
  ..\..\..\subversion\libsvn_delta\cancel.c:120: (apr_err=155015)
  ..\..\..\subversion\libsvn_delta\cancel.c:120: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\repos_diff.c:710: (apr_err=155015)
  ..\..\..\subversion\libsvn_client\merge.c:2234: (apr_err=155015)
  ..\..\..\subversion\libsvn_wc\adm_ops.c:1069: (apr_err=155015)
  ..\..\..\subversion\libsvn_wc\adm_ops.c:937: (apr_err=155015)
  svn: E155015:
'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\working_copies\merge_tree_conflict_tests-24\branch\C'
is an existing item in conflict; please mark the conflict as resolved
before adding a new item here

  >svn st
  ?     C branch\C
        >   local delete, incoming delete upon merge
  Summary of conflicts:
    Tree conflicts: 1

### Prior to r1127134 the merge raised a tree conflict:

  trunk@1127133>svn merge ^^/A branch
  --- Merging r2 through r4 into 'branch':
     C branch\C
  --- Recording mergeinfo for merge of r2 through r4 into 'branch':
   U   branch
  Summary of conflicts:
    Tree conflicts: 1

  trunk@1127133>svn st
   M      branch
  !     C branch\C
        >   local delete, incoming delete upon merge
  Summary of conflicts:
    Tree conflicts: 1

Not exactly sure how to fix this...I can look at it further tomorrow,
just wanted to get your thoughts if you have time.

Paul

>         switch (status)
>           {
>             case svn_wc__db_status_not_present:
>
> Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Tue May 24 16:35:14
2011
> @@ -1134,23 +1134,17 @@ def actual_only_node_behaviour(sbox):
>
>   # add
>   expected_stdout = None
> -  expected_stderr = ".*foo.*not found.*"
> +  expected_stderr = ".*foo.*is an existing item in conflict.*"
>   run_and_verify_svn(None, expected_stdout, expected_stderr,
>                      "add", foo_path)
>
>   # add (with an existing obstruction of foo)
> -  ### this does not error out -- needs review
>   svntest.main.file_write(foo_path, "This is an obstruction of foo.\n")
> -  expected_stdout = "A.*foo"
> -  expected_stderr = []
> +  expected_stdout = None
> +  expected_stderr = ".*foo.*is an existing item in conflict.*"
>   run_and_verify_svn(None, expected_stdout, expected_stderr,
>                      "add", foo_path)
> -  ### for now, ignore the fact that add succeeds, revert the entire
> -  ### working copy, and repeat the merge so we can test more commands
> -  svntest.main.run_svn(None, "revert", "-R", wc_dir)
>   os.remove(foo_path) # remove obstruction
> -  svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
> -                       os.path.join(wc_dir, 'A'))
>
>   # blame (praise, annotate, ann)
>   expected_stdout = None
> @@ -1267,17 +1261,10 @@ def actual_only_node_behaviour(sbox):
>   run_and_verify_svn(None, expected_stdout, expected_stderr,
>                      "mergeinfo", A_copy_url + '/foo', foo_path)
>   # mkdir
> -  ### this does not error out -- needs review
>   expected_stdout = None
> -  expected_stderr = []
> +  expected_stderr = ".*foo.*is an existing item in conflict.*"
>   run_and_verify_svn(None, expected_stdout, expected_stderr,
>                      "mkdir", foo_path)
> -  ### for now, ignore the fact that mkdir succeeds, revert the entire
> -  ### working copy, and repeat the merge so we can test more commands
> -  svntest.main.run_svn(None, "revert", "-R", wc_dir)
> -  os.rmdir(foo_path) # remove obstruction
> -  svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
> -                       os.path.join(wc_dir, 'A'))
>
>   # move (mv, rename, ren)
>   expected_stdout = None
>
>
>

Mime
View raw message