Re: Implicit keep-alive after reintegrate merge

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@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.  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.   * 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. 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>.  Briefly:  When we make a new change in A@10 and merge that into B as B@12, we get two physical changes in the repository.  B@12 is a physical manifestation of the same logical change as A@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@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@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". Step 2: traverse the tree of merged revisions, seeing whether all of the logical changes that comprise A@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. > (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