imgpkg shouldn't error when no command is given

[benmoss]

What steps did you take: Ran imgpkg

What happened: I got an error:

imgpkg: Error: Use one of available subcommands: completion [bash|zsh|fish|powershell], copy, describe, help [command], pull, push, tag, version

What did you expect: To see the help text like in imgpkg --help

Anything else you would like to add: This seems like a standard CLI behavior, sed, grep, kubectl, most CLIs that don't have a useful argument-less behavior just print help

---

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

[github-actions[bot]]

This issue is being marked as stale due to a long period of inactivity and will be closed in 5 days if there is no response.

[joaopapereira]

Marked as accepted so that we can have a more consistent output with other tools

[ashpect]

Hey! I was going through this issue and the imgpkg cli in general for the past days.

Why failing ?

I went through the code base and realised this is due to the classic fallback mechanism of cobra which happens when you provide no arguments to the cli, but it is failing here due to the wrapper cobracli written and added upon cobra.

Where in codebase ?

https://github.com/carvel-dev/imgpkg/blob/1a02bc4ced5e9b0d1cd13ae080a1f830ef8a4ee8/pkg/imgpkg/cmd/imgpkg.go#L61 You can try this by commenting out all the cobrautil commands after the above given link and running ./imgpkg by default.

Reasons ?

Cobrautils make change to the cmd context given to cobra and thus the fallback fails. For example : I commented out all the cobrautils, except this one initial command on line 61 to check the issue and yes, it did not fall back to regular fallback : https://github.com/carvel-dev/imgpkg/blob/1a02bc4ced5e9b0d1cd13ae080a1f830ef8a4ee8/pkg/imgpkg/cmd/imgpkg.go#L61 I then dived the wholecodebase and also how the cobrautil and cobra was working and found that basically : cobra util checks for cmd.RunE which is a field of a cobra.Command instance. It represents the function that gets executed when the command is run. And cobrautil was giving it a func value, hence the inability of cobra to fallback. ShowSubcommands function is used as the default execution function as shown here. https://github.com/cppforlife/cobrautil/blob/acdfead391ef15cb1680064ef5e7afb9437a7d95/misc.go#L60C11-L60C11

As you can see the diff, when I print the cmd context. The is replaced by a pointer. Hence the changed context, which is probably the function pointer for custom function execution.

Questions

1. What is the significance of ShowSubcommands ? As far as I understood, it's made as a fallback for cases when no commands are associated with cobra. I think the author might have considered that the default fallback was also associated with a cmd type, and weird empty cases will get redirected to ShowSubcommands. Although I am not sure. I would be glad if @cppforlife provide some info.

Possible fix

1. We can make the default fallback as default by removing the parts where custom function invocations are added when there are none.
2. If we don't want to mess with the imported library (cobrautils), we can always make a custom check before the entry point of cobrautils package and redirect it. Although not the best way, but it definitely works.

Although the fix is in sight, I am not opening a PR with a fix as I would love to get inputs on which way should I proceed. Thanks..!!

[praveenrewar]

Hi @ashpect! Thanks for looking into this ❤️ The cobrautil library has a ShowHelp function which can be used with the root command (like this) to get the desired result.

[ashpect]

Oh.Great..!! I checked it by changing

	if cmd.RunE == nil {
		cmd.RunE = ShowSubcommands
	}

to

	if cmd.RunE == nil {
		cmd.RunE = ShowHelp
	}

and as expected, instead of populating it with Showsubcommands function when it's empty, it populates with Showhelp. 🥳 .

The best fix would be in cobrautils here : https://github.com/cppforlife/cobrautil/blob/acdfead391ef15cb1680064ef5e7afb9437a7d95/misc.go#L60C11-L60C11

But ofc, I can't change the vendors, they'll get overwritten. So, I am writing a check here, just before cobrautils.VisitCommands is called. https://github.com/carvel-dev/imgpkg/blob/1a02bc4ced5e9b0d1cd13ae080a1f830ef8a4ee8/pkg/imgpkg/cmd/imgpkg.go#L59

Upon realisation that RunE is always initialised to nil, no need of a check, we can directly initialise RunE to cobrautil.showcommands. Fixed it and opened a PR https://github.com/carvel-dev/imgpkg/blob/1a02bc4ced5e9b0d1cd13ae080a1f830ef8a4ee8/pkg/imgpkg/cmd/imgpkg.go#L37

[joaopapereira]

Fixed and will be released with version v0.38.0