-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
tap_auditor: check renamed formula exists #18741
Conversation
I guess tap audit isn't tested in brew CI. Locally, before Homebrew/homebrew-core#197128: ❯ brew audit --tap homebrew/core --only tap_formula_lists
homebrew/core
* formula_renames.json references
formulae or casks that are not found in the homebrew/core tap.
Invalid formulae or casks: antlr@2, elasticsearch@6, gcc@5, gcc@6, gnuplot@4, [email protected], tomcat@7, [email protected], [email protected]
Error: 1 problem in 1 tap detected. And after: ❯ brew audit --tap homebrew/core --only tap_formula_lists
❯ echo $?
0 EDIT: I allowed references via chains of renames and/or tap migrations. An alternative is failing and forcing these to be squashed to 1-level, e.g.
Didn't seem that important to fail tap audit on. |
It should be: https://github.com/Homebrew/brew/actions/runs/11751234155/job/32740921538?pr=18741#step:5:1 Sounds like the audit might not be working correctly? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK with this when CI is fixed
Not sure if it is running off of API or something as can't reproduce locally. Adding back one of the old renames and running same command: ❯ brew audit --skip-style --except=version --tap=homebrew/core
homebrew/core
* formula_renames.json references
formulae or casks that are not found in the homebrew/core tap.
Invalid formulae or casks: [email protected] Bit tricky now to test CI as Homebrew/core side has already been merged so there are no more renames that trigger audit. |
CI intentionally doesn't set |
I locally tried running without and with Can try re-running CI with all rename values, which should still fail on chained renames:
|
3206cdc
to
40cecdf
Compare
Probably good to remove the recursive names as looks like we never implemented support in some commands, e.g. ❯ brew info gtef
Warning: Formula gtef was renamed to tepl.
Warning: Formula gtef was renamed to tepl.
Warning: Formula gtef was renamed to tepl.
Error: No available formula with the name "gtef".
Warning: Formula gtef was renamed to tepl.
==> Searching for a previously deleted formula (in the last month)...
Error: No previously deleted formula found. And I guess API may prune out some of these? ❯ unset HOMEBREW_NO_INSTALL_FROM_API
❯ brew info gtef
Error: No available formula with the name "gtef".
==> Searching for a previously deleted formula (in the last month)...
Error: No previously deleted formula found. If this approach is fine, can merge Homebrew/core PR and then re-run CI here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on ✅ when CI is 🟢
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?Audit renames so we remove old ones that can no longer be resolved to an actual formula/alias,
further renames, or a tap migration.EDIT: remove support for latter and instead require cleaning those old renames.