All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Teach --no-ff option to 'rebase -i'.
@ 2010-03-16 16:08 Marc Branchaud
  2010-03-16 19:19 ` Marc Branchaud
  2010-03-16 19:42 ` [PATCHv2] " Marc Branchaud
  0 siblings, 2 replies; 32+ messages in thread
From: Marc Branchaud @ 2010-03-16 16:08 UTC (permalink / raw)
  To: git; +Cc: Marc Branchaud

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.

(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.)

		M.

 Documentation/git-rebase.txt |   13 ++++++++++++-
 git-rebase--interactive.sh   |   10 ++++++++--
 t/t3417-rebase-no-ff.sh      |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100755 t/t3417-rebase-no-ff.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 823f2a4..01f1476 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -316,7 +316,18 @@ which makes little sense.
 	commit to be modified, and change the action of the moved
 	commit from `pick` to `squash` (or `fixup`).
 +
-This option is only valid when '--interactive' option is used.
+This option is only valid when the '--interactive' option is used.
+
+--no-ff::
+	Cherry-pick all rebased commits instead of fast-forwarding over
+	the unchanged ones.  This ensures that the entire history of the
+	rebased branch is composed of new commits.  You may find this
+	helpful after reverting a topic branch merge, as this option
+	recreates the topic branch with fresh commits so it can be remerged
+	successfully without needing to "reverting the reversion" (as described
+	in the link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To]).
++
+This option is only valid when the '--interactive' option is used.
 
 include::merge-strategies.txt[]
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3e4fd14..aecac3e 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -20,6 +20,7 @@ v,verbose          display a diffstat of what changed upstream
 onto=              rebase onto given branch instead of upstream
 p,preserve-merges  try to recreate merges instead of ignoring them
 s,strategy=        use the given merge strategy
+no-ff              never fast-forward any commits, even if they're unchanged
 m,merge            always used (no-op)
 i,interactive      always used (no-op)
  Actions:
@@ -103,6 +104,7 @@ VERBOSE=
 OK_TO_SKIP_PRE_REBASE=
 REBASE_ROOT=
 AUTOSQUASH=
+NO_SKIP=
 
 GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
 mark the corrected paths with 'git add <paths>', and
@@ -222,7 +224,7 @@ do_with_author () {
 }
 
 pick_one () {
-	no_ff=
+	no_ff=$NO_SKIP
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
 	test -d "$REWRITTEN" &&
@@ -742,6 +744,10 @@ first and then run 'git rebase --continue' again."
 	-i)
 		# yeah, we know
 		;;
+	--no-ff)
+		# Set -n parameter to pass to pick_one function
+		NO_SKIP=t
+		;;
 	--root)
 		REBASE_ROOT=t
 		;;
@@ -927,7 +933,7 @@ EOF
 		has_action "$TODO" ||
 			die_abort "Nothing to do"
 
-		test -d "$REWRITTEN" || skip_unnecessary_picks
+		test -d "$REWRITTEN" || ( test -z "$NO_SKIP" && skip_unnecessary_picks )
 
 		git update-ref ORIG_HEAD $HEAD
 		output git checkout $ONTO && do_rest
diff --git a/t/t3417-rebase-no-ff.sh b/t/t3417-rebase-no-ff.sh
new file mode 100755
index 0000000..29455da
--- /dev/null
+++ b/t/t3417-rebase-no-ff.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Marc Branchaud
+#
+
+test_description='git rebase -i --no-ff tests'
+
+. ./test-lib.sh
+
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+set_fake_editor
+
+test_expect_success setup '
+	echo hello > hello &&
+	git add hello &&
+	git commit -m "hello" &&
+
+	echo world >> hello &&
+	git commit -a -m "hello world" &&
+
+	echo goodbye >> hello &&
+	git commit -a -m "goodbye" &&
+
+	git tag old_head
+	'
+# Pause to ensure that the cherry-picked commits have a different
+# timestamp.
+sleep 1
+
+test_expect_success rebase '
+	git rebase -i --no-ff HEAD~2 &&
+	test ! $(git rev-parse HEAD) = $(git rev-parse old_head)
+	'
+
+test_done
-- 
1.7.0.2.dirty

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH] Teach --no-ff option to 'rebase -i'.
  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
  1 sibling, 0 replies; 32+ messages in thread
From: Marc Branchaud @ 2010-03-16 19:19 UTC (permalink / raw)
  To: git

Apologies -- this patch fails several rebase unit tests.

Back to the drawing board...

		M.


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.
> 
> (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.)
> 
> 		M.
> 
>  Documentation/git-rebase.txt |   13 ++++++++++++-
>  git-rebase--interactive.sh   |   10 ++++++++--
>  t/t3417-rebase-no-ff.sh      |   36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+), 3 deletions(-)
>  create mode 100755 t/t3417-rebase-no-ff.sh
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 823f2a4..01f1476 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -316,7 +316,18 @@ which makes little sense.
>  	commit to be modified, and change the action of the moved
>  	commit from `pick` to `squash` (or `fixup`).
>  +
> -This option is only valid when '--interactive' option is used.
> +This option is only valid when the '--interactive' option is used.
> +
> +--no-ff::
> +	Cherry-pick all rebased commits instead of fast-forwarding over
> +	the unchanged ones.  This ensures that the entire history of the
> +	rebased branch is composed of new commits.  You may find this
> +	helpful after reverting a topic branch merge, as this option
> +	recreates the topic branch with fresh commits so it can be remerged
> +	successfully without needing to "reverting the reversion" (as described
> +	in the link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To]).
> ++
> +This option is only valid when the '--interactive' option is used.
>  
>  include::merge-strategies.txt[]
>  
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 3e4fd14..aecac3e 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -20,6 +20,7 @@ v,verbose          display a diffstat of what changed upstream
>  onto=              rebase onto given branch instead of upstream
>  p,preserve-merges  try to recreate merges instead of ignoring them
>  s,strategy=        use the given merge strategy
> +no-ff              never fast-forward any commits, even if they're unchanged
>  m,merge            always used (no-op)
>  i,interactive      always used (no-op)
>   Actions:
> @@ -103,6 +104,7 @@ VERBOSE=
>  OK_TO_SKIP_PRE_REBASE=
>  REBASE_ROOT=
>  AUTOSQUASH=
> +NO_SKIP=
>  
>  GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
>  mark the corrected paths with 'git add <paths>', and
> @@ -222,7 +224,7 @@ do_with_author () {
>  }
>  
>  pick_one () {
> -	no_ff=
> +	no_ff=$NO_SKIP
>  	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
>  	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
>  	test -d "$REWRITTEN" &&
> @@ -742,6 +744,10 @@ first and then run 'git rebase --continue' again."
>  	-i)
>  		# yeah, we know
>  		;;
> +	--no-ff)
> +		# Set -n parameter to pass to pick_one function
> +		NO_SKIP=t
> +		;;
>  	--root)
>  		REBASE_ROOT=t
>  		;;
> @@ -927,7 +933,7 @@ EOF
>  		has_action "$TODO" ||
>  			die_abort "Nothing to do"
>  
> -		test -d "$REWRITTEN" || skip_unnecessary_picks
> +		test -d "$REWRITTEN" || ( test -z "$NO_SKIP" && skip_unnecessary_picks )
>  
>  		git update-ref ORIG_HEAD $HEAD
>  		output git checkout $ONTO && do_rest
> diff --git a/t/t3417-rebase-no-ff.sh b/t/t3417-rebase-no-ff.sh
> new file mode 100755
> index 0000000..29455da
> --- /dev/null
> +++ b/t/t3417-rebase-no-ff.sh
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2010 Marc Branchaud
> +#
> +
> +test_description='git rebase -i --no-ff tests'
> +
> +. ./test-lib.sh
> +
> +. "$TEST_DIRECTORY"/lib-rebase.sh
> +
> +set_fake_editor
> +
> +test_expect_success setup '
> +	echo hello > hello &&
> +	git add hello &&
> +	git commit -m "hello" &&
> +
> +	echo world >> hello &&
> +	git commit -a -m "hello world" &&
> +
> +	echo goodbye >> hello &&
> +	git commit -a -m "goodbye" &&
> +
> +	git tag old_head
> +	'
> +# Pause to ensure that the cherry-picked commits have a different
> +# timestamp.
> +sleep 1
> +
> +test_expect_success rebase '
> +	git rebase -i --no-ff HEAD~2 &&
> +	test ! $(git rev-parse HEAD) = $(git rev-parse old_head)
> +	'
> +
> +test_done

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCHv2] Teach --no-ff option to 'rebase -i'.
  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 ` Marc Branchaud
  2010-03-16 21:47   ` Jonathan Nieder
  1 sibling, 1 reply; 32+ messages in thread
From: Marc Branchaud @ 2010-03-16 19:42 UTC (permalink / raw)
  To: git; +Cc: Marc Branchaud

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>
---

Now passes all rebase unit tests.  I'll repeat my rationale from the v1
posting:

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.

(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.)

		M.

 Documentation/git-rebase.txt |   13 ++++++++++++-
 git-rebase--interactive.sh   |   10 ++++++++--
 t/t3417-rebase-no-ff.sh      |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100755 t/t3417-rebase-no-ff.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 823f2a4..01f1476 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -316,7 +316,18 @@ which makes little sense.
 	commit to be modified, and change the action of the moved
 	commit from `pick` to `squash` (or `fixup`).
 +
-This option is only valid when '--interactive' option is used.
+This option is only valid when the '--interactive' option is used.
+
+--no-ff::
+	Cherry-pick all rebased commits instead of fast-forwarding over
+	the unchanged ones.  This ensures that the entire history of the
+	rebased branch is composed of new commits.  You may find this
+	helpful after reverting a topic branch merge, as this option
+	recreates the topic branch with fresh commits so it can be remerged
+	successfully without needing to "reverting the reversion" (as described
+	in the link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To]).
++
+This option is only valid when the '--interactive' option is used.
 
 include::merge-strategies.txt[]
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3e4fd14..35943c0 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -20,6 +20,7 @@ v,verbose          display a diffstat of what changed upstream
 onto=              rebase onto given branch instead of upstream
 p,preserve-merges  try to recreate merges instead of ignoring them
 s,strategy=        use the given merge strategy
