Wrong option -h in grep, ls-remote, and show-ref.

7 messages, 4 authors, 2021-09-24 · open the first message on its own page

Wrong option -h in grep, ls-remote, and show-ref.

From: Ignacy Gawedzki <hidden>
Date: 2021-09-24 14:35:24

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

git ls-remote -h

What did you expect to happen? (Expected behavior)

The same as git ls-remote --heads.

What happened instead? (Actual behavior)

Displayed the git ls-remote usage.

What's different between what you expected and what actually happened?

The usage indicates -h is the same as --heads, while -h is handled
upstream and always displays the usage of the command.

Anything else you want to add:

The same problem exists with the following commands:

  grep -h is supposed to not show filenames according to its usage string.
  show-ref -h is defined as some hidden option equivalent to --head.

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.30.2
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.11.0-31-lowlatency #33-Ubuntu SMP PREEMPT Wed Aug 11 14:21:21 UTC 2021 x86_64
compiler info: gnuc: 10.2
libc info: glibc: 2.33
$SHELL (typically, interactive shell): /bin/zsh


[Enabled Hooks]
not run from a git repository - no hooks to show

[PATCH 0/3] show-ref, ls-remote, grep: fix -h handling

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-24 16:51:52

This fixes the issues reported at
http://lore.kernel.org/git/20210924141920.ezfpyf4uutob2h5z@zenon.in.qult.net,
thanks to Ignacy Gawedzki for the report!

Ævar Arnfjörð Bjarmason (3):
  show-ref: remove unused custom handling of -h
  ls-remote: remove documentation for custom -h option
  grep: pass PARSE_OPT_NO_INTERNAL_HELP, fix odd -h handling

 Documentation/git-ls-remote.txt | 1 -
 builtin/grep.c                  | 3 ++-
 builtin/ls-remote.c             | 2 +-
 builtin/show-ref.c              | 2 --
 t/t0012-help.sh                 | 4 +++-
 t/t7810-grep.sh                 | 4 ++++
 6 files changed, 10 insertions(+), 6 deletions(-)

-- 
2.33.0.1285.g7aff81f6560

[PATCH 1/3] show-ref: remove unused custom handling of -h

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-24 16:51:54

Since b92891f9783 (parseopt: add PARSE_OPT_NO_INTERNAL_HELP,
2009-03-08) parse_options() has handled "-h" unless told not to, so
when show-ref was migrated to parse_options() in
69932bc6117 (show-ref: migrate to parse-options, 2009-06-20) the
custom "-h" handling that was retained did nothing.

The option was then hidden in e62b3935056 (Show usage string for 'git
show-ref -h', 2009-11-09), but that OPT_BOOLEAN didn't do
anything. Let's just remove this dead code.

Reported-by: Ignacy Gawedzki <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 builtin/show-ref.c | 2 --
 1 file changed, 2 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 7f8a5332f83..8cefb663282 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -163,8 +163,6 @@ static const struct option show_ref_options[] = {
 	OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
 	OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
 		    "requires exact ref path")),
-	OPT_HIDDEN_BOOL('h', NULL, &show_head,
-			N_("show the HEAD reference, even if it would be filtered out")),
 	OPT_BOOL(0, "head", &show_head,
 	  N_("show the HEAD reference, even if it would be filtered out")),
 	OPT_BOOL('d', "dereference", &deref_tags,
-- 
2.33.0.1285.g7aff81f6560

[PATCH 2/3] ls-remote: remove documentation for custom -h option

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-24 16:51:54

The custom handling of the "-h" option was broken in
ba5f28bf79e (ls-remote: use parse-options api, 2016-01-19), first
released with Git v2.8.0. We've been promising that it's a synonym of
--head, but it's not.

We could make this work again by supplying the
PARSE_OPT_NO_INTERNAL_HELP flag to parse_options(), but if we were
writing this command today we wouldn't make this an exception. Since
it's been such a long time let's just remove this rather than
restoring the exception to "-h" handling.

Reported-by: Ignacy Gawedzki <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 Documentation/git-ls-remote.txt | 1 -
 builtin/ls-remote.c             | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 492e573856f..6e241640bd9 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -21,7 +21,6 @@ commit IDs.
 
 OPTIONS
 -------
--h::
 --heads::
 -t::
 --tags::
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index f4fd823af83..c5e68918b78 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -64,7 +64,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			   N_("path of git-upload-pack on the remote host"),
 			   PARSE_OPT_HIDDEN },
 		OPT_BIT('t', "tags", &flags, N_("limit to tags"), REF_TAGS),
-		OPT_BIT('h', "heads", &flags, N_("limit to heads"), REF_HEADS),
+		OPT_BIT(0, "heads", &flags, N_("limit to heads"), REF_HEADS),
 		OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
 		OPT_BOOL(0, "get-url", &get_url,
 			 N_("take url.<base>.insteadOf into account")),
-- 
2.33.0.1285.g7aff81f6560

[PATCH 3/3] grep: pass PARSE_OPT_NO_INTERNAL_HELP, fix odd -h handling

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-24 16:51:56

The "grep" command supports both "-h" and "-H" options, along with a
mandatory pattern, but this has been partially usurped by the "-h"
handling in parse_options().

