[PATCH 0/6] am/cherry-pick/rebase/revert --rerere-autoupdate fixes

[PATCH 0/6] am/cherry-pick/rebase/revert --rerere-autoupdate fixes

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

With the exception of 'rebase -m' when continuing after stopping for
the user to resolve conflicts, they all forget the setting of
--rerere-autoupdate.

Phillip Wood (6):
  am: remember --rerere-autoupdate setting
  rebase: honor --rerere-autoupdate
  rebase -i: honor --rerere-autoupdate
  t3504: use test_commit
  cherry-pick/revert: remember --rerere-autoupdate
  cherry-pick/revert: reject --rerere-autoupdate when continuing

 builtin/am.c                  | 12 ++++++
 builtin/revert.c              |  2 +
 git-rebase--am.sh             |  3 +-
 git-rebase--interactive.sh    |  7 ++--
 sequencer.c                   | 20 +++++++++-
 t/t3418-rebase-continue.sh    | 85 +++++++++++++++++++++++++++-------------
 t/t3504-cherry-pick-rerere.sh | 90 +++++++++++++++++++++++++++++++++++--------
 7 files changed, 170 insertions(+), 49 deletions(-)

-- 
2.13.3

[PATCH 1/6] am: remember --rerere-autoupdate setting

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Save the rerere-autoupdate setting so that it is remembered after
stopping for the user to resolve conflicts.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---

There are no new tests, but this code is exercised by the new rebase
tests in the next patch.

builtin/am.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index c973bd96dcb5d630d56e935733bfa4530ccd2872..6962d4db5ffceef3022b7f877a43c8833a839b31 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -431,6 +431,14 @@ static void am_load(struct am_state *state)
 	read_state_file(&sb, state, "utf8", 1);
 	state->utf8 = !strcmp(sb.buf, "t");
 
+	if (file_exists(am_path(state, "rerere-autoupdate"))) {
+		read_state_file(&sb, state, "rerere-autoupdate", 1);
+		state->allow_rerere_autoupdate = strcmp(sb.buf, "t") ?
+			RERERE_NOAUTOUPDATE : RERERE_AUTOUPDATE;
+	} else {
+		state->allow_rerere_autoupdate = 0;
+	}
+
 	read_state_file(&sb, state, "keep", 1);
 	if (!strcmp(sb.buf, "t"))
 		state->keep = KEEP_TRUE;
@@ -1003,6 +1011,10 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
 	write_state_bool(state, "sign", state->signoff);
 	write_state_bool(state, "utf8", state->utf8);
 
