On Wed, Jan 18, 2012 at 8:45 AM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> Hi Paul.
>
> Paul Burba wrote:
>
>> Julian Foad wrote:
>>> I (Julian Foad) wrote:
>> I think I understand now. Your patch works fine when the only cyclic
>> merges are done via reintegrate (i.e. r24).
>
> [...]
>
>> I thought you were going to tackle more complex reflective merge
>> cases, such as this example:
>
>
> Yes, exactly!
>
>> A_at_1---r4---r5---r6---r7----------------r11----------->
>> |\ ^ |
>> | \ | |
>> | \ reintegrate |
>> | V | |
>> | A_COPY_2-----------------r9---r10--- |
>> | ^ sync merge
>> | / |
>> | cherry-pick merge of r8 |
>> V / V
>> A_COPY-------------------r8------------------------->
>
>
> Thanks for this good example of a more complex use case. I've included it in <http://wiki.apache.org/subversion/KeepingReintegratedBranchAlive> and written up a discussion of the options there.
>
> What do we want in this case? The options are basically:
>
> (1) try to merge (as we do now), despite knowing it will conflict;
> (2) skip r11 (not usually good in this kind of case);
> (3) figure out what really "should" be merged and merge it;
>
> (4) stop and tell the user what's up.
>
> I'm not aiming for (3) because I think that's much more complex than it sounds.
Actually it *sounds* pretty complex ;-) Seriously, if you solve #3
then I think you will also have solved issue #2897.
> With any other option, I believe it is essential to tell the user why Subversion can't simply include or exclude r11, with details.
>
>
> Any of these options could be best for the user. It is not at all the case that (1) is best. It depends how big the changes in r8/r9 are compared with those in r10, and how much they conflict, and how the user prefers to resolve the situation.
>
> I'm aiming for:
>
> * Detect whether we have the simple case (the rev is effectively already merged) or the complex case (partially merged).
> * Simple case: Do The Right Thing which is skip it but record it as merged.
>
> * Complex case: let user choose (1) or (2) or (4), in advance or interactively.
+1
> * Complex case: always issue a diagnostic message that clearly explains what's up.
>
>> As your patch stands, the above use-case causes some serious problems.
>
> Sure, my last patch only detects a Go/No-go and chooses No-go because that's sufficient for the simple use case. But now we can get to the interesting bit: how to detect the "Partially merged" situation.
Sorry, I was initially unclear on how far you intended to take this.
I'm with you now.
> The principle that we are grasping as humans but haven't yet codified formally is that we want the merge to know whether all the "original changes" have been made already (directly or via any series of merges) in the target branch.
>
> I describe the idea of "logical changes" in <http://wiki.apache.org/subversion/MergeTrackingIdeas>.
Most of the ideas there make sense, though I'm a bit foggy as to how
the mergeinfo format will change and how we'll migrate existing
repositories.
> Briefly: When we make a new change in A_at_10 and merge that into B as B_at_12, we get two physical changes in the repository. B_at_12 is a physical manifestation of the same logical change as A_at_10, but it looks a bit different because it is adjusted (automatically and/or manually) to fit the physical form and logical requirements of branch B. I say that there is one "logical change" there, which we refer to by the co-ordinates where it originated, A_at_10.
>
> Whenever we ask Subversion to track merges, although the current implementation just tracks physical changes from the immediate source branch, I believe that what we (as users) really expect is for it to track logical changes. The "partial merge" use case above is an example of where we have to make a distinction and explicitly ask whether all the required logical changes are present.
>
> ALGORITHM
>
> Purpose: To determine whether all logical changes comprising a given physical change (e.g. A_at_11) are already present in the target.
>
> Step 1: build a tree of merged revisions (i.e. mergeinfo changes), recursing until we reach "logical changes".
I know you are still early in the proof-of-concept phase, but have you
given much thought to performance? In particular I'm thinking of how
mergeinfo_graph_populate recursively scans the history of reflective
merges to build the mergeinfo graph. In doing so it ultimately calls
svn_client__get_repos_mergeinfo twice for each revision (operative or
not) described in the reflected mergeinfo. That could get slow in a
hurry.
> Step 2: traverse the tree of merged revisions, seeing whether all of the logical changes that comprise A_at_11 are recorded in the mergeinfo of the target (A_COPY).
>
> While doing this we need to ignore no-op revisions because we don't care whether they are recorded in the target or not.
>
> I've written code for step 1 (the attached 'merge-avoid-reflective-5.patch' is my current state) and am starting on step 2.
I look forward to see what you come up with for step 2.
Paul
>> (The attached patch is an update of yours with a new
>
>> merge_reintegrate.py test #20, which demonstrates the above use case)
>
> Thanks; that's very useful.
>
>
> - Julian
Received on 2012-01-18 19:35:54 CET