Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible breaking change: help now goes to STDERR #1002

Closed
marckhouzam opened this issue Dec 13, 2019 · 6 comments · Fixed by #1004
Closed

Possible breaking change: help now goes to STDERR #1002

marckhouzam opened this issue Dec 13, 2019 · 6 comments · Fixed by #1004

Comments

@marckhouzam
Copy link
Collaborator

As part of #922 the help message was recently changed from being printed to STDOUT to now being printed to STDERR.

This sounds like a breaking change. Some scripts could be written with the expectation that the help message is on stdout, and changing it would break such scripts. I know the change broke one of Helm's acceptance testing scripts (helm/acceptance-testing#64). We've discussed this at the Helm developper call and we felt it was worth bringing up to the Cobra project.

Doing a little digging I found that sending the help message to STDOUT was a conscious decision done in #303, 3 years ago. Changing back to STDERR however seemed more of a "why not" decision, as can be interpreted from the original code change text from #642 (comment) from which #922 was based, which states:

Usage output in this scenario is also now sent to stderr instead of stdout, which is better for scripting and seems more consistent with surrounding code.

(Note that although the text says "Usage output" it is actually the Help output that was changed.)

The specific change in question is not part of a COBRA release yet so could easily be reverted to have the help message be sent to STDOUT again and avoid breaking automation scripts.

I can provide a PR if there is agreement to revert.

Thank you.

/cc @bruceadowns @BoGeM

@bruceadowns
Copy link
Contributor

@marckhouzam, thank you for the reasoned background.

Imho, help output should go to stderr. It makes more common sense to me. That said, I understand there is a downstream dependency concern that needs to be communicated.

I set out to back up my opinion with checking various cli and surprisingly there is little consistency.

For example docker vs go:

$ docker -h > /dev/null
$ go -h 2> /dev/null

Docker help goes to stdout and go help goes to stderr.

Another example of sed vs awk:

$ awk -h > /dev/null
$ sed -h 2> /dev/null

And bzip vs gzip:

$ gzip -h > /dev/null
$ bzip2 -h 2> /dev/null

And various others:

$ date --help > /dev/null
$ grep --help > /dev/null
$ ping -h 2> /dev/null
$ ln --help > /dev/null
$ curl --help > /dev/null
$ bzcat -help 2> /dev/null

@marckhouzam
Copy link
Collaborator Author

@bruceadowns Thank you for taking the time to investigate.

Your findings make me feel that if Cobra had output the help message on stderr from the beginning it would have been fine; however #303 actually purposefully changed it from stderr to stdout. Maybe @fabianofranz can shed some light about that decision?

That being said, even though a valid argument can be made about using stderr we are probably stuck because of the backwards compatibility need.

@jharshman
Copy link
Collaborator

So it looks like #303 ensured that the help command for both top level commands and subcommands printed to stdout.
kubernetes/kubernetes#26077 (comment)

The offending line in #922 is https://github.com/spf13/cobra/pull/922/files#diff-9c42b524cb14ca001c61267826cbefb1R375, where it changes that decision and prints all help command text to stderr.

It also might be worth mentioning GNU standards here which suggest that help output is sent to stdout.
https://www.gnu.org/prep/standards/html_node/_002d_002dhelp.html

@marckhouzam
Copy link
Collaborator Author

So it looks like #303 ensured that the help command for both top level commands and subcommands printed to stdout.
kubernetes/kubernetes#26077 (comment)

Thanks @jharshman for finding this! Seems enough to revert the change. I will post a PR.

marckhouzam added a commit to marckhouzam/cobra that referenced this issue Dec 22, 2019
Fixes spf13#1002
For backwards compatibility reasons, and to follow the need of
kubernetes/kubernetes#26077 (comment)
the help message should be printed on stdout.

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
marckhouzam added a commit to marckhouzam/cobra that referenced this issue Dec 22, 2019
Fixes spf13#1002
For backwards compatibility reasons, and to follow the need of
kubernetes/kubernetes#26077 (comment)
the help message should be printed on stdout.

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
eparis pushed a commit that referenced this issue Dec 23, 2019
Fixes #1002
For backwards compatibility reasons, and to follow the need of
kubernetes/kubernetes#26077 (comment)
the help message should be printed on stdout.

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
@fabianofranz
Copy link
Contributor

The change to print to STDOUT instead of STDERR in #303 back in 2016 was conscious and based on the GNU Standards for CLI --help. GNU Standards and POSIX must be, when available, the single source of thruth for these kinds of decisions. Thanks for reverting it.

@bruceadowns
Copy link
Contributor

The original intent of #922 was to ensure that a non-zero exit code is emitted in the case of invalid subcommands or arguments. I've verified that this behavior was preserved through #1004.

I agree that if help is explicitly requested, it should be printed to stdout with a zero exit code per gnu standards. This tracking issue provided great clarity.

hhhapz added a commit to hhhapz/gunk that referenced this issue Nov 18, 2021
This change migrates all of the current CLI code (and related tests) to
now use cobra. All functionality should stay identical with the flags.

The only changes present within the tests are the formatting of specific
errors, and how they differ between kingpin/cobra.

One potential breaking change that could impact users is that all
`gunk help...` commands now print to stdout, even if invalid. This is
not true for other errors, so it probably should not break any real
world applications.

This change can be linked back to spf13/cobra#1002.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants