Re: Sparse working copy in 1.8.0 causes problems for others' merges; 1.7.x didn't complain

On Thu, May 16, 2013 at 11:14 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>> -----Original Message-----
>> From: Bert Huijben [mailto:bert_at_qqmail.nl]
>> Sent: donderdag 16 mei 2013 17:07
>> To: 'Paul Burba'
>> Cc: 'Bob Cardillo'; 'dev'
>> Subject: RE: Sparse working copy in 1.8.0 causes problems for others'
>> merges; 1.7.x didn't complain
>>
>>
>>
>> > -----Original Message-----
>> > From: Paul Burba [mailto:ptburba_at_gmail.com]
>> > Sent: donderdag 16 mei 2013 16:10
>> > To: Bert Huijben
>> > Cc: Bob Cardillo; dev
>> > Subject: Re: Sparse working copy in 1.8.0 causes problems for others'
>> > merges; 1.7.x didn't complain
>> >
>> > On Thu, May 16, 2013 at 3:59 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>> > >
>> > >> -----Original Message-----
>> > >> From: Paul Burba [mailto:ptburba_at_gmail.com]
>> > >> Sent: donderdag 16 mei 2013 01:47
>> > >> To: Bob Cardillo
>> > >> Cc: dev
>> > >> Subject: Re: Sparse working copy in 1.8.0 causes problems for others'
>> > >> merges; 1.7.x didn't complain
>> > >>
>> > >> On Wed, May 15, 2013 at 1:06 PM, Paul Burba <ptburba_at_gmail.com>
>> > wrote:
>> > >> > On Mon, May 13, 2013 at 9:58 PM, Bob Cardillo
>> > <bob.cardillo_at_gmail.com>
>> > >> wrote:
>> > >> >> I'm running Subversion 1.8.0-dev on Windows 7 Pro SP1.
>> > >> >>
>> > >> >> The following steps went through without error on 1.7.x, but they fail
>> > with
>> > >> >> an error on the last step when run on 1.8.0 (see below for full
>> > >> reproducible
>> > >> >> recipe):
>> > >> >
>> > >> > Hi Bob,
>> > >> >
>> > >> > Thanks for the detailed script. Comments follow inline.
>> > >> >
>> > >> >> 1. make a copy (branch) of your trunk
>> > >> >> 2. Harry checks out the branch in full
>> > >> >> 3. Sally sparsely checks out the branch with just a subset of subtrees
>> > >> >> 4. someone adds something in trunk under one of the subtrees that
>> > Sally
>> > >> has
>> > >> >> excluded
>> > >> >> 5. someone removes something from trunk under the subtree
>> added
>> > in
>> > >> step 4
>> > >> >> 6. Sally merges trunk into the branch (remember she has the sparse
>> > >> working
>> > >> >> copy)
>> > >> >> 7. Harry merges trunk into the branch
>> > >> >> BAM! Harry can't commit the merge because:
>> > >> >> svn: E155011: Commit failed (details follow):
>> > >> >> svn: E155011: Directory 'C:\testbranch1_userX\B\B1\B1a' is out of
>> date
>> > >> >> svn: E160028: '/branches/branch1/B/B1/B1a' is out of date
>> > >> >>
>> > >> >> I suspect this has something to do with one or both of these two
>> > issues,
>> > >> >> completed in 1.8.0:
>> > >> >> - http://subversion.tigris.org/issues/show_bug.cgi?id=4305
>> > >> >> - http://subversion.tigris.org/issues/show_bug.cgi?id=4169
>> > >> >>
>> > >> >> Can someone confirm?
>> > >> >>
>> > >> >> Is this a new bug introduced in 1.8.0 or a correction of an oversight in
>> > >> >> 1.7.x? Either way, what is the workaround? It seems to me that a
>> > merge
>> > >> into
>> > >> >> a sparse working copy either shouldn't be allowed,
>> > >> >
>> > >> > Merges into sparse working copies, while not encouraged, should
>> work.
>> > >> > It's one of the reasons we have non-inheritable mergeinfo. Here's a
>> > >> > brief explanation before we dive into the bug in the final merge.
>> > >> >
>> > >> > Right before r6, Sally has this shallow WC:
>> > >> >
>> > >> > C:\SVN\sandbox\shallow-merge-targets-1.7\testbranch1_userY>svn
>> st
>> > -v
>> > >> > 2 2 pburba . <--Depth=empty
>> > >> > 2 1 pburba C <-- Depth=infinity
>> > >> >
>> > >> > The merge adds some new subtrees in the children that are present
>> > >> > (i.e. C). Changes that affect the missing subtree 'B' are skipped:
>> > >> >
>> > >> > C:\SVN\sandbox\shallow-merge-targets-1.7\testbranch1_userY>svn
>> > >> merge ^^/trunk
>> > >> > Skipped 'B'
>> > >> > --- Merging r2 through r5 into 'C':
>> > >> > A C\C1
>> > >> > A C\C1\test.txt
>> > >> > --- Recording mergeinfo for merge of r2 through r5 into '.':
>> > >> > U .
>> > >> > --- Recording mergeinfo for merge of r2 through r5 into 'C':
>> > >> > U C
>> > >> > Summary of conflicts:
>> > >> > Skipped paths: 1
>> > >> >
>> > >> > Mergeinfo describing the merge is as expected. Non-inheritable
>> > >> > mergeinfo is set on the target, which indicates that any missing
>> > >> > children (e.g. 'B') didn't get those changes. 'C' get's it's own full
>> > >> > set of (normal inheritable) mergeinfo describing the change.
>> > >> >
>> > >> > C:\SVN\sandbox\shallow-merge-targets-1.7\testbranch1_userY>svn
>> pl
>> > -vR
>> > >> > Properties on '.':
>> > >> > svn:mergeinfo
>> > >> > /trunk:2-5*
>> > >> > Properties on 'C':
>> > >> > svn:mergeinfo
>> > >> > /trunk/C:2-5
>> > >> >
>> > >> > This is all expected and by design. Do note that with 1.8, this
>> > >> > missing paths affected under 'B' do get a special conflict
>> > >> > notification:
>> > >> >
>> > >> > C:\SVN\sandbox\shallow-merge-targets\testbranch1_userY>svn
>> merge
>> > >> ^^/trunk
>> > >> > --- Merging r2 through r5 into '.':
>> > >> > Skipped missing target: 'B'
>> > >> > A B\B1\test.txt <- A'dd' in the fourth column let's users know
>> > >> > what was skipped under a missing subtree.
>> > >> > A B\B1 <-- Ditto
>> > >> > A C\C1
>> > >> > A C\C1\test.txt
>> > >> > --- Recording mergeinfo for merge of r2 through r5 into '.':
>> > >> > U .
>> > >> > --- Recording mergeinfo for merge of r2 through r5 into 'C':
>> > >> > U C
>> > >> > Summary of conflicts:
>> > >> > Skipped paths: 1
>> > >> >
>> > >> > C:\SVN\sandbox\shallow-merge-targets\testbranch1_userY>svn st
>> > >> > M .
>> > >> > M C
>> > >> > A + C\C1
>> > >> >
>> > >> > C:\SVN\sandbox\shallow-merge-targets\testbranch1_userY>svn pl -
>> vR
>> > >> > Properties on '.':
>> > >> > svn:mergeinfo
>> > >> > /trunk:2-5*
>> > >> > Properties on 'C':
>> > >> > svn:mergeinfo
>> > >> > /trunk/C:2-5
>> > >> >
>> > >> >> or it should work
>> > >> >> correctly. In other words, this recipe should either fail on step 6
>> above
>> > >> >> (instead of 7) or it should go all the way through correctly, including
>> > step
>> > >> >> 7.
>> > >> >
>> > >> > So yes, that final merge by Harry into a infinite depth WC should work
>> > >> > like it does in 1.7. I'll try to figure out what is wrong here.
>> > >>
>> > >> Haven't been able to figure out what the problem is. I filed an issue
>> > >> to track this: http://subversion.tigris.org/issues/show_bug.cgi?id=4367
>> > >
>> > > I haven't looked at the specific details yet, but a specific behavior change
>> > between <= 1.7 and 1.8 is that Subversion 1.8 now compares the incoming
>> > delete with the local version of all nodes. (A TODO item since Subversion
>> 1.6)
>> >
>> > Hi Bert,
>> >
>> > Is there an issue associated with this? Or could you just point me to
>> > a particular revision(s)?
>> >
>> > > If the incoming delete describes the delete of something else that what is
>> > available locally then the node is marked as tree conflicted.
>> > >
>> > > One very common case that this helps for is when nodes are moved. If a
>> > directory tree was changed before it was moved, then Subversion <= 1.7
>> > would just delete the old tree and insert a copy of whatever is merged in. -
>> ->
>> > You would lose track of the changes in the original location.
>> > >
>> > > I don't think this comparison code was really tested with sparse trees in
>> > mind. (We usually don't write detailed tests of cases we don't support)
>> >
>> > I'm confused...we don't support sparse working copies anymore? Also,
>> > to be clear, the problem doesn't appear to be with merges to shallow
>> > WCs. That seems to work as it did in 1.7. The problem occurs when
>> > merging to a uniform revision WC at infinite depth.
>>
>> I think we still do, but how can we determine if an incoming 'delete <this-
>> exact-tree>' should apply to a node that is not there?
>>
>> In 1.6 and 1.7 we just deleted every directory tree, without verifying
>> anything (and for files we did a limited check). This was a MAJOR todo left in
>> our tree conflict implementation.
>>
>> Even in a shallow working copy scenario, the delete of exactly what is not
>> there is an unlikely operation: usually you would merges that apply to your
>> tree.. Not merges that delete exactly what is not in your tree.
>>
>> So if you commit you delete everything, and the mergetracking won't help
>> you in any way as we can't store mergeinfo on a deleted node.
>>
>>
>> I would say that noting that you do something dangerous like this would be a
>> good reason to give a tree conflict. The user can then decide: "yes this is
>> what I intended to do", or "no, I shouldn't have applied this merge".
>>
>> Maybe there is some middle ground where a few levels of directories apply
>> to your working copy, and some deeper subdirectory is sparse. In this case I
>> can see why you would want to apply the delete without further
>> verifications... But in other cases I don't think you want that to be the default
>> behavior.
>
> I think the primary issue is
> http://subversion.tigris.org/issues/show_bug.cgi?id=3210
> Issue#3210 "Merge should raise a tree-conflict when deleting a modified"

I traced the test failure in the test I added (merge_tests.py 139:
repeat merge to infinite depth WC conflicts) to r1443112, which were
fixes for issue #2282 and issue #3150. Bob's scenario however, still
fails, so my initial suspicion that
http://subversion.tigris.org/issues/show_bug.cgi?id=4367#desc2
described the same problem he saw was incorrect. There are two
different problems.

> But it goes back to the design of tree conflicts: when to raise delete tree conflicts.
> (use case 5)
>
> The design says that we should raise a tree conflict when what is to be deleted is not an exact match to what exists locally.
>
> Bert
>

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