+no-ff              never fast-forward any commits, even if they're unchanged
 m,merge            always used (no-op)
 i,interactive      always used (no-op)
  Actions:
@@ -103,6 +104,7 @@ VERBOSE=
 OK_TO_SKIP_PRE_REBASE=
 REBASE_ROOT=
 AUTOSQUASH=
+NO_SKIP=
 
 GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
 mark the corrected paths with 'git add <paths>', and
@@ -222,7 +224,7 @@ do_with_author () {
 }
 
 pick_one () {
-	no_ff=
+	no_ff=$NO_SKIP
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
 	test -d "$REWRITTEN" &&
@@ -742,6 +744,10 @@ first and then run 'git rebase --continue' again."
 	-i)
 		# yeah, we know
 		;;
+	--no-ff)
+		# Set -n parameter to pass to pick_one function
+		NO_SKIP=t
+		;;
 	--root)
 		REBASE_ROOT=t
 		;;
@@ -927,7 +933,7 @@ EOF
 		has_action "$TODO" ||
 			die_abort "Nothing to do"
 
-		test -d "$REWRITTEN" || skip_unnecessary_picks
+		test -d "$REWRITTEN" || test -n "$NO_SKIP" || skip_unnecessary_picks
 
 		git update-ref ORIG_HEAD $HEAD
 		output git checkout $ONTO && do_rest
diff --git a/t/t3417-rebase-no-ff.sh b/t/t3417-rebase-no-ff.sh
new file mode 100755
index 0000000..2a2be81
--- /dev/null
+++ b/t/t3417-rebase-no-ff.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Marc Branchaud
+#
+
+test_description='git rebase -i --no-ff tests'
+
+. ./test-lib.sh
+
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+set_fake_editor
+
+test_expect_success setup '
+	echo hello > hello &&
+	git add hello &&
+	git commit -m "hello" &&
+
+	echo world >> hello &&
+	git commit -a -m "hello world" &&
+
+	echo goodbye >> hello &&
+	git commit -a -m "goodbye" &&
+
+	git tag old_head
+	'
+# Pause to ensure that the cherry-picked commits have a different
+# timestamp.
+sleep 1
+
+test_expect_success 'rebase -i --no-ff' '
+	git rebase -i --no-ff HEAD~2 &&
+	test ! $(git rev-parse HEAD) = $(git rev-parse old_head)
+	'
+
+test_done
-- 
1.7.0.2.1.g6d104.dirty

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  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:56     ` [PATCHv2] Teach --no-ff option to 'rebase -i' Marc Branchaud
  0 siblings, 2 replies; 32+ messages in thread
From: Jonathan Nieder @ 2010-03-16 21:47 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  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:03       ` Marc Branchaud
  2010-03-17 15:56     ` [PATCHv2] Teach --no-ff option to 'rebase -i' Marc Branchaud
  1 sibling, 2 replies; 32+ messages in thread
From: Johannes Sixt @ 2010-03-17  6:59 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Marc Branchaud, git

Jonathan Nieder schrieb:
> 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.

If I were to re-merge topic into master a second time after this
situation, I would install a temporary graft that removes the second
parent of M and repeat the merge. After the graft is removed, the history
would look like this:

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

Are there any downsides? I don't know - I haven't thought it through.

-- Hannes

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  2010-03-16 21:47   ` Jonathan Nieder
  2010-03-17  6:59     ` Johannes Sixt
@ 2010-03-17 15:56     ` Marc Branchaud
  2010-03-17 17:53       ` Jonathan Nieder
  1 sibling, 1 reply; 32+ messages in thread
From: Marc Branchaud @ 2010-03-17 15:56 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git

Jonathan Nieder wrote:
> 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.

Easy to do, but how necessary?  The rationale is already in the description
of --no-ff in rebase's man page.

There's an extra aspect to my motivation that I failed to mention, which is
that I would like the topic branch to stay rooted at its original commit.
This is because in our workflow that original commit is a synchronization
point for several maintenance and "next"-style branches, and keeping the
topic rooted there makes it easy to merge into different branches.

> 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.

Yup, you got it!

> Side note:
> 
> After this maneuver, I would want to merge ‘master’ into ‘topic’ as
> soon as possible.

Well, I wouldn't want to do that because it would make it impossible to merge
the topic into some other branch without also bringing in all of master.

> 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.

If the re-cast topic1 doesn't rewrite those commits, then the merge will
simply succeed because the code is already identical in both branches.

But if topic1 does rewrite those commits then there'll be a conflict.  IMO
that's correct, because with the merging of topic2 the code in master really
did diverge in a relevant way from what's in topic1, so that conflict should
get resolved in the normal way.

> 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.

Yes, but it also means that all the history of master since commit A is now
part of the topic, which isn't acceptable to me.

> 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]

I understand how this achieves your goal but, aside from dragging all of
master into the topic, this is way more complicated than recreating the topic
or even doing the double-revert.  By creating T with "-s ours" U becomes like
a surrogate ancestor of the topic -- it's there, but it doesn't share any
genetic material with the topic.  I think that's a recipe for confusion to
future explorers of the repo's history.

Besides, I do expect at least one of the topic's original commits to actually
be different in the recast topic -- the original merge was reverted because
the topic was faulty in some way, so there needs to be a real change made
somewhere.  If the topic could simply be fixed with an extra commit or two
after D then re-merged, there wouldn't be a need to revert the original merge
in the first place.

> 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.

This still brings all of the master's A..M^ commits into the topic.

>> (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.

No, the man page says --force-rebase does something else.

Thanks for the feedback!

		M.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  2010-03-17  6:59     ` Johannes Sixt
@ 2010-03-17 15:58       ` Peter Baumann
  2010-03-17 16:07         ` Johannes Sixt
  2010-03-17 16:03       ` Marc Branchaud
  1 sibling, 1 reply; 32+ messages in thread
From: Peter Baumann @ 2010-03-17 15:58 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jonathan Nieder, Marc Branchaud, git

On Wed, Mar 17, 2010 at 07:59:19AM +0100, Johannes Sixt wrote:
> Jonathan Nieder schrieb:
> > 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.
> 
> If I were to re-merge topic into master a second time after this
> situation, I would install a temporary graft that removes the second
> parent of M and repeat the merge. After the graft is removed, the history
> would look like this:
> 
>      B --- C --- D --------------.   [topic]
>    /              \               \
>   A ---  ...   --- M ... --- U ... N [master]
> 
> Are there any downsides? I don't know - I haven't thought it through.
> 

Might be. If there is any branch starting anywhere in between M and U
which also needs to merge [topic] will also cause you headaches :-)

       B --- C --- D --------------.   [topic]
     /              \               \
    A ---  ...   --- M ... --- U ... N [master]
                         \
                          x --- y [side_branch wich needs to merge topic] 

--
Peter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  2010-03-17  6:59     ` Johannes Sixt
  2010-03-17 15:58       ` Peter Baumann
@ 2010-03-17 16:03       ` Marc Branchaud
  2010-03-17 16:19         ` Johannes Sixt
  1 sibling, 1 reply; 32+ messages in thread
From: Marc Branchaud @ 2010-03-17 16:03 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jonathan Nieder, git

Johannes Sixt wrote:
> Jonathan Nieder schrieb:
>> 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.
> 
> If I were to re-merge topic into master a second time after this
> situation, I would install a temporary graft that removes the second
> parent of M and repeat the merge. After the graft is removed, the history
> would look like this:
> 
>      B --- C --- D --------------.   [topic]
>    /              \               \
>   A ---  ...   --- M ... --- U ... N [master]
> 
> Are there any downsides? I don't know - I haven't thought it through.

I'm not sure I follow how to create that graft.

But the original point (which I hadn't made clear) is that at least one of
the topic's commits needs to change in some substantial way.  So it's not
just a straight re-merge but a new take on the topic.

Consider that if the topic's first commit (B) needed to be rewritten then the
repaired topic would contain only new commits and it could be merged into
master without reverting the first merge's reversion.

What "rebase -i --no-ff" does is allow you to ensure that this will always be
the case, even if you don't actually need to change the topic's first commit.

		M.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  2010-03-17 15:58       ` Peter Baumann
@ 2010-03-17 16:07         ` Johannes Sixt
  2010-03-17 18:42           ` Peter Baumann
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Sixt @ 2010-03-17 16:07 UTC (permalink / raw)
  To: Peter Baumann; +Cc: Jonathan Nieder, Marc Branchaud, git

Peter Baumann schrieb:
> On Wed, Mar 17, 2010 at 07:59:19AM +0100, Johannes Sixt wrote:
>> If I were to re-merge topic into master a second time after this
>> situation, I would install a temporary graft that removes the second
>> parent of M and repeat the merge. After the graft is removed, the history
>> would look like this:
>>
>>      B --- C --- D --------------.   [topic]
>>    /              \               \
>>   A ---  ...   --- M ... --- U ... N [master]
>>
>> Are there any downsides? I don't know - I haven't thought it through.
>>
> 
> Might be. If there is any branch starting anywhere in between M and U
> which also needs to merge [topic] will also cause you headaches :-)
> 
>        B --- C --- D --------------.   [topic]
>      /              \               \
>     A ---  ...   --- M ... --- U ... N [master]
>                          \
>                           x --- y [side_branch wich needs to merge topic] 

?? I don't follow you. The side branch already contains the topic. What do
you want to merge?

