Hi Paul. Thanks for your thoughts. Paul Burba wrote:
> Julian Foad wrote: >> We can say for sure that when we reintegrated B to A (in A:40), that >> will have added new mergeinfo on A describing merges from B. However, >> if change A:40 had instead been a different merge into A, let's say >> from C, it is still possible that merge might have brought along some >> new mergeinfo describing merges from B, because of the way mergeinfo >> is propagated from branch to branch. Therefore, if we find that >> change A:40 adds new mergeinfo about merges from B, we cannot simply >> say that A:40 describes a reintegrate merge from B. > > Absolutely! There's also the similar case where 'B' is reintegrated > to 'A', additional edits are made to 'A', and then the whole thing is > committed as r40. Now r40 represents both the reintegrate change and > unrelated edits -- but of course the smallest unit mergetracking works > with is a revision, so how does it classify r40? Actually, I don't think we need to be concerned about the "mixture of merges and new changes" scenario, and here's why. As we know, merging is not a deterministic operation, and requires user input in general [1]. When the user does a merge and commits the resulting working copy, the result is tracked as a merge. There's no conceptual way Subversion could distinguish the two kinds of change without the user telling it. The way to tell Subversion about two separate changes is by putting them in two separate revisions [2]. So, "Don't commit new changes with a merge" is a rule we have to embrace if we want to make progress in defining more useful merge behaviour. Or rather the rule is a bit softer: "You shouldn't commit new changes with a merge, because Subversion won't track the new changes separately but will assume they are part of the merge conflict resolution." In fact the only cases in which Subversion's behaviour will actually be affected by this rule are multi-path or cyclic merges, that is the cases where we want to avoid merging a change to a branch that already has "it". The only form of cyclic merge we've supported up till now has been the reintegrate merge, and that is a special case because it is limited to cases where it doesn't need to track individual changes, it simply compares at the entire states of the two branches. The effect is that it doesn't matter whether the user included new changes along with any merges into the source (child) branch. In a multi-path merge, such as the third step of A->B, A->C, B->C, we don't yet support automatically skipping any changes coming via B that have already been merged directly from A to C. Subversion will still try to merge that whole change from B to C, and the already-merged part of it will conflict (logically if not physically). The user's options are to avoid merging that specific revision from B to C (perhaps by record-only merging it), or to deal with that conflict. If a new change had been committed along with that A->B merge, neither option successfully merges that new change to C. So the reintegrate merge doesn't care if you have mixed a merge with a new change, and for the multi-path scenario the user already needs to keep merges separate from new changes. Now, what about the case I'm addressing? That is, the sync merge into a branch that has already been reintegrated. If the user wants to carry on using the branch in this way, up till now it has been the user's responsibility to tell Subversion to ignore the reintegrate merge commit, by the record-only merge known as the "keep-alive dance". If there had been a new change mixed up with that reintegration, the user would be telling Subversion to ignore it. So here again we already had the rule that you have to keep new changes separate. [1] Of course we have automatic text merge tools which give the user a good guess at a likely answer, which often turns out to need no further editing in simple cases. [2] I don't believe it would be productive to invent a mechanism for recording two separate changes within one revision, and if we did the user would still have to tell Subversion about them. >> We need to look more closely. That's what I'm currently working on. > > This runs up against issue #2897 'Reflective merges are faulty'. You > seem to be acknowledging this with your comments in the patch itself: > > "Note that with all these improvements, the result might approach a more > flexible merge system in which any reflected or cyclic changes are skipped, > *BUT WOULD NOT COPE* (emphasis mine) with the case where a revision > in the source branch contains a mixture of changes merged from the target > branch and other changes." > > But if we can't cope with that case how can we do anything that's an > improvement over what we do today (i.e. just merge r40)? We can't > skip r40 based on mergeinfo alone because there might be other > legitimate changes. There are two potential kinds of "other" changes. New changes mixed in with the merge are against the rules, as explained above. Other merges mixed in with that merge are, I think, legitimate. If we can calculate that r40 contains nothing but changes merged directly from branch B, then the Right Thing is to skip it. If it contains other changes (or if we can't determine for sure), then we can tell the user what's happening (that it would be wrong to try to merge this change because it contains "...") and we can either skip it or stop the merge. My contention is that that would be better than what we do today. Note that what we do today is unconditionally *attempt* to merge r40 even though (with my new code) we could know for sure that it will conflict logically (and will probably but not necessarily conflict physically). Another way of looking at it is that when we merge a revision that will logically conflict with the target, today the default action is to attempt to merge it anyway, and my proposed action is to tell the user what's happening and allow them to take corrective action in advance. The user could choose a different merge source, or perhaps realize that they were trying to merge into the wrong target branch, or something else. Does that make sense? - Julian
