Re: [RFC PATCH] add t3420-rebase-topology

From: Martin von Zweigbergk <martinvonz@gmail.com>
Date: Sep 28, 2012, 18:03 UTC
Message-ID: <CANiSa6hvs2253yb4s0PpaAfG=hoBK2psP896GMFWT-UjiPZDwA@mail.gmail.com>
In-Reply-To: <20120927122035.GL16059@arachsys.com>

On Thu, Sep 27, 2012 at 5:20 AM, Chris Webb <chris@arachsys.com> wrote:

> You're right that rebase --root without --onto always creates a brand new
> root as a result of the implementation using a sentinel commit. Clearly this
> is what's wanted with --interactive

That's not as clear as one might first think. "git rebase -i" actually honors --force; if one marks a commit for "edit", but then --continue without making any changes, the next commit(s) will be fast-forwarded. See the first few lines of pick_one (or pick_one_preserving_merges).

Show 7 quoted lines

> but rebase --root with neither --onto
> nor --interactive is a slightly odd combination for which I struggle to
> imagine a natural use. Perhaps you're right that for consistency it should
> be a no-op unless --force-rebase is given?
>
> If we did this, this combination would be a no-op unconditionally as by
> definition we're always descended from the root of our current commit.

For consistency, it seems like "git rebase -p --root" should always be a no-op, while "git rebase [-i/-m] --root" should be no-op if the history has no merges. Also, since "git rebase -i" tries to fast-forward through existing commits, it seems like "git rebase -i --root" should ideally not create a sentinel commit, but instead stop at the first commit marked for editing.

If, OTOH, --force-rebase is given, we should rewrite history from the first commit, which in the case of --root would mean creating a sentinel commit.

So, in short, I have a feeling that the sentinel commit should be created if and only if both --root and --force-rebase (but not --onto) are given.

> However, given the not-very-useful behaviour, I suspect that rebase --root
> is much more likely to be a mistyped version of rebase -i --root than rebase
> --root --force-rebase. (Unless I'm missing a reasonable use for this?
> History linearisation perhaps?)

Yes, the "not-very-useful"-ness of this is the clear argument against making them no-ops. But I have to say I was slightly surprised when I tried "git rebase --root" for the first time and it created completely new history for me. As you say, "git rebase --root" is probably often a mistyped "git rebase -i --root", and if that is the case, it seems nicer (in addition to being more consistent) if we don't do anything rather than rewriting the history. The history rewriting might even go unnoticed and come as an unpleasant surprise later.

When working on a new project, "git rebase -i --root" might even be a convenient replacement for "git rebase -i <initial commit>" even when one does not want to rewrite the initial commit itself, and in such a case, the user would clearly not want a sentinel commit either.

So I'm getting more and more convinced that the sentinel commit should only be created if --force-rebase was given. Let me know if I'm missing something.

Martin