-- Hannes

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  2010-03-17 16:03       ` Marc Branchaud
@ 2010-03-17 16:19         ` Johannes Sixt
  2010-03-17 18:10           ` Marc Branchaud
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Sixt @ 2010-03-17 16:19 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: Jonathan Nieder, git

Marc Branchaud schrieb:
> Johannes Sixt wrote:
>> If I were to re-merge topic into master a second time after this
>> situation, I would install a temporary graft that removes the second
>> parent of M and repeat the merge. After the graft is removed, the history
>> would look like this:
>>
>>      B --- C --- D --------------.   [topic]
>>    /              \               \
>>   A ---  ...   --- M ... --- U ... N [master]
>>
>> Are there any downsides? I don't know - I haven't thought it through.
> 
> I'm not sure I follow how to create that graft.

  $ echo $(git rev-parse M M^) >> .git/info/grafts

> But the original point (which I hadn't made clear) is that at least one of
> the topic's commits needs to change in some substantial way.  So it's not
> just a straight re-merge but a new take on the topic.
> 
> Consider that if the topic's first commit (B) needed to be rewritten then the
> repaired topic would contain only new commits and it could be merged into
> master without reverting the first merge's reversion.

You don't need --ff nor --no-ff in this case.

> What "rebase -i --no-ff" does is allow you to ensure that this will always be
> the case, even if you don't actually need to change the topic's first commit.

But why do you base the reworked topic on A instead of U or later?

Or why don't you just mark the first commit as r(eword) and just exit the
editor; it would rewrite the commit and all subsequent ones will be
rewritten as well. Never in my life would I have searched for a *option*
that achieves the goal. It is such a rare situation that we don't need an
option, do we?

-- Hannes

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  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
  0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Nieder @ 2010-03-17 17:53 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git

Marc Branchaud wrote:

> If the re-cast topic1 doesn't rewrite those commits, then the merge will
> simply succeed because the code is already identical in both branches.
> 
> But if topic1 does rewrite those commits then there'll be a conflict.  IMO
> that's correct, because with the merging of topic2 the code in master really
> did diverge in a relevant way from what's in topic1, so that conflict should
> get resolved in the normal way.

Sorry, I did not think it through all the way.  Here is an example of
what I meant by trouble.  Suppose you have re-cast topic using
rebase --no-ff:

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

topic represents a new feature that was merged prematurely and then
reverted, and topic2 represents some helpful new plumbing.
(introduction of a few functions, maybe).

To take advantage of the changes from F, someone merges topic2 into
topic and builds on it:

                    .. X --- Y [topic]
                   /  /
   B' --- C' --- D'  F [topic2]
  /                 /
 A -- ... -- M --- E ... --- U [master]

Now someone decides it is time to merge topic into master.  The
merge-base for Y and U is E, and the result is a that the changes from
topic are reverted.

What I had missed: it would be just as dangerous to simply merge topic2
directly.  Merging ‘master’ into ‘topic’ does nothing to prevent that.

The new advice: when using rebase --no-ff this way, be sure to rewrite
_every_ branch that includes those commits but doesn’t include U.

Jonathan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  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
  0 siblings, 1 reply; 32+ messages in thread
From: Marc Branchaud @ 2010-03-17 18:10 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jonathan Nieder, git

Johannes Sixt wrote:
> Marc Branchaud schrieb:
>> Johannes Sixt wrote:
>>> If I were to re-merge topic into master a second time after this
>>> situation, I would install a temporary graft that removes the second
>>> parent of M and repeat the merge. After the graft is removed, the history
>>> would look like this:
>>>
>>>      B --- C --- D --------------.   [topic]
>>>    /              \               \
>>>   A ---  ...   --- M ... --- U ... N [master]
>>>
>>> Are there any downsides? I don't know - I haven't thought it through.
>> I'm not sure I follow how to create that graft.
> 
>   $ echo $(git rev-parse M M^) >> .git/info/grafts
> 
>> But the original point (which I hadn't made clear) is that at least one of
>> the topic's commits needs to change in some substantial way.  So it's not
>> just a straight re-merge but a new take on the topic.
>>
>> Consider that if the topic's first commit (B) needed to be rewritten then the
>> repaired topic would contain only new commits and it could be merged into
>> master without reverting the first merge's reversion.
> 
> You don't need --ff nor --no-ff in this case.
> 
>> What "rebase -i --no-ff" does is allow you to ensure that this will always be
>> the case, even if you don't actually need to change the topic's first commit.
> 
> But why do you base the reworked topic on A instead of U or later?

I want the topic based on A so that I can merge it into other branches that
are also based on A.

> Or why don't you just mark the first commit as r(eword) and just exit the
> editor; it would rewrite the commit and all subsequent ones will be
> rewritten as well.

Yes, that works just as well (at least until someone optimizes the reword
command).

> Never in my life would I have searched for a *option*
> that achieves the goal. It is such a rare situation that we don't need an
> option, do we?

That's a more fundamental question.  I don't feel strongly either way.  The
main advantage I see with having the option is that it codifies the process,
with documentation and a unit test to help make sure it doesn't break.  So if
nobody wants the new option, I would then like to add a unit test to ensure
that rebase's reword command doesn't get optimized (if such a test doesn't
already exist), and maybe also add a note to the revert-a-faulty-merge howto.

		M.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  2010-03-17 17:53       ` Jonathan Nieder
@ 2010-03-17 18:13         ` Jonathan Nieder
  0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Nieder @ 2010-03-17 18:13 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git

Hi again,

Sorry to keep talking to myself here...

Jonathan Nieder wrote:

> To take advantage of the changes from F, someone merges topic2 into
> topic and builds on it:
> 
>                     .. X --- Y [topic]
>                    /  /
>    B' --- C' --- D'  F [topic2]
>   /                 /
>  A -- ... -- M --- E ... --- U [master]
> 
> Now someone decides it is time to merge topic into master.  The
> merge-base for Y and U is E, and the result is a that the changes from
> topic are reverted.

Almost certainly bad.

> What I had missed: it would be just as dangerous to simply merge topic2
> directly.  Merging ‘master’ into ‘topic’ does nothing to prevent that.

Might be bad, might not, depending on what topic2 is about.

> The new advice: when using rebase --no-ff this way, be sure to rewrite
> _every_ branch that includes those commits but doesn’t include U.

Or rather: rewrite every branch that you want to make supersede U.

Surprisingly tricky.

I prefer the “revert the faulty revert” technique because it is more
transparent.  If it is only possible to re-merge now because of
developments that occured in topic, a simple revert would create a
non-bisectable history, but Johannes’s trick (or the uglier method I
described before) should work.

 git checkout master
 : >> .git/info/grafts
 cp .git/info/grafts .git/info/saved-grafts
 echo $(git rev-parse M M^) >> .git/info/grafts
 git merge topic;	# being sure to mention M in the commit message!
 mv .git/info/saved-grafts .git/info/grafts

Jonathan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  2010-03-17 16:07         ` Johannes Sixt
@ 2010-03-17 18:42           ` Peter Baumann
  2010-03-18  7:08             ` Johannes Sixt
  0 siblings, 1 reply; 32+ messages in thread
From: Peter Baumann @ 2010-03-17 18:42 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jonathan Nieder, Marc Branchaud, git

On Wed, Mar 17, 2010 at 05:07:53PM +0100, Johannes Sixt wrote:
> Peter Baumann schrieb:
> > On Wed, Mar 17, 2010 at 07:59:19AM +0100, Johannes Sixt wrote:
> >> If I were to re-merge topic into master a second time after this
> >> situation, I would install a temporary graft that removes the second
> >> parent of M and repeat the merge. After the graft is removed, the history
> >> would look like this:
> >>
> >>      B --- C --- D --------------.   [topic]
> >>    /              \               \
> >>   A ---  ...   --- M ... --- U ... N [master]
> >>
> >> Are there any downsides? I don't know - I haven't thought it through.
> >>
> > 
> > Might be. If there is any branch starting anywhere in between M and U
> > which also needs to merge [topic] will also cause you headaches :-)
> > 
> >        B --- C --- D --------------.   [topic]
> >      /              \               \
> >     A ---  ...   --- M ... --- U ... N [master]
> >                          \
> >                           x --- y [side_branch wich needs to merge topic] 
> 
> ?? I don't follow you. The side branch already contains the topic. What do
> you want to merge?
> 

Won't it loose the revert 'U' after merging side_branch back to master?

Ah. Looking at the picture more closely, I could answer myself and say it would
only cause a huge mergeconflict, won't it?.

--
Peter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  2010-03-17 18:42           ` Peter Baumann
@ 2010-03-18  7:08             ` Johannes Sixt
  2010-03-18  8:03               ` Peter Baumann
  0 siblings, 1 reply; 32+ messages in thread
From: Johannes Sixt @ 2010-03-18  7:08 UTC (permalink / raw)
  To: Peter Baumann; +Cc: Jonathan Nieder, Marc Branchaud, git

Peter Baumann schrieb:
> On Wed, Mar 17, 2010 at 05:07:53PM +0100, Johannes Sixt wrote:
>> Peter Baumann schrieb:
>>> On Wed, Mar 17, 2010 at 07:59:19AM +0100, Johannes Sixt wrote:
>>>> If I were to re-merge topic into master a second time after this
>>>> situation, I would install a temporary graft that removes the second
>>>> parent of M and repeat the merge. After the graft is removed, the history
>>>> would look like this:
>>>>
>>>>      B --- C --- D --------------.   [topic]
>>>>    /              \               \
>>>>   A ---  ...   --- M ... --- U ... N [master]
>>>>
>>>> Are there any downsides? I don't know - I haven't thought it through.
>>>>
>>> Might be. If there is any branch starting anywhere in between M and U
>>> which also needs to merge [topic] will also cause you headaches :-)
>>>
>>>        B --- C --- D --------------.   [topic]
>>>      /              \               \
>>>     A ---  ...   --- M ... --- U ... N [master]
>>>                          \
>>>                           x --- y [side_branch wich needs to merge topic] 
>> ?? I don't follow you. The side branch already contains the topic. What do
>> you want to merge?
>>
> 
> Won't it loose the revert 'U' after merging side_branch back to master?
> 
> Ah. Looking at the picture more closely, I could answer myself and say it would
> only cause a huge mergeconflict, won't it?.

No. N and the merge-base of N and y are identical (wrt changes introduced
by B,C,D). At least this part will not cause any conflicts.

-- Hannes

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv2] Teach --no-ff option to 'rebase -i'.
  2010-03-18  7:08             ` Johannes Sixt
