The method only returned the executable name and not the full path,
leading to a swallowed error, because brew gracefully ignores a
failing lipo command.
Currently `brew audit` only audits the first binary in a cask.
For example the cask `wiso-steuer-2024` contains multiple binaries in
`Contents/MacOS`:
- `btssysteminfo`
- `whilfe`
- `wmain24`
The first binary (some telemetry tool) is not the main binary and not
a universal binary, but the other two are. Given that `wmain24` is
defined as the main binary in the `Contents/Info.plist`, brew probably
should audit that binary rather than just checking the first one.
- Previously I thought that comments were fine to discourage people from
wasting their time trying to bump things that used `undef` that Sorbet
didn't support. But RuboCop is better at this since it'll complain if
the comments are unnecessary.
- Suggested in https://github.com/Homebrew/brew/pull/18018#issuecomment-2283369501.
- I've gone for a mixture of `rubocop:disable` for the files that can't
be `typed: strict` (use of undef, required before everything else, etc)
and `rubocop:todo` for everything else that should be tried to make
strictly typed. There's no functional difference between the two as
`rubocop:todo` is `rubocop:disable` with a different name.
- And I entirely disabled the cop for the docs/ directory since
`typed: strict` isn't going to gain us anything for some Markdown
linting config files.
- This means that now it's easier to track what needs to be done rather
than relying on checklists of files in our big Sorbet issue:
```shell
$ git grep 'typed: true # rubocop:todo Sorbet/StrictSigil' | wc -l
268
```
- And this is confirmed working for new files:
```shell
$ git status
On branch use-rubocop-for-sorbet-strict-sigils
Untracked files:
(use "git add <file>..." to include in what will be committed)
Library/Homebrew/bad.rb
Library/Homebrew/good.rb
nothing added to commit but untracked files present (use "git add" to track)
$ brew style
Offenses:
bad.rb:1:1: C: Sorbet/StrictSigil: Sorbet sigil should be at least strict got true.
^^^^^^^^^^^^^
1340 files inspected, 1 offense detected
```
This improves the load time of most brew commands. For an example of
one of the simplest commands this speeds up:
Without Bootsnap:
```
$ hyperfine 'git checkout master; brew help' 'git checkout optimise_requires; brew help'
Benchmark 1: git checkout master; brew help
Time (mean ± σ): 525.0 ms ± 35.8 ms [User: 229.9 ms, System: 113.1 ms]
Range (min … max): 465.3 ms … 576.6 ms 10 runs
Benchmark 2: git checkout optimise_requires; brew help
Time (mean ± σ): 383.3 ms ± 25.1 ms [User: 133.0 ms, System: 72.1 ms]
Range (min … max): 353.0 ms … 443.6 ms 10 runs
Summary
git checkout optimise_requires; brew help ran
1.37 ± 0.13 times faster than git checkout master; brew help
```
With Bootsnap:
```
$ hyperfine 'git checkout master; brew help' 'git checkout optimise_requires; brew help'
Benchmark 1: git checkout master; brew help
Time (mean ± σ): 386.0 ms ± 30.9 ms [User: 130.2 ms, System: 93.8 ms]
Range (min … max): 359.5 ms … 469.3 ms 10 runs
Benchmark 2: git checkout optimise_requires; brew help
Time (mean ± σ): 330.2 ms ± 32.4 ms [User: 93.4 ms, System: 73.0 ms]
Range (min … max): 302.9 ms … 413.9 ms 10 runs
Summary
git checkout optimise_requires; brew help ran
1.17 ± 0.15 times faster than git checkout master; brew help
```
The `#page_headers` and `#page_content` methods in
`Livecheck::Strategy` will fetch a URL using our default user agent
but if the request fails it will retry with the `:browser` user agent.
[For context, it was added as an interim measure to make URLs work
that require a different user agent but I aim to remove it in the
future in favor of specifying the user agent in a `livecheck` block
(so we don't make unnecessary requests that we know will fail).]
`Cask::Audit#audit_livecheck_https_availability` checks the
`livecheck` block URL but it only does so using our default user
agent (i.e., it calls `#validate_url_for_https_availability` which
calls `Utils::Curl#curl_check_http_content` which has a `user_agents:
[:default]` parameter). Due to this behavioral mismatch, it's possible
for a `livecheck` block to work but for this cask audit to fail.
This addresses the issue by adding `user_agents: [:default, :browser]`
to the arguments the audit uses, which aligns its behavior with
livecheck's.
Fixes edge cases where nested containers are used. Extraction for auditing artifacts did not pull the secondary container, which tried to audit the container instead of the contents.
I previously introduced a finalizer method in `Cask::Audit` to remove
the created `@tmpdir` once it's no longer needed but the existing
approach produces a `finalizer references object to be finalized`
warning when `brew audit` is run. I didn't see this warning when I
was originally testing it but now it reliably appears.
This reworks the finalizer to define it within the
`#extract_artifacts` method and use `@tmpdir` as the target object.
`Cask::Audit#extract_artifacts` is used in the `#audit_signing` and
`#cask_plist_min_os` methods to create a directory in `/tmp` and
extract cask artifacts without duplicating the work if it's already
done. However, due to how this is set up, `tmpdir` isn't removed
afterward and the extracted artifacts will take up disk space until
the `tmp` directory is cleaned up. As a result, running
`brew audit --strict --online` locally can chew through disk space
and it may not be clear to the user where their free space has gone.
This adds a finalizer method to `Cask::Audit` to remove the created
`@tmpdir` (if any) once it's no longer needed. There may be a better
way of addressing the issue but this works for now without having to
restructure how these audits work.
