git add -p: use non-zero exit code when the diff generation failed

The first thing `git add -p` does is to generate a diff. If this diff cannot be generated, `git add -p` should not continue as if nothing happened, but instead fail. What we *actually* do here is much broader: we now verify for *every* `run_cmd_pipe()` call that the spawned process actually succeeded. Note that we have to change two callers in this patch, as we need to store the spawned process' output in a local variable, which means that the callers can no longer decide whether to interpret the `return <$fh>` in array or in scalar context. This bug was noticed while writing a test case for the diff.algorithm feature, and we let that test case double as a regression test for this fixed bug, too. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Dec 6, 2019 at 13:08 UTC 89c8559367aae771006cc0956b6f5e54cc8c614c

2 files changed +6 -4

git-add--interactive.perl

+5 -3

@@ -177,7 +177,9 @@ sub run_cmd_pipe {

177 } else {

178 my $fh = undef;

179 open($fh, '-|', @_) or die;

180 - return <$fh>;

180 + my @out = <$fh>;

181 + close $fh || die "Cannot close @_ ($!)";

182 + return @out;

183 }

184 }

185

@@ -224,7 +226,7 @@ my $status_head = sprintf($status_fmt, __('staged'), __('unstaged'), __('path'))

226 sub get_empty_tree {

227 return $empty_tree if defined $empty_tree;

228

227 - $empty_tree = run_cmd_pipe(qw(git hash-object -t tree /dev/null));

229 + ($empty_tree) = run_cmd_pipe(qw(git hash-object -t tree /dev/null));

230 chomp $empty_tree;

231 return $empty_tree;

232 }

@@ -1127,7 +1129,7 @@ aborted and the hunk is left unchanged.

1129 EOF2

1130 close $fh;

1131

1130 - chomp(my $editor = run_cmd_pipe(qw(git var GIT_EDITOR)));

1132 + chomp(my ($editor) = run_cmd_pipe(qw(git var GIT_EDITOR)));

1133 system('sh', '-c', $editor.' "$@"', $editor, $hunkfile);

1134

1135 if ($? != 0) {

t/t3701-add-interactive.sh

+1 -1

@@ -530,7 +530,7 @@ test_expect_success 'diff.algorithm is passed to `git diff-files`' '

530 >file &&

531 git add file &&

532 echo changed >file &&

533 - git -c diff.algorithm=bogus add -p 2>err &&

533 + test_must_fail git -c diff.algorithm=bogus add -p 2>err &&

534 test_i18ngrep "error: option diff-algorithm accepts " err

535 '

536