@ 2010-03-18  8:03               ` Peter Baumann
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Baumann @ 2010-03-18  8:03 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jonathan Nieder, Marc Branchaud, git

On Thu, Mar 18, 2010 at 08:08:51AM +0100, Johannes Sixt wrote:
> Peter Baumann schrieb:
> > On Wed, Mar 17, 2010 at 05:07:53PM +0100, Johannes Sixt wrote:
> >> Peter Baumann schrieb:
> >>> On Wed, Mar 17, 2010 at 07:59:19AM +0100, Johannes Sixt wrote:
> >>>> If I were to re-merge topic into master a second time after this
> >>>> situation, I would install a temporary graft that removes the second
> >>>> parent of M and repeat the merge. After the graft is removed, the history
> >>>> would look like this:
> >>>>
> >>>>      B --- C --- D --------------.   [topic]
> >>>>    /              \               \
> >>>>   A ---  ...   --- M ... --- U ... N [master]
> >>>>
> >>>> Are there any downsides? I don't know - I haven't thought it through.
> >>>>
> >>> Might be. If there is any branch starting anywhere in between M and U
> >>> which also needs to merge [topic] will also cause you headaches :-)
> >>>
> >>>        B --- C --- D --------------.   [topic]
> >>>      /              \               \
> >>>     A ---  ...   --- M ... --- U ... N [master]
> >>>                          \
> >>>                           x --- y [side_branch wich needs to merge topic] 
> >> ?? I don't follow you. The side branch already contains the topic. What do
> >> you want to merge?
> >>
> > 
> > Won't it loose the revert 'U' after merging side_branch back to master?
> > 
> > Ah. Looking at the picture more closely, I could answer myself and say it would
> > only cause a huge mergeconflict, won't it?.
> 
> No. N and the merge-base of N and y are identical (wrt changes introduced
> by B,C,D). At least this part will not cause any conflicts.
> 

You are right. How could I missed that.

Thanks for the clarification.

--
Peter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH] Test that the 'rebase -i' "reword" command always cherry-picks a commit.
  2010-03-17 18:10           ` Marc Branchaud
@ 2010-03-22 19:25             ` Marc Branchaud
  2010-03-22 20:23               ` Avery Pennarun
                                 ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Marc Branchaud @ 2010-03-22 19:25 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt, Jonathan Nieder, Marc Branchaud

In particular, "reword" should cherry-pick a reworded commit even if the
commit's message is unchanged.

This behaviour provides a way to deal with a situation that can arise when
a merge had to be reverted.  Added an addendum to revert-a-faulty-merge.txt
describing the situation and how to use "reword" to handle it.
---

Is this more acceptable than adding --no-ff to rebase--interactive?

I wasn't sure how to integrate the new text into revert-a-faulty-merge.txt.
I went with an addendum, but I'm open to other approaches.

		M.

 Documentation/howto/revert-a-faulty-merge.txt |   64 +++++++++++++++++++++++++
 t/t3417-rebase-reword-forces-cherry-pick.sh   |   59 +++++++++++++++++++++++
 2 files changed, 123 insertions(+), 0 deletions(-)
 create mode 100755 t/t3417-rebase-reword-forces-cherry-pick.sh

diff --git a/Documentation/howto/revert-a-faulty-merge.txt b/Documentation/howto/revert-a-faulty-merge.txt
index 3b4a390..892433d 100644
--- a/Documentation/howto/revert-a-faulty-merge.txt
+++ b/Documentation/howto/revert-a-faulty-merge.txt
@@ -142,6 +142,8 @@ different resolution strategies:
    revert of a merge was rebuilt from scratch (i.e. rebasing and fixing,
    as you seem to have interpreted), then re-merging the result without
    doing anything else fancy would be the right thing to do.
+   (See the ADDENDUM below for how to rebuild a branch from scratch
+   without changing its original branching-off point.)
 
 However, there are things to keep in mind when reverting a merge (and
 reverting such a revert).
@@ -177,3 +179,65 @@ the answer is: "oops, I really shouldn't have merged it, because it wasn't
 ready yet, and I really need to undo _all_ of the merge"). So then you
 really should revert the merge, but when you want to re-do the merge, you
 now need to do it by reverting the revert.
+
+ADDENDUM
+
+Sometimes you're in a situation like this
+
+ P---o---o---M---x---x---W---x
+  \         /
+   A---B---C
+
+where 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.
+
+However, you could avoid these problems if you recreated 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
+
+Now you can merge A'-B'-C' into the mainline branch without worrying about
+first reverting W.
+
+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.  One way to do this
+is to use the "reword" command in "rebase -i".  If you:
+
+ - Checkout the A-B-C branch
+
+ - Run "rebase -i P" to rebase the entire branch
+
+ - "reword" commit A (the first commit in the rebase script)
+
+ - Make NO changes to commit A's message
+
+then you'll end up with a new branch A'-B'-C' with all-new commits (all the
+SHA IDs will be different) but that has the same changes that were in the
+original A-B-C branch.
+
+You can then modify commit B' as you like and merge the new A'-B''-C' branch
+into the mainline branch without first reverting commit W.
diff --git a/t/t3417-rebase-reword-forces-cherry-pick.sh b/t/t3417-rebase-reword-forces-cherry-pick.sh
new file mode 100755
index 0000000..3e29f0f
--- /dev/null
+++ b/t/t3417-rebase-reword-forces-cherry-pick.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Marc Branchaud
+#
+
+test_description='git rebase interactive "reword" always forces cherry-pick
+
+This test ensures that the "reword" command of "git rebase -i" cherry-picks
+a reworded commit even if no changes are made to the commit message.
+
+This behaviour provides a way to deal with a situation that can arise when a
+merge had to be reverted.  For details, see the ADDENDUM in
+Documentation/howto/revert-a-faulty-merge.txt.
+'
+
+. ./test-lib.sh
+
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+set_fake_editor
+
+# A simple linear history.
+test_expect_success 'setup' '
+	test_commit A file1 &&
+	test_commit B file1 &&
+	test_commit C file1 &&
+	test_commit D file1 &&
+	git branch old-master HEAD
+'
+
+# Ensure that the rebased commits get a different timestamp.
+test_tick
+
+# Reword the first commit, but don't actually change the message
+test_expect_success 'reword' '
+	FAKE_LINES="reword 1 2 3" git rebase -i A
+'
+
+# There should be no textual differences between the old and new branches.
+# However:
+#  - Commit A should be completely unchanged.
+#  - Commits B, C and D should have different SHA IDs.
+test_expect_success 'verify' '
+	touch empty &&
+	test ! $(git rev-parse ) = $(git rev-parse master) &&
+	git diff old-master master > out &&
+	test_cmp empty out &&
+	test ! $(git rev-parse C) = $(git rev-parse master^) &&
+	git diff old-master^ master^ > out &&
+	test_cmp empty out &&
+	test ! $(git rev-parse B) = $(git rev-parse master^^) &&
+	git diff old-master^^ master^^ > out &&
+	test_cmp empty out &&
+	test $(git rev-parse A) = $(git rev-parse master^^^) &&
+	git diff old-master^^^ master^^^ > out &&
+	test_cmp empty out
+'
+
+test_done
-- 
1.7.0.3

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH] Test that the 'rebase -i' "reword" command always  cherry-picks a commit.
  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-22 22:09               ` Jonathan Nieder
  2 siblings, 1 reply; 32+ messages in thread
From: Avery Pennarun @ 2010-03-22 20:23 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git, Johannes Sixt, Jonathan Nieder

On Mon, Mar 22, 2010 at 3:25 PM, Marc Branchaud <marcnarc@xiplink.com> wrote:
> +Sometimes you're in a situation like this
> +
> + P---o---o---M---x---x---W---x
> +  \         /
> +   A---B---C
> +
> +where 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 think you need to clarify in the above text that W is a revert of M.
 I was very confused by this at first.

Other than that, I'll leave it to others more opinionated than me to
comment on whether regenerating a commit just for the sake of
regenerating it is actually desirable or not :)

Have fun,

Avery

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Test that the 'rebase -i' "reword" command always cherry-picks a commit.
  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 20:46               ` Junio C Hamano
  2010-03-23 14:38                 ` Marc Branchaud
  2010-03-22 22:09               ` Jonathan Nieder
  2 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2010-03-22 20:46 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git, Johannes Sixt, Jonathan Nieder

Marc Branchaud <marcnarc@xiplink.com> writes:

> In particular, "reword" should cherry-pick a reworded commit even if the
> commit's message is unchanged.
>
> This behaviour provides a way to deal with a situation that can arise when
> a merge had to be reverted.  Added an addendum to revert-a-faulty-merge.txt
> describing the situation and how to use "reword" to handle it.
> ---
>
> Is this more acceptable than adding --no-ff to rebase--interactive?
>
> I wasn't sure how to integrate the new text into revert-a-faulty-merge.txt.
> I went with an addendum, but I'm open to other approaches.

The addendum looked readable, but I am a bit puzzled.  "rebase -i --no-ff"
already exists, and is probably a more natural way to do this than saying
"reword" but not rewording anything, no?

I would actually say "rebase -f P" would be even easier and clearer,
especially as...

> ...
> +However, you could avoid these problems if you recreated 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
> +
> +Now you can merge A'-B'-C' into the mainline branch without worrying about
> +first reverting W.
> +
> +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.

... this part seems to talk about working around the tendency of 'rebase -i'
to fast-forward.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Test that the 'rebase -i' "reword" command always  cherry-picks a commit.
  2010-03-22 20:23               ` Avery Pennarun
@ 2010-03-22 22:06                 ` Marc Branchaud
  0 siblings, 0 replies; 32+ messages in thread
From: Marc Branchaud @ 2010-03-22 22:06 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: git, Johannes Sixt, Jonathan Nieder

