Merge branch 'pt/am-builtin-options'

After "git am --opt1" stops, running "git am --opt2" pays attention
to "--opt2" only for the patch that caused the original invocation
to stop.

* pt/am-builtin-options:
  am: let --signoff override --no-signoff
  am: let command-line options override saved options
  test_terminal: redirect child process' stdin to a pty

builtin/am.c

@@ -98,6 +98,12 @@ enum scissors_type {
 	SCISSORS_TRUE        /* pass --scissors to git-mailinfo */
 };
 
+enum signoff_type {
+	SIGNOFF_FALSE = 0,
+	SIGNOFF_TRUE = 1,
+	SIGNOFF_EXPLICIT /* --signoff was set on the command-line */
+};
+
 struct am_state {
 	/* state directory path */
 	char *dir;
@@ -123,7 +129,7 @@ struct am_state {
 	int interactive;
 	int threeway;
 	int quiet;
-	int signoff;
+	int signoff; /* enum signoff_type */
 	int utf8;
 	int keep; /* enum keep_type */
 	int message_id;
@@ -1185,6 +1191,18 @@ static void NORETURN die_user_resolve(const struct am_state *state)
 	exit(128);
 }
 
+/**
+ * Appends signoff to the "msg" field of the am_state.
+ */
+static void am_append_signoff(struct am_state *state)
+{
+	struct strbuf sb = STRBUF_INIT;
+
+	strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);
+	append_signoff(&sb, 0, 0);
+	state->msg = strbuf_detach(&sb, &state->msg_len);
+}
+
 /**
  * Parses `mail` using git-mailinfo, extracting its patch and authorship info.
  * state->msg will be set to the patch message. state->author_name,
@@ -1779,7 +1797,6 @@ static void am_run(struct am_state *state, int resume)
 
 		if (resume) {
 			validate_resume_state(state);
-			resume = 0;
 		} else {
 			int skip;
 
@@ -1841,6 +1858,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"))) {
@@ -1895,6 +1916,7 @@ static void am_resolve(struct am_state *state)
 
 next:
 	am_next(state);
+	am_load(state);
 	am_run(state, 0);
 }
 
@@ -2022,6 +2044,7 @@ static void am_skip(struct am_state *state)
 		die(_("failed to clean index"));
 
 	am_next(state);
+	am_load(state);
 	am_run(state, 0);
 }
 
@@ -2132,6 +2155,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>)...]"),
@@ -2147,8 +2171,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('3', "3way", &state.threeway,
 			N_("allow fall back on 3way merging if needed")),
 		OPT__QUIET(&state.quiet, N_("be quiet")),
-		OPT_BOOL('s', "signoff", &state.signoff,
-			N_("add a Signed-off-by line to the commit message")),
+		OPT_SET_INT('s', "signoff", &state.signoff,
+			N_("add a Signed-off-by line to the commit message"),
+			SIGNOFF_EXPLICIT),
 		OPT_BOOL('u', "utf8", &state.utf8,
 			N_("recode into utf8 (default)")),
 		OPT_SET_INT('k', "keep", &state.keep,
@@ -2227,6 +2252,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)
@@ -2239,7 +2268,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:
@@ -2258,7 +2287,8 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		if (resume == RESUME_FALSE)
 			resume = RESUME_APPLY;
 
-		am_load(&state);
+		if (state.signoff == SIGNOFF_EXPLICIT)
+			am_append_signoff(&state);
 	} else {
 		struct argv_array paths = ARGV_ARRAY_INIT;
 		int i;

t/t4153-am-resume-override-opts.sh

@@ -0,0 +1,102 @@
+#!/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 '--signoff overrides --no-signoff' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout first &&
+
+	test_must_fail git am --no-signoff side[12].eml &&
+	test_path_is_dir .git/rebase-apply &&
+	echo side1 >file &&
+	git add file &&
+	git am --signoff --continue &&
+
+	# Applied side1 will be signed off
+	echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >expected &&
+	git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
+	test_cmp expected actual &&
+
+	# Applied side2 will not be signed off
+	test $(git cat-file commit HEAD | grep -c "Signed-off-by:") -eq 0
+'
+
+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

t/test-terminal.perl

@@ -5,15 +5,17 @@
 use IO::Pty;
 use File::Copy;
 
-# Run @$argv in the background with stdio redirected to $out and $err.
+# Run @$argv in the background with stdio redirected to $in, $out and $err.
 sub start_child {
-	my ($argv, $out, $err) = @_;
+	my ($argv, $in, $out, $err) = @_;
 	my $pid = fork;
 	if (not defined $pid) {
 		die "fork failed: $!"
 	} elsif ($pid == 0) {
+		open STDIN, "<&", $in;
 		open STDOUT, ">&", $out;
 		open STDERR, ">&", $err;
+		close $in;
 		close $out;
 		exec(@$argv) or die "cannot exec '$argv->[0]': $!"
 	}
@@ -49,6 +51,17 @@ sub xsendfile {
 	copy($in, $out, 4096) or $!{EIO} or die "cannot copy from child: $!";
 }
 
+sub copy_stdin {
+	my ($in) = @_;
+	my $pid = fork;
+	if (!$pid) {
+		xsendfile($in, \*STDIN);
+		exit 0;
+	}
+	close($in);
+	return $pid;
+}
+
 sub copy_stdio {
 	my ($out, $err) = @_;
 	my $pid = fork;
@@ -67,14 +80,25 @@ sub copy_stdio {
 if ($#ARGV < 1) {
 	die "usage: test-terminal program args";
 }
+my $master_in = new IO::Pty;
 my $master_out = new IO::Pty;
 my $master_err = new IO::Pty;
+$master_in->set_raw();
 $master_out->set_raw();
 $master_err->set_raw();
+$master_in->slave->set_raw();
 $master_out->slave->set_raw();
 $master_err->slave->set_raw();
-my $pid = start_child(\@ARGV, $master_out->slave, $master_err->slave);
+my $pid = start_child(\@ARGV, $master_in->slave, $master_out->slave, $master_err->slave);
+close $master_in->slave;
 close $master_out->slave;
 close $master_err->slave;
+my $in_pid = copy_stdin($master_in);
 copy_stdio($master_out, $master_err);
-exit(finish_child($pid));
+my $ret = finish_child($pid);
+# If the child process terminates before our copy_stdin() process is able to
+# write all of its data to $master_in, the copy_stdin() process could stall.
+# Send SIGTERM to it to ensure it terminates.
+kill 'TERM', $in_pid;
+finish_child($in_pid);
+exit($ret);