+	if (state->allow_rerere_autoupdate)
+		write_state_bool(state, "rerere-autoupdate",
+			 state->allow_rerere_autoupdate == RERERE_AUTOUPDATE);
+
 	switch (state->keep) {
 	case KEEP_FALSE:
 		str = "f";
-- 
2.13.3

[PATCH 2/6] rebase: honor --rerere-autoupdate

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Rebase accepts '--rerere-autoupdate' as an option but only honors it
if '-m' is also given. Fix it for a non-interactive rebase by passing
on the option to 'git am' and 'git cherry-pick'. Rework the tests so
that they can be used for each rebase flavor and extend them.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-rebase--am.sh          |  3 +-
 t/t3418-rebase-continue.sh | 82 +++++++++++++++++++++++++++++++---------------
 2 files changed, 57 insertions(+), 28 deletions(-)

diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 375239341fbfe885e51a25e9e0dc2d4fee791345..319933e70a34f9da4ec93d063eb102eff33b6787 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -45,7 +45,7 @@ then
 	# itself well to recording empty patches.  fortunately, cherry-pick
 	# makes this easy
 	git cherry-pick ${gpg_sign_opt:+"$gpg_sign_opt"} --allow-empty \
-		--right-only "$revisions" \
+		$allow_rerere_autoupdate --right-only "$revisions" \
 		${restrict_revision+^$restrict_revision}
 	ret=$?
 else
@@ -82,6 +82,7 @@ else
 	fi
 
 	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
+		$allow_rerere_autoupdate \
 		${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches"
 	ret=$?
 
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 4428b9086e8bcb383df801834d0de323f316f4fa..2b746f1559ad21a5ebf3bebd726c9a1b3d071c5a 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -40,25 +40,6 @@ test_expect_success 'non-interactive rebase --continue works with touched file'
 	git rebase --continue
 '
 
-test_expect_success 'non-interactive rebase --continue with rerere enabled' '
-	test_config rerere.enabled true &&
-	test_when_finished "test_might_fail git rebase --abort" &&
-	git reset --hard commit-new-file-F2-on-topic-branch &&
-	git checkout master &&
-	rm -fr .git/rebase-* &&
-
-	test_must_fail git rebase --onto master master topic &&
-	echo "Resolved" >F2 &&
-	git add F2 &&
-	cp F2 F2.expected &&
-	git rebase --continue &&
-
-	git reset --hard commit-new-file-F2-on-topic-branch &&
-	git checkout master &&
-	test_must_fail git rebase --onto master master topic &&
-	test_cmp F2.expected F2
-'
-
 test_expect_success 'rebase --continue can not be used with other options' '
 	test_must_fail git rebase -v --continue &&
 	test_must_fail git rebase --continue -v
@@ -93,25 +74,72 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '
 	test -f funny.was.run
 '
 
-test_expect_success 'rebase --continue remembers --rerere-autoupdate' '
+test_expect_success 'setup rerere database' '
 	rm -fr .git/rebase-* &&
 	git reset --hard commit-new-file-F3-on-topic-branch &&
 	git checkout master &&
 	test_commit "commit-new-file-F3" F3 3 &&
-	git config rerere.enabled true &&
+	test_config rerere.enabled true &&
 	test_must_fail git rebase -m master topic &&
 	echo "Resolved" >F2 &&
+	cp F2 expected-F2 &&
 	git add F2 &&
 	test_must_fail git rebase --continue &&
 	echo "Resolved" >F3 &&
+	cp F3 expected-F3 &&
 	git add F3 &&
 	git rebase --continue &&
-	git reset --hard topic@{1} &&
-	test_must_fail git rebase -m --rerere-autoupdate master &&
-	test "$(cat F2)" = "Resolved" &&
-	test_must_fail git rebase --continue &&
-	test "$(cat F3)" = "Resolved" &&
-	git rebase --continue
+	git reset --hard topic@{1}
 '
 
+prepare () {
+	rm -fr .git/rebase-* &&
+	git reset --hard commit-new-file-F3-on-topic-branch &&
+	git checkout master &&
+	test_config rerere.enabled true
+}
+
+test_rerere_autoupdate () {
+	action=$1 &&
+	test_expect_success "rebase $action --continue remembers --rerere-autoupdate" '
+		prepare &&
+		test_must_fail git rebase $action --rerere-autoupdate master topic &&
+		test_cmp expected-F2 F2 &&
+		git diff-files --quiet &&
+		test_must_fail git rebase --continue &&
+		test_cmp expected-F3 F3 &&
+		git diff-files --quiet &&
+		git rebase --continue
+	'
+
+	test_expect_success "rebase $action --continue honors rerere.autoUpdate" '
+		prepare &&
+		test_config rerere.autoupdate true &&
+		test_must_fail git rebase $action master topic &&
+		test_cmp expected-F2 F2 &&
+		git diff-files --quiet &&
+		test_must_fail git rebase --continue &&
+		test_cmp expected-F3 F3 &&
+		git diff-files --quiet &&
+		git rebase --continue
+	'
+
+	test_expect_success "rebase $action --continue remembers --no-rerere-autoupdate" '
+		prepare &&
+		test_config rerere.autoupdate true &&
+		test_must_fail git rebase $action --no-rerere-autoupdate master topic &&
+		test_cmp expected-F2 F2 &&
+		test_must_fail git diff-files --quiet &&
+		git add F2 &&
+		test_must_fail git rebase --continue &&
+		test_cmp expected-F3 F3 &&
+		test_must_fail git diff-files --quiet &&
+		git add F3 &&
+		git rebase --continue
+	'
+}
+
+test_rerere_autoupdate
+test_rerere_autoupdate -m
+
 test_done
-- 
2.13.3

[PATCH 3/6] rebase -i: honor --rerere-autoupdate

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Interactive rebase was ignoring '--rerere-autoupdate'. Fix this by
reading it appropriate file when restoring the sequencer state for an
interactive rebase and passing '--rerere-autoupdate' to merge and
cherry-pick when rebasing with '--preserve-merges'.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-rebase--interactive.sh |  7 ++++---
 sequencer.c                | 10 ++++++++++
 t/t3418-rebase-continue.sh |  3 +++
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 90b1fbe9cf6e8dfb2f4331916809fa40bf9050d2..29b7e8824b53abeaa68780b95d5954f67f734098 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -281,7 +281,7 @@ pick_one () {
 
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output eval git cherry-pick \
+	output eval git cherry-pick $allow_rerere_autoupdate \
 			${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")} \
 			"$strategy_args" $empty_args $ff "$@"
 
@@ -393,7 +393,8 @@ pick_one_preserving_merges () {
 			merge_args="--no-log --no-ff"
 			if ! do_with_author output eval \
 			'git merge ${gpg_sign_opt:+"$gpg_sign_opt"} \
-				$merge_args $strategy_args -m "$msg_content" $new_parents'
+				$allow_rerere_autoupdate $merge_args \
+				$strategy_args -m "$msg_content" $new_parents'
 			then
 				printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG
 				die_with_patch $sha1 "$(eval_gettext "Error redoing merge \$sha1")"
@@ -401,7 +402,7 @@ pick_one_preserving_merges () {
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
 		*)
-			output eval git cherry-pick \
+			output eval git cherry-pick $allow_rerere_autoupdate \
 				${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")} \
 				"$strategy_args" "$@" ||
 				die_with_patch $sha1 "$(eval_gettext "Could not pick \$sha1")"
diff --git a/sequencer.c b/sequencer.c
index 3010faf86398697469e903318a35421d911acb23..7dc0670d902291b8054072d32cc0c8979c13598c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -127,6 +127,7 @@ static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto")
 static GIT_PATH_FUNC(rebase_path_autostash, "rebase-merge/autostash")
 static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
 static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
+static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
 
 static inline int is_rebase_i(const struct replay_opts *opts)
 {
@@ -1479,6 +1480,15 @@ static int read_populate_opts(struct replay_opts *opts)
 				free(opts->gpg_sign);
 				opts->gpg_sign = xstrdup(buf.buf + 2);
 			}
+			strbuf_reset(&buf);
+		}
+
+		if (read_oneliner(&buf, rebase_path_allow_rerere_autoupdate(), 1)) {
+			if (!strcmp(buf.buf, "--rerere-autoupdate"))
+				opts->allow_rerere_auto = RERERE_AUTOUPDATE;
+			else if (!strcmp(buf.buf, "--no-rerere-autoupdate"))
+				opts->allow_rerere_auto = RERERE_NOAUTOUPDATE;
+			strbuf_reset(&buf);
 		}
 
 		if (file_exists(rebase_path_verbose()))
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 2b746f1559ad21a5ebf3bebd726c9a1b3d071c5a..fcfdd197bd352a9dca10233c2ba6d2aa4a66149e 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -141,5 +141,8 @@ test_rerere_autoupdate () {
 
 test_rerere_autoupdate
 test_rerere_autoupdate -m
+GIT_SEQUENCE_EDITOR=: && export GIT_SEQUENCE_EDITOR
+test_rerere_autoupdate -i
+test_rerere_autoupdate --preserve-merges
 
 test_done
-- 
2.13.3

[PATCH 4/6] t3504: use test_commit

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Using test_commit is simpler than chaining echo && git add &&
test_tick && commit. Also having tags makes it clearer which commit
is being selecting by reset.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t3504-cherry-pick-rerere.sh | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/t/t3504-cherry-pick-rerere.sh b/t/t3504-cherry-pick-rerere.sh
index e6a64816efef0e53018c7a56784d1af62602e9d3..33f902b1b0d8eb651e0e6857f8f5b86ef633ef4a 100755
--- a/t/t3504-cherry-pick-rerere.sh
+++ b/t/t3504-cherry-pick-rerere.sh
@@ -5,14 +5,11 @@ test_description='cherry-pick should rerere for conflicts'
 . ./test-lib.sh
 
 test_expect_success setup '
-	echo foo >foo &&
-	git add foo && test_tick && git commit -q -m 1 &&
-	echo foo-master >foo &&
-	git add foo && test_tick && git commit -q -m 2 &&
+	test_commit foo &&
+	test_commit foo-master foo &&
 
-	git checkout -b dev HEAD^ &&
-	echo foo-dev >foo &&
-	git add foo && test_tick && git commit -q -m 3 &&
+	git checkout -b dev foo &&
+	test_commit foo-dev foo &&
 	git config rerere.enabled true
 '
 
@@ -21,10 +18,10 @@ test_expect_success 'conflicting merge' '
 '
 
 test_expect_success 'fixup' '
-	echo foo-dev >foo &&
-	git add foo && test_tick && git commit -q -m 4 &&
-	git reset --hard HEAD^ &&
-	echo foo-dev >expect
+	echo foo-resolved >foo &&
+	git commit -am resolved &&
+	cp foo expect &&
+	git reset --hard HEAD^
 '
 
 test_expect_success 'cherry-pick conflict' '
-- 
2.13.3

[PATCH 5/6] cherry-pick/revert: remember --rerere-autoupdate

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

When continuing after conflicts, cherry-pick forgot if the user had specified
'--rerere-autoupdate'.

Redo the cherry-pick rerere tests to check --rerere-autoupdate works
as expected.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 sequencer.c                   | 10 +++++++-
 t/t3504-cherry-pick-rerere.sh | 60 ++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 7dc0670d902291b8054072d32cc0c8979c13598c..e0e66b987b27072da4aea6304a565ab708be91e4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1439,7 +1439,11 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
 	else if (!strcmp(key, "options.strategy-option")) {
 		ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
 		opts->xopts[opts->xopts_nr++] = xstrdup(value);
-	} else
+	} else if (!strcmp(key, "options.allow-rerere-auto"))
+		opts->allow_rerere_auto =
+			git_config_bool_or_int(key, value, &error_flag) ?
+				RERERE_AUTOUPDATE : RERERE_NOAUTOUPDATE;
+	else
 		return error(_("invalid key: %s"), key);
 
 	if (!error_flag)
@@ -1752,6 +1756,10 @@ static int save_opts(struct replay_opts *opts)
 							"options.strategy-option",
 							opts->xopts[i], "^$", 0);
 	}
+	if (opts->allow_rerere_auto)
+		res |= git_config_set_in_file_gently(opts_file, "options.allow-rerere-auto",
+						     opts->allow_rerere_auto == RERERE_AUTOUPDATE ?
+						     "true" : "false");
 	return res;
 }
 
diff --git a/t/t3504-cherry-pick-rerere.sh b/t/t3504-cherry-pick-rerere.sh
index 33f902b1b0d8eb651e0e6857f8f5b86ef633ef4a..af316cb40b7b16c95881eb8483eea4f6191c7cfa 100755
--- a/t/t3504-cherry-pick-rerere.sh
+++ b/t/t3504-cherry-pick-rerere.sh
@@ -7,9 +7,11 @@ test_description='cherry-pick should rerere for conflicts'
 test_expect_success setup '
 	test_commit foo &&
 	test_commit foo-master foo &&
+	test_commit bar-master bar &&
 
 	git checkout -b dev foo &&
 	test_commit foo-dev foo &&
+	test_commit bar-dev bar &&
 	git config rerere.enabled true
 '
 
@@ -19,22 +21,66 @@ test_expect_success 'conflicting merge' '
 
 test_expect_success 'fixup' '
 	echo foo-resolved >foo &&
+	echo bar-resolved >bar &&
 	git commit -am resolved &&
-	cp foo expect &&
+	cp foo foo-expect &&
+	cp bar bar-expect &&
 	git reset --hard HEAD^
 '
 
-test_expect_success 'cherry-pick conflict' '
-	test_must_fail git cherry-pick master &&
-	test_cmp expect foo
+test_expect_success 'cherry-pick conflict with --rerere-autoupdate' '
+	test_must_fail git cherry-pick --rerere-autoupdate foo..bar-master &&
+	test_cmp foo-expect foo &&
+	git diff-files --quiet &&
+	test_must_fail git cherry-pick --continue &&
+	test_cmp bar-expect bar &&
+	git diff-files --quiet &&
+	git cherry-pick --continue &&
+	git reset --hard bar-dev
 '
 
-test_expect_success 'reconfigure' '
-	git config rerere.enabled false &&
-	git reset --hard
+test_expect_success 'cherry-pick conflict repsects rerere.autoUpdate' '
+	test_config rerere.autoUpdate true &&
+	test_must_fail git cherry-pick foo..bar-master &&
+	test_cmp foo-expect foo &&
+	git diff-files --quiet &&
+	test_must_fail git cherry-pick --continue &&
+	test_cmp bar-expect bar &&
+	git diff-files --quiet &&
+	git cherry-pick --continue &&
+	git reset --hard bar-dev
+'
+
+test_expect_success 'cherry-pick conflict with --no-rerere-autoupdate' '
+	test_config rerere.autoUpdate true &&
+	test_must_fail git cherry-pick --no-rerere-autoupdate foo..bar-master &&
+	test_cmp foo-expect foo &&
+	test_must_fail git diff-files --quiet &&
+	git add foo &&
+	test_must_fail git cherry-pick --continue &&
+	test_cmp bar-expect bar &&
+	test_must_fail git diff-files --quiet &&
+	git add bar &&
+	git cherry-pick --continue &&
+	git reset --hard bar-dev
+'
+
+test_expect_success 'cherry-pick --rerere-autoupdate more than once' '
+	test_must_fail git cherry-pick --rerere-autoupdate --rerere-autoupdate foo..bar-master &&
+	test_cmp foo-expect foo &&
+	git diff-files --quiet &&
+	git cherry-pick --abort &&
+	test_must_fail git cherry-pick --rerere-autoupdate --no-rerere-autoupdate --rerere-autoupdate foo..bar-master &&
+	test_cmp foo-expect foo &&
+	git diff-files --quiet &&
+	git cherry-pick --abort &&
+	test_must_fail git cherry-pick --rerere-autoupdate --no-rerere-autoupdate foo..bar-master &&
+	test_must_fail git diff-files --quiet &&
+	git cherry-pick --abort
 '
 
 test_expect_success 'cherry-pick conflict without rerere' '
+	test_config rerere.enabled false &&
 	test_must_fail git cherry-pick master &&
 	test_must_fail test_cmp expect foo
 '
-- 
2.13.3

[PATCH 6/6] cherry-pick/revert: reject --rerere-autoupdate when continuing

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

cherry-pick and revert should not accept --[no-]rerere-autoupdate once
they have started.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---

This will break scripts that pass --[no-]rerere-autoupdate to 'git
cherry-pick --continue'. I don't think that this will be an issue for
the vast majority of users as I think most people will have assumed
that you cannot pass any other options with '--continue'.
'--rerere-autoupdate' is mentioned by 'git cherry-pick -h' but it is
not mentioned in the documentation. Hopefully a note in the release
notes should be enough to alert anyone who is affected by this.

builtin/revert.c              |  2 ++
 t/t3504-cherry-pick-rerere.sh | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/builtin/revert.c b/builtin/revert.c
index 16028b9ea82edee9cf41044c69a47e8994d78fc6..b9d927eb09c9ed87c84681df1396f4e6d9b13c97 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -155,6 +155,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
 				"--strategy-option", opts->xopts ? 1 : 0,
 				"-x", opts->record_origin,
 				"--ff", opts->allow_ff,
+				"--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,
+				"--no-rerere-autoupdate", opts->allow_rerere_auto == RERERE_NOAUTOUPDATE,
 				NULL);
 	}
 