Avery Pennarun wrote:
> On Mon, Mar 22, 2010 at 3:25 PM, Marc Branchaud <marcnarc@xiplink.com> wrote:
>> +Sometimes you're in a situation like this
>> +
>> + P---o---o---M---x---x---W---x
>> +  \         /
>> +   A---B---C
>> +
>> +where 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 think you need to clarify in the above text that W is a revert of M.
>  I was very confused by this at first.

Someone who reads through the whole file will see that W is the reversion of
M.  It's probably good to repeat that in the addendum for readers who jump to
the addendum right away.

> Other than that, I'll leave it to others more opinionated than me to
> comment on whether regenerating a commit just for the sake of
> regenerating it is actually desirable or not :)

I'm all ears!

		M.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Test that the 'rebase -i' "reword" command always cherry-picks a commit.
  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 20:46               ` Junio C Hamano
@ 2010-03-22 22:09               ` Jonathan Nieder
  2 siblings, 0 replies; 32+ messages in thread
From: Jonathan Nieder @ 2010-03-22 22:09 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git, Johannes Sixt

Marc Branchaud wrote:

> Is this more acceptable than adding --no-ff to rebase--interactive?

No, in my opinion the latter is better, and I suspect it will go in. :)

No one has objected to the --no-ff option.  I like it: if someone is
trying to reset the committer date on a collection of patches, I’d
rather they use rebase -f or rebase -i --no-ff than format-patch and am.
It’s cooking in pu for the moment, and from here the natural progression
is to next, master, then maybe maint.

Hope that helps,
Jonathan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] Test that the 'rebase -i' "reword" command always cherry-picks a commit.
  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 19:16                   ` [PATCH] Test that the 'rebase -i' "reword" command always cherry-picks a commit Jonathan Nieder
  0 siblings, 2 replies; 32+ messages in thread
From: Marc Branchaud @ 2010-03-23 14:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Sixt, Jonathan Nieder

Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>> In particular, "reword" should cherry-pick a reworded commit even if the
>> commit's message is unchanged.
>>
>> This behaviour provides a way to deal with a situation that can arise when
>> a merge had to be reverted.  Added an addendum to revert-a-faulty-merge.txt
>> describing the situation and how to use "reword" to handle it.
>> ---
>>
>> Is this more acceptable than adding --no-ff to rebase--interactive?
>>
>> I wasn't sure how to integrate the new text into revert-a-faulty-merge.txt.
>> I went with an addendum, but I'm open to other approaches.
> 
> The addendum looked readable, but I am a bit puzzled.

Ya think _you're_ puzzled?  :)

> "rebase -i --no-ff"
> already exists, and is probably a more natural way to do this than saying
> "reword" but not rewording anything, no?
> 
> I would actually say "rebase -f P" would be even easier and clearer,
> especially as...
> 
>> ...
>> +However, you could avoid these problems if you recreated 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
>> +
>> +Now you can merge A'-B'-C' into the mainline branch without worrying about
>> +first reverting W.
>> +
>> +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.
> 
> ... this part seems to talk about working around the tendency of 'rebase -i'
> to fast-forward.

Yes.  Thanks for pointing this out, it's cleared up a lot for me.

I was confused about the purpose of "rebase -f".  Jonathan Nieder even
pointed me to it when I posted my original patch for "rebase -i --no-ff", but
the description in the man page threw me:

	Force the rebase even if the current branch is a descendant of
	the commit you are rebasing onto. Normally the command will
	exit with the message "Current branch is up to date" in such a
	situation.

I didn't realize that this is exactly the situation that "rebase -i" normally
deals with (-i basically implies -f), and that "rebase -f" would do exactly
what I wanted "rebase -i --no-ff" to do.

But I think I see an approach that might make sense:

 - Teach "rebase -i" to recognize the -f parameter (instead of --no-ff).

 - Update rebase's man page to better explain -f.

 - Update revert-a-faulty-merge.txt to explain how to use "rebase [-i] -f".

I'll submit a new patch shortly.

		M.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCHv3] Teach -f/--force-rebase option to 'rebase -i'.
  2010-03-23 14:38                 ` Marc Branchaud
@ 2010-03-23 16:19                   ` Marc Branchaud
  2010-03-23 22:42                     ` Junio C Hamano
  2010-03-23 19:16                   ` [PATCH] Test that the 'rebase -i' "reword" command always cherry-picks a commit Jonathan Nieder
  1 sibling, 1 reply; 32+ messages in thread
From: Marc Branchaud @ 2010-03-23 16:19 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt, Jonathan Nieder, Junio C Hamano, Marc Branchaud

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

Also expanded -f's description in rebase's man page.

-f offers an alterntive way to deal with reverted merges.  Instead of
"reverting the revert" you can use "rebase -f [-i]" 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.  Added an
addendum to revert-a-faulty-merge.txt describing the situation and how to
use -f to handle it.

Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---

It seems more natural to me to just teach -f to "rebase -i" instead of adding
a new --no-ff option.

This patch combines my initial "--no-ff" patch with the documentation changes
in my "reword" patch.  I moved the unit test for "rebase -f -i" into
t3404-rebase-interactive.sh.

		M.


 Documentation/git-rebase.txt                  |   10 ++++-
 Documentation/howto/revert-a-faulty-merge.txt |   57 +++++++++++++++++++++++++
 git-rebase--interactive.sh                    |    9 +++-
 t/t3404-rebase-interactive.sh                 |   36 ++++++++++++++--
 4 files changed, 104 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 823f2a4..1fd8d1c 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -274,9 +274,15 @@ which makes little sense.
 -f::
 --force-rebase::
 	Force the rebase even if the current branch is a descendant
-	of the commit you are rebasing onto.  Normally the command will
+	of the commit you are rebasing onto.  Normally non-interactive rebase will
 	exit with the message "Current branch is up to date" in such a
-	situation.
+	situation.  With --interactive, this option forces rebase to
+	cherry-pick all commits, even if they're not actually changed.
++
+--force-rebase can sometimes be helpful after reverting a topic branch merge,
+as it recreates the topic branch with fresh commits so it can be remerged
+successfully without needing to "revert the reversion" (see the
+link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To]).
 
 --ignore-whitespace::
 --whitespace=<option>::
diff --git a/Documentation/howto/revert-a-faulty-merge.txt b/Documentation/howto/revert-a-faulty-merge.txt
index 3b4a390..994bd7f 100644
--- a/Documentation/howto/revert-a-faulty-merge.txt
+++ b/Documentation/howto/revert-a-faulty-merge.txt
@@ -142,6 +142,8 @@ different resolution strategies:
    revert of a merge was rebuilt from scratch (i.e. rebasing and fixing,
    as you seem to have interpreted), then re-merging the result without
    doing anything else fancy would be the right thing to do.
+   (See the ADDENDUM below for how to rebuild a branch from scratch
+   without changing its original branching-off point.)
 
 However, there are things to keep in mind when reverting a merge (and
 reverting such a revert).
@@ -177,3 +179,58 @@ the answer is: "oops, I really shouldn't have merged it, because it wasn't
 ready yet, and I really need to undo _all_ of the merge"). So then you
 really should revert the merge, but when you want to re-do the merge, you
 now need to do it by reverting the revert.
+
+ADDENDUM
+
+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.
+
+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
+
+Now you can merge A'-B'-C' into the mainline branch without worrying about
+first reverting W.
+
+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.  One way to do this
+is to rebase the entire branch from the branching-off point (commit P) using
+the -f parameter to ensure that all the commits get recreated:
+
+    $ git rebase -f -i P
+
+This creates a new branch A'-B'-C' with all-new commits (all the SHA IDs will
+be different) even if you only actually modify commit B.  You can then merge
+this new branch directly into the mainline branch and be sure you'll get all
+of the branch's changes.
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3e4fd14..61ccf4c 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -20,6 +20,7 @@ v,verbose          display a diffstat of what changed upstream
 onto=              rebase onto given branch instead of upstream
 p,preserve-merges  try to recreate merges instead of ignoring them
 s,strategy=        use the given merge strategy
+f,force-rebase     cherry-pick all commits, even if unchanged
 m,merge            always used (no-op)
 i,interactive      always used (no-op)
  Actions:
@@ -103,6 +104,7 @@ VERBOSE=
 OK_TO_SKIP_PRE_REBASE=
 REBASE_ROOT=
 AUTOSQUASH=
+FORCE=
 
 GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
 mark the corrected paths with 'git add <paths>', and
@@ -222,7 +224,7 @@ do_with_author () {
 }
 
 pick_one () {
-	no_ff=
+	no_ff=$FORCE
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
 	test -d "$REWRITTEN" &&
@@ -742,6 +744,9 @@ first and then run 'git rebase --continue' again."
 	-i)
 		# yeah, we know
 		;;
+	-f)
+		FORCE=t
+		;;
 	--root)
 		REBASE_ROOT=t
 		;;
@@ -927,7 +932,7 @@ EOF
 		has_action "$TODO" ||
 			die_abort "Nothing to do"
 
-		test -d "$REWRITTEN" || skip_unnecessary_picks
+		test -d "$REWRITTEN" || test -n "$FORCE" || skip_unnecessary_picks
 
 		git update-ref ORIG_HEAD $HEAD
 		output git checkout $ONTO && do_rest
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4e35137..a169470 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -22,12 +22,18 @@ set_fake_editor
 # | \
 # |   F - G - H                (branch1)
 # |     \
-#  \      I                    (branch2)
-#   \
-#     J - K - L - M            (no-conflict-branch)
+# |\      I                    (branch2)
+# | \
+# |   J - K - L - M            (no-conflict-branch)
+#  \
+#    N - O - P                 (force-branch)
 #
 # where A, B, D and G all touch file1, and one, two, three, four all
 # touch file "conflict".
+#
+# WARNING: Modifications to the initial repository can change the SHA ID used
+# in the expect2 file for the 'stop on conflicting pick' test.
+
 
 test_expect_success 'setup' '
 	test_commit A file1 &&
@@ -50,6 +56,11 @@ test_expect_success 'setup' '
 	for n in J K L M
 	do
 		test_commit $n file$n
+	done &&
+	git checkout -b force-branch A &&
+	for n in N O P
+	do
+		test_commit $n file$n
 	done
 '
 
@@ -113,7 +124,7 @@ cat > expect2 << EOF
 D
 =======
 G
->>>>>>> 51047de... G
+>>>>>>> 5d18e54... G
 EOF
 
 test_expect_success 'stop on conflicting pick' '
@@ -553,4 +564,21 @@ test_expect_success 'reword' '
 	git show HEAD~2 | grep "C changed"
 '
 
+test_tick # Ensure that the rebased commits get a different timestamp.
+test_expect_success 'always cherry-pick with -f' '
+	git checkout force-branch &&
+	git tag original-force-branch &&
+	git rebase -i -f A &&
+	touch empty &&
+	for p in 0 1 2
+	do
+		test ! $(git rev-parse HEAD~$p) = $(git rev-parse original-force-branch~$p) &&
+		git diff HEAD~$p original-force-branch~$p > out &&
+		test_cmp empty out
+	done &&
+	test $(git rev-parse HEAD~3) = $(git rev-parse original-force-branch~3) &&
+	git diff HEAD~3 original-force-branch~3 > out &&
+	test_cmp empty out
+'
+
 test_done
-- 
1.7.0.3.dirty

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH] Test that the 'rebase -i' "reword" command always cherry-picks a commit.
  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 19:16                   ` Jonathan Nieder
  1 sibling, 0 replies; 32+ messages in thread
