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>

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index ed200c8af1285ed4ac45a50aaa6275a739585836..293428fa0d21407fe465c1ab2a00ae8f5c336713 100644 (file)
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -622,6 +622,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
        for (i = 1; i < argc; i++) {
                const char *arg = argv[i];
 
+               if (as_is) {
+                       if (show_file(arg, output_prefix) && as_is < 2)
+                               verify_filename(prefix, arg, 0);
+                       continue;
+               }
+
                if (!strcmp(arg, "--local-env-vars")) {
                        int i;
                        for (i = 0; local_repo_env[i]; i++)
@@ -655,11 +661,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
                        i++;
                        continue;
                }
-               if (as_is) {
-                       if (show_file(arg, output_prefix) && as_is < 2)
-                               verify_filename(prefix, arg, 0);
-                       continue;
-               }
                if (!strcmp(arg,"-n")) {
                        if (++i >= argc)
                                die("-n requires an argument");

diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
index 3e657e693b272dec5711593ae85eec7c6725b9dc..2ed5d50059e7bff527e6714319db90a87cfe8034 100755 (executable)
--- a/t/t1506-rev-parse-diagnosis.sh
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -254,4 +254,13 @@ test_expect_success 'escaped char does not trigger wildcard rule' '
        test_must_fail git rev-parse "foo\\*bar"
 '
 
+test_expect_success 'arg after dashdash not interpreted as option' '
+       cat >expect <<-\EOF &&
+       --
+       --local-env-vars
+       EOF
+       git rev-parse -- --local-env-vars >actual &&
+       test_cmp expect actual
+'
+
 test_done