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

On Sat, Jan 26, 2013 at 5:29 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>> -----Original Message-----
>> From: rhuijben_at_apache.org [mailto:rhuijben_at_apache.org]
>> Sent: zaterdag 26 januari 2013 00:55
>> To: commits_at_subversion.apache.org
>> Subject: svn commit: r1438778 -
>> /subversion/trunk/subversion/libsvn_client/merge.c
>>
>> Author: rhuijben
>> Date: Fri Jan 25 23:54:39 2013
>> New Revision: 1438778
>>
>> URL: http://svn.apache.org/viewvc?rev=1438778&view=rev
>> Log:
>> * subversion/libsvn_client/merge.c
>> (merge_dir_props_changed): Add note about merge_tests.py failure.
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_client/merge.c
>>
>> Modified: subversion/trunk/subversion/libsvn_client/merge.c
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/m
>> erge.c?rev=1438778&r1=1438777&r2=1438778&view=diff
>> ==========================================================
>> ====================
>> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
>> +++ subversion/trunk/subversion/libsvn_client/merge.c Fri Jan 25 23:54:39
>> 2013
>> @@ -1514,6 +1514,18 @@ merge_dir_props_changed(svn_wc_notify_st
>> ### merges, but in this case the fix added here should also be applied
>> ### for added files! */
>>
>> + /* ### The old code performed (via prepare_merge_props_changed):
>> + if (apr_hash_get(new_pristine_props, SVN_PROP_MERGEINFO,
>> + APR_HASH_KEY_STRING))
>> + {
>> + alloc_and_store_path(&merge_b->paths_with_new_mergeinfo,
>> + local_abspath, merge_b->pool);
>> + }
>> + ### which is something merge_add_file() doesn't do, but
>> + ### merge_tests.py 95: no self referential filtering on added path
>> + ### fails if this block is not enabled.
>> + */
>
> If I enable this here, and in the merge_add_file() function all tests will pass with only slight 'U' vs 'G' tweaks, but I'm not sure if this is really the behavior we want here.
>
> I expect that we really want different behavior on changed directories vs added directories.

Agreed, these should be consistent. I see you did that in r1440647.

> I don't see why we want to describe the whole merge on a node if the merge just added the final version.

We need to describe the merge on the added node because the merge
might represent changes other than simply the add.

Here's an example:

### Starting with a vanilla Greek tree, make two branches:

>svn copy A branch-1
A branch-1

>svn copy A branch-2
A branch-2

>svn ci -m "" -q

### Move a directory on our "trunk":

>svn move A\D A\D-MOVED
A A\D-MOVED
D A\D
D A\D\G
D A\D\G\pi
D A\D\G\rho
D A\D\G\tau
D A\D\H
D A\D\H\chi
D A\D\H\omega
D A\D\H\psi
D A\D\gamma

>svn ci -m "" -q

### Sync the "trunk" to the first branch:

>svn merge ^^/A branch-1
--- Merging r2 through r3 into 'branch-1':
A branch-1\D-MOVED
A branch-1\D-MOVED\H
A branch-1\D-MOVED\H\psi
A branch-1\D-MOVED\H\chi
A branch-1\D-MOVED\H\omega
A branch-1\D-MOVED\gamma
A branch-1\D-MOVED\G
A branch-1\D-MOVED\G\pi
A branch-1\D-MOVED\G\rho
A branch-1\D-MOVED\G\tau
D branch-1\D
--- Recording mergeinfo for merge of r2 through r3 into 'branch-1':
 U branch-1

>svn ci -m "" -q

>svn up -q

### Make some file edits within the moved subtree on the first branch:

>echo branch1 edit> branch-1\D-MOVED\H\chi

>svn ci -m "" -q

>echo branch1 edit> branch-1\D-MOVED\G\pi

>svn ci -m "" -q

### Perform file level and directory level subtree merges from the
first branch back to "trunk":

>svn merge ^^/branch-1/D-MOVED/G/pi A\D-MOVED\G\pi
--- Merging r4 through r6 into 'A\D-MOVED\G\pi':
U A\D-MOVED\G\pi
--- Recording mergeinfo for merge of r4 through r6 into 'A\D-MOVED\G\pi':
 U A\D-MOVED\G\pi

>svn merge ^^/branch-1/D-MOVED/H A\D-MOVED\H
--- Merging r4 through r6 into 'A\D-MOVED\H':
U A\D-MOVED\H\chi
--- Recording mergeinfo for merge of r4 through r6 into 'A\D-MOVED\H':
 U A\D-MOVED\H

>svn st
MM A\D-MOVED\G\pi
 M A\D-MOVED\H
M A\D-MOVED\H\chi

>svn pl -vR A
Properties on 'A\D-MOVED\G\pi':
  svn:mergeinfo
    /branch-1/D-MOVED/G/pi:4-6
Properties on 'A\D-MOVED\H':
  svn:mergeinfo
    /branch-1/D-MOVED/H:4-6

>svn ci -m "subtree cherry picks back from branch"
Sending A\D-MOVED\G\pi
Sending A\D-MOVED\H
Sending A\D-MOVED\H\chi
Transmitting file data ..
Committed revision 7.

>svn up -q

### Now sync the "trunk" with the *second* branch:

>svn merge ^^/A branch-2
--- Merging r2 through r7 into 'branch-2':
A branch-2\D-MOVED
A branch-2\D-MOVED\gamma
A branch-2\D-MOVED\G
A branch-2\D-MOVED\G\pi
A branch-2\D-MOVED\G\rho
A branch-2\D-MOVED\G\tau
A branch-2\D-MOVED\H
A branch-2\D-MOVED\H\chi
A branch-2\D-MOVED\H\omega
A branch-2\D-MOVED\H\psi
D branch-2\D
--- Recording mergeinfo for merge of r2 through r7 into 'branch-2':
 U branch-2

### Note that with your issue #3405 fixes we no longer record any mergeinfo
### describing the merge on the two added subtrees with explicit mergeinfo:

>svn st
 M branch-2
D branch-2\D
D branch-2\D\G
D branch-2\D\G\pi
D branch-2\D\G\rho
D branch-2\D\G\tau
D branch-2\D\H
D branch-2\D\H\chi
D branch-2\D\H\omega
D branch-2\D\H\psi
D branch-2\D\gamma
A + branch-2\D-MOVED

>svn pl -vR branch-2
Properties on 'branch-2':
  svn:mergeinfo
    /A:2-7
Properties on 'branch-2\D-MOVED\G\pi':
  svn:mergeinfo
    /branch-1/D-MOVED/G/pi:4-6
Properties on 'branch-2\D-MOVED\H':
  svn:mergeinfo
    /branch-1/D-MOVED/H:4-6

>svn ci -m "" -q

### The previous behavior did describe the merge on the directory subtree
### (the fact that it doesn't on the file is another bug attributable to the
### inconsistency between files and dirs which you pointed out above):

old>svn pl -vR branch-2
Properties on 'branch-2':
  svn:mergeinfo
    /A:2-7
Properties on 'branch-2\D-MOVED\G\pi':
  svn:mergeinfo
    /branch-1/D-MOVED/G/pi:4-6 <-- Nothing for the file! That's an old
bug (or at least pre-dates Bert's change)
Properties on 'branch-2\D-MOVED\H':
  svn:mergeinfo
    /A/D-MOVED/H:3-7
    /branch-1/D-MOVED/H:4-6

### Ok, returning to the new behavior, if we repeat the merge, the
mergeinfo indicates that r7
### hasn't been merged to the two subtrees (which we know from above
it has been) and tries
### to remerge that revision. That's a big no-no for mergetracking
since one of it's primary
### purposes in life is to avoid repeat merges (and the spurious
conflicts that go with them):

>svn merge ^^/A branch-2
--- Merging r7 into 'branch-2\D-MOVED\G\pi':
 G branch-2\D-MOVED\G\pi
--- Merging r7 into 'branch-2\D-MOVED\H':
 G branch-2\D-MOVED\H
--- Recording mergeinfo for merge of r2 through r8 into 'branch-2':
 U branch-2
--- Recording mergeinfo for merge of r2 through r8 into 'branch-2\D-MOVED\G\pi':
 U branch-2\D-MOVED\G\pi
--- Recording mergeinfo for merge of r2 through r8 into 'branch-2\D-MOVED\H':
 U branch-2\D-MOVED\H

-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba