Re: Merge policies

On Fri, Apr 20, 2012 at 5:07 PM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Fri, Apr 20, 2012 at 6:17 AM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
>> Paul Burba wrote:
>>
>>> On Thu, Apr 19, 2012 at 12:57 PM, Julian Foad wrote:
>>>>  Branko Čibej wrote:
>>>>>  By the way, I'm all for removing support for merging into
>>>>> mixed-revision
>>>>>  and/or switched-subtree working copies. There's too much room for
>>>>>  unexpected results in these cases.
>>>>
>>>>  I don't necessarily disagree, but that sounds a bit unsubstantiated.
>>>> Is there too much "room for unexpected results" because you don't  have
>>>> a mental model of what to expect?  If I were to figure out and describe
>>>> some sensible semantics and a use case, might you then change your mind?
>>>>
>>>>  FWIW, I have an inkling of how a mixed-rev target WC should work: it's
>>>> logically very similar to having different mergeinfo on different subtrees,
>>>> except that the differences are on the target side of the merge rather than
>>>> the  source side of the merge.  It requires some care to be sure that the
>>>> parent-to-child inheritance of mergeinfo in the WC remains valid where base
>>>> revision numbers differ.
>>>>
>>>>  That said, I can't imagine any valid reason to need to support a
>>>> mixed-rev target WC.
>>>>
>>>>  I have thought very little about about how switched subtrees should
>>>> work [and use cases].
>>>>
>>>>  Maybe Paul can help fill in some of the "how" and "why" gaps here?
>>>
>>> I believe I answered this question already, see:
>>> http://svn.haxx.se/dev/archive-2012-03/0632.shtml%c2  If that doesn't
>>> sufficiently answer your questions let me know.
>>
>> Hi Paul.  Yes, what you wrote there makes sense as to how we handle switched subtrees and "why" in a logical sense.  I've just followed up with a reply to that email going into a bit more detail.
>
> I replied on that thread.
>
>> I feel that with this level of functional spec, Switched Subtrees is something I can now add to the Symmetric Merge design proposal.
>>
>> I see there's already a brief statement about sparse WCs too: <http://svn.apache.org/repos/asf/subversion/trunk/notes/merge-tracking/func-spec.html#sparse-checkouts>.  I'm not clear how the whole 'depth' thing works, though, if you add children to a directory that was initially sparse or exclude children from a directory that was initially depth non-empty.
>
> Ah, welcome to my world!  Yes, this is a tricky situation.  If our
> merge target is shallow (i.e. depth < infinity) and the merge applies
> changes that would effect paths *deeper* than the immediate children
> currently present, then those children are skipped and we set
> non-inheritable mergeinfo to reflect that these changes were not
> merged.  Where it gets whacky is if the merge *adds* paths which are
> immediate children of the children currently present (e.g. merge
> target at depth empty, merge adds a immediate file to the merge
> target).  Today we add the file, despite the fact that the target's
> depth is empty.
>
> Before you claw your eyes out in frustration, read the issue I just
> filed: http://subversion.tigris.org/issues/show_bug.cgi?id=4164  It
> spells out the problem fairly clearly (I hope!).
>
> Paul
>
>> Do you have a pointer to some details about how a mixed-rev target WC is handled?

(Julian, apologies if most of this is old-hat for you, I'm writing as
much for the newly involved folks as I am for you)

At the most basic level, mixed-revision WCs impact how mergeinfo is
(or isn't) inherited[1]. The best place to start is looking at the
doc string for svn_client__get_wc_mergeinfo. Essentially, within the
working copy, a CHILD path without explicit mergeinfo may inherit[2]
mergeinfo from its working copy PARENT if:

PARENT_CHANGED_REV <= CHILD_BASE_REV <= PARENT_BASE_REV

If the above condition doesn't hold then we need to contact the
repository for CHILD's inherited mergeinfo @BASE_REV.

~~~~~

Ok, so what? How does this impact merge-tracking aware merges?

1) When the root of a merge target has no explicit mergeinfo. We need
to know the target's "merge history" before we can decide what to
merge, and this "merge history" is effectively made up of the target's
inherited or explicit mergeinfo and its implicit mergeinfo (a.k.a. its
natural history). If we can find the target's inherited mergeinfo
from it's WC parents we do so, otherwise we contact the repository.
That's pretty straightforward.

2) Any time a merge has cause to set explicit mergeinfo on a path that
previously had no explicit mergeinfo, we need to find the path's
inherited mergeinfo (if any) and combine that with the new mergeinfo
we want to record (otherwise merge history can be lost). There are
several different cases in which we do this, for example during a
shallow merge where we need to record non-inheritable mergeinfo on the
roots of subtrees into which the merge did not extend.

3) The third impact is probably the most important and is where
inherited mergeinfo is *not* considered: Mixed revision merge targets
where subtrees without explicit mergeinfo appear to be missing merge
history that is actually present. A simple example will explain what
I mean:

### Given the WC for a branch at a uniform revision, with no missing
subtrees, and no
### explicit mergeinfo:

>svn up
Updating '.':
At revision 7.

>svn st

>svn pl -vR

### Cherry pick a single revision and commit that merge:

>svn merge ^^/A . -c4
--- Merging r4 into '.':
U D\H\psi
--- Recording mergeinfo for merge of r4 into '.':
 U .

>svn ci -m "cherry pick r4 from ^/A"
Sending .
Sending D\H\psi
Transmitting file data .
Committed revision 8.

### We obviously have a mixed-rev WC:

>svn st -v
                 8 8 pburba .
                 7 1 jrandom mu
                 7 1 jrandom B
                 7 1 jrandom B\lambda
                 7 1 jrandom B\E
                 7 1 jrandom B\E\alpha
                 7 1 jrandom B\E\beta
                 7 1 jrandom B\F
                 7 1 jrandom C
                 7 1 jrandom D
                 7 1 jrandom D\gamma
                 7 1 jrandom D\G
                 7 1 jrandom D\G\rho
                 7 1 jrandom D\G\pi
                 7 1 jrandom D\G\tau
                 7 1 jrandom D\H
                 7 1 jrandom D\H\omega
                 8 8 pburba D\H\psi
                 7 1 jrandom D\H\chi

### Now make a change to the file which was modified by the cherry pick:

>echo branch edit > D\H\psi

>svn ci -m ""
Sending D\H\psi
Transmitting file data .
Committed revision 9.

### Still have that mixed-rev WC:

>svn st -v
                 8 8 pburba .
                 7 1 jrandom mu
                 7 1 jrandom B
                 7 1 jrandom B\lambda
                 7 1 jrandom B\E
                 7 1 jrandom B\E\alpha
                 7 1 jrandom B\E\beta
                 7 1 jrandom B\F
                 7 1 jrandom C
                 7 1 jrandom D
                 7 1 jrandom D\gamma
                 7 1 jrandom D\G
                 7 1 jrandom D\G\rho
                 7 1 jrandom D\G\pi
                 7 1 jrandom D\G\tau
                 7 1 jrandom D\H
                 9 9 pburba D\H\psi
                 7 1 jrandom D\H\chi
                 7 1 jrandom D\H\omega

### A repeat of the cherry pick to the root of our branch (where the explicit
### mergeinfo is found) is a no-op:

>svn merge ^^/A . -c4 --allow-mixed-revisions
--- Recording mergeinfo for merge of r4 into '.':
 U .

>svn st

### But if we perform a subtree merge to 'D', the target doesn't inherit the
### explicit mergeinfo on '.' because of the mixed-rev inheritance rules, so the
### repository is contacted to see what mergeinfo D_at_7 inherits (which is none).
### The fact that a subtree of the target, D/H/psi *does* inherit mergeinfo
### reflecting the previous cherry-pick is not considered, so the merge is
### repeated, producing a spurious text conflict:

>svn merge ^^/A/D D -c4 --allow-mixed-revisions
Conflict discovered in
'C:/SVN/src-branch-1.7.x/Debug/subversion/tests/cmdline/svn-test-work/working_copies/merge_tests-125/A_COPY/D/H/psi'.
Select: (p) postpone, (df) diff-full, (e) edit,
        (mc) mine-conflict, (tc) theirs-conflict,
        (s) show all options: p
--- Merging r4 into 'D':
C D\H\psi
--- Recording mergeinfo for merge of r4 into 'D':
 U D
Summary of conflicts:
  Text conflicts: 1

Situations similar to this are what lead us to disallow merges to
mixed-rev working copies by default (i.e. adoption of the
--allow-mixed-revisions option
http://svn.haxx.se/dev/archive-2010-10/0000.shtml).

Certainly it would be possible to detect cases like this, but doing so
in a performant manner is another question.

Paul

[1] Anything that affects inheritance obviously also effects elision.
I.e. is the explicit mergeinfo on CHILD identical to the mergeinfo
CHILD would inherit from PARENT if CHILD had not explicit mergeinfo?
If so then the explicit mergeinfo on CHILD is redundant and can be
elided/removed.

[2] This is an oversimplification, really what we are talking about is
the whether the child may inherit across this mixed-revision boundary,
even if the inherited mergeinfo in question doesn't explicitly exist
on the parent.

Grand-parent (explicit mergeinfo)
  |
inheritance
allowed?
 |
 V
parent (no explicit mergeinfo)
  |
inheritance
allowed?
 |
 V
child (no explicit mergeinfo)

>> I have now added a section "Mixed-Rev, Switched, or Sparse WC" to <http://wiki.apache.org/subversion/SymmetricMerge>.
>>
>>
>>> As to removing support for merging into WCs with switched subtrees,
>>> let's be clear that this *does* work today (if you think otherwise
>>> please provide a use-case demonstrating what is broken).  Now maybe we
>>> want to restrict these types of merges to make further improvements
>>> possible (e.g. Julian's symmetric merge work), that I won't argue
>>> against, but I want to be clear that we are making the choice to
>>> remove some existing functionality as a trade-off in implementing new
>>> functionality, rather than fixing a broken feature.
>>
>> Totally clear, yes, if that's what we decide to do.
>>
>> - Julian
Received on 2012-04-23 18:54:45 CEST