Homebrew-cask: Some non-existing stanzas do not error out with `_stanza`

[vitorgalvao]

Solution to be implemented

On brew cask _stanza {{whatever}} {{cask}}:

1. Check if {{whatever}} is a valid stanza (in HBC as a whole).
   1.1. If yes, continue to 2.
   1.2. If no, print Warning/Error and immediately abort with exit code 1.

2. Check if {{whatever}} exists in {{cask}}.
   2.1. If yes, print it and exit code 0.
   2.2. If no, print the Warning/Error and exit code 1.

---

Problem

On non-existing stanzas, _stanza correctly errors on some but not others. Will use ableton-live in the examples.

First, some stanzas the cask has:

$ brew cask _stanza version ableton-live
9.7.1
$ echo $?
0

$ brew cask _stanza name ableton-live
["Ableton Live"]
$ echo $?
0

Correct exit code, correct output.

Now, some stanzas the cask does not have. First exists (just not in the cask), the other is fake.

$ brew cask _stanza installer ableton-live
Warning: no such stanza 'installer' on Cask 'ableton-live'

Error: nothing to print
$ echo $?
1

$ brew cask _stanza fake_stanza ableton-live
Warning: no such stanza 'fake_stanza' on Cask 'ableton-live'

Error: nothing to print
$ echo $?
1

Correct exit code, correct output.

Some more stanzas the cask does not have. They all exist (just not in the cask).

$ brew cask _stanza appcast ableton-live

$ echo $?
0

$ brew cask _stanza gpg ableton-live

$ echo $?
0

Incorrect exit code, incorrect output (empty line, yes, but no error).

At first I thought it was misbehaving on optional stanzas, but then I tried preflight (which is optional).

$ brew cask _stanza preflight ableton-live
Warning: no such stanza 'preflight' on Cask 'ableton-live'

Error: nothing to print
$ echo $?
1

[Git-Jiro]

Hi! I would like to debug this!

[Git-Jiro]

I think I know where the bug is coming from:
'Hbc::CLI::InternalStanza::run' calls 'cask.send(stanza)'
Which means the search for 'stanza' is successful if the Cask instance or any of it's ancestor classes have a corresponding method.

[vitorgalvao]

Thank you for the interest, @Git-Jiro. Feel free to submit a PR for this. Any specific code questions you have along the way can be directed at @reitermarkus, who has the most familiarity with the core.

[Git-Jiro]

Here is my first idea for this: https://github.com/Homebrew/brew/pull/3258
This will abort when doing stuff like: 'brew cask _stanza dumpcask ableton-live'
I still have no idea how to solve the 'brew cask _stanza gpg ableton-live' use case...

[claui]

Thanks for your contribution @Git-Jiro. It’s a small step in the right direction but it’s a step.

@vitorgalvao @reitermarkus It seems that starting with commit 9c8f7138f3 earlier this year, brew cask _stanza has stopped erroring out altogether.

If I run this Bash snippet:

for subcommand in \
  version name installer fake_stanza appcast gpg preflight; do
  bash -x -c "brew cask _stanza ${subcommand} ableton-live"
  echo $?
done

I get the following output:

+ brew cask _stanza version ableton-live
9.7.4
0
+ brew cask _stanza name ableton-live
["Ableton Live"]
0
+ brew cask _stanza installer ableton-live
Warning: no such stanza 'installer' on Cask 'ableton-live'

0
+ brew cask _stanza fake_stanza ableton-live
Warning: no such stanza 'fake_stanza' on Cask 'ableton-live'

0
+ brew cask _stanza appcast ableton-live

0
+ brew cask _stanza gpg ableton-live

0
+ brew cask _stanza preflight ableton-live
Warning: no such stanza 'preflight' on Cask 'ableton-live'

0

Note that the exit codes are now 0/0/0/0/0/0/0 (which is at odds with the observations reported at the beginning of this thread, which were 0/0/1/1/0/0/1).

@vitorgalvao Are we still committed to the original expectation of 0/0/1/1/1/1/1?

I’m asking because Codecov needs tests. It has already started lamenting, which blocks Git-Jiro’s PR.

[vitorgalvao]

Option 1

Don’t care about what stanzas exist in HBC; only about what stanzas exist in a given cask.

On brew cask _stanza {{whatever}} {{cask}}:

1. Check if {{whatever}} exists in {{cask}}.
   1.1. If yes, print it and exit code 0.
   1.2. If no, print the Warning/Error and exit code 1.

Option 2

On brew cask _stanza {{whatever}} {{cask}}:

