attestation: remove gh version detection #17899
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
3 tasks
|
Makes sense for now, thanks @woodruffw!
If we get too many of these errors: we should probably just only attempt attestations if |
4 tasks
mmrwoods
referenced
this pull request
Aug 1, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm declaring bankruptcy on this entire approach:
ghinstalled is built fromHEADor similar.gh --versionalso 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
ghis present, we use it. Ifghis not present, we attempt to install it withensure_executable!. If the user'sghis 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 stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?See https://github.com/orgs/Homebrew/discussions/5530 for the originating failure here.
Fixes #17898.