-
Notifications
You must be signed in to change notification settings - Fork 137
Assorted release maintenance #1891
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @jtobin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the reliability, security, and user experience of the project's release process. It introduces robust dependency checks in release scripts, enhances the verifiability of official Docker images by embedding necessary tools, and modernizes the release documentation to provide clearer guidance. These changes collectively streamline the release pipeline and empower users with better tools for verifying software integrity. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces several valuable improvements to the release process, including adding dependency checks to the release scripts and significantly updating the release documentation for better clarity. The changes make the release process more robust and user-friendly. However, I've identified a critical issue in scripts/release.sh where the new logic for handling checksum commands will cause the script to fail on certain systems due to incorrect variable quoting. I've provided detailed comments and suggestions to resolve this.
a25fc54 to
5728c95
Compare
We rely on a few utilities in our release toolchain scripts that may be missing in not-terribly-uncommon situations. get-git-tag-name.sh implicitly requires GNU grep due to the use of Perl-compatible regular expressions (-P, in particular due to the \K assertion, which has no extended regexp equivalent in BSD grep). We now check that the user is running the required grep variant. zip(1) commonly, and Perl somewhat less commonly, may be missing on Linux, so checks are added to scripts/release.sh to confirm their presence as well. If shasum is not present, we check for coreutils and, if it's available, swap in sha256sum instead.
The release workflow previously called 'make release' to produce the release artifacts. This commit changes the workflow to first build our helper Docker image and then use it to produce the release artifacts, ensuring they're constructed in a consistent environment that can be easily reproduced outside of CI. Building the Docker image here adds on perhaps 2-3 minutes to the workflow, but release workflows are performed very infrequently (only on a v* tag push) . Note also that this eliminates a location where Go toolchain version state needs to be tracked.
Pull Request Test Coverage Report for Build 20024021332Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
5728c95 to
f568e46
Compare
Modernizes the text of this document, primarily by: * Recommending the use of 'make docker-release' for builds, * Pointing at the 'Assets' section of GitHub releases, and * Fleshing out the section on signing a manifest.
These were missing from the constructed image, so a recommended
verification method in our docs didn't work. Now
$ docker run --rm --entrypoint="" \
"$IMAGE" /verify-install.sh "$TAG"
will run the verification script on the binaries in the image, as
expected.
Since 81f787a, this image is used to build release artifacts, so pinning it by digest is good paranoia practice.
| $ git clone https://github.com/lightninglabs/taproot-assets.git | ||
| $ cd taproot-assets | ||
| $ git checkout "$TAG" | ||
| $ make docker-release tag="$TAG" |
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.
I think we should be able to speed up the docker-release (at least on mac) using a similar technique as my recent PR to speed up linting.
| the desired archive(s) containing the release binaries from the | ||
| Taproot Assets [Releases page][ghrelease] on GitHub. | ||
|
|
||
| 4. Recompute the SHA256 hash of each artifact with e.g. `shasum -a 256 |
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.
On the lnd side, we've recently started to use commands like this that wrap everything all into one:
wget https://dl.google.com/go/go1.24.9.linux-amd64.tar.gz
echo "5b7899591c2dd6e9da1809fde4a2fad842c45d3f6b9deb235ba82216e31e34a6 go1.24.9.linux-amd64.tar.gz" | sha256sum --check
We don't need an interactive environment, so there's no need for this flag.
These fixes stop Docker from warning about the deprecated MAINTAINER instruction and legacy key/value format.
|
I've since tested CI with the changes here and can confirm that everything works (no disk issues, etc.). A stripped-down log for the release workflow can be seen here, demonstrating that the image builds, and that the release artifacts are produced as expected. |
Closes #1874, #1877.
See the individual commits for more details, but the TLDR version:
docker-release(see commentary in [feature]: unify release build methods #1877 for rationale).