diff --git a/t/t3504-cherry-pick-rerere.sh b/t/t3504-cherry-pick-rerere.sh
index af316cb40b7b16c95881eb8483eea4f6191c7cfa..a267b2d144df4a84f18ba4907b317e757ba98f16 100755
--- a/t/t3504-cherry-pick-rerere.sh
+++ b/t/t3504-cherry-pick-rerere.sh
@@ -65,6 +65,19 @@ test_expect_success 'cherry-pick conflict with --no-rerere-autoupdate' '
 	git reset --hard bar-dev
 '
 
+test_expect_success 'cherry-pick --continue rejects --rerere-autoupdate' '
+	test_must_fail git cherry-pick --rerere-autoupdate foo..bar-master &&
+	test_cmp foo-expect foo &&
+	git diff-files --quiet &&
+	test_must_fail git cherry-pick --continue --rerere-autoupdate >actual 2>&1 &&
+	echo "fatal: cherry-pick: --rerere-autoupdate cannot be used with --continue" >expect &&
+	test_i18ncmp expect actual &&
+	test_must_fail git cherry-pick --continue --no-rerere-autoupdate >actual 2>&1 &&
+	echo "fatal: cherry-pick: --no-rerere-autoupdate cannot be used with --continue" >expect &&
+	test_i18ncmp expect actual &&
+	git cherry-pick --abort
+'
+
 test_expect_success 'cherry-pick --rerere-autoupdate more than once' '
 	test_must_fail git cherry-pick --rerere-autoupdate --rerere-autoupdate foo..bar-master &&
 	test_cmp foo-expect foo &&
