Git branch outputs usage message on stderr

Hey, please find the report below.

What did you do before the bug happened? (Steps to reproduce your issue) I opened a terminal on Arch Linux with a bash shell and called `git branch -h` to get a usage overview of git's `branch` command. I then tried processing the output with `grep` by `git branch -h | grep list` which gave the whole (unfiltered) output, i.e., the displayed message was not processed by `grep`.

What did you expect to happen? (Expected behavior) Pipe redirects stdout to grep (here: usage info of git branch) and grep can filter it.

What happened instead? (Actual behavior) `git branch -h` output was not redirected to `grep`. Instead, it went to stderr, bypassing the pipe. Using `|&` as pipe gives the expected result and confirms that the usage output is indeed redirected to stderr.

What's different between what you expected and what actually happened? To me, `git branch -h` (invoked without any non-valid option) shall output its help/usage message to stdout, and exit without error status (exit status 0).

Anything else you want to add: In fact, `git branch -h` exits with a non-zero status. This is coherent with outputting the usage message on stderr and made me think the behavior could be intended. I can hardly imagine, as outputting usage info to stderr to me only made sense if `git branch` is invoked with, e.g., a non-valid option (if at all). I would prefer the usage output on stderr even in this case, but allow for a non-zero exit status of git branch. Also, `git -h` gives its output on stdout.

[System Info]
git version:
git version 2.48.1
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
libcurl: 8.11.1
OpenSSL: OpenSSL 3.4.0 22 Oct 2024
zlib: 1.3.1
uname: Linux 6.12.8-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 02 Jan 2025 
22:52:26 +0000 x86_64
compiler info: gnuc: 14.2
libc info: glibc: 2.40
$SHELL (typically, interactive shell): /bin/bash

Best,

On Wed Jan 15, 2025 at 12:22 PM CET, Jonas Konrad wrote:

And that is exactly the correct behaviour. In the world of UNIX, where pipes are normal, utilities should send to the stdout only substantial material, which could be processed down the pipeline. Error messages, help, and similar diagnostics, should go to stderr. Also, you know about `|&`, right?

Best,

Matěj

