monotone-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Monotone-devel] [BUG] merge_into_workspace fails on second merge at


From: Nathaniel Smith
Subject: Re: [Monotone-devel] [BUG] merge_into_workspace fails on second merge attempt
Date: Fri, 12 Oct 2007 14:48:10 -0700
User-agent: Mutt/1.5.13 (2006-08-11)

On Fri, Oct 12, 2007 at 02:32:19PM +0200, Ralf S. Engelschall wrote:
> -    I(left_uncommon_ancestors.find(left_rid) != 
> left_uncommon_ancestors.end());
[...]
> ...the "merge_into_workspace" command works just fine. I've reviewed
> even the results and everything looks just fine. Unfortunately, I still
> do not _understand_ why this assertion exists and why it is false only
> for "merge_into_workspace". But I guess this assertion is from times
> before "merge_into_workspace" was invented and the workflow in the
> implementation of "merge_into_workspace" results in a special case which
> triggers the assertion. I don't know whether one can really _safely_
> remove the assertion or not. Can somebody shed some light on us here?

Ah, I see.  Here's what's going on: "left_uncommon_ancestors" is the
set of all revisions that are ancestors of the left revision being
merged, but are not ancestors of the right revision being merged.
(Compare to the notion of "common ancestors"; every ancestor of the
revisions being merged is exactly one of common, left uncommon, or
right uncommon.)

Asserting that left_rid is in left_uncommon_ancestors, then, is
asserting that left_rid is an ancestor of itself (which it is by
definition) and that it is not an ancestor of right_rid (which is the
point that's causing the problem).

It does not make sense to merge an ancestor with its own descendent --
the descendent *already* contains everything that the ancestor has,
plus it creates pointless merge nodes in the graph that prevent
branches from converging.  The other merge commands with more polished
UIs contain special case code to detect this and skip entering the
actual merge engine; merge_into_workspace ATM does not.  (For
instance, check out the code follow "// check for special cases" on
line 510 of cmd_merging.cc.)

So the short-term workaround for this case would be to just use
'propagate otherbranch thisbranch' or 'cert h:otherbranch branch
thisbranch' instead of 'workspace_merge otherbranch thisbranch'.

The long term solution is probably:
  -- if someone tries to do a workspace merge like this, then print a
     message and just update to the descendent version
  -- teach commit that if you try to do a commit with no changes, but
     the branch has changed versus the parent, then put that branch
     cert on the parent instead of just erroring out.

-- Nathaniel

-- 
Eternity is very long, especially towards the end.
  -- Woody Allen




reply via email to

[Prev in Thread] Current Thread [Next in Thread]