-- 
2.13.3

Re: [PATCH 6/6] cherry-pick/revert: reject --rerere-autoupdate when continuing

[Junio C Hamano]

Phillip Wood <phillip.wood@talktalk.net> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> cherry-pick and revert should not accept --[no-]rerere-autoupdate once
> they have started.

Hmph, why shouldn't they?  In other words, shouldn't the usual "try
to carry forward from the original invocation (saved in the state
file), but allow overriding from the command line" rule apply?

I took a brief look at earlier steps and I thought 1-4/6 were all
good.  I am not sure (yet) about these last two patches.

Thanks for working on this topic.

Re: [PATCH 6/6] cherry-pick/revert: reject --rerere-autoupdate when continuing

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Phillip Wood <phillip.wood@talktalk.net> writes:
>
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> cherry-pick and revert should not accept --[no-]rerere-autoupdate once
>> they have started.
>
> Hmph, why shouldn't they?  In other words, shouldn't the usual "try
> to carry forward from the original invocation (saved in the state
> file), but allow overriding from the command line" rule apply?

Actually, I do not care _too_ deeply between

 * You can only give "--[no-]rerere-autoupdate" at the beginning and
   cannot change your mind later.

and

 * The "--[no-]rerere-autoupdate" you give at the beginning is used
   throughout your multi-commit cherry-pick session, but you can
   give an opposite one from the command line when you say
   "--continue", and in that case it takes effect only for a single
   commit.

