rev-parse: correctly diagnose revision errors before "--"

Rev-parse understands that a "--" may separate revisions and filenames, and that anything after the "--" is taken as-is. However, it does not understand that anything before the token must be a revision (which is the usual rule implemented by the setup_revisions parser). Since rev-parse prefers revisions to files when parsing before the "--", we end up with the correct result (if such an argument is a revision, we parse it as one, and if it is not, it is an error either way). However, we misdiagnose the errors: $ git rev-parse foobar -- >/dev/null fatal: ambiguous argument 'foobar': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' $ >foobar $ git rev-parse foobar -- >/dev/null fatal: bad flag '--' used after filename In both cases, we should know that the real error is that "foobar" is meant to be a revision, but could not be resolved. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Dec 6, 2013 at 17:05 UTC 141856738152d02beac5d4270a310a6007597282
2 files changed +34
builtin/rev-parse.c
+10
@@ -476,6 +476,7 @@ N_("git rev-parse --parseopt [options] -- [<args>...]\n"
476 int cmd_rev_parse(int argc, const char **argv, const char *prefix)
477 {
478 int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
479 + int has_dashdash = 0;
480 int output_prefix = 0;
481 unsigned char sha1[20];
482 const char *name = NULL;
@@ -489,6 +490,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
490 if (argc > 1 && !strcmp("-h", argv[1]))
491 usage(builtin_rev_parse_usage);
492
493 + for (i = 1; i < argc; i++) {
494 + if (!strcmp(argv[i], "--")) {
495 + has_dashdash = 1;
496 + break;
497 + }
498 + }
499 +
500 prefix = setup_git_directory();
501 git_config(git_default_config, NULL);
502 for (i = 1; i < argc; i++) {
@@ -765,6 +773,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
773 }
774 if (verify)
775 die_no_single_rev(quiet);
776 + if (has_dashdash)
777 + die("bad revision '%s'", arg);
778 as_is = 1;
779 if (!show_file(arg, output_prefix))
780 continue;
t/t1506-rev-parse-diagnosis.sh
+24
@@ -196,4 +196,28 @@ test_expect_success 'dotdot is not an empty set' '
196 test_cmp expect actual
197 '
198
199 +test_expect_success 'arg before dashdash must be a revision (missing)' '
200 + test_must_fail git rev-parse foobar -- 2>stderr &&
201 + test_i18ngrep "bad revision" stderr
202 +'
203 +
204 +test_expect_success 'arg before dashdash must be a revision (file)' '
205 + >foobar &&
206 + test_must_fail git rev-parse foobar -- 2>stderr &&
207 + test_i18ngrep "bad revision" stderr
208 +'
209 +
210 +test_expect_success 'arg before dashdash must be a revision (ambiguous)' '
211 + >foobar &&
212 + git update-ref refs/heads/foobar HEAD &&
213 + {
214 + # we do not want to use rev-parse here, because
215 + # we are testing it
216 + cat .git/refs/heads/foobar &&
217 + printf "%s\n" --
218 + } >expect &&
219 + git rev-parse foobar -- >actual &&
220 + test_cmp expect actual
221 +'
222 +
223 test_done