Re: [Monotone-devel] non-content conflict messages

[Zack Weinberg]

On 9/23/07, Derek Scherger <address@hidden> wrote:
> I've been playing around a bit with the rather cryptic messages that
> monotone generates on so called non-content conflicts and hopefully have
> improved things a bit.

Cool!  Your revisions are definitely better than what we had, but I
think they could be even better still.

The term 'node' is only clear if you're familiar with monotone
internals.  I'd suggest checking what kind of node it is and
substituting 'file' or 'directory' as appropriate.  (Yes,
unfortunately, this does mean you have to duplicate the entire
diagnostic so the translators can get the grammar right.)

It's not clear to me if the names printed are leaf names or full
file_paths.  IMO they should be full file_paths.

I don't think we should print the node IDs; to user's they're
mysterious numbers that bear no relation to anything they know about,
and mysterious numbers are one of the chief evils of computer
diagnostic messages.  What would be good to have, though, is ancestral
names, whenever that makes sense.

Your proposals, while improved over what we already have, could still
be better about showing the user what actually happened to create the
situation.  This is especially important for the more exotic
conflicts, where the naive user -- or even a relatively savvy user,
like me -- would wonder 'how is that even possible?'

As a tangential nit, these are _not_ warnings, we're just using W()
for them because we don't have a better way to do it.  Can we have D()
[stands for "detailed error"] whose semantics are: it prints a
diagnostic which is a hard error, but then it returns to its caller
rather than terminating the process; furthermore, the "error: " leader
appears only the first time D() is called, and _not_ subsequently when
E() is called; and finally, it sets a flag in the ui object which
causes an invariant failure if D() has been used and then the program
attempts to exit successfully.  [Or some more elegant way of ensuring
that code that uses D() always follows it up with an E().]

Finally, your proposed diagnostics don't make it sufficiently clear
which side of the merge is which.  This will be especially important
with workspace merge, because the primitive there is 'merge this head
into my current workspace', so you really want the pathnames that
refer to your current workspace to appear in the same place always.
To deal with this, I suggest we should print a header whenever we're
about to report conflicts, reading something like this:

mtn: error: non-content conflicts found while merging:
mtn:   left revision <LEFTID>
mtn:  right revision <RIGHTID>
mtn: common ancestor <LCAID>
mtn: details follow:

and then consistently refer to LEFTID as left and RIGHTID as right
throughout.

Rephrasing all your diagnostics according to the above suggestions:

> [OLD] mtn: warning: name conflict on node 2: [parent 1, self right] vs.
> [parent 1, self left]
> [NEW] mtn: warning: divergent name conflict: one node (2) wants two
> names ('right' and 'left')

mtn: name split: ancestral (file|directory) 'LCA_FILE'
mtn: renamed to 'LEFT_FILE' on the left
mtn: renamed to 'RIGHT_FILE' on the right

(I'm using 'on the (left|right)' instead of 'in (left|right) rev'
because the actual commit(s) causing the problem might be ancestors of
the heads that we're trying to merge, not the heads themselves.)

> [OLD] mtn: warning: rename target conflict: nodes 6, 5, both want parent
> 3, name bar
> [NEW] mtn: warning: convergent name conflict: two nodes (5 and 6) want
> one name ('bar')

This case needs to distinguish four subcases:

mtn: name collision: (file|directory) 'TARGET_FILE'
mtn: renamed from 'LEFT_LCA_FILE' on the left
mtn: renamed from 'RIGHT_LCA_FILE' on the right

mtn: name collision: (file|directory) 'TARGET_FILE'
mtn: renamed from 'LEFT_LCA_FILE' on the left
mtn: created as 'TARGET_FILE' on the left

mtn: name collision: (file|directory) 'TARGET_FILE'
mtn: created as 'TARGET_FILE' on the left
mtn: renamed from 'RIGHT_LCA_FILE' on the right

mtn: name collision: (file|directory) 'TARGET_FILE'
mtn: created independently in left and right revs

> [OLD] mtn: warning: directory loop conflict: node 9, wanted parent 8,
> name foo
> [NEW] mtn: warning: directory loop conflict: between 'foo' (node 9) and
> 'bar' (node 8)

If this happens the way I think it happens,

mtn: attempt to create a directory loop:
mtn: 'foo' renamed to 'bar/foo' on the left
mtn: and 'foo/bar' renamed to 'bar' on the right

> [OLD] mtn: warning: orphaned node conflict on node 15, dead parent 13,
> name bar
> [NEW] mtn: warning: orphaned node conflict: parent of 'foo/bar' was
> removed (node 15 parent 13)

mtn: orphaned (file|directory):
mtn: directory 'bar' was deleted in (left|right) rev
mtn: (file|directory) 'bar/foo' was (created|renamed from 'baz') in
(right|left) rev
mtn: [... repeat the third line for all immediate children of 'bar';
do *not* spam
          the user with messages about descendants of those children]

> [OLD] mtn: warning: illegal name conflict: node 16, wanted parent 13,
> name _MTN
> [NEW] mtn: warning: illegal name conflict: between 'foo/_MTN' (node 16)
> and '' (node 13)

(Can we please avoid the term 'illegal' in user diagnostics?  Nobody
broke any laws.)

This is another case that I'm not sure how to produce; guessing
something like

mtn: name collision with the bookkeeping directory:
mtn: directory 'foo/_MTN' created on the left
mtn: directory 'foo' pivoted to root on the right

with variations, as above, for all the different actual operations
that could do this.  I don't think it's possible to put a versioned
file inside the bookkeeping directory without also causing a versioned
directory to have the same name as the bookkeeping directory itself,
so we should not spam the user for children of 'foo/_MTN' in the
example.  (Also, my brain insists, contrary to Ispell, that 'pivoted'
is misspelt - but doesn't like 'pivotted' either.  How the heck do you
spell that word?)

> [OLD] <nothing>
> [NEW] mtn: warning: missing root conflict: root directory has been removed

Guessing again:

mtn: merged revision has no root directory:
mtn: directory 'foo' deleted on the left
mtn: directory 'foo' pivoted to root on the right

> [OLD] mtn: warning: attribute conflict on node 19, key test-attr: [1,
> right-value] vs. [1, left-value]
> [NEW] mtn: warning: attribute conflict: attribute 'test-attr' on 'foo'
> (node 19) wants two values ('right-value' and 'left-value')

mtn: attribute conflict: 'test-attr' on 'foo'
mtn: given value 'left-value' on the left
mtn: given value 'right-value' on the right

(do we get an attribute conflict when an attr is changed in one rev
and deleted in another?  If so, that case should be distinguished.)

> The other thing we should do is add a more detailed section on conflicts
> to the manual that lists all of these with some hints on how they occur
> and what to do about them. I'll probably make a first pass at that as I
> clean this stuff up.

This would definitely be good, but I don't think it substitutes for
the diagnostics themselves making clear how the situation occurred.

zw