Skip to content
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

Use artifact inventory #814

Merged
merged 35 commits into from
May 9, 2024
Merged

Use artifact inventory #814

merged 35 commits into from
May 9, 2024

Conversation

runesoerensen
Copy link
Contributor

@runesoerensen runesoerensen commented Apr 18, 2024

This PR introduces support for the artifact inventory code shared with the Go buildpack, and updates the GitHub actions for node.js to write a corresponding inventory.toml file in the new format (which includes the sha256 checksum that is used to verify the download, as well as the upstream URL rather than our own S3 mirrored artifact store).

A few notes:

  • The shared inventory code is being pulled directly from the Go buildpack. This will be moved to libherokubuildpack at some point.
  • The inventory file includes arm64 and amd64 linux artifacts and are resolved based on the current host OS and arch.
  • darwin artifacts and the staging channel releases are no longer included in the inventory.
  • As the classic nodejs buildpack uses the current implementation for inventory updates, this PR does not update or remove any of the shared code.
  • When/if the classic node.js buildpack if migrated to use the new inventory utilities we can remove the node specific code from this repository.

We may also want to consider migrating NPM and Yarn over to the new format at some point (so we can avoid mirroring, and simplify our codebase), but that's probably out of scope for the time being.

@runesoerensen runesoerensen added skip changelog rust Pull requests that update Rust code labels Apr 18, 2024
@runesoerensen runesoerensen self-assigned this Apr 18, 2024
@runesoerensen runesoerensen force-pushed the use-artifact-inventory branch from 8693c51 to f0879eb Compare April 23, 2024 13:44
@runesoerensen runesoerensen marked this pull request as ready for review April 23, 2024 14:42
@runesoerensen runesoerensen requested review from joshwlewis, colincasey and a team as code owners April 23, 2024 14:42
.github/workflows/inventory.yml Outdated Show resolved Hide resolved
.github/workflows/inventory.yml Outdated Show resolved Hide resolved
.github/workflows/inventory.yml Outdated Show resolved Hide resolved
.github/workflows/inventory.yml Show resolved Hide resolved
buildpacks/nodejs-engine/src/main.rs Show resolved Hide resolved
common/nodejs-utils/src/bin/update_node_inventory.rs Outdated Show resolved Hide resolved
common/nodejs-utils/src/bin/update_node_inventory.rs Outdated Show resolved Hide resolved
common/nodejs-utils/src/bin/update_node_inventory.rs Outdated Show resolved Hide resolved
common/nodejs-utils/src/bin/update_node_inventory.rs Outdated Show resolved Hide resolved
Copy link
Member

@joshwlewis joshwlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Direction looks good to me. I tend to agree with Colin's comments, so looks good once those are resolved.

common/nodejs-utils/src/bin/update_node_inventory.rs Outdated Show resolved Hide resolved
common/nodejs-utils/src/bin/update_node_inventory.rs Outdated Show resolved Hide resolved
@runesoerensen runesoerensen force-pushed the use-artifact-inventory branch from 2db2178 to 59cfa15 Compare April 25, 2024 20:46
@runesoerensen
Copy link
Contributor Author

runesoerensen commented Apr 26, 2024

The PR is ready to be merged, but I'll hold off doing that until we hear back from the SA team

@runesoerensen runesoerensen force-pushed the use-artifact-inventory branch 2 times, most recently from 22e0f81 to 47e772f Compare April 29, 2024 22:11
Non-zero exit code values aren't used anywhere
Avoid dependency on an archived github action, and take the same approach as other workflows in this repository
It's clear what is happening here, so avoid assigning an "earliest version" variable/providing context for an error that'll be trivial to identify if it's ever changed.
When an error is encountered, the process will still be terminated with a non-zero exit code with adequate error information for debugging an issue written to stderr
@runesoerensen runesoerensen force-pushed the use-artifact-inventory branch from 47e772f to d93b035 Compare May 8, 2024 16:18
@runesoerensen runesoerensen merged commit aeb50fe into main May 9, 2024
26 checks passed
@runesoerensen runesoerensen deleted the use-artifact-inventory branch May 9, 2024 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Pull requests that update Rust code skip changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants