checkout-index: propagate errors to exit code

If we encounter an error while checking out an explicit path, we print a message to stderr but do not actually exit with a non-zero code. While this is a plumbing command and the behavior goes all the way back to 33db5f4d90 (Add a "checkout-cache" command which does what the name suggests., 2005-04-09), this is almost certainly an oversight: - we _do_ return an exit code from checkout_file(); the caller just never reads it - errors while checking out all paths (with "-a") do result in a non-zero exit code. - it would be quite unusual not to use the exit code for an error, as otherwise the caller has no idea the command failed except by scraping stderr To keep our tests simple and portable, we can use the most obvious error: asking to checkout a path which is not in the index at all. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 27, 2020 at 03:37 UTC 7e41061588eab25d0c015e79aa8d04956bee482a
3 files changed +18 -3
builtin/checkout-index.c
+6 -2
@@ -167,6 +167,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
167 int prefix_length;
168 int force = 0, quiet = 0, not_new = 0;
169 int index_opt = 0;
170 + int err = 0;
171 struct option builtin_checkout_index_options[] = {
172 OPT_BOOL('a', "all", &all,
173 N_("check out all files in the index")),
@@ -231,7 +232,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
232 if (read_from_stdin)
233 die("git checkout-index: don't mix '--stdin' and explicit filenames");
234 p = prefix_path(prefix, prefix_length, arg);
234 - checkout_file(p, prefix);
235 + err |= checkout_file(p, prefix);
236 free(p);
237 }
238
@@ -253,13 +254,16 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
254 strbuf_swap(&buf, &unquoted);
255 }
256 p = prefix_path(prefix, prefix_length, buf.buf);
256 - checkout_file(p, prefix);
257 + err |= checkout_file(p, prefix);
258 free(p);
259 }
260 strbuf_release(&unquoted);
261 strbuf_release(&buf);
262 }
263
264 + if (err)
265 + return 1;
266 +
267 if (all)
268 checkout_all(prefix, prefix_length);
269
t/t2004-checkout-cache-temp.sh
+1 -1
@@ -88,7 +88,7 @@ test_expect_success 'checkout all stage 2 to temporary files' '
88 done
89 '
90
91 -test_expect_failure 'checkout all stages of unknown path' '
91 +test_expect_success 'checkout all stages of unknown path' '
92 rm -f path* .merge_* actual &&
93 test_must_fail git checkout-index --stage=all --temp \
94 -- does-not-exist 2>stderr &&
t/t2006-checkout-index-basic.sh
+11
@@ -21,4 +21,15 @@ test_expect_success 'checkout-index -h in broken repository' '
21 test_i18ngrep "[Uu]sage" broken/usage
22 '
23
24 +test_expect_success 'checkout-index reports errors (cmdline)' '
25 + test_must_fail git checkout-index -- does-not-exist 2>stderr &&
26 + test_i18ngrep not.in.the.cache stderr
27 +'
28 +
29 +test_expect_success 'checkout-index reports errors (stdin)' '
30 + echo does-not-exist |
31 + test_must_fail git checkout-index --stdin 2>stderr &&
32 + test_i18ngrep not.in.the.cache stderr
33 +'
34 +
35 test_done