From 18d8c26930acbba1627d2a8b7323be30ac2bd9bb Mon Sep 17 00:00:00 2001 From: Paul Tan Date: Tue, 4 Aug 2015 22:08:49 +0800 Subject: test_terminal: redirect child process' stdin to a pty When resuming, git-am detects if we are trying to feed it patches or not by checking if stdin is a TTY. However, the test library redirects stdin to /dev/null. This makes it difficult, for instance, to test the behavior of "git am -3" when resuming, as git-am will think we are trying to feed it patches and error out. Support this use case by extending test-terminal.perl to create a pseudo-tty for the child process' standard input as well. Note that due to the way the code is structured, the child's stdin pseudo-tty will be closed when we finish reading from our stdin. This means that in the common case, where our stdin is attached to /dev/null, the child's stdin pseudo-tty will be closed immediately. Some operations like isatty(), which git-am uses, require the file descriptor to be open, and hence if the success of the command depends on such functions, test_terminal's stdin should be redirected to a source with large amount of data to ensure that the child's stdin is not closed, e.g. test_terminal git am --3way Cc: Jeff King Signed-off-by: Paul Tan Signed-off-by: Junio C Hamano --- t/test-terminal.perl | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/t/test-terminal.perl b/t/test-terminal.perl index 1fb373f25b..96b6a03e1c 100755 --- a/t/test-terminal.perl +++ b/t/test-terminal.perl @@ -5,15 +5,17 @@ use warnings; 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); -- cgit v1.2.3 From 852a171018be4695f21848b03d001b5ed3ee96a0 Mon Sep 17 00:00:00 2001 From: Paul Tan Date: Tue, 4 Aug 2015 22:08:50 +0800 Subject: am: let command-line options override saved options 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 Helped-by: Junio C Hamano Helped-by: Jeff King Signed-off-by: Paul Tan Signed-off-by: Junio C Hamano --- 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 1399c8dd88..f81b74dd0b 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1779,7 +1779,6 @@ static void am_run(struct am_state *state, int resume) if (resume) { validate_resume_state(state); - resume = 0; } else { int skip; @@ -1841,6 +1840,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 +1898,7 @@ static void am_resolve(struct am_state *state) next: am_next(state); + am_load(state); am_run(state, 0); } @@ -2022,6 +2026,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 +2137,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] [(|)...]"), @@ -2227,6 +2233,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 +2249,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: @@ -2257,8 +2267,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 0000000000..39fac7993e --- /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 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 Date: Tue, 4 Aug 2015 22:08:51 +0800 Subject: am: let --signoff override --no-signoff After resolving a conflicting patch, a user may wish to sign off the patch to declare that the patch has been modified. As such, the user will expect that running "git am --signoff --continue" will append the signoff to the commit message. However, the --signoff option is only taken into account during the mail-parsing stage. If the --signoff option is set, then the signoff will be appended to the commit message. Since the mail-parsing stage comes before the patch application stage, the --signoff option, if provided on the command-line when resuming, will have no effect at all. We cannot move the append_signoff() call to the patch application stage as the applypatch-msg hook and interactive mode, which run before patch application, may expect the signoff to be there. Fix this by taking note if the user explictly set the --signoff option on the command-line, and append the signoff to the commit message when resuming if so. Signed-off-by: Paul Tan Signed-off-by: Junio C Hamano --- builtin/am.c | 28 +++++++++++++++++++++++++--- t/t4153-am-resume-override-opts.sh | 20 ++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index f81b74dd0b..634f7a7aa7 100644 --- a/builtin/am.c +++ b/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, @@ -2153,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, @@ -2267,6 +2286,9 @@ int cmd_am(int argc, const char **argv, const char *prefix) if (resume == RESUME_FALSE) resume = RESUME_APPLY; + + if (state.signoff == SIGNOFF_EXPLICIT) + am_append_signoff(&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 index 39fac7993e..7c013d84d5 100755 --- a/t/t4153-am-resume-override-opts.sh +++ b/t/t4153-am-resume-override-opts.sh @@ -64,6 +64,26 @@ test_expect_success '--no-quiet overrides --quiet' ' 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 && -- cgit v1.2.3