Hi Paul.
Thanks for looking at this and the other ('deleted subtrees') thread. Replies in line below...
Paul Burba wrote:
> Julian Foad wrote:
>> There are some merge scenarios for which it's not clear whether the
>> user should specify '--reintegrate' or not. We need to decide what the
>> 'symmetric' (i.e. automatically-choosing) code should do in those cases.
>>
>> The following example is adapted from merge_symmetric_tests.py 18,
>> subtree_to_and_fro(), "reintegrate considers source subtree
>> mergeinfo":
>>
>> reintegrate 'everything'
>> |
>> A ------o-s--------------x
>> \ \ /
>> \ \ /
>> B o-----s----s----s---
>> |
>> merge the subtree 'D' only
>
> What does 's' represent in this diagram?
Something happening to the subtree.
> In merge_symmetric_tests.py's key 's' represents 'a subtree
> merge', but that's not the case here, AFAICT.
Yup, sorry for the unclarity.
>> In terms of commands (assume a commit after each step):
>>
>> svn cp A B
>> edit A
>> edit A/D
>> edit B/D svn merge ^/A/D B/D
>> edit B/D
>> svn merge --reintegrate ^/B A
>>
>> The output from "merge --reintegrate" is:
>>
>> svn: E195016: Reintegrate can only be used if
>> revisions 2 through 8 were previously merged [...]
>>
>> But why did the user choose a 'reintegrate' merge in this example,
>> when there has been no merge of the branch root in the A->B
>> direction?
>
> No, but we could easily have had such a merge,
>
> reintegrate 'everything'
> |
> A -----o----o------------x
> \ \ \ /
> \ \ \ /
> B o----x-----s----s---
> | |
> | merge the subtree 'D' only
> |
> merge 'everything'
>
> and still be facing the same problem.
I don't think so, because now it is clearly a 'reintegrate' that is needed. I say 'clearly' because now the root path of the merge has mergeinfo on one side, showing that a merge was done in one direction on this *same root path* that we're now wanting to merge. The rule for using "reintegrate", as far as I am aware, is that you use it when merging in the opposite direction to "the previous merge". No matter whether "the previous one" means the most recent merge in the tree or the most recent merge of the given root path, in this scenario those are both in the direction A->B, and so both the user and the current symmetric merge code know what to do. (I should check that it currently does.)
On the other hand, in the test scenario, if "the previous one" means the most recent merge of the given root path (which is what I assumed, but who's to say?), then it's ambiguous because there was no such merge in either direction.
>> Or, from another angle, what if we have this similar scenario?
>>
>> merge 'everything'
>> |
>> A o-o-s--------------x
>> / \ /
>> / \ /
>> B ----------s----s----s---
>>
>> Given this example, it looks like a non-reintegrate merge might be
>> the user's choice for the final merge (and for the intermediate
>> merge... either a reintegrate or non-reintegrate); but all that's
>> changed in this example is which branch was branched from which, and
>> that shouldn't make any difference. AFAIK we don't document a rule
>> for whether '--reintegrate' should be used; it's ambiguous.
>>
>> We have three possible results when we request a merge:
>>
>> (1) a correct merge
>> (2) a wrong merge, with spurious conflicts
>> (3) bail out
>>
>> Currently, 'merge' gives (2), 'merge --reintegrate' gives
>> (3), and 'merge --symmetric' gives (2).
>>
>> The subtree_to_and_fro() test expects 'merge --symmetric' to give
>> result (3), but the symmetric merge currently sees that case as one
>> in which a *non-reintegrate* merge is needed, and goes ahead to
>> produce a merged result (with a conflict):
>>
>> --- Merging r2 through r8 into 'A':
>> U A/D/gamma
>> C A/D/H/psi
>> [...]
>>
>> Is the symmetric merge 'broken'? This test claims so, on the basis
>> of expecting it to behave like a reintegrate merge. However, we can't
>> be strictly backward-compatible with both the reintegrate and the
>> non-reintegrate behaviours if we have to pick one, because they differ.
>> What do we want to see here?
>>
>> I think it depends what the user is thinking. If the user is thinking
>> "just help me merge everything that needs to be merged", then the user
>> probably would have used the plain 'merge' command in 1.7, and would
>> prefer a wrong merge with spurious conflicts, given that we haven't yet
>> implemented a correct merge. If the user is thinking "this is a
>> reintegration, and I believe my branch was sync'd with the trunk
>> recently", then the user probably would prefer this merge to bail out
>> like 'reintegrate' does.
>>
>> It almost goes without saying that if and when we have implemented a
>> correct merge then that is the right thing to do, and that will resolve
>> the ambiguity. (When we do that, there is still the possibility of
>> giving the user an option to make Subversion detect and report whether
>> the required merge has anything complex about it -- subtrees,
>> cherry-picked revisions, and so on -- because the user might want the
>> mental security of confirming that in fact the merge is as simple as
>> the user expects it to be. But that is another topic.)
>>
>> Our options for what 'symmetric' merge will do, in cases like this
>> [1], are:
>>
>> (1) Implement correct merging.
>> (2) Merge wrongly, with spurious conflicts, like plain 1.7 'merge'.
>> (2a) (2) by default, with an option to check and bail out like (3).
>> (3) Bail out like 1.7 'reintegrate'.
>> (3a) (3) by default, with an option to force the merge to proceed
>> like (2).
>>
>> If we choose (2), then we break strict compatibility with 1.7
>> 'reintegrate' merge. If we choose (3), then we break strict
>> compatibility with 1.7 'non-reintegrate' merge.
>>
>> I'd like to do (1), but, in the meantime, would we be happy with one of
>> the alternatives?
>
> #1 would be awesome :-) but in the meantime...
>
> I take your points about user intent but if the symmetric merge code
> is meant to do away with the --reintegrate option, I think we should
> take the more cautious approach and assume the user intends a
> reintegrate, so IMO (3a) is the best choice.
OK, yes. If I can try to interpret your meaning without saying "reintegrate", it would be: we should take the more cautious approach which is to bail out because of the inconsistent subtree state. The rule would be, for a request to merge in the direction A->B, roughly speaking:
* if every node in the tree was last merged A->B or not at all -> merge in the non-reintegrate style
* if every node in the tree was last merged B->A, up to the same
B revision
-> merge in the reintegrate style
* otherwise
-> bail out and report the subtree inconsistency
I'll try to formulate that rule more precisely. In particular, that rough statement doesn't address the difference between 'complete' merges (which don't leave unmerged gaps before any merged revs) and 'cherry-pick' merges (which do).
- Julian
Received on 2012-08-02 16:46:34 CEST