Well, let me add that one detail: I could not find any other git sub command which behaves the same (admittedly haven't tried all). Instead, the ones I have tried give their help output usually on stdout. Try yourself with `git -h`. It perfectly acts according to what I'd expect (exit status 0). Where as `git reflog -h`outputs to stdout but still exit with non-zero status (which seems okay). `git branch -h` adds a third way, as described. I argue: That diverging behavior is confusing to the user.

If your claim was right, this bug report got even bigger as a lot of behavior from other subcommands would have to be changed. In fact, what "substantial material" is, has to be defined for every piece of software regarding each output. However, I cannot see how one subcommand's usage message is considered "substantial" whereas another subcommand's usage message is not (besides, I guess, `git branch` is more frequently used than `git reflog`).

Best, Jonas

On 15.01.25 12:36, Matěj Cepl wrote:

Matěj Cepl <mcepl@cepl.eu> writes:

If I understand the case Jonas reports correctly, he is talking about "git branch -h<RETURN>", and the "substantial material" (I'd rather phrase it as the primary output in response to the end user request) in that case is the help text.

Somebody may want to go over "git help --all" and for each of them try "git $cmd -h >/dev/null" to find those that give the help output to their standard error stream.

Thanks, both.

On Wed, Jan 15, 2025, at 16:28, Junio C Hamano wrote:

    #!/bin/sh

    for cmd in $(git --list-cmds=builtins); do
        git $cmd -h >/dev/null
    done 2>&1 | grep '^usage: ' \
        | perl -pe 's/^usage:\s*(\(EXPERIMENTAL!\)\s*)?//; s/^(git\s+[a-zA-Z0-9-]+).*/\1/'

Gives

    git am
    git branch
    git check-ref-format
    git checkout--worker
    git checkout-index
    git commit
    git commit-tree
    git credential
    git diff
    git diff-files
    git diff-index
    git diff-tree
    git fast-import
    git fetch-pack
    git fsmonitor--daemon
    git gc
    git get-tar-commit-id
    git index-pack
    git ls-files
    git mailsplit
    git merge
    git merge-index
    git merge-ours
    git merge-recursive
    git merge-recursive-ours
    git merge-recursive-theirs
    git merge-subtree
    git pack-redundant
    git rebase
    git remote-ext
    git remote-fd
    git rev-list
    git rev-parse
    git status
    git unpack-file
    git unpack-objects
    git update-index
    git upload-archive
    git upload-archive
    git var

For

    $ git version
    git version 2.48.0

On Wed, Jan 15, 2025 at 05:55:19PM +0100, Kristoffer Haugsbakk wrote:

We may want:

Jeff King <peff@peff.net> writes:

Yup, I was looking at the code involved and came to the same conclusion.

Jeff King <peff@peff.net> writes:

It would look something like this; I am not sure if I like the "this may show help and exit if the user requested, but otherwise it is a no-op" semantics, though.

 builtin/am.c    |  3 +--
 parse-options.c | 10 ++++++++++
 parse-options.h |  4 ++++
 3 files changed, 15 insertions(+), 2 deletions(-)

On Wed, Jan 15, 2025 at 09:56:23AM -0800, Junio C Hamano wrote:

Yeah, I agree it is funny to have a "maybe noop, maybe exit" function. Perhaps a different name would help? I'd expect show_usage_help() to always do what the name says. Maybe check_help_option() or something?

I think parse-options will exit(129) in this case, and that's what t0012 insists upon.

-Peff

Jeff King <peff@peff.net> writes:

maybe_show_usage_help()?

Yeah, but the test can be adjusted to updated reality if needed.  

In this case, the command is doing what the end-user asked it to do, and if we were writing the system from scratch, 0 would certainly be the right exit status in this case. If hit usage_with_options() because the command line option supplied by the user was nonsense, we should exit with non-zero, but I am not sure if exit(129) is a good idea here.

On Wed, Jan 15, 2025 at 01:16:50PM -0800, Junio C Hamano wrote:

Heh, I almost suggested that one, too, but worried it was too clunky. But maybe since we both thought of it...

I certainly see an argument for exit(0), but whatever we do should be consistent with how parse-options handles it (since whether we use this or leave it to parse-options is purely an implementation detail that the user should not need to be aware of).

And it uses code 129, even for "-h". I don't see any explicit rationale for that in the history; I think it goes back to the beginning of parse-options. It happens via the PARSE_OPT_HELP flag, but curiously we also trigger that for ambiguous options (which should exit with error). That might be a bug-in-waiting if we start handling PARSE_OPT_HELP differently.

-Peff

Jeff King <peff@peff.net> writes:

Here is what I have as v2; there will be patches that touch builtin/*.c in between and I expect that the last patch to conclude the series will end with an update to parse-options.c (to exit with 0 when asked to give a help) and t0012 (to stop expecting 129).

--- >8 ---
Subject: [PATCH v2] parse-options: add show_usage_help_and_exit_if_asked()

Many commands call usage_with_options() when they are asked to give the help message, but it incorrectly sends the help text to the standard error stream. When the user asked for it with "git cmd -h", the help message is the primary output from the command, hence we should send it to the standard output stream.

Introduce a helper function that captures the common pattern

	if (argc == 2 && !strcmp(argv[1], "-h"))
		usage_with_options(usage, options);

and replaces it with

	show_usage_help_and_exit_if_asked(argc, argv, usage, options);

to help correct code paths (there are 40 or so of them).

Note that this helper function still exits with status 129, and t0012 insists on it. After converting all the mistaken callers of usage_with_options() to call this new helper, we may want to address it---the end user is asking us to give the help text, and we are doing exactly as asked, so there is no reason to exit with non-zero status.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 parse-options.c | 10 ++++++++++
 parse-options.h |  4 ++++
 2 files changed, 14 insertions(+)

On Wed, Jan 15, 2025 at 01:56:31PM -0800, Junio C Hamano wrote:

Thanks, this looks fine. The name is clunky but probably OK. ;)

I don't know if we'd want something like this on top. If somebody is interested in just doing all the conversions in the near-term, we could do without the optional flag.

-- >8 --
Subject: [PATCH] t0012: optionally check that "-h" output goes to stdout

For most commands, "git foo -h" will send the help output to stdout, as this is what parse-options.c does. But some commands send it to stderr instead. This is usually because they call usage_with_options(), and should be switched to show_usage_help_and_exit_if_asked().

Currently t0012 is permissive and allows either behavior. We'd like it to eventually enforce that help goes to stdout, and teaching it to do so identifies the commands that need to be changed. But during the transition period, we don't want to enforce that for most test runs.

So let's introduce a flag that will let most test runs use the permissive behavior, and people interested in converting commands can run:

  GIT_TEST_HELP_MUST_BE_STDOUT=1 ./t0012-help.sh

to see the failures. Eventually (when all builtins have been converted) we'll remove this flag entirely and always check the strict behavior.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t0012-help.sh | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Jeff King <peff@peff.net> writes:

Ah, you are much more practical than I am ;-) I was wondering if we want a list of "these commands have already been updated" and behave differently.

Yeah, that is why the "git branch -h" thing has been left broken for such a long time.

Nice.

It is a tangent, but I wonder how many among the 40 really needed to use usage_with_options() to react to "-h" in the first place. In other words, these manual checks for "-h" are done only because the code _wants_ to react to "-h" before it calls parse_options(), but does everybody who _wants_ to do so really _needs_ to do so? You already have shown that "gir branch" did not have to, and to me, 40 among 100+ felt way too many.

This may be a bit stricter than needed (things other than usage may need to be spitted out), but it is sufficent to declare that we will deal with any potential fallout only after it becomes necessary ;-).

Will queue.  Thanks.

Junio C Hamano <gitster@pobox.com> writes:

It turns out that some of the actually cannot be helped, as they do not even use parse-options and calling usage() themselves is the only way to show the usage text for them.

I'll send a 6-patch series out shortly.

On Wed, Jan 15, 2025 at 03:32:29PM -0800, Junio C Hamano wrote:

Heh, I started writing it that way but then got turned off by how annoying it is to look up in a list of strings in bash. ;)

Yes, I had the same thought. Unfortunately it is a lot of brain-power to examine each one, with relatively little gain.

Yep. I'd hope for the most part that "-h" would spew help and nothing else, but I guess we could get hit with a deprecation notice or something. I agree on crossing that bridge when we come to it.

-Peff

Jeff King <peff@peff.net> writes:

There is another class of callers that are protected by the same "argc == 2 && !strcmp(argv[1], "-h")" condition, and they call usage.c:usage(), instead of calling usage_with_options(). These calls (but not all calls to usage()) need to be updated to use a similar helper, say, show_usage_and_exit_if_asked(). Sigh...

On Wed, Jan 15, 2025 at 02:11:56PM -0800, Junio C Hamano wrote:

Oof. And that uses vreportf(), which always writes to stderr. So more refactoring.

-Peff

Jeff King <peff@peff.net> writes:

Yes, it's ugly ;-)

In any case, this is a result of my sweek in builtin/ for usage_with_options(). The remaining

	if (argc == 2 && !strcmp(argv[1], "-h"))

are all protecting usage() call.

---- >8 ----
Subject: [PATCH] builtins: send help text to standard output

Using the show_usage_help_and_exit_if_asked() helper we introduced earlier, fix callers of usage_with_options() that want to show the help text when explicitly asked by the end-user. The help text now goes to the standard output stream for them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/am.c                | 3 +--
 builtin/branch.c            | 4 ++--
 builtin/checkout--worker.c  | 6 +++---
 builtin/checkout-index.c    | 6 +++---
 builtin/commit-tree.c       | 4 ++--
 builtin/commit.c            | 8 ++++----
 builtin/fsmonitor--daemon.c | 4 ++--
 builtin/gc.c                | 4 ++--
 builtin/ls-files.c          | 4 ++--
 builtin/merge.c             | 4 ++--
 builtin/rebase.c            | 6 +++---
 builtin/update-index.c      | 4 ++--
 t/t7600-merge.sh            | 2 +-
 13 files changed, 29 insertions(+), 30 deletions(-)

Junio C Hamano <gitster@pobox.com> writes:

After thinking about it a bit more, I am starting to like it, not because it reduces a few more lines from the calling site, but because it makes it almost impossible to use for a careless and incorrect conversion from usage_with_options(). Any existing callsite of usage_with_options() MUST be inspected to make sure it is responding to an end-user request to give "-h"(elp) text before being replaced with the call to a new helper, and if I made show_usage_help() without argc/argv, a careless developer can easily and blindly do string replacement to send a ton of patch that reviewers need to waste time to do the inspection for them.

But with your "argc and argv checking included" approach, it is harder to do the blind replacement.

--- >8 ---
From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 15 Jan 2025 09:56:23 -0800
Subject: [PATCH] parse-options: add show_usage_help()

Many commands call usage_with_options() when they are asked to give the help message, but it incorrectly sends the help text to the standard error stream. When the user asked for it with "git cmd -h", the help message is the primary output from the command, hence we should send it to the standard output stream.

Introduce a helper function that captures the common pattern

	if (argc == 2 && !strcmp(argv[1], "-h"))
		usage_with_options(usage, options);

and replaces it with

	show_usage_help(argc, argv, usage, options);

to help correct code paths (there are 40 or so of them).

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 parse-options.c | 10 ++++++++++
 parse-options.h |  4 ++++
 2 files changed, 14 insertions(+)

On Wed, Jan 15, 2025, at 19:29, Junio C Hamano wrote:

+Reported-by: Jonas Konrad <jonas.konrad@uni-muenster.de>

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

Not on this patch, but it would be a good addition to a follow-up patch that uses this new API function to update builtin/branch.c (which I do not plan to do myself).

Thanks.

On 15.01.25 17:55, Kristoffer Haugsbakk wrote:

Was just about to share the very same results, leading to 40 commands out of 142 built-ins outputting their usage info to stderr. Some additions on non-builtins: git-scalar also outputs its usage info to stderr. git-lfs does not have "usage text for -h". I have not tested git-svn and git-cvsserver properly (do not have installed the respective modules). On another note, git-p4 does not know "-h", but then gives usage info - to stdout(!)). Lastly, if you still read, test git-http-backend and git-filter-branch, as they show special behavior.

On Wed, Jan 15, 2025, at 18:14, Jonas Konrad wrote:

Okay, `git-http-backend` is listed in `git --list-cmds=main`.

That one doesn’t support `-h`.

    $ git http-backend -h >/dev/null
    fatal: No REQUEST_METHOD from server

and

    $ git http-backend -h 2>/dev/null
    Status: 500 Internal Server Error
    Expires: Fri, 01 Jan 1980 00:00:00 GMT
    Pragma: no-cache
    Cache-Control: no-cache, max-age=0, must-revalidate

git-filter-branch(1) prints a warning about “glut of gotchas”, waits 10 seconds, then prints usage to stdout.

    $ time git filter-branch -h >/dev/null

    real	0m10,058s
    user	0m0,023s
    sys	0m0,041s

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

Being consistent is a good idea, and I wanted to first gauge which way we should unify. It seems that those who spit their help text into their standard error stream are indeed in minority?

Thanks.

On Wed, Jan 15, 2025, at 18:19, Junio C Hamano wrote:

Yes: 40 of those stderr `-h` outputs.

Versus 102 that use stdout.[1]

Trying a random command with usage-on-error:

    $ git-upload-pack >/dev/null
    usage: [snip]

Does give usage on stderr.

† 1:
    git add
    git annotate
    git apply
    git archive
    git bisect
    git blame
    git bugreport
    git bundle
    git cat-file
    git check-attr
    git check-ignore
    git check-mailmap
    git checkout
    git cherry
    git cherry-pick
    git clean
    git clone
    git column
    git commit-graph
    git config
    git count-objects
    git credential-cache
    git credential-cache--daemon
    git credential-store
    git describe
    git diagnose
    git difftool
    git fast-export
    git fetch
    git fmt-merge-msg
    git for-each-ref
    git for-each-repo
    git format-patch
    git fsck
    git fsck
    git grep
    git hash-object
    git help
    git hook
    git init
    git init
    git interpret-trailers
    git log
    git ls-remote
    git ls-tree
    git mailinfo
    git maintenance
    git merge-base
    git merge-file
    git merge-tree
    git mktag
    git mktree
    git multi-pack-index
    git mv
    git name-rev
    git notes
    git pack-objects
    git pack-refs
    git patch-id
    git blame
    git prune
    git prune-packed
    git pull
    git push
    git range-diff
    git read-tree
    git receive-pack
    git reflog
    git refs
    git remote
    git repack
    git replace
    git replay
    git rerere
    git reset
    git restore
    git revert
    git rm
    git send-pack
    git shortlog
    git log
    git show-branch
    git show-index
    git show-ref
    git sparse-checkout
    git add
    git stash
    git stripspace
    git submodule--helper
    git switch
    git symbolic-ref
    git tag
    git update-ref
    git update-server-info
    git-upload-pack
    git verify-commit
    git verify-pack
    git verify-tag
    git version
    git log
    git worktree
    git write-tree

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

Good; that is what we want to see.  Thanks.