If I understand correctly, the former is what 5-6/6 implements.  The
latter makes it more in line with how "am -3" followed by "am --no-3
--continue" behaves.

Thanks.

Re: [PATCH 6/6] cherry-pick/revert: reject --rerere-autoupdate when continuing

[Phillip Wood]

Hi Junio

Thanks for your comments.

On 02/08/17 23:29, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Phillip Wood <phillip.wood@talktalk.net> writes:
>>
>>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>>
>>> cherry-pick and revert should not accept --[no-]rerere-autoupdate once
>>> they have started.
>>
>> Hmph, why shouldn't they?  In other words, shouldn't the usual "try
>> to carry forward from the original invocation (saved in the state
>> file), but allow overriding from the command line" rule apply?
> 
> Actually, I do not care _too_ deeply between
> 
>  * You can only give "--[no-]rerere-autoupdate" at the beginning and
>    cannot change your mind later.
> 
> and
> 
>  * The "--[no-]rerere-autoupdate" you give at the beginning is used
>    throughout your multi-commit cherry-pick session, but you can
>    give an opposite one from the command line when you say
>    "--continue", and in that case it takes effect only for a single
>    commit.
> 
> If I understand correctly, the former is what 5-6/6 implements.

Yes, that's correct. It was easier to implement it that way

