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:07 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>
>> -----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.

Don't become fixated on the mention of shallow WCs. They are not the
problem here.

Please look at both Bob's reproduction script and the test I added for
issue #4367 (merge_tests.py 139: repeat merge to infinite depth WC
conflicts) because there is *no* problem with the merge to the shallow
WC! His problem is a merge to an infinite depth working copy (which
succeeds but cannot be committed). Similarly, with merge test #139,
the merge to the shallow WC works as expected, it's the merge to an
infinite depth WC that fails.

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