git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Marc Branchaud <marcnarc@xiplink.com>
Cc: git@vger.kernel.org, Johannes Sixt <j.sixt@viscovery.net>,
	Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [PATCH 2/2] Teach the --no-ff option to 'rebase -i'.
Date: Wed, 24 Mar 2010 12:06:54 -0700	[thread overview]
Message-ID: <7vfx3pilo1.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1269445261-2941-3-git-send-email-marcnarc@xiplink.com> (Marc Branchaud's message of "Wed\, 24 Mar 2010 11\:41\:01 -0400")

Marc Branchaud <marcnarc@xiplink.com> writes:

> This option tells git-rebase--interactive to cherry-pick all the commits in
> the rebased branch, instead of fast-forwarding over any unchanged commits.

Thanks.

> +Sometimes you're in a situation like this
> +
> + P---o---o---M---x---x---W---x
> +  \         /
> +   A---B---C
> +
> +where W is the reversion of merge M and you:
> +
> + - Need to rewrite one of the commits on the A-B-C branch; and
> +
> + - Want the rewritten A-B-C branch to still start at commit P (perhaps P
> +   is a branching-off point for yet another branch, and you want be able to
> +   merge A-B-C into both branches).
> +
> +The natural thing to do in this case is to checkout the A-B-C branch and use
> +"rebase -i A" to change commit B.  However, this does not rewrite commit A,
> +and you end up with this:
> +
> + P---o---o---M---x---x---W---x
> +  \         /
> +   A---B---C   <-- old branch
> +   \
> +    B'---C'    <-- rewritten branch
> +
> +To merge A-B'-C' into the mainline branch you would still have to first revert
> +commit W in order to pick up the changes in A, but then it's likely that the
> +changes in B' will conflict with the original B changes re-introduced by the
> +reversion of W.

I find it somewhat a contrived and unconvincing explanation.  A moderately
intelligent reader would wonder "Why did you choose to 'rebase -i A' in
the first place?  if you did 'rebase -i P' instead, you didn't have to
suffer from this issue, no?", and you are not answering that question.

Two things that you are assuming but not telling the user to make this
explanation appear unsatisfactory are:

 - The true reason you reverted M turns out to be that B was bad, and A
   was fine, so you want to fix B in A-B-C and merge it to master; and

 - Because "rebase -i" by default fast-forwards earlier "pick", even
   "rebase -i P" wouldn't have given you a rewritten A.

If you spell these out, then the explanation starts to make sense.

> +However, you can avoid these problems if you recreate the entire branch,
> +including commit A:
> +
> + P---o---o---M---x---x---W---x
> + |\         /
> + | A---B---C   <-- old branch
> + \
> +  A'---B'---C' <-- entirely recreated branch

This is just an advice on drawing, but you can avoid this ugly picture by
starting your discussion with:

      A---B---C
     /         \
    P---o---o---M---x---x---W---x

and adding the reproduction like this:

      A---B---C
     /         \
    P---o---o---M---x---x---W---x
     \
      A'--B'--C'

> +But if you don't actually need to change commit A, then you need some way to
> +recreate it as a new commit with the same changes in it.  The rebase commmand's
> +two modes (interactive and non-interactive) provide options to do this.  With
> +"rebase -i" use the --no-ff option:
> +
> +    $ git rebase -i --no-ff P
> +
> +With plain "rebase" use the -f option:
> +
> +    $ git rebase -f P

This is exactly why I don't mind giving a --no-ff synonym for --force to
non-interactive rebase.  That would make the choice between -i and no -i
made by the reader solely depend on the need to change anything in the
commits.  If you are fixing B in A-B-C, you would use "rebase -i".  If
instead you queued a fix-up as D, like so:

      A---B---C-------------------D
     /         \
    P---o---o---M---x---x---W---x

then you can rebase non-interactively the whole thing in preparation for
another merge:

      A---B---C---D
     /         \
    P---o---o---M---x---x---W---x...*
     \                             /
      A'--B'--C'------------------D'

  reply	other threads:[~2010-03-24 19:07 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-16 16:08 [PATCH] Teach --no-ff option to 'rebase -i' Marc Branchaud
2010-03-16 19:19 ` Marc Branchaud
2010-03-16 19:42 ` [PATCHv2] " Marc Branchaud
2010-03-16 21:47   ` Jonathan Nieder
2010-03-17  6:59     ` Johannes Sixt
2010-03-17 15:58       ` Peter Baumann
2010-03-17 16:07         ` Johannes Sixt
2010-03-17 18:42           ` Peter Baumann
2010-03-18  7:08             ` Johannes Sixt
2010-03-18  8:03               ` Peter Baumann
2010-03-17 16:03       ` Marc Branchaud
2010-03-17 16:19         ` Johannes Sixt
2010-03-17 18:10           ` Marc Branchaud
2010-03-22 19:25             ` [PATCH] Test that the 'rebase -i' "reword" command always cherry-picks a commit Marc Branchaud
2010-03-22 20:23               ` Avery Pennarun
2010-03-22 22:06                 ` Marc Branchaud
2010-03-22 20:46               ` Junio C Hamano
2010-03-23 14:38                 ` Marc Branchaud
2010-03-23 16:19                   ` [PATCHv3] Teach -f/--force-rebase option to 'rebase -i' Marc Branchaud
2010-03-23 22:42                     ` Junio C Hamano
2010-03-24 15:40                       ` [PATCHv4 0/2] Teach the --no-ff " Marc Branchaud
2010-03-24 17:13                         ` Junio C Hamano
2010-03-24 20:34                           ` [PATCHv5] Teach rebase the --no-ff option Marc Branchaud
2010-03-24 21:45                             ` Junio C Hamano
2010-03-24 15:41                       ` [PATCH 1/2] Teach 'rebase -i' to accept and ignore the -f/--force-rebase option Marc Branchaud
2010-03-24 15:41                       ` [PATCH 2/2] Teach the --no-ff option to 'rebase -i' Marc Branchaud
2010-03-24 19:06                         ` Junio C Hamano [this message]
2010-03-23 19:16                   ` [PATCH] Test that the 'rebase -i' "reword" command always cherry-picks a commit Jonathan Nieder
2010-03-22 22:09               ` Jonathan Nieder
2010-03-17 15:56     ` [PATCHv2] Teach --no-ff option to 'rebase -i' Marc Branchaud
2010-03-17 17:53       ` Jonathan Nieder
2010-03-17 18:13         ` Jonathan Nieder

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=7vfx3pilo1.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j.sixt@viscovery.net \
    --cc=jrnieder@gmail.com \
    --cc=marcnarc@xiplink.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).