git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Paul Tan <pyokagan@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Stefan Beller <sbeller@google.com>,
	Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>,
	Jeff King <peff@peff.net>, Paul Tan <pyokagan@gmail.com>
Subject: [PATCH v2 2/3] am: let command-line options override saved options
Date: Tue,  4 Aug 2015 22:08:50 +0800	[thread overview]
Message-ID: <1438697331-29948-3-git-send-email-pyokagan@gmail.com> (raw)
In-Reply-To: <1438697331-29948-1-git-send-email-pyokagan@gmail.com>

When resuming, git-am mistakenly ignores command-line options.

For instance, when a patch fails to apply with "git am patch",
subsequently running "git am --3way" would not cause git-am to fall
back on attempting a threeway merge.  This occurs because by default
the --3way option is saved as "false", and the saved am options are
loaded after the command-line options are parsed, thus overwriting
the command-line options when resuming.

Fix this by moving the am_load() function call before parse_options(),
so that command-line options will override the saved am options.

The purpose of supporting this use case is to enable users to "wiggle"
that one conflicting patch. As such, it is expected that the
command-line options do not affect subsequent applied patches. Implement
this by calling am_load() once we apply the conflicting patch
successfully.

Noticed-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/am.c                       | 16 ++++++--
 t/t4153-am-resume-override-opts.sh | 82 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 4 deletions(-)
 create mode 100755 t/t4153-am-resume-override-opts.sh

diff --git a/builtin/am.c b/builtin/am.c
index 84d57d4..0961304 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1777,7 +1777,6 @@ static void am_run(struct am_state *state, int resume)
 
 		if (resume) {
 			validate_resume_state(state);
-			resume = 0;
 		} else {
 			int skip;
 
@@ -1839,6 +1838,10 @@ static void am_run(struct am_state *state, int resume)
 
 next:
 		am_next(state);
+
+		if (resume)
+			am_load(state);
+		resume = 0;
 	}
 
 	if (!is_empty_file(am_path(state, "rewritten"))) {
@@ -1893,6 +1896,7 @@ static void am_resolve(struct am_state *state)
 
 next:
 	am_next(state);
+	am_load(state);
 	am_run(state, 0);
 }
 
@@ -2020,6 +2024,7 @@ static void am_skip(struct am_state *state)
 		die(_("failed to clean index"));
 
 	am_next(state);
+	am_load(state);
 	am_run(state, 0);
 }
 
@@ -2130,6 +2135,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	int keep_cr = -1;
 	int patch_format = PATCH_FORMAT_UNKNOWN;
 	enum resume_mode resume = RESUME_FALSE;
