-
-
Notifications
You must be signed in to change notification settings - Fork 10k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
attestation: remove gh version detection #17899
Conversation
I'm declaring bankruptcy on this entire approach: 1. We can attempt to match on versions, but this will fail when the version of `gh` installed is built from `HEAD` or similar. 2. We can match on dates instead (since `gh --version` also includes the date), but this is even more brittle + implies a support contract we don't actually have (we don't actually want to say we support random dated builds between public releases of `gh`). This moves us back to a simpler approach: if `gh` is present, we use it. If `gh` is not present, we attempt to install it with `ensure_executable!`. If the user's `gh` is present but too old, it'll fail during attestation verification with a reasonable error, which IMO is fine for now since this is all still in beta. Signed-off-by: William Woodruff <william@yossarian.net>
Thanks for the quick fix! linking #17898 , the issue that came out of the linked discussion |
Signed-off-by: William Woodruff <william@yossarian.net>
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.
Thanks, @woodruffw!
Makes sense for now, thanks @woodruffw!
If we get too many of these errors: we should probably just only attempt attestations if |
Take 2 of #17692 but with: - provide and document `HOMEBREW_NO_VERIFY_ATTESTATIONS` - don't try to run unless there's GitHub credentials - don't try to run unless `gh` is installed - don't try to run in CI While we're here: - split out a `Homebrew::EnvConfig.devcmdrun?` helper method - add some missing `Homebrew::EnvConfig.github_api_token` presence checks
I'm declaring bankruptcy on this entire approach:
gh
installed is built fromHEAD
or similar.gh --version
also includes the date), but this is even more brittle + implies a support contract we don't actually have (we don't actually want to say we support random dated builds between public releases ofgh
).This moves us back to a simpler approach: if
gh
is present, we use it. Ifgh
is not present, we attempt to install it withensure_executable!
. If the user'sgh
is present but too old, it'll fail during attestation verification with a reasonable error, which IMO is fine for now since this is all still in beta.CC @nandahkrishna
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?See https://github.com/orgs/Homebrew/discussions/5530 for the originating failure here.
Fixes #17898.