The reason it's just been "odd" instead of a bug is that we'll only
print out "-h" usage with parse_options() if there's no further
non-option arguments, so instead of printing this brief blurb on a
stand-alone -h we'd print out the full usage:

    $ git grep -H
    fatal: no pattern given

But for the aforementioned reason a "git grep -h <pattern>" would
work, we wouldn't take the !PARSE_OPT_NO_INTERNAL_HELP branch in
parse_options_step(), would handle our own custom 'h' option, and
builtin/grep.c itself would know what to do at that point.

Reported-by: Ignacy Gawedzki <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 builtin/grep.c  | 3 ++-
 t/t0012-help.sh | 4 +++-
 t/t7810-grep.sh | 4 ++++
 3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 51278b01fa2..a2d6704aa41 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -981,7 +981,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	 */
 	argc = parse_options(argc, argv, prefix, options, grep_usage,
 			     PARSE_OPT_KEEP_DASHDASH |
-			     PARSE_OPT_STOP_AT_NON_OPTION);
+			     PARSE_OPT_STOP_AT_NON_OPTION |
+			     PARSE_OPT_NO_INTERNAL_HELP);
 	grep_commit_pattern_type(pattern_type_arg, &opt);
 
 	if (use_index && !startup_info->have_repository) {
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 913f34c8e9d..0b623fe794a 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -90,7 +90,9 @@ test_expect_success 'git help succeeds without git.html' '
 '
 
 test_expect_success 'generate builtin list' '
-	git --list-cmds=builtins >builtins
+	git --list-cmds=builtins >builtins &&
+	grep -v "^grep$" builtins >builtins+ &&
+	mv builtins+ builtins
 '
 
 while read builtin
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 6b6423a07c3..4c2777120b0 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -11,6 +11,10 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+test_expect_success 'usage' '
+	test_expect_code 128 git grep -h
+'
+
 test_invalid_grep_expression() {
 	params="$@" &&
 	test_expect_success "invalid expression: grep $params" '
-- 
2.33.0.1285.g7aff81f6560

Re: [PATCH 2/3] ls-remote: remove documentation for custom -h option

From: SZEDER Gábor <hidden>
Date: 2021-09-24 17:11:23

On Fri, Sep 24, 2021 at 06:51:45PM +0200, Ævar Arnfjörð Bjarmason wrote:
The custom handling of the "-h" option was broken in
ba5f28bf79e (ls-remote: use parse-options api, 2016-01-19), first
released with Git v2.8.0. We've been promising that it's a synonym of
--head, but it's not.

We could make this work again by supplying the
PARSE_OPT_NO_INTERNAL_HELP flag to parse_options(), but if we were
writing this command today we wouldn't make this an exception. Since
it's been such a long time let's just remove this rather than
restoring the exception to "-h" handling.
This breaks the case when '-h' is used in combination with a remote:

  $ git ls-remote -h origin
  225bc32a989d7a22fa6addafd4ce7dcd04675dbf	refs/heads/maint
  ddb1055343948e0d0bc81f8d20245f1ada6430a0	refs/heads/master
  4c38ced6901a8523cea197b31b2616240ec9fb6e	refs/heads/next
  ee03ddbf0ea6a78ad9a229bd029408bbff85232e	refs/heads/seen
  687d33056ee28fd03567f3725150e3fcd0582979	refs/heads/todo
quoted hunk
Reported-by: Ignacy Gawedzki <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 Documentation/git-ls-remote.txt | 1 -
 builtin/ls-remote.c             | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 492e573856f..6e241640bd9 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -21,7 +21,6 @@ commit IDs.
 
 OPTIONS
 -------
--h::
The description of this option contains the following:

  Note that git ls-remote -h used without anything else on the command
  line gives help, consistent with other git subcommands.
quoted hunk
 --heads::
 -t::
 --tags::
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index f4fd823af83..c5e68918b78 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -64,7 +64,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			   N_("path of git-upload-pack on the remote host"),
 			   PARSE_OPT_HIDDEN },
 		OPT_BIT('t', "tags", &flags, N_("limit to tags"), REF_TAGS),
-		OPT_BIT('h', "heads", &flags, N_("limit to heads"), REF_HEADS),
+		OPT_BIT(0, "heads", &flags, N_("limit to heads"), REF_HEADS),
 		OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
 		OPT_BOOL(0, "get-url", &get_url,
 			 N_("take url.<base>.insteadOf into account")),
-- 
2.33.0.1285.g7aff81f6560

Re: [PATCH 2/3] ls-remote: remove documentation for custom -h option

From: Jeff King <hidden>
Date: 2021-09-24 17:54:18

On Fri, Sep 24, 2021 at 07:11:13PM +0200, SZEDER Gábor wrote:
quoted
diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 492e573856f..6e241640bd9 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -21,7 +21,6 @@ commit IDs.
 
 OPTIONS
 -------
--h::
The description of this option contains the following:

  Note that git ls-remote -h used without anything else on the command
  line gives help, consistent with other git subcommands.
Yup. That comes from this thread discussing an identical patch:

  https://lore.kernel.org/git/b0397b3285eab3448a3fd5dd2c50abb9@pascalroeleven.nl/

which in turn references more discussion of this behavior:

  https://lore.kernel.org/git/xmqqk1ztmkbn.fsf@gitster.mtv.corp.google.com/

It's gross and weird, but kept for backwards compatibility.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help