From: Jonathan Nieder @ 2010-03-23 19:16 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: Junio C Hamano, git, Johannes Sixt

Marc Branchaud wrote:
> Junio C Hamano wrote:

>> "rebase -i --no-ff"
>> already exists, and is probably a more natural way to do this than saying
>> "reword" but not rewording anything, no?
>> 
>> I would actually say "rebase -f P" would be even easier and clearer,
>> especially as...
[...]
> I was confused about the purpose of "rebase -f".  Jonathan Nieder even
> pointed me to it when I posted my original patch for "rebase -i --no-ff", but
> the description in the man page threw me:
>
> 	Force the rebase even if the current branch is a descendant of
> 	the commit you are rebasing onto. Normally the command will
> 	exit with the message "Current branch is up to date" in such a
> 	situation.
>
> I didn't realize that this is exactly the situation that "rebase -i" normally
> deals with (-i basically implies -f), and that "rebase -f" would do exactly
> what I wanted "rebase -i --no-ff" to do.

Yes, sorry for not following up on that comment.  The point is that
rebase without --interactive never fast-forwards.  So if you do not need
to edit your branch interactively, it should work for your purpose.

> But I think I see an approach that might make sense:
> 
>  - Teach "rebase -i" to recognize the -f parameter (instead of --no-ff).

I like --no-ff better. :)

>  - Update rebase's man page to better explain -f.

While at it, I should mention another documentation problem: rebase’s
man page does not explain that rebase --interactive will fast-forward at
the beginning of the series.  It should say that rebase -i will do that
and rebase without -i will not.  I can prepare a patch to fix it this
evening.

>  - Update revert-a-faulty-merge.txt to explain how to use "rebase [-i] -f".

Sounds interesting.  Simpler to use "rebase (-f | -i --no-ff)", I think.
Or maybe rebase could learn a no-op --no-ff option to make it easier to
switch between it and rebase -i, and this would become
"rebase (-f | -i) --no-ff".

Jonathan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCHv3] Teach -f/--force-rebase option to 'rebase -i'.
  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
                                         ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Junio C Hamano @ 2010-03-23 22:42 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git, Johannes Sixt, Jonathan Nieder

Marc Branchaud <marcnarc@xiplink.com> writes:

> This option tells rebase--interactive to cherry-pick all the commits in the
> rebased branch, instead of fast-forwarding over any unchanged commits.
>
> Also expanded -f's description in rebase's man page.
>
> -f offers an alterntive way to deal with reverted merges.  Instead of
> "reverting the revert" you can use "rebase -f [-i]" 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.  Added an
> addendum to revert-a-faulty-merge.txt describing the situation and how to
> use -f to handle it.
>
> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
> ---
>
> It seems more natural to me to just teach -f to "rebase -i" instead of adding
> a new --no-ff option.
>
> This patch combines my initial "--no-ff" patch with the documentation changes
> in my "reword" patch.  I moved the unit test for "rebase -f -i" into
> t3404-rebase-interactive.sh.

Hmm, to be honest, this is not what I expected.

A non-interactive "rebase" will _refuse_ to do anything, because it knows
that the end result will be exactly the same, in a fast-forward situation.
In general, the option "--force" is "I know the precondition I am in will
trigger your logic to refuse this request---but please do so anyway", and
the use of that option in non-interactive one is in line with that
definition.  So "--force" makes perfect sense to non-interactive rebase.

But "rebase -i" cannot _refuse_ when the range to be rebased is already
fast-forward, as the whole point of the command is to be able to rewrite
chosen commits, and that "rewriting chosen commits" is the reason why it
makes sense for the command to fast-forward the earlier picks that were
not chosen to be rewritten.  But "forcing" doesn't make sense, if the
command does not even refuse to begin with.  So "--no-ff" is sensible
there.

What I meant was that as a measure to prepare for a future merge of
(corrected) side branch whose earlier merge was once reverted, it would
make sense to use "rebase -f" without any "interactive", compared to the
"start rebase -i, and to ensure it wouldn't fast-forward, lie to it that
you will reword the first one and do nothing in reality", which your
earlier documentation suggested.

If anything, teaching --no-ff to non-interactive rebase would also make
sense.  Just like saying "I know you will refuse if it is a fast forward,
but please do so anyway" with --force makes sense, it is sensible to say
"I know you will make this a no-op if it is a fast forward, but please
rewrite all of them" with --no-ff.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCHv4 0/2] Teach the --no-ff option to 'rebase -i'.
  2010-03-23 22:42                     ` Junio C Hamano
@ 2010-03-24 15:40                       ` Marc Branchaud
  2010-03-24 17:13                         ` 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
  2 siblings, 1 reply; 32+ messages in thread
From: Marc Branchaud @ 2010-03-24 15:40 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt, Jonathan Nieder, Junio C Hamano

Thanks to both Junio and Jonathan for your patience in working through this.

Junio, I think I see what you mean.  I like teaching --no-ff to 'rebase -i'
because it allows me to combine two commands into one for this situation.

I've split this work into two patches:

The first one simply teaches "rebase -i" to accept and ignore -f.  I feel
this is better than adding text to the man page explaining why interactive
rebase has --no-ff but not -f, while non-interactive has the opposite.

The second is a re-roll of the --no-ff patch.  The only significant
differences are in the documentation:

 - The rebase man page mentions reverting a merge under both the -f and
   --no-ff options.

 - Rewrote the last 3 paragraphs of the revert-a-faulty-merge.txt howto
   (starting at "But if you don't ...").

I'm wondering now if it would make sense to also teach non-interactive
rebase to accept --no-ff as a synonym for -f.  Thoughts?

		M.

 Documentation/git-rebase.txt                  |   23 +++++++++-
 Documentation/howto/revert-a-faulty-merge.txt |   61 +++++++++++++++++++++++++
 git-rebase--interactive.sh                    |   13 +++++-
 t/t3404-rebase-interactive.sh                 |   36 +++++++++++++--
 4 files changed, 125 insertions(+), 8 deletions(-)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/2] Teach 'rebase -i' to accept and ignore the -f/--force-rebase option.
  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 15:41                       ` Marc Branchaud
  2010-03-24 15:41                       ` [PATCH 2/2] Teach the --no-ff option to 'rebase -i' Marc Branchaud
  2 siblings, 0 replies; 32+ messages in thread
From: Marc Branchaud @ 2010-03-24 15:41 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt, Jonathan Nieder, Junio C Hamano, Marc Branchaud

An interactive rebase implies -f:  The user wants to rewrite some of the
branch's own commits.
---
 git-rebase--interactive.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3e4fd14..aa0ab1a 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -22,6 +22,7 @@ p,preserve-merges  try to recreate merges instead of ignoring them
 s,strategy=        use the given merge strategy
 m,merge            always used (no-op)
 i,interactive      always used (no-op)
+f,force-rebase     always used (no-op)
  Actions:
 continue           continue rebasing process
 abort              abort rebasing process and restore original branch
@@ -733,6 +734,9 @@ first and then run 'git rebase --continue' again."
 	-m)
 		# we use merge anyway
 		;;
+	-f)
+		# we don't need to be forced
+		;;
 	-v)
 		VERBOSE=t
 		;;
-- 
1.7.0.3.1.g59254.dirty

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/2] Teach the --no-ff option to 'rebase -i'.
  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 15:41                       ` [PATCH 1/2] Teach 'rebase -i' to accept and ignore the -f/--force-rebase option Marc Branchaud
@ 2010-03-24 15:41                       ` Marc Branchaud
  2010-03-24 19:06                         ` Junio C Hamano
  2 siblings, 1 reply; 32+ messages in thread
From: Marc Branchaud @ 2010-03-24 15:41 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt, Jonathan Nieder, Junio C Hamano, Marc Branchaud

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

--no-ff offers an alterntive way to deal with reverted merges.  Instead of
"reverting the revert" you can use "rebase -i --no-ff" 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.  Added an
addendum to revert-a-faulty-merge.txt describing the situation and how to
use --no-ff to handle it.

--force-rebase also provides this alternative with non-interactive rebase,
so updated rebase's man page to mention it.

Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---
 Documentation/git-rebase.txt                  |   23 +++++++++-
 Documentation/howto/revert-a-faulty-merge.txt |   61 +++++++++++++++++++++++++
 git-rebase--interactive.sh                    |    9 +++-
 t/t3404-rebase-interactive.sh                 |   36 +++++++++++++--
 4 files changed, 121 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 823f2a4..ca06c55 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -274,9 +274,15 @@ which makes little sense.
 -f::
 --force-rebase::
 	Force the rebase even if the current branch is a descendant
