root-ids changing for some merges

[vila]

>>>>> Aaron Bentley <aaron at aaronbentley.com> writes:

    > On 11-07-06 02:10 AM, vila wrote:
    >>>>>>> Aaron Bentley <aaron at aaronbentley.com> writes:

    >> > I pointed
    >> > this change out when I proposed the merge, so by approving the
    >> > proposal, Jelmer approved the change in behaviour.
    >> 
    >> Neither the submitter nor the reviewer are required to be perfect,
    >> errors happen. And Jelmer wasn't involved, that was jam and me and none
    >> of us caught the issue.

    > I'm sorry if I was vague or misleading, but I really did try to bring it
    > to the reviewers' attention.  It's very strange to see it called an
    > unexpected behaviour now.

Sorry about that, I really understood what was going on with these
root-ids by first investigating bug http://pad.lv/805809 where I
realized that debian brancheS were using one root-id and the ubuntu
brancheS another.

I wrote a test for that and it failed without the fix for 2.2 and
succeed with the fix. Same thing for 2.3. But merging to trunk it failed
with the fix and that's how I realized the impact of your change, not
before.

So at this point it became an unexpected behaviour because:

- when doing merge-upstream we *may* want this behaviour,
- when doing a "random" merge, we don't want it.

<snip/>

    >> The one we had was fine (bar the empty branch/tree merge bugs).

    > It's not helpful to repeat this without the *why*.

Hence that test added at the end of the mail you're replying to so you
can see for yourself: confusing and useless renames. I'd rather keep the
root-id and avoid them completely.

<snip/>

    > And again, what are the unwanted changes, as opposed to the
    > expected and deliberately introduced changes?

I think the unwanted change is that the new root-id is propagated in all
cases (and triggers confusing renames as explained in http://pad.lv/806536)

    >> Why the test suite didn't expose this in a
    >> more obvious way is another debate.

    > It's not ideal, but not very surprising.

When one realizes what is going on, it's not surprising anymore but
that's not the case for the average user.

    > Root ids are pretty unimportant most of the time, and we tend to
    > hide them.

Exactly, that's the main issue: since they are hidden there is no way
for the user to modify them so we need to make sure we always do the
right choice.

AIUI, there are only two cases where we want to propagate the root-it:
- merging into an empty tree,
- merge-upstream.

In all other cases, we've been fine *not* propagating it, that's what I
meant initially in this thread.

    > For example, we don't bother telling the user when a tree root is
    > newly-added (i.e.  before the first commit).

And what if the empty tree had *no* root-id instead like the 'null:'
revision tree ?

Wouldn't that make it easier to recognize that case automatically ?

merge-upstream can force it via the API (hand wawing), it doesn't so far
but this can wait and be done later.

    >> > Okay, so there's bad UI consequences.
    >> 
    >> Yes and I'm saying they are bad enough to be fixed before we release
    >> 2.4b5. If we can't, then I'd rather revert this change.

    > This seems like an overreaction to me.  It is a beta, after all.

Well, may be, but on the other hand we don't need this change nor do we
want beta users losing their root-id with no way to restore them easily
either, so the sooner we fix this, the better.

    > But I will change the behaviour, and I expect it to be easy.

Cool, thanks,

      Vincent