On Fri, Apr 16, 2010 at 11:45 AM, C. Michael Pilato <cmpil...@collab.net> wrote:
> Stefan Sperling wrote:
>> On Fri, Apr 16, 2010 at 09:45:16AM -0400, Paul Burba wrote:
>>> I just double checked this test, running with both a debug and release
>>> build, both pass.
>>>
>>> You probably already noticed, but the dry-run and full-merge outputs
>>> are semantically equivalent. The only differences are the ordering of
>>> the notifications and that the
>>>
>>> "--- Merging r5 through r9 into
>>> 'svn-test-work/working_copies/merge_tests-63.files':"
>>>
>>> notification appears twice. This strikes me more as a limitation in
>>> our test suite, rather than a problem with Subversion.
>>
>> Yes, it does not seem to be a serious problem.
>> It's not release blocker.
>> I do not compile serf support into the subversion binaries for OpenBSD,
>> so in any case this test failure won't affect svn users on this platform.
>>
>> However, it's interesting that the test fails reliably for me with
>> fsfs and serf, and passes reliably with bdb and serf.
>> That's kind of odd. Maybe the problem is related to the fact that
>> I build APR and Subversion without support for threads? I guess ordering
>> of notifications with serf is timing dependent? In what way do fsfs
>> and bdb differ in this respect? Should we try to hunt down the cause of
>> this or just dismiss it as noise and see if it pops up again later?
>
> Actually, I'm really surprised by this result, and can't think of a good
> explanation even with Serf's ordering ... characteristics.
>
> Paul, I thought the merge process always handled subtrees before their
> parents. And their two different editor drives altogether. Is that not the
> case?
Hi Mike (you'll want to be sitting down for this),
Not necessarily. Multiple editor drives are dictated by what
revisions the WC target, taken as a whole, need. The mere presence of
subtrees with explicit mergeinfo doesn't imply anything re the number
of editor drives. If the needed ranges are contiguous then there is
only one editor drive, if not, then there are multiple editor drives.
Hopefully the following examples will show what I mean (examples with
tr...@934970):
1) Discrete Ranges --> Multiple Editor Drives
We have a branch 'A_COPY' with some mergeinfo on the root and on a subtree:
>svn pl -vR A_COPY
Properties on 'A_COPY':
svn:mergeinfo
/A:5
Properties on 'A_COPY\D':
svn:mergeinfo
/A/D:3
The revisions eligible to be merged to A_COPY are not contiguous:
>svn mergeinfo --show-revs eligible -R ^^/A A_COPY
r4
r6
So when we do the merge there are two editor drives:
>svn merge ^^/A A_COPY
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
XX
XX EDITOR DRIVE
XX
XX Merge ^/a...@2 ^/a...@4
XX
XX A_COPY/D already has r3, so it only needs r4
XX No need for a separate editor drive, we just use
XX our svn_ra_reporter3_t->set_path callback to let
XX the repos know this subtree is already at r3
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
--- Merging r4 into 'A_COPY\D':
U A_COPY\D\G\rho
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
XX
XX What about r5?
XX
XX r5 would be inoperative if merged to A_COPY, as it
XX affects only A_COPY/B/E/beta and this was already
XX merged. This is one difference between trunk and
XX 1.6.x, as the former does away with inoperative
XX editor drives - see
XX http://svn.haxx.se/dev/archive-2009-08/0045.shtml
XX and r878767
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
XX
XX EDITOR DRIVE
XX
XX Merge ^/a...@5 ^/a...@7
XX
XX No such set_path trickery needed here, r6 is the
XX the only operative rev and it affects
XX A_COPY/D/H/omega, so effectively the "whole" tree
XX rooted at A_COPY needs it.
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
--- Merging r6 through r7 into 'A_COPY\D':
U A_COPY\D\H\omega
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
XX No more editor drives, the following is simply
XX merge tracking house-keeping.
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
--- Recording mergeinfo for merge of r2 through r7 into 'A_COPY':
U A_COPY
--- Recording mergeinfo for merge of r2 through r7 into 'A_COPY\D':
U A_COPY\D
--- Eliding mergeinfo from 'A_COPY\D':
U A_COPY\D
2) One Contiguous Range --> One Editor Drive
Again, we have a branch 'A_COPY' with some mergeinfo on the root and
on a subtree:
>svn pl -vR
Properties on 'A_COPY':
svn:mergeinfo
/A:4
Properties on 'A_COPY\D\H\psi':
svn:mergeinfo
/A/D/H/psi:2-6
But this time, the eligible ranges are contiguous:
>svn mergeinfo --show-revs eligible -R ^^/A A_COPY
r5*
r6*
(The -R and '*' are a new 1.7 feature of the svn mergeinfo subcommand.
Without -R, it would examine only the mergeinfo on the target A_COPY
and conclude that r3,r5,and r6 were needed. With -R it realizes that
r3 is already fully merged into the only subtree it affects
A_COPY/D/H/psi, and that only r5 and r6 are needed and further that
they are needed only by other subtrees - though in this case the
"other" subtree is actually the remainder of the tree rooted at A_COPY
less A_COPY\D\H\psi.)
>svn merge ^^/A A_COPY
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
XX
XX EDITOR DRIVE
XX
XX Merge ^/a...@4 ^/a...@7
XX
XX A_COPY/D/H/psi already has r4:7, so it only needs
XX r7. Again, no need for a separate editor drive, we
XX let the repos know this subtree is already at r6
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
--- Merging r5 through r7 into 'A_COPY':
U A_COPY\D\H\omega
U A_COPY\B\E\beta
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
XX Merge tracking house-keeping.
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
--- Recording mergeinfo for merge of r2 through r7 into 'A_COPY':
U A_COPY
SIDEBAR: I didn't show this, but there is an inoperative editor drive
of ^/a...@1 ^/a...@2 at the start of both these examples (r2 was the copy of
A to A_COPY). Apparently the subtree-mergeinfo branch work is not
picking up the fact that this is inoperative. I'll look into that.
> Is it possible that ra_serf is actually overlapping whole editor
> drives?
I don't think so, otherwise we'd see multiple notifications as the
merge was repeated.
One thing to keep in mind: Multiple notification headers don't
necessarily imply multiple editor drives.
Take this output for example:
>svn pl -vR
Properties on 'A_COPY\D':
svn:mergeinfo
/A/D:3
>svn mergeinfo --show-revs eligible -R ^^/A A_COPY
r4
r5
r6
r8
>svn merge ^^/A A_COPY
--- Merging r4 through r9 into 'A_COPY\D':
U A_COPY\D\gamma
U A_COPY\D\G\rho
U A_COPY\D\H\omega
--- Merging r3 through r9 into 'A_COPY':
U A_COPY\B\E\beta
--- Recording mergeinfo for merge of r2 through r9 into 'A_COPY':
U A_COPY
--- Recording mergeinfo for merge of r2 through r9 into 'A_COPY\D':
U A_COPY\D
--- Eliding mergeinfo from 'A_COPY\D':
U A_COPY\D
How many editor drives? (excluding the inoperative one of ^/a...@1 ^/a...@2
for the branch creation I mentioned in the SIDEBAR above)
There is only one editor drive, ^/a...@2 ^/a...@9
Since A_COPY/D already has r3 merged to it, when notifications for
paths under A_COPY/D are made, they get their own header starting at
r4. Changes made elsewhere get a notification header starting at r3.
The 'into' path is just the nearest ancestor path with mergeinfo to
the path being notified.
Let's look at what Stefan saw (note that when I debug this it is a
single editor drive of merge_tests-62/a...@4 merge_tests-62/a...@9...and I
see no reason to expect that things are any different for Stefan):
The full merge output:
--- Merging r5 through r9 into
'svn-test-work/working_copies/merge_tests-63.files/mu':
U svn-test-work/working_copies/merge_tests-63.files/mu
--- Merging r5 through r9 into
'svn-test-work/working_copies/merge_tests-63.files':
C svn-test-work/working_copies/merge_tests-63.files/D
C svn-test-work/working_copies/merge_tests-63.files/B
U svn-test-work/working_copies/merge_tests-63.files
Summary of conflicts:
Tree conflicts: 2
Ok, that looks fine (to me at any rate). But if you look at the test,
you might notice that merge_tests-63.files/mu doesn't actually have
any mergeinfo prior to the merge, nor does the root
merge_tests-63.files for that matter. What is going on here is an
unintended side effect of dealing with shallow merge targets. Since
merge_tests-63.files is at depth=files, is treated as if it does have
mergeinfo, since its immediate parent is missing its directory
children, once the merge is complete it will get non-inheritable
mergeinfo describing the merge. So merge_tests-63.files/mu must get
its own explicit mergeinfo. The CHILDREN_WITH_MERGEINFO array that
merge.c uses to keep track of all subtrees with mergeinfo just
includes merge_tests-63.files/mu right from the beginning, knowing
that it will end up with mergeinfo at the end of the merge...(well
that was the original intent, but in 1.7, since unaffected subtrees
won't have their mergeinfo updated, children of shallow parents won't
always have their mergeinfo updated. I'll fix this.
Anyhow, the test failed for Stefan because the --dry-run output was different:
The dry-run merge output:
--- Merging r5 through r9 into
'svn-test-work/working_copies/merge_tests-63.files':
C svn-test-work/working_copies/merge_tests-63.files/B
--- Merging r5 through r9 into
'svn-test-work/working_copies/merge_tests-63.files/mu':
U svn-test-work/working_copies/merge_tests-63.files/mu
--- Merging r5 through r9 into
'svn-test-work/working_copies/merge_tests-63.files':
C svn-test-work/working_copies/merge_tests-63.files/D
U svn-test-work/working_copies/merge_tests-63.files
Summary of conflicts:
Tree conflicts: 2
It's different yes, but merge_tests-63.files/mu,
merge_tests-63.files/B, and merge_tests-63.files/D are just notified
in a different order, there is nothing wrong with that that I can see,
they are all at the same depth.
Paul
> If so, that seems (to me) like a big-time problem in that layer. I
> still don't think it should block the release, but it's something I'd want
> to see investigated.