-	of the commit you are rebasing onto.  Normally the command will
+	of the commit you are rebasing onto.  Normally non-interactive rebase will
 	exit with the message "Current branch is up to date" in such a
 	situation.
++
+You may find this (or --no-ff with an interactive rebase) helpful after
+reverting a topic branch merge, as this option recreates the topic branch with
+fresh commits so it can be remerged successfully without needing to "reverting
+the reversion" (as described in the
+link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To]).
 
 --ignore-whitespace::
 --whitespace=<option>::
@@ -316,7 +322,20 @@ which makes little sense.
 	commit to be modified, and change the action of the moved
 	commit from `pick` to `squash` (or `fixup`).
 +
-This option is only valid when '--interactive' option is used.
+This option is only valid when the '--interactive' option is used.
+
+--no-ff::
+	Cherry-pick all rebased commits instead of fast-forwarding over
+	the unchanged ones.  This ensures that the entire history of the
+	rebased branch is composed of new commits.
++
+This option is only valid when the '--interactive' option is used.
++
+You may find this (or --force-rebase with a non-interactive rebase) helpful
+after reverting a topic branch merge, as this option recreates the topic
+branch with fresh commits so it can be remerged successfully without needing
+to "reverting the reversion" (as described in the
+link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To]).
 
 include::merge-strategies.txt[]
 
diff --git a/Documentation/howto/revert-a-faulty-merge.txt b/Documentation/howto/revert-a-faulty-merge.txt
index 3b4a390..eaa8b32 100644
--- a/Documentation/howto/revert-a-faulty-merge.txt
+++ b/Documentation/howto/revert-a-faulty-merge.txt
@@ -142,6 +142,8 @@ different resolution strategies:
    revert of a merge was rebuilt from scratch (i.e. rebasing and fixing,
    as you seem to have interpreted), then re-merging the result without
    doing anything else fancy would be the right thing to do.
+   (See the ADDENDUM below for how to rebuild a branch from scratch
+   without changing its original branching-off point.)
 
 However, there are things to keep in mind when reverting a merge (and
 reverting such a revert).
@@ -177,3 +179,62 @@ the answer is: "oops, I really shouldn't have merged it, because it wasn't
 ready yet, and I really need to undo _all_ of the merge"). So then you
 really should revert the merge, but when you want to re-do the merge, you
 now need to do it by reverting the revert.
+
+ADDENDUM
+
+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.
+
+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
+
+Now you can merge A'-B'-C' into the mainline branch without worrying about
+first reverting W.
+
+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
+
+Both these commands create a new branch A'-B'-C' with all-new commits (all the
+SHA IDs will be different) even if in the interactive case you only actually
+modify commit B.  You can then merge this new branch directly into the mainline
+branch and be sure you'll get all of the branch's changes.
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index aa0ab1a..42aa705 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -20,6 +20,7 @@ v,verbose          display a diffstat of what changed upstream
 onto=              rebase onto given branch instead of upstream
 p,preserve-merges  try to recreate merges instead of ignoring them
 s,strategy=        use the given merge strategy
+no-ff              cherry-pick all commits, even if unchanged
 m,merge            always used (no-op)
 i,interactive      always used (no-op)
 f,force-rebase     always used (no-op)
@@ -104,6 +105,7 @@ VERBOSE=
 OK_TO_SKIP_PRE_REBASE=
 REBASE_ROOT=
 AUTOSQUASH=
+NEVER_FF=
 
 GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
 mark the corrected paths with 'git add <paths>', and
@@ -223,7 +225,7 @@ do_with_author () {
 }
 
 pick_one () {
-	no_ff=
+	no_ff=$NEVER_FF
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
 	test -d "$REWRITTEN" &&
@@ -746,6 +748,9 @@ first and then run 'git rebase --continue' again."
 	-i)
 		# yeah, we know
 		;;
+	--no-ff)
+		NEVER_FF=t
+		;;
 	--root)
 		REBASE_ROOT=t
 		;;
@@ -931,7 +936,7 @@ EOF
 		has_action "$TODO" ||
 			die_abort "Nothing to do"
 
-		test -d "$REWRITTEN" || skip_unnecessary_picks
+		test -d "$REWRITTEN" || test -n "$NEVER_FF" || skip_unnecessary_picks
 
 		git update-ref ORIG_HEAD $HEAD
 		output git checkout $ONTO && do_rest
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4e35137..624e78e 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -22,12 +22,18 @@ set_fake_editor
 # | \
 # |   F - G - H                (branch1)
 # |     \
-#  \      I                    (branch2)
-#   \
-#     J - K - L - M            (no-conflict-branch)
+# |\      I                    (branch2)
+# | \
+# |   J - K - L - M            (no-conflict-branch)
+#  \
+#    N - O - P                 (no-ff-branch)
 #
 # where A, B, D and G all touch file1, and one, two, three, four all
 # touch file "conflict".
+#
+# WARNING: Modifications to the initial repository can change the SHA ID used
+# in the expect2 file for the 'stop on conflicting pick' test.
+
 
 test_expect_success 'setup' '
 	test_commit A file1 &&
@@ -50,6 +56,11 @@ test_expect_success 'setup' '
 	for n in J K L M
 	do
 		test_commit $n file$n
+	done &&
+	git checkout -b no-ff-branch A &&
+	for n in N O P
+	do
+		test_commit $n file$n
 	done
 '
 
@@ -113,7 +124,7 @@ cat > expect2 << EOF
 D
 =======
 G
->>>>>>> 51047de... G
+>>>>>>> 5d18e54... G
 EOF
 
 test_expect_success 'stop on conflicting pick' '
@@ -553,4 +564,21 @@ test_expect_success 'reword' '
 	git show HEAD~2 | grep "C changed"
 '
 
+test_tick # Ensure that the rebased commits get a different timestamp.
+test_expect_success 'always cherry-pick with --no-ff' '
+	git checkout no-ff-branch &&
+	git tag original-no-ff-branch &&
+	git rebase -i --no-ff A &&
+	touch empty &&
+	for p in 0 1 2
+	do
+		test ! $(git rev-parse HEAD~$p) = $(git rev-parse original-no-ff-branch~$p) &&
+		git diff HEAD~$p original-no-ff-branch~$p > out &&
+		test_cmp empty out
+	done &&
+	test $(git rev-parse HEAD~3) = $(git rev-parse original-no-ff-branch~3) &&
+	git diff HEAD~3 original-no-ff-branch~3 > out &&
+	test_cmp empty out
+'
+
 test_done
-- 
1.7.0.3.1.g59254.dirty

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCHv4 0/2] Teach the --no-ff option to 'rebase -i'.
  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
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2010-03-24 17:13 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git, Johannes Sixt, Jonathan Nieder

Marc Branchaud <marcnarc@xiplink.com> writes:

> Thanks to both Junio and Jonathan for your patience in working through this.
>
> Junio, I think I see what you mean.  I like teaching --no-ff to 'rebase -i'
> because it allows me to combine two commands into one for this situation.
>
> I've split this work into two patches:
>
> The first one simply teaches "rebase -i" to accept and ignore -f.  I feel
> this is better than adding text to the man page explaining why interactive
> rebase has --no-ff but not -f, while non-interactive has the opposite.

I am mildly against that change; I cannot explain why "rebase -i" accepts
and then ignores "-f" myself to the users.  "Non interactive one knows -f
but interactive doesn't" is not necessarily an inconsistency.  "--force"
is about forcing what would normally rejected, and if the latter doesn't
have anything that normally rejects, it doesn't need to have a "--force".

On the other hand, I wouldn't mind at all if you were to add "--no-ff" to
non-interactive rebase; that has a much easier and logical explanation.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] Teach the --no-ff option to 'rebase -i'.
  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
  0 siblings, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2010-03-24 19:06 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git, Johannes Sixt, Jonathan Nieder

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'

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCHv5] Teach rebase the --no-ff option.
  2010-03-24 17:13                         ` Junio C Hamano
@ 2010-03-24 20:34                           ` Marc Branchaud
  2010-03-24 21:45                             ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Marc Branchaud @ 2010-03-24 20:34 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt, Jonathan Nieder, Junio C Hamano, Marc Branchaud

For git-rebase.sh, --no-ff is a synonym for --force-rebase.

For git-rebase--interactive.sh, --no-ff cherry-picks all the commits in
the rebased branch, instead of fast-forwarding over any unchanged commits.

--no-ff offers an alternative way to deal with reverted merges.  Instead of
"reverting the revert" you can use "rebase --no-ff" 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.  Added an
addendum to revert-a-faulty-merge.txt describing the situation and how to
use --no-ff to handle it.

Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---

Now also makes git-rebase.sh treat --no-ff as a synonym for -f.  I think I
got the man page right...

Also rewrote the addendum to revert-a-faulty-merge.txt to (hopefully) reflect
Junio's comments.

I agree about not teaching -f to rebase--interative.  I was only mildly for
it to begin with.  It also occurred to me that we don't want users to think
that "rebase -i" treats -f the same way as it does --no-ff, but making
rebase--interactive silently accept -f might encourage that mistake.

		M.

 Documentation/git-rebase.txt                  |   23 ++++++-
 Documentation/howto/revert-a-faulty-merge.txt |   90 +++++++++++++++++++++++++
 git-rebase--interactive.sh                    |    9 ++-
 git-rebase.sh                                 |    4 +-
 t/t3404-rebase-interactive.sh                 |   36 +++++++++-
 5 files changed, 152 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 823f2a4..0d07b1b 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -274,9 +274,16 @@ which makes little sense.
 -f::
 --force-rebase::
 	Force the rebase even if the current branch is a descendant
-	of the commit you are rebasing onto.  Normally the command will
+	of the commit you are rebasing onto.  Normally non-interactive rebase will
 	exit with the message "Current branch is up to date" in such a
 	situation.
+	Incompatible with the --interactive option.
++
+You may find this (or --no-ff with an interactive rebase) helpful after
+reverting a topic branch merge, as this option recreates the topic branch with
+fresh commits so it can be remerged successfully without needing to "revert
+the reversion" (see the
+link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To] for details).
 
 --ignore-whitespace::
 --whitespace=<option>::