> The
> latter makes it more in line with how "am -3" followed by "am --no-3
> --continue" behaves.

I'm a bit confused about what am does when you pass extra options to
--continue. It looks like they do not persist if there's another
conflict and may only apply to the first patch that is applied when
resuming - I'd need to spend more time looking at the code or run a test
to be sure.

Best Wishes

Phillip

Re: [PATCH 6/6] cherry-pick/revert: reject --rerere-autoupdate when continuing

[Junio C Hamano]

Phillip Wood <phillip.wood@talktalk.net> writes:

> On 02/08/17 23:29, Junio C Hamano wrote:
> ...
>> The
>> latter makes it more in line with how "am -3" followed by "am --no-3
>> --continue" behaves.
>
> I'm a bit confused about what am does when you pass extra options to
> --continue. It looks like they do not persist if there's another
> conflict and may only apply to the first patch that is applied when
> resuming - I'd need to spend more time looking at the code or run a test
> to be sure.

I think you got what "am" wants to do.  

The idea is that the user would say she does not trust the three-way
fallback when she starts to apply many patches in an mbox, i.e.

   $ git am mbox

Upon seeing a message that does not apply, she would examine the
patch that caused _this_ stoppage, and then decide that it is safe
to apply _this_ patch (but not necessarily later ones) with
three-way fallback and move on:

    $ git am -3 --continue