1. Check if {{whatever}} is a valid stanza (in HBC as a whole).
   1.1. If yes, exit code will be 0. Continue to 2.
   1.2. If no, print Warning/Error and immediately abort with exit code 1.
2. Check if {{whatever}} exists in {{cask}}.
   2.1. If yes, print it.
   2.2. If no, print an empty line.

---

I’m more inclined towards option 1.

[reitermarkus]

Option 3

On brew cask _stanza {{whatever}} {{cask}}:

1. Check if {{whatever}} is a valid stanza (in HBC as a whole).
   1.1. If yes, continue to 2.
   1.2. If no, print Warning/Error and immediately abort with exit code 1.

2. Check if {{whatever}} exists in {{cask}}.
   2.1. If yes, print it and exit code 0.
   2.2. If no, print the Warning/Error and exit code 1.

… which is basically the same as option 1 in terms of exit code, but differentiates between not in HBC and not in the Cask for the error message.

[vitorgalvao]

@reitermarkus So you mean giving a different error message for each case? I’m fine with that.

[claui]

What I really like about options 1 and 3 is that both allow the use case: “Programmatically check whether all cask have this stanza or not”

Another advantage of options 2 and 3 would be: Suppose a cask uses an unsupported stanza. The error message will correctly state that. (I think this is a useful feature in the light of deprecating stanzas.) Option 1 would just error out with a misleading message, stating the cask did not have the stanza, even though it actually has it.

I’m leaning towards option 3.

[vitorgalvao]

Option 3 it is. Will update the top post with it.

[claui]

I feel we may have forgotten to discuss how to handle the following usage modes of _stanza:

1. calling _stanza with multiple Cask tokens, and
2. calling _stanza without any Cask token at all. (This means “all the Casks” as of today.)

The current implementation allows either. But how is the new “early abort/exit” requirement supposed to work in these cases?

Do we want to continue supporting these usage modes? Does anyone (except me) even _use_ _stanza like that?

[commitay]

Does anyone (except me) even _use_ _stanza like that?

I didn't know _stanza supported anything except a single token.

Do we want to continue supporting these usage modes?

Multiple tokens (1) could be useful, but I'm okay with dropping support for without tokens (2).

This would align with most brew cask commands: Error: This command requires a Cask token.

[claui]

Thanks @commitay. With multiple tokens, what is the command supposed to do if {{stanza}} is only missing in {{cask1}}? Error out early without looking at {{cask2}}?

Or should it carry on with {{cask2}}, display its stanza, and only after that exit with code 1?

[commitay]

That's a good point, I hadn't fully considered that.

We could follow the behaviour of info, install, etc which will error out on the first token but it might be easier to only support a single token.

[vitorgalvao]

I’m not seeing good use cases for doing _stanza on multiple casks at once.

I do see the case for doing it sequentially and doing something with the result, for which we’d need a loop anyway.

I’m for the easiest solution: only allow one at a time, and if more than one (or none) are given, error out.

[claui]

I’m not seeing good use cases for doing _stanza on multiple casks at once.

I concur. The only thing I’d miss would be the _all casks_ usage. For example:

• What artifacts are hosted on CloudFront? → brew cask _stanza url | grep cloudfront
• Are there Casks that require an Adobe product? → brew cask _stanza depends_on | grep adobe

That said, I’m inclined to drop support even for that usage mode. We can always run grep -R --include='*.rb' on the Taps directory directly, which is a lot faster and, thanks to the strict enforcement of style rules, works well enough for most purposes.

For a crude example:

• grep -R --include='*.rb' 'url.*cloudfront' "$(brew --prefix)/Homebrew/Library/Taps"

[vitorgalvao]

• What artifacts are hosted on CloudFront? → brew cask _stanza url | grep cloudfront
• Are there Casks that require an Adobe product? → brew cask _stanza depends_on | grep adobe

We could:

for cask in $(brew cask search); do
  brew cask _stanza url "${cask}"
done | grep cloudfront

and

for cask in $(brew cask search); do
  brew cask _stanza depends_on "${cask}"
done | grep adobe

Not as short and straightforward, but it’s not that much of a hassle and still works.

[reitermarkus]

I also think more than one token would not work well, or be useful. No strong opinion on the “all” case though.

[claui]

Sounds reasonable. Thank you all for your opinions.

[claui]

Looks like deprecating both the multi-token and the all-token usage seems OK for everyone. Can we call that consensus?

Deprecating both would help get the current PR merged.

[vitorgalvao]

Can we call that consensus?

Looks like we can!