Skip to contentSkip to conversation

Do not ignore merge options in interactive rebase

PATCH In review Arnaud Fontaine 2 participants 9 messages Jun 15, 2016

lore
Share & tools
lore

Arnaud Fontaine

author
#0 lore raw

PATCH 1/1 Do not ignore merge options in interactive rebase

Merge strategy and its options can be specified in git rebase, but with --interactive, they were completely ignored.

Signed-off-by Arnaud Fontaine <[email protected]>
---
 git-rebase--interactive.sh    | 11 ++++++++++-
 t/t3404-rebase-interactive.sh | 11 +++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)
 mode change 100644 => 100755 git-rebase--interactive.sh
git-rebase--interactive.sh +10 -1
old mode 100644
new mode 100755
index f953d8d..c157fdf
@@ -239,7 +239,16 @@ pick_one () {
 
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output git cherry-pick $empty_args $ff "$@"
+
+	if test -n "$do_merge"
+	then
+		test -z "$strategy" && strategy=recursive
+		output git cherry-pick --strategy=$strategy \
+			$(echo $strategy_opts | sed "s/'--\([^']*\)'/-X\1/g") \
+			$empty_args $ff "$@"
+	else
+		output git cherry-pick $empty_args $ff "$@"
+	fi
 }
 
 pick_one_preserving_merges () {
t/t3404-rebase-interactive.sh +11 -0
index 79e8d3c..8b6a36f 100755
@@ -947,4 +947,15 @@ test_expect_success 'rebase -i respects core.commentchar' '
 	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
 
+test_expect_success 'rebase -i with --strategy and -X' '
+	git checkout -b conflict-merge-use-theirs conflict-branch &&
+	git reset --hard HEAD^ &&
+	echo five >conflict &&
+	echo Z >file1 &&
+	git commit -a -m "one file conflict" &&
+	EDITOR=true git rebase -i --strategy=recursive -Xours conflict-branch &&
+	test $(git show conflict-branch:conflict) = $(cat conflict) &&
+	test $(cat file1) = Z
+'
+
 test_done
Signature · 1 line
1.8.3.GIT
#1 lore raw
Quoted 2 lines · Arnaud Fontaine

Merge strategy and its options can be specified in git rebase, but with --interactive, they were completely ignored.

And why is it a bad thing? If you meant s/--interactive/-m/ in the above, then I can sort of understand the justification, though.

git-rebase--interactive.sh

Quoted 3 lines
git-rebase--interactive.sh +0 -0
old mode 100644
new mode 100755

I see an unjustifiable mode change here.

git-rebase--interactive.sh pick_one () { · line 244

Quoted 11 lines

index f953d8d..c157fdf

git-rebase--interactive.sh +3 -1
@@ -239,7 +239,16 @@ pick_one () {
 
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output git cherry-pick $empty_args $ff "$@"
+
+	if test -n "$do_merge"
+	then

So you _did_ mean "rebase -m"?

git-rebase--interactive.sh pick_one () { · line 246

Quoted 2 lines
git-rebase--interactive.sh +2 -0
+		test -z "$strategy" && strategy=recursive
+		output git cherry-pick --strategy=$strategy \

This is a bad change.

I would understand if the above were:

	git cherry-pick ${strategy+--strategy=$strategy} ...

in other words, "if there is no strategy specified, do not override the configured default that might be different from recursive" (pull.twohead may be set to resolve).

git-rebase--interactive.sh pick_one () { · line 247

Quoted 1 line
git-rebase--interactive.sh +1 -0
+			$(echo $strategy_opts | sed "s/'--\([^']*\)'/-X\1/g") \

Is it guaranteed $startegy_opts do not have a space in it?

There is a call to "git merge" that uses "${strategy+-s $strategy}", but it does not seem to propagate the strategy option. Does it need a similar change? It seems that the first step might be to factor out these calls to the "git cherry-pick" and "git merge" to helper functions to make it easier to call them with -s/-X options in a consistent way.

git-rebase--interactive.sh pick_one () { · line 251

Quoted 4 lines
git-rebase--interactive.sh +4 -0
+			$empty_args $ff "$@"
+	else
+		output git cherry-pick $empty_args $ff "$@"
+	fi

It seems that there is another call to "git cherry-pick" in the script ("git grep" for it). Does it need a similar change?

author
#2 lore raw
Quoted 7 lines · Junio C Hamano
Quoted 2 lines · Arnaud Fontaine

Merge strategy and its options can be specified in git rebase, but with --interactive, they were completely ignored.

And why is it a bad thing? If you meant s/--interactive/-m/ in the above, then I can sort of understand the justification, though.

Sorry, it was not clear. I meant that you can do 'rebase -m --strategy recursive'. But with 'rebase --interactive -m --strategy recursive', '-m --strategy recursive' is ignored. To me, this is not consistent because
the behavior is different in interactive mode... Personally, I needed
to specify the strategy and its options while using interactive mode and it seems I'm not the only one[0].

Quoted 5 lines

git-rebase--interactive.sh

Quoted 3 lines
git-rebase--interactive.sh +0 -0
old mode 100644
new mode 100755

I see an unjustifiable mode change here.

Sorry about that, I fixed it.

Quoted 5 lines
Quoted 5 lines

index f953d8d..c157fdf

git-rebase--interactive.sh +0 -0
@@ -239,7 +239,16 @@ pick_one () {
Quoted 8 lines

git-rebase--interactive.sh pick_one () { · line 244

Quoted 6 lines
git-rebase--interactive.sh +3 -1
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output git cherry-pick $empty_args $ff "$@"
+
+	if test -n "$do_merge"
+	then

So you _did_ mean "rebase -m"?

I really meant 'rebase --interactive -m'. do_merge is set to true if either '--strategy' or '-m' or '-X' is given according to git-rebase.sh.

Quoted 5 lines

git-rebase--interactive.sh pick_one () { · line 246

Quoted 2 lines
git-rebase--interactive.sh +2 -0
+		test -z "$strategy" && strategy=recursive
+		output git cherry-pick --strategy=$strategy \

This is a bad change.

Quoted 7 lines

I would understand if the above were:

	git cherry-pick ${strategy+--strategy=$strategy} ...

in other words, "if there is no strategy specified, do not override the configured default that might be different from recursive" (pull.twohead may be set to resolve).

Indeed, I did not know about that. I wrongly thought it was a good idea
to do the same as both git-rebase (when -X is given) and
git-rebase--merge which do the same test ('test -z "$strategy" && strategy=recursive'). However after checking more carefully, I guess that, for the former case, it is because only recursive currently takes options, whereas, for the latter case, it is to call a default git-rebase-$strategy.

Quoted 3 lines

git-rebase--interactive.sh pick_one () { · line 247

Quoted 1 line
git-rebase--interactive.sh +1 -0
+			$(echo $strategy_opts | sed "s/'--\([^']*\)'/-X\1/g") \

Is it guaranteed $startegy_opts do not have a space in it?

strategy_opts may be something like (git-rebase.sh): "'--foo' '--bar'", but I'm not sure what is wrong if there is a space in it though.

Quoted 6 lines

There is a call to "git merge" that uses "${strategy+-s $strategy}", but it does not seem to propagate the strategy option. Does it need a similar change? It seems that the first step might be to factor out these calls to the "git cherry-pick" and "git merge" to helper functions to make it easier to call them with -s/-X options in a consistent way.

As far as I understand, yes. I changed it. As it is really short, I just added an if/else inside the script itself, not sure if that's ok...

Quoted 7 lines

git-rebase--interactive.sh pick_one () { · line 251

Quoted 4 lines
git-rebase--interactive.sh +4 -0
+			$empty_args $ff "$@"
+	else
+		output git cherry-pick $empty_args $ff "$@"
+	fi

It seems that there is another call to "git cherry-pick" in the script ("git grep" for it). Does it need a similar change?

As far as I understand, yes. So I changed it as well.

I have sent the fixed patch in my next email. Many thanks for the review!

Cheers,

Signature · 3 lines
Arnaud Fontaine

[0] http://git.661346.n2.nabble.com/git-rebase-interactive-does-not-respect-merge-options-td7500093.html
#3 lore raw
Quoted 6 lines · Arnaud Fontaine
Quoted 3 lines

git-rebase--interactive.sh pick_one () { · line 247

Quoted 1 line
git-rebase--interactive.sh +1 -0
+			$(echo $strategy_opts | sed "s/'--\([^']*\)'/-X\1/g") \

Is it guaranteed $startegy_opts do not have a space in it?

strategy_opts may be something like (git-rebase.sh): "'--foo' '--bar'", but I'm not sure what is wrong if there is a space in it though.

I was primarily worried about "'--frotz=nitfol xyzzy'", where you need to pass -X='frotz=nitfol xyzzy' so that 'xyzzy' part does not become a separate argument directly given to 'git merge' and friends.

And adding '' around \1 is not sufficient, because the value given to the "--frotz" may have to be "nitfol 'n xyzzy".

A comment next to that sed script that says "Currently there is no strategy option that needs quoting" (if it is the case; I didn't check), together with a guard to protect us against unexpected strategy-opts, might be a workable band-aid, though.

author
#4 lore raw

PATCH 1/1 Do not ignore merge options in interactive rebase

In reply to Arnaud Fontaine

Fix inconsistency where --strategy and/or --strategy-option can be specified in git rebase, but with --interactive argument only there were completely ignored.

Signed-off-by Arnaud Fontaine <[email protected]>
---
 git-rebase--interactive.sh    | 13 ++++++++++---
 t/t3404-rebase-interactive.sh | 11 +++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)
git-rebase--interactive.sh +10 -3
index f953d8d..e558397 100644
@@ -80,6 +80,13 @@ amend="$state_dir"/amend
 rewritten_list="$state_dir"/rewritten-list
 rewritten_pending="$state_dir"/rewritten-pending
 
+strategy_args=
+if test -n "$do_merge"
+then
+	strategy_args="${strategy+--strategy=$strategy}
+ $(echo $strategy_opts | sed "s/'--\([^']*\)'/-X\1/g")"
+fi
+
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
 

@@ -239,7 +246,7 @@ pick_one () {
 
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output git cherry-pick $empty_args $ff "$@"
+	output git cherry-pick $strategy_args $empty_args $ff "$@"
 }
 
 pick_one_preserving_merges () {

@@ -341,7 +348,7 @@ pick_one_preserving_merges () {
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
 			if ! do_with_author output \
-				git merge --no-ff ${strategy:+-s $strategy} -m \
+				git merge --no-ff $strategy_args -m \
 					"$msg_content" $new_parents
 			then
 				printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG

@@ -350,7 +357,7 @@ pick_one_preserving_merges () {
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
 		*)
-			output git cherry-pick "$@" ||
+			output git cherry-pick $strategy_args "$@" ||
 				die_with_patch $sha1 "Could not pick $sha1"
 			;;
 		esac
t/t3404-rebase-interactive.sh +11 -0
index 79e8d3c..8b6a36f 100755
@@ -947,4 +947,15 @@ test_expect_success 'rebase -i respects core.commentchar' '
 	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
 
+test_expect_success 'rebase -i with --strategy and -X' '
+	git checkout -b conflict-merge-use-theirs conflict-branch &&
+	git reset --hard HEAD^ &&
+	echo five >conflict &&
+	echo Z >file1 &&
+	git commit -a -m "one file conflict" &&
+	EDITOR=true git rebase -i --strategy=recursive -Xours conflict-branch &&
+	test $(git show conflict-branch:conflict) = $(cat conflict) &&
+	test $(cat file1) = Z
+'
+
 test_done
Signature · 1 line
1.8.3.GIT
#5 lore raw
Quoted 15 lines · Arnaud Fontaine

Fix inconsistency where --strategy and/or --strategy-option can be specified in git rebase, but with --interactive argument only there were completely ignored.

Signed-off-by Arnaud Fontaine <[email protected]>
---
 git-rebase--interactive.sh    | 13 ++++++++++---
 t/t3404-rebase-interactive.sh | 11 +++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)
git-rebase--interactive.sh +0 -0
index f953d8d..e558397 100644

git-rebase--interactive.sh amend="$state_dir"/amend · line 88

Quoted 9 lines
git-rebase--interactive.sh +6 -0
@@ -80,6 +80,13 @@ amend="$state_dir"/amend
 rewritten_list="$state_dir"/rewritten-list
 rewritten_pending="$state_dir"/rewritten-pending
 
+strategy_args=
+if test -n "$do_merge"
+then
+	strategy_args="${strategy+--strategy=$strategy}
+ $(echo $strategy_opts | sed "s/'--\([^']*\)'/-X\1/g")"
+fi

The "${var+if_set_use_this}" interpolates if $var is set (as opposed to "unset"); this will cause it to have "--strategy= " in strategy_args when $do_merge is set. If you ran t3421, you would have noticed breakage.

You should use ${var:+if_set_to_nonempty_use_this} here.

stragety_opts is designed to be a valid shell scriptlet that can be inserted as a part of eval'ed string. git-rebase.sh does this:

	-X)
		shift
		strategy_opts="$strategy_opts $(git rev-parse --sq-quote "--$1")"
		do_merge=t

so that git-rebase--merge.sh can use it like so:

	eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'

without having to worry about quoting issues when future strategy options have letters that need quoting, e.g.

	$ git rebase -X foo="bar ' baz"

git rev-parse --sq-quote turns it into '--foo=bar '\'' baz' and then in the eval'ed string, it becomes a single argument given to the git-merge-$strategy command, even though it may have spaces and single quotes and other characters that may be tricky to quote manually without mistakes.

So munging it manually with sloppy "sed" script is simply wrong.

Quoted 50 lines
git-rebase--interactive.sh +3 -3
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
 

@@ -239,7 +246,7 @@ pick_one () {
 
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output git cherry-pick $empty_args $ff "$@"
+	output git cherry-pick $strategy_args $empty_args $ff "$@"
 }
 
 pick_one_preserving_merges () {

@@ -341,7 +348,7 @@ pick_one_preserving_merges () {
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
 			if ! do_with_author output \
-				git merge --no-ff ${strategy:+-s $strategy} -m \
+				git merge --no-ff $strategy_args -m \
 					"$msg_content" $new_parents
 			then
 				printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG

@@ -350,7 +357,7 @@ pick_one_preserving_merges () {
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
 		*)
-			output git cherry-pick "$@" ||
+			output git cherry-pick $strategy_args "$@" ||
 				die_with_patch $sha1 "Could not pick $sha1"
 			;;
 		esac
t/t3404-rebase-interactive.sh +11 -0
index 79e8d3c..8b6a36f 100755
@@ -947,4 +947,15 @@ test_expect_success 'rebase -i respects core.commentchar' '
 	test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
 
+test_expect_success 'rebase -i with --strategy and -X' '
+	git checkout -b conflict-merge-use-theirs conflict-branch &&
+	git reset --hard HEAD^ &&
+	echo five >conflict &&
+	echo Z >file1 &&
+	git commit -a -m "one file conflict" &&
+	EDITOR=true git rebase -i --strategy=recursive -Xours conflict-branch &&
+	test $(git show conflict-branch:conflict) = $(cat conflict) &&
+	test $(cat file1) = Z
+'
+
 test_done
#6 lore raw
Quoted 3 lines · Junio C Hamano

You should use ${var:+if_set_to_nonempty_use_this} here.
...
So munging it manually with sloppy "sed" script is simply wrong.

Perhaps something like this on top of your patch squashed in?

The eval magic lets the shell to split $strategy_opts back into individual words (e.g. "--subtree=My Project" is a single word), strip the leading "--", and then uses "rev-parse --sq-quote" again to turn them into "-X 'subtree=My Project'" (two words), which can be inserted into a string later to be eval'ed.

 git-rebase--interactive.sh | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
git-rebase--interactive.sh +11 -7
index e558397..ae2da7a 100644
@@ -83,8 +83,13 @@ rewritten_pending="$state_dir"/rewritten-pending
 strategy_args=
 if test -n "$do_merge"
 then
-	strategy_args="${strategy+--strategy=$strategy}
- $(echo $strategy_opts | sed "s/'--\([^']*\)'/-X\1/g")"
+	strategy_args=${strategy:+--strategy=$strategy}
+	eval '
+		for strategy_opt in '"$strategy_opts"'
+		do
+			strategy_args="$strategy_args -X$(git rev-parse --sq-quote "${strategy_opt#--}")"
+		done
+	'
 fi
 
 GIT_CHERRY_PICK_HELP="$resolvemsg"

@@ -246,7 +251,7 @@ pick_one () {
 
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output git cherry-pick $strategy_args $empty_args $ff "$@"
+	output eval git cherry-pick "$strategy_args" $empty_args $ff "$@"
 }
 
 pick_one_preserving_merges () {

@@ -347,9 +352,8 @@ pick_one_preserving_merges () {
 			msg_content="$(commit_message $sha1)"
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
-			if ! do_with_author output \
-				git merge --no-ff $strategy_args -m \
-					"$msg_content" $new_parents
+			if ! do_with_author output eval \
+			'git merge --no-ff $strategy_args -m "$msg_content" $new_parents'
 			then
 				printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG
 				die_with_patch $sha1 "Error redoing merge $sha1"

@@ -357,7 +361,7 @@ pick_one_preserving_merges () {
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
 		*)
-			output git cherry-pick $strategy_args "$@" ||
+			output eval git cherry-pick "$strategy_args" "$@" ||
 				die_with_patch $sha1 "Could not pick $sha1"
 			;;
 		esac
author
#7 lore raw

PATCH 1/1 Do not ignore merge options in interactive rebase

Merge strategy and its options can be specified in git rebase, but with --interactive, they were completely ignored.

Signed-off-by Arnaud Fontaine <[email protected]>
---
 git-rebase--interactive.sh    | 21 ++++++++++++++++-----
 t/t3404-rebase-interactive.sh | 11 +++++++++++
 2 files changed, 27 insertions(+), 5 deletions(-)
git-rebase--interactive.sh +16 -5
index 169e876..157690b 100644
@@ -80,6 +80,18 @@ amend="$state_dir"/amend
 rewritten_list="$state_dir"/rewritten-list
 rewritten_pending="$state_dir"/rewritten-pending
 
+strategy_args=
+if test -n "$do_merge"
+then
+	strategy_args=${strategy:+--strategy=$strategy}
+	eval '
+		for strategy_opt in '"$strategy_opts"'
+		do
+			strategy_args="$strategy_args -X$(git rev-parse --sq-quote "${strategy_opt#--}")"
+		done
+	'
+fi
+
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
 

@@ -239,7 +251,7 @@ pick_one () {
 
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output git cherry-pick $empty_args $ff "$@"
+	output eval git cherry-pick "$strategy_args" $empty_args $ff "$@"
 }
 
 pick_one_preserving_merges () {

@@ -340,9 +352,8 @@ pick_one_preserving_merges () {
 			msg_content="$(commit_message $sha1)"
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
-			if ! do_with_author output \
-				git merge --no-ff ${strategy:+-s $strategy} -m \
-					"$msg_content" $new_parents
+			if ! do_with_author output eval \
+			'git merge --no-ff $strategy_args -m "$msg_content" $new_parents'
 			then
 				printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG
 				die_with_patch $sha1 "Error redoing merge $sha1"

@@ -350,7 +361,7 @@ pick_one_preserving_merges () {
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
 		*)
-			output git cherry-pick "$@" ||
+			output eval git cherry-pick "$strategy_args" "$@" ||
 				die_with_patch $sha1 "Could not pick $sha1"
 			;;
 		esac
t/t3404-rebase-interactive.sh +11 -0
index d6b4143..8a6ec03 100755
@@ -950,4 +950,15 @@ test_expect_success 'rebase -i, with <onto> and <upstream> specified as :/quuxer
 	git checkout branch1
 '
 
+test_expect_success 'rebase -i with --strategy and -X' '
+	git checkout -b conflict-merge-use-theirs conflict-branch &&
+	git reset --hard HEAD^ &&
+	echo five >conflict &&
+	echo Z >file1 &&
+	git commit -a -m "one file conflict" &&
+	EDITOR=true git rebase -i --strategy=recursive -Xours conflict-branch &&
+	test $(git show conflict-branch:conflict) = $(cat conflict) &&
+	test $(cat file1) = Z
+'
+
 test_done
Signature · 1 line
1.8.3.GIT
author
#8 lore raw
In reply to Junio C Hamano

Hello,

Sorry for the late reply.

Quoted 59 lines · Junio C Hamano
Quoted 3 lines · Junio C Hamano

You should use ${var:+if_set_to_nonempty_use_this} here.
...
So munging it manually with sloppy "sed" script is simply wrong.

Perhaps something like this on top of your patch squashed in?

The eval magic lets the shell to split $strategy_opts back into individual words (e.g. "--subtree=My Project" is a single word), strip the leading "--", and then uses "rev-parse --sq-quote" again to turn them into "-X 'subtree=My Project'" (two words), which can be inserted into a string later to be eval'ed.

 git-rebase--interactive.sh | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
git-rebase--interactive.sh +10 -6
index e558397..ae2da7a 100644
@@ -83,8 +83,13 @@ rewritten_pending="$state_dir"/rewritten-pending
 strategy_args=
 if test -n "$do_merge"
 then
-	strategy_args="${strategy+--strategy=$strategy}
- $(echo $strategy_opts | sed "s/'--\([^']*\)'/-X\1/g")"
+	strategy_args=${strategy:+--strategy=$strategy}
+	eval '
+		for strategy_opt in '"$strategy_opts"'
+		do
+			strategy_args="$strategy_args -X$(git rev-parse --sq-quote "${strategy_opt#--}")"
+		done
+	'
 fi
 
 GIT_CHERRY_PICK_HELP="$resolvemsg"

@@ -246,7 +251,7 @@ pick_one () {
 
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output git cherry-pick $strategy_args $empty_args $ff "$@"
+	output eval git cherry-pick "$strategy_args" $empty_args $ff "$@"
 }
 
 pick_one_preserving_merges () {

@@ -347,9 +352,8 @@ pick_one_preserving_merges () {
 			msg_content="$(commit_message $sha1)"
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
-			if ! do_with_author output \
-				git merge --no-ff $strategy_args -m \
-					"$msg_content" $new_parents
+			if ! do_with_author output eval \
+			'git merge --no-ff $strategy_args -m "$msg_content" $new_parents'
 			then
 				printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG
 				die_with_patch $sha1 "Error redoing merge $sha1"

git-rebase--interactive.sh pick_one_preserving_merges () { · line 367

Quoted 8 lines
git-rebase--interactive.sh +1 -1
@@ -357,7 +361,7 @@ pick_one_preserving_merges () {
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
 		*)
-			output git cherry-pick $strategy_args "$@" ||
+			output eval git cherry-pick "$strategy_args" "$@" ||
 				die_with_patch $sha1 "Could not pick $sha1"
 			;;
 		esac

Indeed, this is much better this way. Thank you very much! I have just squashed your patch into mine and ran successfully t3404 and t3421 tests after rebasing my branch on top of current master.

I will send the final patch in another email. Of course, feel free to take authorship of the patch or add yourself as Signed-off-by.

Cheers,

Signature · 1 line
Arnaud Fontaine