I have not thought too deeply if the parallel applies to
multi-commit pick, though.  

"am" (rather, its underlying machinery "apply") is designed to be
all-or-none, so a failed --no-3way application would leave the index
and the working tree intact.  "-3 --continue" can retry the failed
step, with "--3way" processing turned on for only one message, from
that state.

But a multi-commit cherry-pick/revert would stop _after_ it munges
the conflicted paths in the index into an unmerged state and writes
the conflicted state into the working tree files.  For "--continue
--rerere-autoupdate" to work more like "am --continue -3", it would
have to learn to reset to the state before the failed cherry-pick
first, before re-attempting the failed cherry-pick with the auto
update enabled only for the single commit and keep going.  So it may
not as trivial as "am --continue", even though it sounds doable.

Re: [PATCH 6/6] cherry-pick/revert: reject --rerere-autoupdate when continuing

[Phillip Wood]

On 03/08/17 18:19, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
> 
>> On 02/08/17 23:29, Junio C Hamano wrote:
>> ...
>>> The
>>> latter makes it more in line with how "am -3" followed by "am --no-3
>>> --continue" behaves.
>>
>> I'm a bit confused about what am does when you pass extra options to
>> --continue. It looks like they do not persist if there's another
>> conflict and may only apply to the first patch that is applied when
>> resuming - I'd need to spend more time looking at the code or run a test
>> to be sure.
> 
> I think you got what "am" wants to do.  
> 
> The idea is that the user would say she does not trust the three-way
> fallback when she starts to apply many patches in an mbox, i.e.
> 
>    $ git am mbox
> 
> Upon seeing a message that does not apply, she would examine the
> patch that caused _this_ stoppage, and then decide that it is safe
> to apply _this_ patch (but not necessarily later ones) with
> three-way fallback and move on:
> 
>     $ git am -3 --continue
> 
> I have not thought too deeply if the parallel applies to
> multi-commit pick, though.  
> 
> "am" (rather, its underlying machinery "apply") is designed to be
> all-or-none, so a failed --no-3way application would leave the index
> and the working tree intact.  "-3 --continue" can retry the failed
> step, with "--3way" processing turned on for only one message, from
> that state.
> 
> But a multi-commit cherry-pick/revert would stop _after_ it munges
> the conflicted paths in the index into an unmerged state and writes
> the conflicted state into the working tree files.  For "--continue
> --rerere-autoupdate" to work more like "am --continue -3", it would
> have to learn to reset to the state before the failed cherry-pick
> first, before re-attempting the failed cherry-pick with the auto
> update enabled only for the single commit and keep going.  So it may
> not as trivial as "am --continue", even though it sounds doable.
> 
Thanks for explaining that, as you say having cherry-pick take
'--rerere-autoupadate' with '--continue' sounds more complicated than
the am case. Also I'm not sure it would be as helpful to toggle
'--rerere-autoupdate' with cherry-pick as it is to toggle '--3way' with
am as it's not that hard for the user to stage the merged files
themselves. If you're happy with the way it is currently implemented I'm
not inclined to change it.

Thanks

Phillip