@@ -316,7 +323,19 @@ which makes little sense.
 	commit to be modified, and change the action of the moved
 	commit from `pick` to `squash` (or `fixup`).
 +
-This option is only valid when '--interactive' option is used.
+This option is only valid when the '--interactive' option is used.
+
+--no-ff::
+	With --interactive, cherry-pick all rebased commits instead of
+	fast-forwarding over the unchanged ones.  This ensures that the
+	entire history of the rebased branch is composed of new commits.
++
+Without --interactive, this is a synonym for --force-rebase.
++
+You may find this helpful after reverting a topic branch merge, as this option
+recreates the topic branch with fresh commits so it can be remerged
+successfully without needing to "revert the reversion" (see the
+link:howto/revert-a-faulty-merge.txt[revert-a-faulty-merge How-To] for details).
 
 include::merge-strategies.txt[]
 
diff --git a/Documentation/howto/revert-a-faulty-merge.txt b/Documentation/howto/revert-a-faulty-merge.txt
index 3b4a390..ff5c0bc 100644
--- a/Documentation/howto/revert-a-faulty-merge.txt
+++ b/Documentation/howto/revert-a-faulty-merge.txt
@@ -142,6 +142,8 @@ different resolution strategies:
    revert of a merge was rebuilt from scratch (i.e. rebasing and fixing,
    as you seem to have interpreted), then re-merging the result without
    doing anything else fancy would be the right thing to do.
+   (See the ADDENDUM below for how to rebuild a branch from scratch
+   without changing its original branching-off point.)
 
 However, there are things to keep in mind when reverting a merge (and
 reverting such a revert).
@@ -177,3 +179,91 @@ the answer is: "oops, I really shouldn't have merged it, because it wasn't
 ready yet, and I really need to undo _all_ of the merge"). So then you
 really should revert the merge, but when you want to re-do the merge, you
 now need to do it by reverting the revert.
+
+ADDENDUM
+
+Sometimes you have to rewrite one of a topic branch's commits *and* you can't
+change the topic's branching-off point.  Consider the following situation:
+
+ P---o---o---M---x---x---W---x
+  \         /
+   A---B---C
+
+where commit W reverted commit M because it turned out that commit B was wrong
+and needs to be rewritten, but you need the rewritten topic to still branch
+from commit P (perhaps P is a branching-off point for yet another branch, and
+you want be able to merge the topic into both branches).
+
+The natural thing to do in this case is to checkout the A-B-C branch and use
+"rebase -i P" to change commit B.  However this does not rewrite commit A,
+because "rebase -i" by default fast-forwards over any initial commits selected
+with the "pick" command.  So you end up with this:
+
+ P---o---o---M---x---x---W---x
+  \         /
+   A---B---C   <-- old branch
+    \
+     B'---C'   <-- naively 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.
+
+However, you can avoid these problems if you recreate the entire branch,
+including commit A:
+
+   A'---B'---C'  <-- completely rewritten branch
+  /
+ P---o---o---M---x---x---W---x
+  \         /
+   A---B---C
+
+You can merge A'-B'-C' into the mainline branch without worrying about first
+reverting W.  Mainline's history would look like this:
+
+   A'---B'---C'------------------
+  /                              \
+ P---o---o---M---x---x---W---x---M2
+  \         /
+   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
+--no-ff option provides a way to do this:
+
+    $ git rebase [-i] --no-ff P
+
+The --no-ff option creates a new branch A'-B'-C' with all-new commits (all the
+SHA IDs will be different) even if in the interactive case you only actually
+modify commit B.  You can then merge this new branch directly into the mainline
+branch and be sure you'll get all of the branch's changes.
+
+You can also use --no-ff in cases where you just add extra commits to the topic
+to fix it up.  Let's revisit the situation discussed at the start of this howto:
+
+ P---o---o---M---x---x---W---x
+  \         /
+   A---B---C----------------D---E   <-- fixed-up topic branch
+
+At this point, you can use --no-ff to recreate the topic branch:
+
+    $ git checkout E
+    $ git rebase --no-ff P
+
+yielding
+
+   A'---B'---C'------------D'---E'  <-- recreated topic branch
+  /
+ P---o---o---M---x---x---W---x
+  \         /
+   A---B---C----------------D---E
+
+You can merge the recreated branch into the mainline without reverting commit W,
+and mainline's history will look like this:
+
+   A'---B'---C'------------D'---E'
+  /                              \
+ P---o---o---M---x---x---W---x---M2
+  \         /
+   A---B---C
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3e4fd14..d5468b0 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -20,6 +20,7 @@ v,verbose          display a diffstat of what changed upstream
 onto=              rebase onto given branch instead of upstream
 p,preserve-merges  try to recreate merges instead of ignoring them
 s,strategy=        use the given merge strategy
+no-ff              cherry-pick all commits, even if unchanged
 m,merge            always used (no-op)
 i,interactive      always used (no-op)
  Actions:
@@ -103,6 +104,7 @@ VERBOSE=
 OK_TO_SKIP_PRE_REBASE=
 REBASE_ROOT=
 AUTOSQUASH=
+NEVER_FF=
 
 GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
 mark the corrected paths with 'git add <paths>', and
@@ -222,7 +224,7 @@ do_with_author () {
 }
 
 pick_one () {
-	no_ff=
+	no_ff=$NEVER_FF
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
 	test -d "$REWRITTEN" &&
@@ -742,6 +744,9 @@ first and then run 'git rebase --continue' again."
 	-i)
 		# yeah, we know
 		;;
+	--no-ff)
+		NEVER_FF=t
+		;;
 	--root)
 		REBASE_ROOT=t
 		;;
@@ -927,7 +932,7 @@ EOF
 		has_action "$TODO" ||
 			die_abort "Nothing to do"
 
-		test -d "$REWRITTEN" || skip_unnecessary_picks
+		test -d "$REWRITTEN" || test -n "$NEVER_FF" || skip_unnecessary_picks
 
 		git update-ref ORIG_HEAD $HEAD
 		output git checkout $ONTO && do_rest
diff --git a/git-rebase.sh b/git-rebase.sh
index fb4fef7..8b23f8b 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2005 Junio C Hamano.
 #
 
-USAGE='[--interactive | -i] [-v] [--force-rebase | -f] [--onto <newbase>] [<upstream>|--root] [<branch>] [--quiet | -q]'
+USAGE='[--interactive | -i] [-v] [--force-rebase | -f] [--no-ff] [--onto <newbase>] [<upstream>|--root] [<branch>] [--quiet | -q]'
 LONG_USAGE='git-rebase replaces <branch> with a new branch of the
 same name.  When the --onto option is provided the new branch starts
 out with a HEAD equal to <newbase>, otherwise it is equal to <upstream>
@@ -347,7 +347,7 @@ do
 	--root)
 		rebase_root=t
 		;;
-	-f|--f|--fo|--for|--forc|force|--force-r|--force-re|--force-reb|--force-reba|--force-rebas|--force-rebase)
+	-f|--f|--fo|--for|--forc|force|--force-r|--force-re|--force-reb|--force-reba|--force-rebas|--force-rebase|--no-ff)
 		force_rebase=t
 		;;
 	--rerere-autoupdate|--no-rerere-autoupdate)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4e35137..624e78e 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -22,12 +22,18 @@ set_fake_editor
 # | \
 # |   F - G - H                (branch1)
 # |     \
-#  \      I                    (branch2)
-#   \
-#     J - K - L - M            (no-conflict-branch)
+# |\      I                    (branch2)
+# | \
+# |   J - K - L - M            (no-conflict-branch)
+#  \
+#    N - O - P                 (no-ff-branch)
 #
 # where A, B, D and G all touch file1, and one, two, three, four all
 # touch file "conflict".
+#
+# WARNING: Modifications to the initial repository can change the SHA ID used
+# in the expect2 file for the 'stop on conflicting pick' test.
+
 
 test_expect_success 'setup' '
 	test_commit A file1 &&
@@ -50,6 +56,11 @@ test_expect_success 'setup' '
 	for n in J K L M
 	do
 		test_commit $n file$n
+	done &&
+	git checkout -b no-ff-branch A &&
+	for n in N O P
+	do
+		test_commit $n file$n
 	done
 '
 
@@ -113,7 +124,7 @@ cat > expect2 << EOF
 D
 =======
 G
->>>>>>> 51047de... G
+>>>>>>> 5d18e54... G
 EOF
 
 test_expect_success 'stop on conflicting pick' '
@@ -553,4 +564,21 @@ test_expect_success 'reword' '
 	git show HEAD~2 | grep "C changed"
 '
 
+test_tick # Ensure that the rebased commits get a different timestamp.
+test_expect_success 'always cherry-pick with --no-ff' '
+	git checkout no-ff-branch &&
+	git tag original-no-ff-branch &&
+	git rebase -i --no-ff A &&
+	touch empty &&
+	for p in 0 1 2
+	do
+		test ! $(git rev-parse HEAD~$p) = $(git rev-parse original-no-ff-branch~$p) &&
+		git diff HEAD~$p original-no-ff-branch~$p > out &&
+		test_cmp empty out
+	done &&
+	test $(git rev-parse HEAD~3) = $(git rev-parse original-no-ff-branch~3) &&
+	git diff HEAD~3 original-no-ff-branch~3 > out &&
+	test_cmp empty out
+'
+
 test_done
-- 
1.7.0.3.1.g7f7ff

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCHv5] Teach rebase the --no-ff option.
  2010-03-24 20:34                           ` [PATCHv5] Teach rebase the --no-ff option Marc Branchaud
@ 2010-03-24 21:45                             ` Junio C Hamano
  0 siblings, 0 replies; 32+ messages in thread
From: Junio C Hamano @ 2010-03-24 21:45 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: git, Johannes Sixt, Jonathan Nieder

Marc Branchaud <marcnarc@xiplink.com> writes:

> For git-rebase.sh, --no-ff is a synonym for --force-rebase.

Thanks, will queue.

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2010-03-24 21:45 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.