monotone-devel
[Top][All Lists]
Advanced

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

[Monotone-devel] disapproving merge (was: New project: libmtn)


From: Daniel Carosone
Subject: [Monotone-devel] disapproving merge (was: New project: libmtn)
Date: Tue, 4 Jul 2006 13:42:06 +1000
User-agent: Mutt/1.5.11

On Fri, Jun 30, 2006 at 01:34:29PM -0700, Nathaniel Smith wrote:
> > - TODO: I cannot disapprove merging

There are two issues that get mixed together a lot, because of a poor
assymmetry in the choice of command naming. I think we need to
disentangle them.

Note also, I'm replying to Nathaniel's mail, but for the benefit of
other readers - Nathaniel is more than familiar enough with this
discussion :)

> Correct.  This is because no-one knows how to make such a thing that
> works.  (The analogy would be, complaining about a math library
> because it does not have a constant-time factoring algorithm.
> Certainly the designers would love to add such a thing, but...)
> 
> We could make disapprove of merge work just like disapprove of
> non-merge, of course, but it turns out that this is very much the
> wrong thing to do, and has very surprising behavior.  If you have:
>    a
>   / \
>  b   c
>   \ /
>    d
>   / \
>  b   c

An aside for the potentially-confused: these are not *really* b and c
at the bottom, though they do have the same content as b and c, which
is what Nathaniel is trying to convey here. Perhaps think of the lower
ones as B and C; same content but different ancestry, and that
difference is what's important when you try again to merge B and C:

> as the result of disapproving a merge, and you re-do the merge now,
> the result is not "d", but "a" (!).

Yes, this is a pain and a difficult problem to solve.

It's also the wrong problem to solve, at least for a command called
'disapprove', and also as a matter of general practice.

The way I envision 'disapprove' *should* work is simply to remove (or
override, revoke, poison, etc) the certificate issued by approve
(usually as implicit in merge or commit).

I don't really care that d exists in my database so much, what I care
about is that it's not a revision I want people updating to or basing
further work from.  In fact, that might only be true for one of the
several branches d is in - it might be a perfectly acceptable revision
in some other branch with different purposes.

If 'disapprove' worked this way, merges wouldn't be special, because
disapprove is nothing to do with the merge algorithm and the
difficulties above.

In general practice, if d was a badly-done (manual) merge, I want to
re-do it properly.  Which means I want to try again merging b and c,
to produce a different merge result e.

With present monotone, that would leave me with two heads, d and e:

   a
  /\
 b  c
 |\/|
 |/\|
 d  e

Assuming these are all in the one branch, I have two choices to
produce a single head again:
 - find a way to kill the branch certificate on d, as above
 - merge d and e to produce f, with the same content as e, which I
   can do now. 

I prefer the latter anyway, because if someone elsewhere on the
project has done some work based on d in the meantime, they'll get the
error fixed when they later merge again because the delta d->f undoes
the 'bad' parts of the merge.

The functionality we currently have as 'disapprove', which commits an
inverse change to a single (non merge) node should be called something
else (though it's hard to think of a good name, revert being taken for
a similar workspace operation).  It's also best thought of as a
convenience wrapper around several other functions that would
otherwise require a workspace, one of which we presently have to do
manually (there is not yet a revert -r, you need a second checkout and
copying files, or to fiddle with _MTN/revision):
 - checkout -r (the revision with the bad change)
 - revert -r (the ancestor with the previous good code)
 - commit
 - merge

If we imagined that monotone had used terminology more like this, what
Nathaniel would be saying above is that because of the difficulties he
describes, we shouldn't have a command called 'unmerge', and indeed we
don't. 

--
Dan.

Attachment: pgpZ01DCuJNJI.pgp
Description: PGP signature


reply via email to

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