rev-parse: don't accept options after dashdash

Because of the order in which we check options in rev-parse, there are a few options we accept even after a "--". This is wrong, because the whole point of "--" is to say "everything after here is a path". Let's move the "did we see a dashdash" check (it's called "as_is" in the code) to the top of the parsing loop. Note there is one subtlety here. The options are ordered so that some are checked before we even see if we're in a repository (they continue the loop, and if we get past a certain point, then we do the repository setup). By moving the as_is check higher, it's also in that "before setup" section, even though it might look at the repository via verify_filename(). However, this works out: we'd never set as_is until we parse "--", and we don't parse that until after doing the setup. An alternative here to avoid the subtlety is to put the as_is check at the top of the post-setup options. But then every pre-setup option would have to remember to check "if (!as_is && !strcmp(...))". So while this is a bit magical, it's harder for future code to get wrong. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Nov 10, 2020 at 16:37 UTC e05e2ae8fe6e5d83254c27664c66bc47b270b6d2
2 files changed +15 -5
builtin/rev-parse.c
+6 -5
@@ -622,6 +622,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
622 for (i = 1; i < argc; i++) {
623 const char *arg = argv[i];
624
625 + if (as_is) {
626 + if (show_file(arg, output_prefix) && as_is < 2)
627 + verify_filename(prefix, arg, 0);
628 + continue;
629 + }
630 +
631 if (!strcmp(arg, "--local-env-vars")) {
632 int i;
633 for (i = 0; local_repo_env[i]; i++)
@@ -655,11 +661,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
661 i++;
662 continue;
663 }
658 - if (as_is) {
659 - if (show_file(arg, output_prefix) && as_is < 2)
660 - verify_filename(prefix, arg, 0);
661 - continue;
662 - }
664 if (!strcmp(arg,"-n")) {
665 if (++i >= argc)
666 die("-n requires an argument");
t/t1506-rev-parse-diagnosis.sh
+9
@@ -254,4 +254,13 @@ test_expect_success 'escaped char does not trigger wildcard rule' '
254 test_must_fail git rev-parse "foo\\*bar"
255 '
256
257 +test_expect_success 'arg after dashdash not interpreted as option' '
258 + cat >expect <<-\EOF &&
259 + --
260 + --local-env-vars
261 + EOF
262 + git rev-parse -- --local-env-vars >actual &&
263 + test_cmp expect actual
264 +'
265 +
266 test_done