Merge branch 'jk/rev-parse-end-of-options'

"git rev-parse" learned the "--end-of-options" to help scripts to
safely take a parameter that is supposed to be a revision, e.g.
"git rev-parse --verify -q --end-of-options $rev".

* jk/rev-parse-end-of-options:
  rev-parse: handle --end-of-options
  rev-parse: put all options under the "-" check
  rev-parse: don't accept options after dashdash

Documentation/git-rev-parse.txt

@@ -109,6 +109,10 @@ names an existing object that is a commit-ish (i.e. a commit, or an
 annotated tag that points at a commit).  To make sure that `$VAR`
 names an existing object of any type, `git rev-parse "$VAR^{object}"`
 can be used.
++
+Note that if you are verifying a name from an untrusted source, it is
+wise to use `--end-of-options` so that the name argument is not mistaken
+for another option.
 
 -q::
 --quiet::
@@ -446,7 +450,7 @@ $ git rev-parse --verify HEAD
 * Print the commit object name from the revision in the $REV shell variable:
 +
 ------------
-$ git rev-parse --verify $REV^{commit}
+$ git rev-parse --verify --end-of-options $REV^{commit}
 ------------
 +
 This will error out if $REV is empty or not a valid revision.
@@ -454,7 +458,7 @@ This will error out if $REV is empty or not a valid revision.
 * Similar to above:
 +
 ------------
-$ git rev-parse --default master --verify $REV
+$ git rev-parse --default master --verify --end-of-options $REV
 ------------
 +
 but if $REV is empty, the commit object name from master will be printed.

builtin/rev-parse.c

@@ -595,6 +595,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 	struct object_context unused;
 	struct strbuf buf = STRBUF_INIT;
 	const int hexsz = the_hash_algo->hexsz;
+	int seen_end_of_options = 0;
 
 	if (argc > 1 && !strcmp("--parseopt", argv[1]))
 		return cmd_parseopt(argc - 1, argv + 1, prefix);
@@ -622,21 +623,29 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 
-		if (!strcmp(arg, "--local-env-vars")) {
+		if (as_is) {
-			int i;
+			if (show_file(arg, output_prefix) && as_is < 2)
-			for (i = 0; local_repo_env[i]; i++)
+				verify_filename(prefix, arg, 0);
-				printf("%s\n", local_repo_env[i]);
 			continue;
 		}
-		if (!strcmp(arg, "--resolve-git-dir")) {
+
-			const char *gitdir = argv[++i];
+		if (!seen_end_of_options) {
-			if (!gitdir)
+			if (!strcmp(arg, "--local-env-vars")) {
-				die("--resolve-git-dir requires an argument");
+				int i;
-			gitdir = resolve_gitdir(gitdir);
+				for (i = 0; local_repo_env[i]; i++)
-			if (!gitdir)
+					printf("%s\n", local_repo_env[i]);
-				die("not a gitdir '%s'", argv[i]);
+				continue;
-			puts(gitdir);
+			}
-			continue;
+			if (!strcmp(arg, "--resolve-git-dir")) {
+				const char *gitdir = argv[++i];
+				if (!gitdir)
+					die("--resolve-git-dir requires an argument");
+				gitdir = resolve_gitdir(gitdir);
+				if (!gitdir)
+					die("not a gitdir '%s'", argv[i]);
+				puts(gitdir);
+				continue;
+			}
 		}
 
 		/* The rest of the options require a git repository. */
@@ -646,41 +655,36 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 			did_repo_setup = 1;
 		}
 
-		if (!strcmp(arg, "--git-path")) {
+		if (!strcmp(arg, "--")) {
-			if (!argv[i + 1])
+			as_is = 2;
-				die("--git-path requires an argument");
+			/* Pass on the "--" if we show anything but files.. */
-			strbuf_reset(&buf);
+			if (filter & (DO_FLAGS | DO_REVS))
-			puts(relative_path(git_path("%s", argv[i + 1]),
+				show_file(arg, 0);
-					   prefix, &buf));
-			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");
-			if ((filter & DO_FLAGS) && (filter & DO_REVS)) {
-				show(arg);
-				show(argv[i]);
-			}
-			continue;
-		}
-		if (starts_with(arg, "-n")) {
-			if ((filter & DO_FLAGS) && (filter & DO_REVS))
-				show(arg);
 			continue;
 		}
 
-		if (*arg == '-') {
+		if (!seen_end_of_options && *arg == '-') {
-			if (!strcmp(arg, "--")) {
+			if (!strcmp(arg, "--git-path")) {
-				as_is = 2;
+				if (!argv[i + 1])
-				/* Pass on the "--" if we show anything but files.. */
+					die("--git-path requires an argument");
-				if (filter & (DO_FLAGS | DO_REVS))
+				strbuf_reset(&buf);
-					show_file(arg, 0);
+				puts(relative_path(git_path("%s", argv[i + 1]),
+						   prefix, &buf));
+				i++;
+				continue;
+			}
+			if (!strcmp(arg,"-n")) {
+				if (++i >= argc)
+					die("-n requires an argument");
+				if ((filter & DO_FLAGS) && (filter & DO_REVS)) {
+					show(arg);
+					show(argv[i]);
+				}
+				continue;
+			}
+			if (starts_with(arg, "-n")) {
+				if ((filter & DO_FLAGS) && (filter & DO_REVS))
+					show(arg);
 				continue;
 			}
 			if (!strcmp(arg, "--default")) {
@@ -937,6 +941,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				puts(the_hash_algo->name);
 				continue;
 			}
+			if (!strcmp(arg, "--end-of-options")) {
+				seen_end_of_options = 1;
+				if (filter & (DO_FLAGS | DO_REVS))
+					show_file(arg, 0);
+				continue;
+			}
 			if (show_flag(arg) && verify)
 				die_no_single_rev(quiet);
 			continue;

t/t1503-rev-parse-verify.sh

@@ -144,4 +144,17 @@ test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
 	test_must_fail git rev-parse --verify broken
 '
 
+test_expect_success 'options can appear after --verify' '
+	git rev-parse --verify HEAD >expect &&
+	git rev-parse --verify -q HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'verify respects --end-of-options' '
+	git update-ref refs/heads/-tricky HEAD &&
+	git rev-parse --verify HEAD >expect &&
+	git rev-parse --verify --end-of-options -tricky >actual &&
+	test_cmp expect actual
+'
+
 test_done

t/t1506-rev-parse-diagnosis.sh

@@ -254,4 +254,29 @@ 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_expect_success 'arg after end-of-options not interpreted as option' '
+	test_must_fail git rev-parse --end-of-options --not-real -- 2>err &&
+	test_i18ngrep bad.revision.*--not-real err
+'
+
+test_expect_success 'end-of-options still allows --' '
+	cat >expect <<-EOF &&
+	--end-of-options
+	$(git rev-parse --verify HEAD)
+	--
+	path
+	EOF
+	git rev-parse --end-of-options HEAD -- path >actual &&
+	test_cmp expect actual
+'
+
 test_done