+	int in_progress;
 
 	const char * const usage[] = {
 		N_("git am [options] [(<mbox>|<Maildir>)...]"),
@@ -2225,6 +2231,10 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 
 	am_state_init(&state, git_path("rebase-apply"));
 
+	in_progress = am_in_progress(&state);
+	if (in_progress)
+		am_load(&state);
+
 	argc = parse_options(argc, argv, prefix, options, usage, 0);
 
 	if (binary >= 0)
@@ -2237,7 +2247,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	if (read_index_preload(&the_index, NULL) < 0)
 		die(_("failed to read the index"));
 
-	if (am_in_progress(&state)) {
+	if (in_progress) {
 		/*
 		 * Catch user error to feed us patches when there is a session
 		 * in progress:
@@ -2255,8 +2265,6 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 
 		if (resume == RESUME_FALSE)
 			resume = RESUME_APPLY;
-
-		am_load(&state);
 	} else {
 		struct argv_array paths = ARGV_ARRAY_INIT;
 		int i;
diff --git a/t/t4153-am-resume-override-opts.sh b/t/t4153-am-resume-override-opts.sh
new file mode 100755
index 0000000..39fac79
--- /dev/null
+++ b/t/t4153-am-resume-override-opts.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+test_description='git-am command-line options override saved options'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
+
+format_patch () {
+	git format-patch --stdout -1 "$1" >"$1".eml
+}
+
+test_expect_success 'setup' '
+	test_commit initial file &&
+	test_commit first file &&
+
+	git checkout initial &&
+	git mv file file2 &&
+	test_tick &&
+	git commit -m renamed-file &&
+	git tag renamed-file &&
+
+	git checkout -b side initial &&
+	test_commit side1 file &&
+	test_commit side2 file &&
+
+	format_patch side1 &&
+	format_patch side2
+'
+
+test_expect_success TTY '--3way overrides --no-3way' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout renamed-file &&
+
+	# Applying side1 will fail as the file has been renamed.
+	test_must_fail git am --no-3way side[12].eml &&
+	test_path_is_dir .git/rebase-apply &&
+	test_cmp_rev renamed-file HEAD &&
+	test -z "$(git ls-files -u)" &&
+
+	# Applying side1 with am --3way will succeed due to the threeway-merge.
+	# Applying side2 will fail as --3way does not apply to it.
+	test_must_fail test_terminal git am --3way </dev/zero &&
+	test_path_is_dir .git/rebase-apply &&
+	test side1 = "$(cat file2)"
+'
+
+test_expect_success '--no-quiet overrides --quiet' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+
+	# Applying side1 will be quiet.
+	test_must_fail git am --quiet side[123].eml >out &&
+	test_path_is_dir .git/rebase-apply &&
+	! test_i18ngrep "^Applying: " out &&
+	echo side1 >file &&
+	git add file &&
+
+	# Applying side1 will not be quiet.
+	# Applying side2 will be quiet.
+	git am --no-quiet --continue >out &&
+	echo "Applying: side1" >expected &&
+	test_i18ncmp expected out
+'
+
+test_expect_success TTY '--reject overrides --no-reject' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+	rm -f file.rej &&
+
+	test_must_fail git am --no-reject side1.eml &&
+	test_path_is_dir .git/rebase-apply &&
+	test_path_is_missing file.rej &&
+
+	test_must_fail test_terminal git am --reject </dev/zero &&
+	test_path_is_dir .git/rebase-apply &&
+	test_path_is_file file.rej
+'
+
+test_done
-- 
2.5.0.280.gd88bd6e

  parent reply	other threads:[~2015-08-04 14:09 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-24 17:48 "git am" and then "git am -3" regression? Junio C Hamano
2015-07-24 18:09 ` Jeff King
2015-07-26  5:03   ` Paul Tan
2015-07-26  5:21     ` Jeff King
2015-07-27  8:09       ` Matthieu Moy
2015-07-27  8:32         ` Jeff King
2015-07-27 14:21     ` Junio C Hamano
2015-07-28 16:43       ` [PATCH] am: let command-line options override saved options Paul Tan
2015-07-28 16:48         ` Junio C Hamano
2015-07-28 17:09         ` Junio C Hamano
2015-07-31 10:58           ` Paul Tan
2015-07-31 16:04             ` Junio C Hamano
2015-08-01  0:59               ` Paul Tan
2015-08-04 14:05         ` [PATCH v2 0/3] " Paul Tan
2015-08-04 21:12           ` Junio C Hamano
2015-08-04 14:08         ` Paul Tan
2015-08-04 14:08           ` [PATCH v2 1/3] test_terminal: redirect child process' stdin to a pty Paul Tan
2015-08-06 22:15             ` Eric Sunshine
2015-08-12  4:16               ` Paul Tan
2015-08-04 14:08           ` Paul Tan [this message]
2015-08-04 14:08           ` [PATCH v2 3/3] am: let --signoff override --no-signoff Paul Tan
2015-08-07  9:29             ` Johannes Schindelin
2015-08-12  3:06               ` Paul Tan
2015-08-12  3:07                 ` Paul Tan
2015-08-05 15:41           ` [PATCH v2 0/3] am: let command-line options override saved options Junio C Hamano
2015-08-05 17:51             ` Paul Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

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

  git send-email \
    --in-reply-to=1438697331-29948-3-git-send-email-pyokagan@gmail.com \
    --to=pyokagan@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --cc=remi.lespinet@ensimag.grenoble-inp.fr \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).