You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2016/06/13 13:51:49 UTC

svn commit: r1748236 - in /subversion/trunk/subversion: libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c

Author: stsp
Date: Mon Jun 13 13:51:49 2016
New Revision: 1748236

URL: http://svn.apache.org/viewvc?rev=1748236&view=rev
Log:
When merging an incoming file move, record this move in the working copy.

This makes merged file moves appear as having been "replayed" on the local
branch, rather than as a delete and a copy from the merge source branch.
File content changes are preserved and may raise a text conflict if applicable.

* subversion/libsvn_client/conflicts.c
  (resolve_incoming_move_file_text_merge): If the conflict was flagged by a
   merge operation, run a meta-data only move instead of just a deletion.

* subversion/tests/libsvn_client/conflicts-test.c
  (test_option_merge_incoming_move_file_text_merge): Update test expectations.

Modified:
    subversion/trunk/subversion/libsvn_client/conflicts.c
    subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c

Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/conflicts.c?rev=1748236&r1=1748235&r2=1748236&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
+++ subversion/trunk/subversion/libsvn_client/conflicts.c Mon Jun 13 13:51:49 2016
@@ -5588,6 +5588,7 @@ resolve_incoming_move_file_text_merge(sv
 {
   svn_client_conflict_option_id_t option_id;
   const char *local_abspath;
+  svn_wc_operation_t operation;
   const char *lock_abspath;
   svn_client_ctx_t *ctx = conflict->ctx;
   svn_error_t *err;
@@ -5612,6 +5613,7 @@ resolve_incoming_move_file_text_merge(sv
   struct conflict_tree_incoming_delete_details *details;
 
   local_abspath = svn_client_conflict_get_local_abspath(conflict);
+  operation = svn_client_conflict_get_operation(conflict);
   details = conflict->tree_conflict_incoming_details;
   if (details == NULL)
     return svn_error_createf(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE, NULL,
@@ -5724,12 +5726,38 @@ resolve_incoming_move_file_text_merge(sv
       ctx->notify_func2(ctx->notify_baton2, notify, scratch_pool);
     }
 
-  /* The merge is done. Local edits are now at the moved-to location.
-   * Delete the tree conflict victim (clears the tree conflict marker). */
-  err = svn_wc_delete4(ctx->wc_ctx, local_abspath, FALSE, FALSE,
-                       NULL, NULL, /* don't allow user to cancel here */
-                       ctx->notify_func2, ctx->notify_baton2,
-                       scratch_pool);
+  /* The merge is done. Local edits are now at the moved-to location. */
+  if (operation == svn_wc_operation_update ||
+      operation == svn_wc_operation_switch)
+    {
+      /* The move operation is part of our natural history.
+       * Delete the tree conflict victim (clears the tree conflict marker). */
+      err = svn_wc_delete4(ctx->wc_ctx, local_abspath, FALSE, FALSE,
+                           NULL, NULL, /* don't allow user to cancel here */
+                           NULL, NULL, /* no extra notification */
+                           scratch_pool);
+      if (err)
+        goto unlock_wc;
+    }
+  else if (operation == svn_wc_operation_merge)
+    {
+      /* The move operation is not part of natural history. We must replicate
+       * this move in our history. Record a move in the working copy. */
+      err = svn_wc__move2(ctx->wc_ctx, local_abspath, details->moved_to_abspath,
+                          TRUE, /* this is a meta-data only move */
+                          FALSE, /* mixed-revisions don't apply to files */
+                          NULL, NULL, /* don't allow user to cancel here */
+                          NULL, NULL, /* no extra notification */
+                          scratch_pool);
+      if (err)
+        goto unlock_wc;
+    }
+  else
+    return svn_error_createf(SVN_ERR_WC_CORRUPT, NULL,
+                             _("Invalid operation code '%d' recorded for "
+                               "conflict at '%s'"), operation,
+                             svn_dirent_local_style(local_abspath,
+                                                    scratch_pool));
 
   if (ctx->notify_func2)
     {

Modified: subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c?rev=1748236&r1=1748235&r2=1748236&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c Mon Jun 13 13:51:49 2016
@@ -1439,9 +1439,10 @@ test_option_merge_incoming_move_file_tex
   SVN_TEST_ASSERT(!status->copied);
   SVN_TEST_ASSERT(!status->switched);
   SVN_TEST_ASSERT(!status->file_external);
-  /* ### Record as moved-away in the WC? */
   SVN_TEST_ASSERT(status->moved_from_abspath == NULL);
-  SVN_TEST_ASSERT(status->moved_to_abspath == NULL);
+  new_file_path = svn_relpath_join(branch_path, new_file_name, b->pool);
+  SVN_TEST_STRING_ASSERT(status->moved_to_abspath,
+                         sbox_wc_path(b, new_file_path));
 
   SVN_ERR(svn_client_conflict_get(&conflict, sbox_wc_path(b, deleted_path),
                                   ctx, b->pool, b->pool));
@@ -1456,7 +1457,6 @@ test_option_merge_incoming_move_file_tex
                   !tree_conflicted);
 
   /* Ensure that the moved file has the expected status. */
-  new_file_path = svn_relpath_join(branch_path, new_file_name, b->pool);
   opt_rev.kind = svn_opt_revision_working;
   sb.result_pool = b->pool;
   SVN_ERR(svn_client_status6(NULL, ctx, sbox_wc_path(b, new_file_path),
@@ -1468,13 +1468,13 @@ test_option_merge_incoming_move_file_tex
   SVN_TEST_ASSERT(status->versioned);
   SVN_TEST_ASSERT(!status->conflicted);
   SVN_TEST_ASSERT(status->node_status == svn_wc_status_added);
-  SVN_TEST_ASSERT(status->text_status == svn_wc_status_modified);
+  SVN_TEST_ASSERT(status->text_status == svn_wc_status_normal);
   SVN_TEST_ASSERT(status->prop_status == svn_wc_status_none);
   SVN_TEST_ASSERT(status->copied);
   SVN_TEST_ASSERT(!status->switched);
   SVN_TEST_ASSERT(!status->file_external);
-  /* ### Record as moved-here in the WC? */
-  SVN_TEST_ASSERT(status->moved_from_abspath == NULL);
+  SVN_TEST_STRING_ASSERT(status->moved_from_abspath,
+                         sbox_wc_path(b, deleted_path));
   SVN_TEST_ASSERT(status->moved_to_abspath == NULL);
 
   /* Ensure that the moved file has the expected content. */



Re: We can now merge file renames (Re: svn commit: r1748236 - in /subversion/trunk/subversion: libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Jun 20, 2016 at 08:34:24AM +0200, Stefan Fuhrmann wrote:
> Can you say what the main limitations are and whether you
> plan to remove them? My guess would be: files only, one
> move per file & conflict resolution cycle, no cyclic renames.
> 
> -- Stefan^2.

I expect to get directories working for 1.10. My plan is to reuse some
of the code we have in wc_db_update_move.c to merge a locally deleted
directory (incoming move source) with a copied one (incoming move target).

As for cycles, I'm not quite sure. The repository scanning code should
handle them fine since it's just building a graph of delete+copy pairs
across history. It's probably a question of doing the right thing in the WC.
Update/merge/switch won't do anything to a node which is already in a tree
conflict, so perhaps there's a possibility we're losing one of the renames?
A regression test will tell us more. Would you like to write one? :)

Some other limitations are already documented in the code.
Look for ### comments in libsvn_client/conflicts.c.

Re: We can now merge file renames (Re: svn commit: r1748236 - in /subversion/trunk/subversion: libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c

Posted by Stefan Fuhrmann <st...@apache.org>.
On 13.06.2016 16:10, Stefan Sperling wrote:
> On Mon, Jun 13, 2016 at 01:51:49PM -0000, stsp@apache.org wrote:
>> Author: stsp
>> Date: Mon Jun 13 13:51:49 2016
>> New Revision: 1748236
>>
>> URL: http://svn.apache.org/viewvc?rev=1748236&view=rev
>> Log:
>> When merging an incoming file move, record this move in the working copy.
>>
>> This makes merged file moves appear as having been "replayed" on the local
>> branch, rather than as a delete and a copy from the merge source branch.
>> File content changes are preserved and may raise a text conflict if applicable.
>>
>> * subversion/libsvn_client/conflicts.c
>>    (resolve_incoming_move_file_text_merge): If the conflict was flagged by a
>>     merge operation, run a meta-data only move instead of just a deletion.
>>
>> * subversion/tests/libsvn_client/conflicts-test.c
>>    (test_option_merge_incoming_move_file_text_merge): Update test expectations.
>>
>> Modified:
>>      subversion/trunk/subversion/libsvn_client/conflicts.c
>>      subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
>   
> Just to raise awareness:
>
> As of this commit, interactive conflict resolution allows simple file
> moves to be merged between branches and boil down the text-conflicts.
> Example below.
Sorry for being late to the party, but this is excellent work!

Can you say what the main limitations are and whether you
plan to remove them? My guess would be: files only, one
move per file & conflict resolution cycle, no cyclic renames.

-- Stefan^2.

Re: We can now merge file renames (Re: svn commit: r1748236 - in /subversion/trunk/subversion: libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c

Posted by Johan Corveleyn <jc...@gmail.com>.
On Mon, Jun 13, 2016 at 4:10 PM, Stefan Sperling <st...@apache.org> wrote:
> On Mon, Jun 13, 2016 at 01:51:49PM -0000, stsp@apache.org wrote:
>> Author: stsp
>> Date: Mon Jun 13 13:51:49 2016
>> New Revision: 1748236
>>
>> URL: http://svn.apache.org/viewvc?rev=1748236&view=rev
>> Log:
>> When merging an incoming file move, record this move in the working copy.
>>
>> This makes merged file moves appear as having been "replayed" on the local
>> branch, rather than as a delete and a copy from the merge source branch.
>> File content changes are preserved and may raise a text conflict if applicable.
>>
>> * subversion/libsvn_client/conflicts.c
>>   (resolve_incoming_move_file_text_merge): If the conflict was flagged by a
>>    merge operation, run a meta-data only move instead of just a deletion.
>>
>> * subversion/tests/libsvn_client/conflicts-test.c
>>   (test_option_merge_incoming_move_file_text_merge): Update test expectations.
>>
>> Modified:
>>     subversion/trunk/subversion/libsvn_client/conflicts.c
>>     subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
>
> Just to raise awareness:
>
> As of this commit, interactive conflict resolution allows simple file
> moves to be merged between branches and boil down the text-conflicts.
> Example below.

Sweet! I've read through your example, and this looks really great!

I can't wait to start using it, and start promoting it to my user base
;-). Time to fire up a trunk build, and start looking at extending
those tests you mentioned.

BTW: until now it looks like the "m" option would usually be the
sanest option ("suggested option", as mentioned by swdev in the other
thread). At least, that's what I would recommend to my users, if they
don't want to think too hard: "just accept the (m)erge option, if
there is one" :-).

Great work!
-- 
Johan

We can now merge file renames (Re: svn commit: r1748236 - in /subversion/trunk/subversion: libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c

Posted by Stefan Sperling <st...@apache.org>.
On Mon, Jun 13, 2016 at 01:51:49PM -0000, stsp@apache.org wrote:
> Author: stsp
> Date: Mon Jun 13 13:51:49 2016
> New Revision: 1748236
> 
> URL: http://svn.apache.org/viewvc?rev=1748236&view=rev
> Log:
> When merging an incoming file move, record this move in the working copy.
> 
> This makes merged file moves appear as having been "replayed" on the local
> branch, rather than as a delete and a copy from the merge source branch.
> File content changes are preserved and may raise a text conflict if applicable.
> 
> * subversion/libsvn_client/conflicts.c
>   (resolve_incoming_move_file_text_merge): If the conflict was flagged by a
>    merge operation, run a meta-data only move instead of just a deletion.
> 
> * subversion/tests/libsvn_client/conflicts-test.c
>   (test_option_merge_incoming_move_file_text_merge): Update test expectations.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_client/conflicts.c
>     subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
 
Just to raise awareness:

As of this commit, interactive conflict resolution allows simple file
moves to be merged between branches and boil down the text-conflicts.
Example below.

$ sh test-merge.sh
+ rm -rf test-merge
+ mkdir -p test-merge
+ mkdir -p test-merge/trunk
+ echo alpha
+ > test-merge/trunk/alpha
+ echo beta
+ > test-merge/trunk/beta
+ mkdir test-merge/trunk/gamma
+ echo delta
+ > test-merge/trunk/gamma/delta
+ mkdir test-merge/trunk/epsilon
+ echo zeta
+ > test-merge/trunk/epsilon/zeta
+ svnadmin create /tmp/test-merge/repos
+ svn import test-merge/trunk file:////tmp/test-merge/repos/trunk -m importing project tree
Adding         test-merge/trunk/alpha
Adding         test-merge/trunk/beta
Adding         test-merge/trunk/epsilon
Adding         test-merge/trunk/epsilon/zeta
Adding         test-merge/trunk/gamma
Adding         test-merge/trunk/gamma/delta
Committing transaction...
Committed revision 1.
+ svn copy file:////tmp/test-merge/repos/trunk file:////tmp/test-merge/repos/branch -m creating branch
Committing transaction...
Committed revision 2.
+ rm -rf test-merge/trunk
+ svn checkout file:////tmp/test-merge/repos/trunk test-merge/trunk
A    test-merge/trunk/alpha
A    test-merge/trunk/beta
A    test-merge/trunk/epsilon
A    test-merge/trunk/epsilon/zeta
A    test-merge/trunk/gamma
A    test-merge/trunk/gamma/delta
Checked out revision 2.
+ svn checkout file:////tmp/test-merge/repos/branch test-merge/branch
A    test-merge/branch/alpha
A    test-merge/branch/beta
A    test-merge/branch/epsilon
A    test-merge/branch/epsilon/zeta
A    test-merge/branch/gamma
A    test-merge/branch/gamma/delta
Checked out revision 2.
+ svn mv test-merge/trunk/alpha test-merge/trunk/alpha-renamed
A         test-merge/trunk/alpha-renamed
D         test-merge/trunk/alpha
+ svn ci -m move alpha to alpha-renamed test-merge/trunk
Deleting       test-merge/trunk/alpha
Adding         test-merge/trunk/alpha-renamed
Committing transaction...
Committed revision 3.
+ echo foo
+ > test-merge/branch/alpha
+ svn ci -m edit alpha on the branch test-merge/branch
Sending        test-merge/branch/alpha
Transmitting file data .done
Committing transaction...
Committed revision 4.
+ echo bar
+ > test-merge/trunk/alpha-renamed
+ svn ci -m edit alpha-renamed on trunk test-merge/trunk
Sending        test-merge/trunk/alpha-renamed
Transmitting file data .done
Committing transaction...
Committed revision 5.
+ svn up test-merge/branch
Updating 'test-merge/branch':
At revision 5.
+ svn merge ^/trunk test-merge/branch
--- Merging r2 through r5 into 'test-merge/branch':
   C test-merge/branch/alpha
A    test-merge/branch/alpha-renamed
--- Recording mergeinfo for merge of r2 through r5 into 'test-merge/branch':
 U   test-merge/branch
Summary of conflicts:
  Tree conflicts: 1
Tree conflict on 'test-merge/branch/alpha':
File merged from
'^/trunk/alpha@1'
to
'^/trunk/alpha@5'
was moved to '^/trunk/alpha-renamed' by stsp in r3.
A file which differs from the corresponding file on the merge source branch was found in the working copy.
Select: (p) postpone, (r) accept current working copy state,
        (i) ignore incoming deletion, (a) accept incoming deletion,
        (m) follow incoming move and merge, (q) quit resolution,
        (h) help: m
C    test-merge/branch/alpha-renamed
Tree conflict at 'test-merge/branch/alpha' marked as resolved.
Summary of conflicts:
  Text conflicts: 1 remaining (and 0 already resolved)
  Tree conflicts: 0 remaining (and 1 already resolved)
+ svn resolve test-merge/branch
Merge conflict discovered in file 'test-merge/branch/alpha-renamed'.
Select: (p) postpone, (df) show diff, (e) edit file, (m) merge,
        (s) show all options: m
Merging 'test-merge/branch/alpha-renamed'.
Conflicting section found during merge:
(1) their version (at line 1)         |(2) your version (at line 1)
--------------------------------------+--------------------------------------
foo                                   |bar
--------------------------------------+--------------------------------------
Select: (1) use their version, (2) use your version,
        (12) their version first, then yours,
        (21) your version first, then theirs,
        (e1) edit their version and use the result,
        (e2) edit your version and use the result,
        (eb) edit both versions and use the result,
        (p) postpone this conflicting section leaving conflict markers,
        (a) abort file merge and return to main menu: 12
Merge of 'test-merge/branch/alpha-renamed' completed.
Select: (p) postpone, (df) show diff, (e) edit file, (m) merge,
        (r) mark resolved, (s) show all options: r
Merge conflicts in 'test-merge/branch/alpha-renamed' marked as resolved.
+ svn ci -m merge trunk to branch test-merge/branch
Sending        test-merge/branch
Deleting       test-merge/branch/alpha
Adding         test-merge/branch/alpha-renamed
Transmitting file data .done
Committing transaction...
Committed revision 6.
+ svn up test-merge/trunk
Updating 'test-merge/trunk':
At revision 6.
+ svn merge ^/branch test-merge/trunk
--- Merging differences between repository URLs into 'test-merge/trunk':
U    test-merge/trunk/alpha-renamed
--- Recording mergeinfo for merge between repository URLs into 'test-merge/trunk':
 U   test-merge/trunk
+ svn ci -m reintegrate branch to trunk test-merge/trunk
Sending        test-merge/trunk
Sending        test-merge/trunk/alpha-renamed
Transmitting file data .done
Committing transaction...
Committed revision 7.
+ svn log -v --diff file:////tmp/test-merge/repos/
------------------------------------------------------------------------
r7 | stsp | 2016-06-13 16:06:34 +0200 (Mon, 13 Jun 2016) | 1 line
Changed paths:
   M /trunk
   M /trunk/alpha-renamed

reintegrate branch to trunk

Index: trunk/alpha-renamed
===================================================================
--- trunk/alpha-renamed (revision 6)
+++ trunk/alpha-renamed (revision 7)
@@ -1 +1,2 @@
+foo
 bar
Index: trunk
===================================================================
--- trunk       (revision 6)
+++ trunk       (revision 7)

Property changes on: trunk
___________________________________________________________________
Added: svn:mergeinfo
## -0,0 +0,1 ##
   Merged /branch:r2-6

------------------------------------------------------------------------
r6 | stsp | 2016-06-13 16:06:33 +0200 (Mon, 13 Jun 2016) | 1 line
Changed paths:
   M /branch
   D /branch/alpha
   A /branch/alpha-renamed (from /branch/alpha:5)

merge trunk to branch

Index: branch/alpha (deleted)
===================================================================
Index: branch/alpha-renamed
===================================================================
--- branch/alpha-renamed        (nonexistent)
+++ branch/alpha-renamed        (revision 6)
@@ -0,0 +1,2 @@
+foo
+bar
Index: branch
===================================================================
--- branch      (revision 5)
+++ branch      (revision 6)

Property changes on: branch
___________________________________________________________________
Added: svn:mergeinfo
## -0,0 +0,1 ##
   Merged /trunk:r2-5

------------------------------------------------------------------------
r5 | stsp | 2016-06-13 16:06:28 +0200 (Mon, 13 Jun 2016) | 1 line
Changed paths:
   M /trunk/alpha-renamed

edit alpha-renamed on trunk

Index: trunk/alpha-renamed
===================================================================
--- trunk/alpha-renamed (revision 4)
+++ trunk/alpha-renamed (revision 5)
@@ -1 +1 @@
-alpha
+bar

------------------------------------------------------------------------
r4 | stsp | 2016-06-13 16:06:28 +0200 (Mon, 13 Jun 2016) | 1 line
Changed paths:
   M /branch/alpha

edit alpha on the branch

Index: branch/alpha
===================================================================
--- branch/alpha        (revision 3)
+++ branch/alpha        (revision 4)
@@ -1 +1 @@
-alpha
+foo

------------------------------------------------------------------------
r3 | stsp | 2016-06-13 16:06:28 +0200 (Mon, 13 Jun 2016) | 1 line
Changed paths:
   D /trunk/alpha
   A /trunk/alpha-renamed (from /trunk/alpha:2)

move alpha to alpha-renamed

Index: trunk/alpha (deleted)
===================================================================
Index: trunk/alpha-renamed
===================================================================
--- trunk/alpha-renamed (nonexistent)
+++ trunk/alpha-renamed (revision 3)
@@ -0,0 +1 @@
+alpha

------------------------------------------------------------------------
r2 | stsp | 2016-06-13 16:06:27 +0200 (Mon, 13 Jun 2016) | 1 line
Changed paths:
   A /branch (from /trunk:1)

creating branch

Index: branch/alpha
===================================================================
--- branch/alpha        (nonexistent)
+++ branch/alpha        (revision 2)
@@ -0,0 +1 @@
+alpha
Index: branch/beta
===================================================================
--- branch/beta (nonexistent)
+++ branch/beta (revision 2)
@@ -0,0 +1 @@
+beta
Index: branch/epsilon/zeta
===================================================================
--- branch/epsilon/zeta (nonexistent)
+++ branch/epsilon/zeta (revision 2)
@@ -0,0 +1 @@
+zeta
Index: branch/gamma/delta
===================================================================
--- branch/gamma/delta  (nonexistent)
+++ branch/gamma/delta  (revision 2)
@@ -0,0 +1 @@
+delta

------------------------------------------------------------------------
r1 | stsp | 2016-06-13 16:06:27 +0200 (Mon, 13 Jun 2016) | 1 line
Changed paths:
   A /trunk
   A /trunk/alpha
   A /trunk/beta
   A /trunk/epsilon
   A /trunk/epsilon/zeta
   A /trunk/gamma
   A /trunk/gamma/delta

importing project tree

Index: trunk/alpha
===================================================================
--- trunk/alpha (nonexistent)
+++ trunk/alpha (revision 1)
@@ -0,0 +1 @@
+alpha
Index: trunk/beta
===================================================================
--- trunk/beta  (nonexistent)
+++ trunk/beta  (revision 1)
@@ -0,0 +1 @@
+beta
Index: trunk/epsilon/zeta
===================================================================
--- trunk/epsilon/zeta  (nonexistent)
+++ trunk/epsilon/zeta  (revision 1)
@@ -0,0 +1 @@
+zeta
Index: trunk/gamma/delta
===================================================================
--- trunk/gamma/delta   (nonexistent)
+++ trunk/gamma/delta   (revision 1)
@@ -0,0 +1 @@
+delta

------------------------------------------------------------------------
$