git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Martin von Zweigbergk <martinvonz@gmail.com>
To: Chris Webb <chris@arachsys.com>
Cc: Johannes Sixt <j.sixt@viscovery.net>, git@vger.kernel.org
Subject: Re: [RFC PATCH] add t3420-rebase-topology
Date: Fri, 28 Sep 2012 11:03:19 -0700	[thread overview]
Message-ID: <CANiSa6hvs2253yb4s0PpaAfG=hoBK2psP896GMFWT-UjiPZDwA@mail.gmail.com> (raw)
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).

> 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

  reply	other threads:[~2012-09-28 18:03 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-18  6:31 [RFC PATCH] add t3420-rebase-topology Martin von Zweigbergk
2012-09-18  7:51 ` Junio C Hamano
2012-09-21 17:06   ` Martin von Zweigbergk
2012-09-18  7:53 ` Johannes Sixt
2012-09-26 17:07   ` Martin von Zweigbergk
2012-09-27 12:20     ` Chris Webb
2012-09-28 18:03       ` Martin von Zweigbergk [this message]
2012-09-29  8:08         ` Chris Webb
2013-05-29  6:39 ` [PATCH v2 0/7] Rebase topology test Martin von Zweigbergk
2013-05-29  6:39   ` [PATCH v2 1/7] add simple tests of consistency across rebase types Martin von Zweigbergk
2013-06-03 17:16     ` Martin von Zweigbergk
2013-06-03 18:05       ` Junio C Hamano
2013-06-03 18:12         ` Martin von Zweigbergk
2013-05-29  6:39   ` [PATCH v2 2/7] add tests for rebasing with patch-equivalence present Martin von Zweigbergk
2013-05-29  7:09     ` Johannes Sixt
2013-05-30  5:30       ` Martin von Zweigbergk
2013-05-30  5:41         ` Felipe Contreras
2013-05-30  6:14           ` Martin von Zweigbergk
2013-05-30  6:40             ` Felipe Contreras
2013-05-30  6:46               ` Martin von Zweigbergk
2013-05-30 12:54         ` Johannes Sixt
2013-05-30 15:01           ` Martin von Zweigbergk
2013-05-29  6:39   ` [PATCH v2 3/7] add tests for rebasing of empty commits Martin von Zweigbergk
2013-05-29  6:39   ` [PATCH v2 4/7] add tests for rebasing root Martin von Zweigbergk
2013-05-29  7:31     ` Johannes Sixt
2013-05-30  5:51       ` Martin von Zweigbergk
2013-05-29  6:39   ` [PATCH v2 5/7] add tests for rebasing merged history Martin von Zweigbergk
2013-05-29  7:57     ` Johannes Sixt
2013-05-31  5:42       ` Martin von Zweigbergk
2013-05-29 10:33     ` Johannes Sixt
2013-05-29  6:39   ` [PATCH v2 6/7] t3406: modernize style Martin von Zweigbergk
2013-05-29  6:39   ` [PATCH v2 7/7] tests: move test for rebase messages from t3400 to t3406 Martin von Zweigbergk
2013-05-29  7:10   ` [PATCH v2 0/7] Rebase topology test Felipe Contreras
2013-05-29 12:50     ` Ramkumar Ramachandra
2013-05-29 13:54       ` Felipe Contreras
2013-05-31  6:49   ` [PATCH v3 " Martin von Zweigbergk
2013-05-31  6:49     ` [PATCH v3 1/7] add simple tests of consistency across rebase types Martin von Zweigbergk
2013-05-31  6:49     ` [PATCH v3 2/7] add tests for rebasing with patch-equivalence present Martin von Zweigbergk
2013-05-31  6:49     ` [PATCH v3 3/7] add tests for rebasing of empty commits Martin von Zweigbergk
2013-05-31  6:49     ` [PATCH v3 4/7] add tests for rebasing root Martin von Zweigbergk
2013-05-31  6:49     ` [PATCH v3 5/7] add tests for rebasing merged history Martin von Zweigbergk
2013-05-31 12:19       ` Johannes Sixt
2013-06-01 21:36         ` [PATCH v4 " Martin von Zweigbergk
2013-05-31  6:49     ` [PATCH v3 6/7] t3406: modernize style Martin von Zweigbergk
2013-05-31  6:49     ` [PATCH v3 7/7] tests: move test for rebase messages from t3400 to t3406 Martin von Zweigbergk
2013-06-03 20:42     ` [PATCH v5 0/7] Rebase topology test Martin von Zweigbergk
2013-06-03 20:42       ` [PATCH v5 1/7] add simple tests of consistency across rebase types Martin von Zweigbergk
2013-06-03 22:28         ` Junio C Hamano
2013-06-04  5:14           ` Martin von Zweigbergk
2013-06-04  5:49             ` Junio C Hamano
2013-06-04  6:15             ` Johannes Sixt
2013-06-05  4:31               ` Martin von Zweigbergk
2013-06-03 20:42       ` [PATCH v5 2/7] add tests for rebasing with patch-equivalence present Martin von Zweigbergk
2013-06-03 20:42       ` [PATCH v5 3/7] add tests for rebasing of empty commits Martin von Zweigbergk
2013-06-03 20:42       ` [PATCH v5 4/7] add tests for rebasing root Martin von Zweigbergk
2013-06-03 20:42       ` [PATCH v5 5/7] add tests for rebasing merged history Martin von Zweigbergk
2013-06-04 17:18         ` Junio C Hamano
2013-06-05  5:44           ` Martin von Zweigbergk
2013-06-05  6:12           ` Johannes Sixt
2013-06-03 20:42       ` [PATCH v5 6/7] t3406: modernize style Martin von Zweigbergk
2013-06-03 20:42       ` [PATCH v5 7/7] tests: move test for rebase messages from t3400 to t3406 Martin von Zweigbergk
2013-06-07  6:11       ` [PATCH v6 0/8] Rebase topology test Martin von Zweigbergk
2013-06-07  6:11         ` [PATCH v6 1/7] add simple tests of consistency across rebase types Martin von Zweigbergk
2013-06-07  6:11         ` [PATCH v6 2/7] add tests for rebasing with patch-equivalence present Martin von Zweigbergk
2013-06-07  6:11         ` [PATCH v6 3/7] add tests for rebasing of empty commits Martin von Zweigbergk
2013-06-07  6:11         ` [PATCH v6 4/7] add tests for rebasing root Martin von Zweigbergk
2013-06-07  6:11         ` [PATCH v6 5/7] add tests for rebasing merged history Martin von Zweigbergk
2013-06-07  6:11         ` [PATCH v6 6/7] t3406: modernize style Martin von Zweigbergk
2013-06-07  6:11         ` [PATCH v6 7/7] tests: move test for rebase messages from t3400 to t3406 Martin von Zweigbergk
2013-06-07 16:43         ` [PATCH v6 0/8] Rebase topology test Junio C Hamano
2013-06-07 19:37         ` Johannes Sixt
2013-06-18  7:28         ` [PATCH mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems Johannes Sixt
2013-06-18 15:45           ` Junio C Hamano
2013-06-18 15:53           ` Martin von Zweigbergk
2013-06-19  5:52             ` Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANiSa6hvs2253yb4s0PpaAfG=hoBK2psP896GMFWT-UjiPZDwA@mail.gmail.com' \
    --to=martinvonz@gmail.com \
    --cc=chris@arachsys.com \
    --cc=git@vger.kernel.org \
    --cc=j.sixt@viscovery.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).