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

Reconcile all Carvel tools to behave identically when no arguments are given #290

Open
pivotaljohn opened this issue Oct 28, 2021 · 4 comments
Labels
discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution enhancement This issue is a feature request

Comments

@pivotaljohn
Copy link
Contributor

pivotaljohn commented Oct 28, 2021

Describe the problem/challenge you have
Reported first as: carvel-dev/kbld#184

For each command-line tool in the Carvel suite, they behave differently from others when no arguments are given.

tool displays help displays error message exit status
imgpkg 1
kapp 1
kbld ❌ (displays "Succeeded") 0
kwt 1
vendir 1
ytt 0

Describe the solution you'd like
For all the tools to behave identically, unless there's a clear reason to be different.

Seems like the most useful behavior would be to:

  • display the help text,
  • report an error message, and
  • exit with a non-zero exit status.

Anything else you would like to add:
[Additional information that will assist in solving the issue.]

@pivotaljohn pivotaljohn added enhancement This issue is a feature request discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution labels Oct 28, 2021
@cppforlife
Copy link
Contributor

both ytt and kbld do not display any error, because it theory its valid to give them to input and have them return no output. other commands do not have such behaviour because they always require some input.

@pivotaljohn
Copy link
Contributor Author

both ytt and kbld do not display any error, because it theory its valid to give them to input and have them return no output. other commands do not have such behaviour because they always require some input.

I don't understand the theory. In the ytt case, it's my understanding that in order to accept input, the --file flag must be given.

It's the difference between:

$ echo "foo: bar" | ytt
$

and

$ echo "foo: bar" | ytt -f-
foo: bar
$

and the same kind of thing seems to apply to kbld as well:

$ echo "image: nginx" | kbld
Succeeded

and

$ echo "image: nginx" | kbld -f-
resolve | final: nginx -> index.docker.io/library/nginx@sha256:644a70516a26004c97d0d85c7fe1d0c3a67ea8ab7ddf4aff193d9f301670cf36
---
image: index.docker.io/library/nginx@sha256:644a70516a26004c97d0d85c7fe1d0c3a67ea8ab7ddf4aff193d9f301670cf36
metadata:
  annotations:
    kbld.k14s.io/images: |
      - origins:
        - resolved:
            tag: latest
            url: nginx
        url: index.docker.io/library/nginx@sha256:644a70516a26004c97d0d85c7fe1d0c3a67ea8ab7ddf4aff193d9f301670cf36

Succeeded

Seems like it would be possible for a user to mistakenly believe that piping STDOUT to ytt/kbld would result in them processing it as input (without a flag). With their current behavior, there's no indication that the input is being ignored.

What am I missing?

@cppforlife
Copy link
Contributor

you are right that -f is needed for input, which means that this could only happen when no flags are given. that may happen if you are writing wrappers (in bash, etc) that allow to deal with 0+ files -- im thinking of it as func ytt(files ...File) with 0 files being valid. there is of course a question whether we should disallow 0 files case entirely (which if we do, i would expect would result in error stating 1 or more files is needed)

@pivotaljohn
Copy link
Contributor Author

pivotaljohn commented Nov 4, 2021

im thinking of it as func ytt(files ...File) with 0 files being valid.

This. This is the bit of context I bet I was missing. Yeah... I can absolutely see it from that perspective.

there is of course a question whether we should disallow 0 files case entirely (which if we do, i would expect would result in error stating 1 or more files is needed)

At first blush, the examples I gave above seem to me to be compelling evidence to make the 0 files case an error case.

@aaronshurley aaronshurley moved this to To Triage in Carvel Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution enhancement This issue is a feature request
Projects
None yet
Development

No branches or pull requests

2 participants