git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Marc Branchaud <marcnarc@xiplink.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
Date: Tue, 16 Mar 2010 16:47:17 -0500	[thread overview]
Message-ID: <20100316214717.GA24880@progeny.tock> (raw)
In-Reply-To: <1268768556-32176-1-git-send-email-marcnarc@xiplink.com>

Hi Marc,

Marc Branchaud wrote:

> This option tells rebase--interactive to cherry-pick all the commits in the
> rebased branch, instead of fast-forwarding over any unchanged commits.
> 
> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
[...]
> This offers an alterntive way to deal with reverted merges (the
> revert-a-faulty-merge.txt howto recommends reverting the initial reversion
> before re-merging a modified topic branch).
> 
> With this change, you can instead use the --no-ff option to recreate the
> branch with entirely new commits (they're new because at the very least the
> committer time is different).  This obviates the need to revert the
> reversion, as you can re-merge the new topic branch directly.

I think this rationale should go in the commit message, for the sake of
future readers studying the code you’ve changed.

But let me make sure I understand it.

If I am understanding properly, your idea is that this would be used on
a branch after “unmerging” it from master:

    B --- C --- D [topic]
  /              \
 A ---  ...   --- M ... --- U [master]

Here M is a merge commit and U a commit reverting the change from M^
to M.

As howto/revert-a-faulty-merge explains, this is an accident waiting to
happen: if you merge master into topic, then it will pull in the revert
along with the rest of the changes.  Similarly, if you merge topic into
master, it will be a no-op (“already up to date”).

Your solution, to rewrite the branch, should address both of these
problems.  The new history looks like this:

    B' --- C' --- D' [topic]
  /
 |  B --- C --- D
 |/              \
 A ---   ...  --- M ... --- U [master]

What happens now if you merge topic into master (or master into topic)?

Git will try to reconcile the changes from the two branches since when
they diverged.  master gained the changes from the old topic branch and
then reverted them since it diverged from topic; since merge algorithm
is a simple three-way merge, all that history is ignored and Git will
just combine any new changes from master with the changes from topic.
Success.

Side note:

After this maneuver, I would want to merge ‘master’ into ‘topic’ as
soon as possible.  Why?  Because there is still a failure mode possible
after this: if you merge any commits from the old topic into the new
one before then, you’re back in trouble.  This could happen if some
topic2 that topic ends up needing is based on a point in master after M
and before U.

After merging master into topic, the _meaning_ of the history is much
clearer: by making U an ancestor, we are saying that as far as this
branch is concerned, the state at the tip of the topic branch is to be
preferred over the reverted state.

This suggests an alternative method to achieve your goal:

 git checkout master
 git revert -m 1 M;	# (1)
 git checkout topic
 git merge master^;	# (2)
 git merge -s ours master;	# (3)

1. Add a commit U removing the changes introduced by topic from master.

2. Merge all changes from master before the revert into topic.

3. Declare the changes from topic to supersede the master branch.
   This way, if topic is ever merged back to master, the changes will be
   reintroduced again.

Resulting history:

    B --- C --- D --- ... --- T^ --- T [topic]
  /              \            /     /
 A ---   ...  --- M ... --- U^ --- U [master]

You can avoid the perhaps pointless commit T^ by using --no-commit in
the command (2).  If not all changes from M..master are suitable for
merging into topic, you can use the same trick on a temporary side
branch:

 git checkout -b revert-topic M
 git revert -m 1 M
 git checkout topic
 git merge --no-commit revert-topic^
 git merge -s ours revert-topic
 git checkout master
 git merge revert-topic

If not all changes from topic..M are suitable for merging into topic,
then things are harder.  I’d suggest making a special unrevert-topic
branch as above to keep in the wings until its time.

> (Honestly, I wouldn't say that this approach is vastly superior to
> reverting the reversion.  I just find it a little less messy and a little
> more intuitive.  It's also a bit easier to explain to people to "use --no-ff
> after reverting a merge" instead of making sure they get the double-
> reversion right.)

Hope this helps clarify things.

Your patch itself might still be a good idea.  I think plain
‘git rebase’ already has something like it, in the form of the
--force-rebase option.  I have used it before, though I don’t remember
why.

Thanks for the food for thought,
Jonathan

  reply	other threads:[~2010-03-16 21:54 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 [this message]
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
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=20100316214717.GA24880@progeny.tock \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --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).