Conversation
This uses the ripunzip releases from github instead of building them ourselves.
There was a problem hiding this comment.
Pull Request Overview
This PR transitions from building ripunzip binaries in-house and storing them in Git LFS to downloading pre-built releases directly from the GoogleChrome/ripunzip GitHub repository. This simplifies the build infrastructure by eliminating the need for a custom build workflow and Git LFS storage.
- Replaces the custom build workflow with direct downloads from GitHub releases
- Implements a new Bazel repository rule that handles platform-specific downloads and extraction
- Updates to ripunzip version 2.0.3 with SHA256 verification for all platforms
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| misc/ripunzip/ripunzip.bzl | New Bazel repository rule implementing platform-specific download and extraction logic for ripunzip releases |
| MODULE.bazel | Replaces LFS-based repositories with the new ripunzip_archive rule, specifying version 2.0.3 and checksums |
| misc/ripunzip/BUILD.bazel | Simplifies alias to reference the unified @ripunzip repository instead of platform-specific repositories |
| misc/ripunzip/BUILD.ripunzip.bazel | Updates glob pattern to match the new bin/ directory structure from GitHub releases |
| .gitattributes | Removes Git LFS configuration for ripunzip binaries that are no longer stored locally |
| .github/workflows/build-ripunzip.yml | Removes the workflow that previously built ripunzip binaries in-house |
| misc/ripunzip/ripunzip-*.tar.zst | Removes locally stored binary archives that are now downloaded on-demand |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
esbena
left a comment
esbena
left a comment
There was a problem hiding this comment.
LGTM modulo comments, but CI disagrees.
| output = "deb", | ||
| ) | ||
| repository_ctx.extract( | ||
| "deb/data.tar.xz", | ||
| strip_prefix = "usr", | ||
| ) |
There was a problem hiding this comment.
Please add a comment here explaining why output is not bin like for the other os cases. In isolation it is fine, but the inconsistency is confusing.
Related:
It did need a bit of bazel tinkering as they only publish debian packages.
Perhap this should be added as a docstrinb on _impl itself
There was a problem hiding this comment.
ok, went for stripping usr/bin and then outputting to bin like the rest
|
The CI failure is unrelated to these changes, and fixed in another internal PR |
2 participants
This uses the ripunzip releases from github instead of building them ourselves.
It did need a bit of bazel tinkering as they only publish debian packages.
This can be tested locally with