Refactor artifact architecture and url#229
Merged
runesoerensen merged 26 commits intomainfrom Mar 13, 2024
Merged
Conversation
colincasey
reviewed
Mar 8, 2024
colincasey
reviewed
Mar 8, 2024
colincasey
reviewed
Mar 8, 2024
colincasey
reviewed
Mar 8, 2024
This was referenced Mar 9, 2024
Closed
colincasey
reviewed
Mar 11, 2024
colincasey
reviewed
Mar 11, 2024
colincasey
reviewed
Mar 11, 2024
Prefer this over the Go specific os and arch naming. Handle this mapping elsewhere when we add support for more OS and arch packages
3005bc3 to
e5dcf1d
Compare
No need look through artifacts if the arch isn't supported
colincasey
approved these changes
Mar 13, 2024
This file contains hidden or 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
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.
This PR builds on the work completed in #216, and focuses on:
Artifact#architecturefield (e.g.linux-amd64) in favor of two new fields,osandarch, with values that we explicitly define (rather than relying on Go's terminology).archfield is set tox86_64rather than the Go idiomatic/specificamd64naming.aarch64andmacos(e.g. values that map to Rust's arch and os rather than Go'sarm64anddarwinterminology).inventory.toml. This eliminates the need to maintain/use Go-specific logic related to the filename, as well as constructing the download URL in code, for consumers of the inventory - this is now only necessary when parsing the Go feed information.inventory.toml-- with a reasonable expectation that the upstream will continue to redirect to the appropriate and currently available target endpoint (when/if it changes again).Finally,
Inventory::resolvenow filters artifacts based on the current OS and ARCH prior to resolving artifacts that satisfy the version string requirements. This is based on the assumption that the target platform/runtime will always be the same as the build platform/environment executing the buildpack.While adding support for updating
and parsingartifacts for other OS and ARCH combinations thanlinux/x86_64is outside the scope of this PR (particularly so theinventory.tomlfield changes can be reviewed on their own, without adding moreartifactentries), this seems to be the appropriate time and place to introduce this constraint - ensuring the new fields are also being utilized in the code that consumes our inventory.