chore: fix the release config for static build#174
Conversation
WalkthroughThe release process has been restructured by removing the previous GoReleaser-based workflow and configuration, and introducing a new GitHub Actions workflow for multi-platform builds and packaging. A new nfpm configuration file was added for Linux packaging, and the release automation now builds, packages, and uploads artifacts for Linux and macOS on tag pushes. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub (tag push)
participant Actions as GitHub Actions
participant Runner as Build Runner (Linux/Mac)
participant nfpm as nfpm
participant Release as GitHub Release
GitHub->>Actions: Push tag v*
Actions->>Runner: Start build matrix (OS/arch)
Runner->>Runner: Checkout code, setup Go, install deps
alt Linux
Runner->>Runner: Build static Go binary<br>Build and install libgit2<br>Create tar.gz, deb, rpm with nfpm
else macOS
Runner->>Runner: Build Go binary<br>Create tar.gz
end
Runner->>Actions: Upload build artifacts
Actions->>Runner: Download all artifacts (create-release job)
Runner->>Runner: Generate SHA-256 checksums
Runner->>Release: Create draft release<br>Upload artifacts and checksums
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🪛 actionlint (1.7.4).github/workflows/release.yaml18-18: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file (runner-label) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
37-40: Enhance temporary build directory handling and error safetyConsider improving the robustness of this block by:
- Adding
set -euo pipefailat the top of the step to ensure immediate exit on any command failure.- Using
mktemp -d /tmp/libgit2_build_XXXXXXinstead of$$to generate a truly unique directory.- Registering a
trap 'cd "$ORIGINAL_DIR"; rm -rf "$TEMP_BUILD_DIR"' EXITto automatically clean up the temp folder.These changes guard against race conditions, ensure failures abort early, and prevent leftover artifacts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/release.yml (1)
49-49: Verify the need forldconfigin CIThe
sudo ldconfigupdates the system library cache, but Go’spkg-configmay already pick up the newlibgit2. Please confirm that this step is necessary for your static build. If it’s redundant, removing it can speed up the workflow.
chlins
force-pushed
the
chore/fix-static-build
branch
3 times, most recently
from
6e36244 to
e2ed0a0
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🧹 Nitpick comments (2)
hack/build-libgit2.sh (1)
12-15: Quote$(nproc)and handle errors explicitly
ShellCheck SC2046 warns about unquoted command substitutions. Also ensuremakefailures stop the script:- cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF && \ - make -j$(nproc) && \ - sudo make install && \ - sudo ldconfig + cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF + make -j"$(nproc)" + sudo make install + sudo ldconfig🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 13-13: Quote this to prevent word splitting.
(SC2046)
.goreleaser.yml (1)
7-9: Ensure the script is executable and path is correct
Verify thathack/build-libgit2.shis checked in with the executable bit (chmod +x) so GoReleaser can run it without permission errors.
🛑 Comments failed to post (3)
hack/build-libgit2.sh (3)
1-2: 🛠️ Refactor suggestion
Use a more robust shell interpreter and enable strict mode
Switching to Bash and turning on strict error handling will make the script fail fast and easier to debug.
Suggestion:- #!/bin/sh + #!/usr/bin/env bash + set -o errexit + set -o nounset + set -o pipefail📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.#!/usr/bin/env bash set -o errexit set -o nounset set -o pipefail
7-11: 🛠️ Refactor suggestion
Isolate build in a temporary directory and clean up afterwards
Right now the workspace is polluted by downloaded archives and extracted folders. Usemktemp -dto create an ephemeral build directory and remove it at the end:+ BUILD_DIR=$(mktemp -d) + cd "$BUILD_DIR" wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz \ -O libgit2-v1.5.1.tar.gz && \ tar -xzf libgit2-v1.5.1.tar.gz && \ cd libgit2-1.5.1 && \ mkdir build && \ cd build && \ + # ... build steps ... + cd / + rm -rf "$BUILD_DIR"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.BUILD_DIR=$(mktemp -d) cd "$BUILD_DIR" wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz \ -O libgit2-v1.5.1.tar.gz && \ tar -xzf libgit2-v1.5.1.tar.gz && \ cd libgit2-1.5.1 && \ mkdir build && \ cd build && \ # ... build steps ... cd / rm -rf "$BUILD_DIR"
3-6: 🛠️ Refactor suggestion
Consolidate and standardize package installation
You currently runapt-get updateand then bothapt-get installandapt install. Consider combining updates and installs into a singleapt-getinvocation to reduce redundancy and ensure consistent flags:- sudo apt-get update - sudo apt-get install -y pkg-config - sudo apt update && \ - sudo DEBIAN_FRONTEND=noninteractive apt install -y build-essential cmake pkg-config libssl-dev libssh2-1-dev zlib1g-dev libhttp-parser-dev python3 wget tar git && \ + sudo apt-get update + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y \ + build-essential cmake pkg-config libssl-dev libssh2-1-dev \ + zlib1g-dev libhttp-parser-dev python3 wget tar git📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.sudo apt-get update sudo DEBIAN_FRONTEND=noninteractive apt-get install -y \ build-essential cmake pkg-config libssl-dev libssh2-1-dev \ zlib1g-dev libhttp-parser-dev python3 wget tar git
chlins
force-pushed
the
chore/fix-static-build
branch
from
e2ed0a0 to
d8e1e6b
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🧹 Nitpick comments (2)
hack/build-libgit2.sh (2)
7-8: Improve download and extraction robustness
It’s helpful to make downloads quieter and to scope extraction into its own directory. For example:-wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz && \ -tar -xzf libgit2-v1.5.1.tar.gz && \ +wget -q https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz +tar -xzf libgit2-v1.5.1.tar.gzOptionally, verify the archive checksum or signature as a security best practice.
9-11: Isolate the build workspace and restore CWD
Rather than polluting the current directory, use a temp build directory and ensure youpopdback. For example:-cd libgit2-1.5.1 && \ -mkdir build && \ -cd build && \ +BUILD_DIR="$(mktemp -d /tmp/libgit2_build_XXXX)" && \ +pushd "$BUILD_DIR" +# unpacking happens here, then: +# popd at the end to return to original directoryThis aligns with the PR’s goal of isolating build artifacts.
🛑 Comments failed to post (3)
hack/build-libgit2.sh (3)
12-13: 🛠️ Refactor suggestion
Quote
$(nproc)to satisfy ShellCheck SC2046
ShellCheck warns that unquoted command substitutions may be split. Change:-make -j$(nproc) && \ +make -j"$(nproc)" && \to eliminate the SC2046 warning and ensure correct CPU-core detection in all shells.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 13-13: Quote this to prevent word splitting.
(SC2046)
1-2: 🛠️ Refactor suggestion
Enable strict error handling and improve portability
To prevent silent failures and catch undefined variables, switch to Bash with strict flags. For example:-#!/bin/sh +#!/usr/bin/env bash +set -euo pipefailThis ensures the script exits on errors, on unset variables, or on pipeline failures.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.#!/usr/bin/env bash set -euo pipefail
3-6: 🛠️ Refactor suggestion
Consolidate and standardize package installation
You currently invokeapt-getandaptseparately and runupdatetwice. Combine these steps, useapt-getconsistently, and applyDEBIAN_FRONTEND=noninteractivefor all installs. For example:-sudo apt-get update -sudo apt-get install -y pkg-config -sudo apt update && \ -sudo DEBIAN_FRONTEND=noninteractive apt install -y build-essential cmake pkg-config libssl-dev libssh2-1-dev zlib1g-dev libhttp-parser-dev python3 wget tar git && \ +sudo apt-get update && \ +DEBIAN_FRONTEND=noninteractive sudo apt-get install -y \ + build-essential cmake pkg-config libssl-dev libssh2-1-dev \ + zlib1g-dev libhttp-parser-dev python3 wget tar gitThis reduces redundant updates and ensures consistent behavior across environments.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.sudo apt-get update && \ DEBIAN_FRONTEND=noninteractive sudo apt-get install -y \ build-essential cmake pkg-config libssl-dev libssh2-1-dev \ zlib1g-dev libhttp-parser-dev python3 wget tar git
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
hack/build-libgit2.sh (5)
3-4: Consolidate and dedupe package installations
We installpkg-configtwice (lines 4 & 6). Combine all dependencies into one install step after a singleapt-get updateto reduce redundancy:- sudo apt-get update - sudo apt-get install -y pkg-config + sudo apt-get update && \ + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y \ + pkg-config build-essential cmake libssl-dev libssh2-1-dev \ + zlib1g-dev libhttp-parser-dev python3 wget tar git
5-6: Unify package manager commands
Mixingaptandapt-getcan lead to subtle differences. Preferapt-getin scripts for consistent behavior.
7-11: Parameterize version and streamline extraction
Hardcoding the version in multiple places is error-prone. Define aLIBGIT2_VERSIONvariable and usetar -Cto extract directly into a clean source folder:+ LIBGIT2_VERSION="1.5.1" + ARCHIVE_URL="https://github.com/libgit2/libgit2/archive/refs/tags/v${LIBGIT2_VERSION}.tar.gz" + wget "$ARCHIVE_URL" -O libgit2-v${LIBGIT2_VERSION}.tar.gz - wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz && \ - tar -xzf libgit2-v1.5.1.tar.gz && \ - cd libgit2-1.5.1 && \ + mkdir -p libgit2-src && \ + tar -xzf libgit2-v${LIBGIT2_VERSION}.tar.gz -C libgit2-src --strip-components=1 && \ + cd libgit2-src && \ mkdir build && \ cd build && \
12-14: Use CMake’s build/install commands and quote subshell expansions
Leveragecmake --build/--installfor consistency, and quote$(nproc)per ShellCheck SC2046:- cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF && \ - make -j$(nproc) && \ - sudo make install && \ + cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF && \ + cmake --build . -- -j"$(nproc)" && \ + sudo cmake --install . && \🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 13-13: Quote this to prevent word splitting.
(SC2046)
15-15: Clean up artifacts after install
After updating the linker cache, remove temporary files and rely on thetrapto restore the working directory:- sudo ldconfig + sudo ldconfig + # Clean up build artifacts + rm -rf libgit2-src libgit2-v${LIBGIT2_VERSION}.tar.gz
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/lint.yml(0 hunks).github/workflows/release.yml(2 hunks).goreleaser.yml(1 hunks)hack/build-libgit2.sh(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/lint.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .goreleaser.yml
- .github/workflows/release.yml
🧰 Additional context used
🪛 Shellcheck (0.10.0)
hack/build-libgit2.sh
[warning] 13-13: Quote this to prevent word splitting.
(SC2046)
chlins
force-pushed
the
chore/fix-static-build
branch
from
d8e1e6b to
0b3a529
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
hack/build-libgit2.sh (1)
1-2: Enable strict error handling and track working directory
In line with the previous suggestion, switch the interpreter to Bash with strict mode, record the original directory, and ensure cleanup with a trap.-#!/bin/sh +#!/usr/bin/env bash +set -euo pipefail + +ORIGINAL_DIR="$(pwd)" +trap 'cd "$ORIGINAL_DIR"' EXIT
🧹 Nitpick comments (3)
hack/build-libgit2.sh (1)
3-7: Consolidate package update and installation commands
You can streamline and make this more robust by usingapt-getconsistently, combining updates, and avoiding duplicate invocations:-sudo apt-get update -sudo apt-get install -y pkg-config -sudo apt update && \ - sudo DEBIAN_FRONTEND=noninteractive apt install -y build-essential cmake pkg-config libssl-dev libssh2-1-dev zlib1g-dev libhttp-parser-dev python3 wget tar git && \ +sudo apt-get update && \ + DEBIAN_FRONTEND=noninteractive apt-get install -y \ + pkg-config build-essential cmake libssl-dev libssh2-1-dev \ + zlib1g-dev libhttp-parser-dev python3 wget tar git.github/workflows/release-v2.yaml (2)
89-93: Explicitly use Bash shell for process substitution
The inline--config <(echo ...)requires Bash; ensure the step runs under Bash on Linux:- uses: goreleaser/nfpm@v2 + shell: bash + uses: goreleaser/nfpm@v2
114-117: Quote globs in checksum step to avoid filename pitfalls
To satisfy SC2035 and guard against filenames starting with dashes, prefix your globs with--:- shasum -a 256 *.tar.gz *.deb *.rpm > checksums.txt + shasum -a 256 -- *.tar.gz *.deb *.rpm > checksums.txt🧰 Tools
🪛 actionlint (1.7.4)
114-114: shellcheck reported issue in this script: SC2035:info:2:15: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
114-114: shellcheck reported issue in this script: SC2035:info:2:24: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
114-114: shellcheck reported issue in this script: SC2035:info:2:30: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release-v2.yaml(1 hunks).github/workflows/release.yml(2 hunks).goreleaser.yml(1 hunks)hack/build-libgit2.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .goreleaser.yml
- .github/workflows/release.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-v2.yaml
44-44: shellcheck reported issue in this script: SC2046:warning:11:8: Quote this to prevent word splitting
(shellcheck)
114-114: shellcheck reported issue in this script: SC2035:info:2:15: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
114-114: shellcheck reported issue in this script: SC2035:info:2:24: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
114-114: shellcheck reported issue in this script: SC2035:info:2:30: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
🪛 Shellcheck (0.10.0)
hack/build-libgit2.sh
[warning] 13-13: Quote this to prevent word splitting.
(SC2046)
chlins
force-pushed
the
chore/fix-static-build
branch
8 times, most recently
from
62f0940 to
a5fda64
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/release-v2.yaml (2)
42-58: DRY up libgit2 build steps for Linux
Inline installation and build commands are verbose and risk drift. Consider centralizing in a script (hack/build-libgit2.sh) for maintainability.- - name: Install CGO dependencies for Linux - if: matrix.goos == 'linux' - run: | - sudo apt-get update - sudo apt-get install -y pkg-config - sudo DEBIAN_FRONTEND=noninteractive apt install -y build-essential cmake pkg-config libssl-dev libssh2-1-dev zlib1g-dev libhttp-parser-dev python3 wget tar git - wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz - tar -xzf libgit2-v1.5.1.tar.gz - cd libgit2-1.5.1 - mkdir build && cd build - cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF - make -j$(nproc) - sudo make install && sudo ldconfig + - name: Install CGO dependencies for Linux (via script) + if: matrix.goos == 'linux' + run: | + chmod +x hack/build-libgit2.sh + hack/build-libgit2.sh
59-72: DRY up libgit2 build steps for macOS
The macOS brew and build logic mirrors Linux. Centralize platform detection and build steps in the samehack/build-libgit2.shto avoid duplication.
🧹 Nitpick comments (2)
.github/workflows/release-v2.yaml (2)
21-23: Consider removing or documenting commented-out matrix entries
The Darwin/amd64 entry is commented out. If it's deprecated, remove it. If you plan to re-enable later, add a comment explaining why it's disabled.
104-169: Clean up commented packaging steps
The large block of commented-out archive and packaging commands should be pruned or extracted to a separate maintenance document. It obscures the active workflow.
🛑 Comments failed to post (1)
.github/workflows/release-v2.yaml (1)
17-19: 💡 Verification agent
❓ Verification inconclusive
Invalid runner label: ubuntu-24.04-arm
The labelubuntu-24.04-armis not a recognized GitHub-hosted runner. This will cause the job to fail.
- Use
ubuntu-22.04orubuntu-latestwith anarm64matrix entry- Or configure a self-hosted runner and register
ubuntu-24.04-arminactionlint.yaml
Invalid runner label:
ubuntu-24.04-armThe label
ubuntu-24.04-armisn’t a GitHub-hosted runner (onlyubuntu-24.04/ubuntu-22.04/ubuntu-latestare available, and they run on x64). This configuration will fail at runtime.• Remove the
runner: ubuntu-24.04-armentry from your matrix.
• Use a singleruns-on: ubuntu-24.04(orubuntu-latest) for all builds and rely on Go’s cross-compile (goarch: arm64) on the x64 VM.
• If you truly need an ARM64 machine, provision a self-hosted ARM64 runner and register its label (e.g.self-hosted,arm64,ubuntu-24.04) inactionlint.yaml.Example update:
strategy: - matrix: - goarch: [amd64, arm64] - runner: [ubuntu-24.04, ubuntu-24.04-arm] + matrix: + goarch: [amd64, arm64] runs-on: ubuntu-24.04Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.4)
18-18: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
chlins
force-pushed
the
chore/fix-static-build
branch
3 times, most recently
from
eb788f4 to
3846c12
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release-v2.yaml (1)
42-58: DRY up libgit2 build steps via a script
Duplicating the inline build logic for Linux and macOS increases maintenance burden and risk of drift. As suggested previously, extracting this into a reusable script (e.g.,hack/build-libgit2.sh) would centralize the commands and make future updates easier.Also applies to: 59-71
🧹 Nitpick comments (5)
.github/workflows/release-v2.yaml (5)
45-48: Combine and streamlineapt-getcommands
You're runningapt-get updatetwice and using bothapt-getandapt. Consider merging these into a single command:- sudo apt-get update - sudo apt-get install -y pkg-config - sudo apt update && DEBIAN_FRONTEND=noninteractive apt install -y build-essential cmake ... + sudo apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y pkg-config build-essential cmake ...This reduces repetition and ensures consistent package management.
39-41: Cache Go modules for faster runs
Adding anactions/cachestep for$GOPATH/pkg/mod(and optionally~/.cache/go-build) can dramatically speed up dependency resolution and subsequent builds.
5-7: Tighten tag trigger pattern
Using'v*'will match any tag starting withv(e.g.,versionX). To only trigger on semantic version tags, consider:on: push: tags: - 'v[0-9]+.[0-9]+.[0-9]+'This avoids unintended runs on non-semver tags.
29-33: Document full-history checkout rationale
You’ve setfetch-depth: 0to fetch the full Git history (necessary for tags and commit metadata). Adding a brief comment explaining this will help future maintainers understand its purpose.
107-172: Clean up or annotate commented-out steps
There are large blocks for archives, packaging, and releases that are fully commented out. If these steps are deprecated, remove them; otherwise, addTODOcomments indicating when or under what conditions they should be re-enabled to prevent them from becoming stale.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/release-v2.yaml(1 hunks).github/workflows/release.yml(2 hunks).goreleaser.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .goreleaser.yml
- .github/workflows/release.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-v2.yaml
18-18: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🔇 Additional comments (1)
.github/workflows/release-v2.yaml (1)
42-58: 🛠️ Refactor suggestionMissing temporary build directory isolation
The PR summary states that the libgit2 build should occur in a temporary directory (/tmp/libgit2_build_$) and then return to the original directory, but the current steps execute in the workspace root. This can clutter the repo and break subsequent steps if the working directory changes. Consider savingPWDand isolating builds:- run: | - sudo apt-get update - sudo apt-get install -y pkg-config build-essential cmake libssl-dev libssh2-1-dev zlib1g-dev libhttp-parser-dev python3 wget tar git && \ - wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz && \ - tar -xzf libgit2-v1.5.1.tar.gz && cd libgit2-1.5.1 && mkdir build && cd build && \ - cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF && make -j$(nproc) && sudo make install && sudo ldconfig + run: | + ORIGINAL_DIR="$PWD" + TMP_BUILD="/tmp/libgit2_build_${{ matrix.goos }}_${{ matrix.goarch }}" + mkdir -p "$TMP_BUILD" && cd "$TMP_BUILD" + sudo apt-get update && \ + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y pkg-config build-essential cmake libssl-dev libssh2-1-dev zlib1g-dev libhttp-parser-dev python3 wget tar git && \ + wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz && \ + tar -xzf libgit2-v1.5.1.tar.gz && cd libgit2-1.5.1 && mkdir build && cd build && \ + cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF && \ + make -j$(nproc) && \ + sudo make install && sudo ldconfig && \ + cd "$ORIGINAL_DIR"Mirror similar isolation in the macOS block.
Also applies to: 59-71
Likely an incorrect or invalid review comment.
chlins
force-pushed
the
chore/fix-static-build
branch
2 times, most recently
from
eadd115 to
a72863f
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release-v2.yaml (1)
42-58: 🛠️ Refactor suggestionExtract Linux libgit2 build into a script and isolate directory
The inline CGO dependency installation and libgit2 build can be centralized inhack/build-libgit2.shand run in a temporary directory to keep the repo root clean. This also aligns with the PR summary's intent to isolate the build process.Apply a diff replacing lines 42–58:
- - name: Install CGO dependencies for Linux - if: matrix.goos == 'linux' - run: | - sudo apt-get update - sudo apt-get install -y pkg-config - sudo DEBIAN_FRONTEND=noninteractive apt install -y build-essential cmake pkg-config libssl-dev libssh2-1-dev zlib1g-dev libhttp-parser-dev python3 wget tar git && \ - wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz && \ - tar -xzf libgit2-v1.5.1.tar.gz && \ - cd libgit2-1.5.1 && \ - mkdir build && \ - cd build && \ - cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF && \ - make -j$(nproc) && \ - sudo make install && \ - sudo ldconfig + - name: Build libgit2 for Linux + if: matrix.goos == 'linux' + run: | + ORIGINAL_DIR=$(pwd) + mkdir -p /tmp/libgit2_build && cd /tmp/libgit2_build + chmod +x hack/build-libgit2.sh + hack/build-libgit2.sh v1.5.1 + cd "$ORIGINAL_DIR"Likely an incorrect or invalid review comment.
🧹 Nitpick comments (3)
.github/workflows/release-v2.yaml (3)
8-24: Clean up or parameterize the matrix include entries
There are several disabled (commented out) OS/architecture entries. If these are long-term changes, consider removing unused entries or parameterizing the matrix via inputs/excludeto avoid manual toggles and improve readability.
59-71: Centralize macOS libgit2 build steps
Similarly, consolidate the macOS CGO dependencies and libgit2 build into the samehack/build-libgit2.shscript for consistency and maintainability.Suggested diff for lines 59–71:
- - name: Install CGO dependencies for macOS - if: matrix.goos == 'darwin' - run: | - brew install cmake wget zlib libiconv && \ - wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.0.tar.gz -O libgit2-v1.5.0.tar.gz && \ - tar -xzf libgit2-1.5.0.tar.gz && \ - cd libgit2-1.5.0 && \ - mkdir build && \ - cd build && \ - cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF && \ - make -j$(nproc) && \ - sudo make install + - name: Build libgit2 for macOS + if: matrix.goos == 'darwin' + run: | + ORIGINAL_DIR=$(pwd) + mkdir -p /tmp/libgit2_build && cd /tmp/libgit2_build + chmod +x hack/build-libgit2.sh + hack/build-libgit2.sh v1.5.0 + cd "$ORIGINAL_DIR"
107-172: Clean up commented packaging steps
There is a large block of commented-out packaging and release steps. Consider removing obsolete code or migrating these to a separate workflow or GoReleaser configuration to reduce clutter.
🛑 Comments failed to post (1)
.github/workflows/release-v2.yaml (1)
72-87: 💡 Verification agent
🧩 Analysis chain
Validate Linux build invocation
Since all matrix entries for Linux are currently commented out, thisgo buildblock will never run. Verify intended matrix configuration or remove this step if not needed.Run this script to check for Linux entries:
🏁 Script executed:
#!/bin/bash rg -n "goos: linux" .github/workflows/release-v2.yamlLength of output: 113
Linux build step is never triggered
Thego buildblock conditioned onmatrix.goos == 'linux'(lines 72–87) will never run because all Linux entries in the matrix are commented out at lines 13 and 16 of.github/workflows/release-v2.yaml. Please update accordingly:
- Either uncomment or add Linux entries under the matrix:
strategy: matrix: include: - goos: linux goarch: amd64 - goos: linux goarch: arm64 # …other combinations- Or remove the entire Linux build step (lines 72–87) if you don’t intend to build for Linux.
chlins
force-pushed
the
chore/fix-static-build
branch
3 times, most recently
from
4189b85 to
3fe437f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release-v2.yaml (1)
43-59: 🛠️ Refactor suggestionDRY up libgit2 build on Linux
Inline Linux build steps are still present. To centralize and maintain consistency, call yourhack/build-libgit2.shscript here instead of inlining all commands.Example diff:
- - name: Install CGO dependencies for Linux - if: matrix.goos == 'linux' - run: | - sudo apt-get update - sudo apt-get install -y pkg-config - # ... - sudo ldconfig + - name: Install CGO dependencies for Linux + if: matrix.goos == 'linux' + run: | + chmod +x hack/build-libgit2.sh + hack/build-libgit2.sh
🧹 Nitpick comments (4)
.github/workflows/release-v2.yaml (4)
12-24: Clean up commented-out matrix entries
The matrixincludeblock contains several commented OS/arch combinations that aren’t currently used. Consider removing or externalizing these entries to reduce noise, or leverage YAML anchors/aliases if you plan to re-enable them in the future.
34-41: Cache Go modules to speed up workflows
Runninggo mod downloadon every run adds latency. Adding a cache step for Go build and module cache can significantly improve performance.Proposed snippet:
- - name: Install dependencies - run: go mod download + - name: Cache Go modules + uses: actions/cache@v3 + with: + path: | + ~/.cache/go-build + ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + + - name: Install dependencies + run: go mod download
73-88: Remove or update dead Linux build step
TheBuild binary for linuxjob is gated onmatrix.goos == 'linux', but no Linux targets are active. Either re-enable the Linux matrix entries or remove this step to prevent confusion.
94-111: Use dynamic workspace paths for CGO_LDFLAGS
Hard-coding/Users/runner/work/modctl/modctl/libgit2-1.5.1/buildmay break if the workspace path changes. Prefer referencing${{ github.workspace }}or relative paths. Please verify that the rpath remains valid after isolating the build directory.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/release-v2.yaml(1 hunks).github/workflows/release.yml(2 hunks).goreleaser.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .goreleaser.yml
- .github/workflows/release.yml
🔇 Additional comments (1)
.github/workflows/release-v2.yaml (1)
1-7: Workflow trigger on version tags is correct
Theon.push.tags: - 'v*'configuration accurately captures version tags for automated releases.
chlins
force-pushed
the
chore/fix-static-build
branch
4 times, most recently
from
41d4749 to
1d7106b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/release-v2.yaml (1)
43-59: DRY and isolatelibgit2build steps for Linux
Inlining the CGO dependency installation and libgit2 build increases maintenance overhead and leaves artifacts in your workspace. Consider invoking a shared script (e.g.,hack/build-libgit2.sh) or extracting this block into a reusable step. Wrap the build in a temporary directory (mktemp -d /tmp/libgit2_build_XXXX) and captureORIGINAL_DIRso you cancdback aftersudo make install.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/release-v2.yaml(1 hunks).github/workflows/release.yml(2 hunks).goreleaser.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .goreleaser.yml
- .github/workflows/release.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-v2.yaml
147-147: could not parse as YAML: yaml: line 147: could not find expected ':'
(syntax-check)
🪛 YAMLlint (1.35.1)
.github/workflows/release-v2.yaml
[warning] 29-29: wrong indentation: expected 6 but found 4
(indentation)
[error] 130-130: duplication of key "name" in mapping
(key-duplicates)
[error] 151-151: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (6)
.github/workflows/release-v2.yaml (6)
11-24: Verify Linux build configurations are intentionally commented out
The matrix currently only includes macOS targets; the Linux entries are commented out. If Linux builds are required for this workflow, please uncomment them and update any runner labels (e.g., replaceubuntu-24.04-armwith a supported label likeubuntu-24.04).
128-148: Validate YAML quoting for the inlinenfpm pkgheredoc
The shell heredoc underrun: |may be misinterpreted by YAML linters (duplicatename:keys, syntax errors). Ensure the<<EOF…EOFblock is indented correctly relative to therun:literal so it stays within the script context. Alternatively, externalize the NFPM config into a file or wrap the heredoc in quotes to avoid YAML parsing issues.🧰 Tools
🪛 actionlint (1.7.4)
147-147: could not parse as YAML: yaml: line 147: could not find expected ':'
(syntax-check)
🪛 YAMLlint (1.35.1)
[error] 130-130: duplication of key "name" in mapping
(key-duplicates)
110-115: Archiving artifacts looks correct
Thetarcommand correctly bundles the binary withLICENSEandREADME.mdintodist/. This aligns with standard practice.
151-156: Checksum generation handles missing packages gracefully
Usingcontinue-on-error: trueallows macOS jobs (which have no.deb/.rpm) to succeed without failing the workflow. Good handling.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 151-151: syntax error: could not find expected ':'
(syntax)
157-162: Artifact upload configuration is sound
Persisting thedist/directory per OS/ARCH withactions/upload-artifact@v4ensures downstream jobs can retrieve the correct builds.
163-184: Draft release creation is correctly configured
Thecreate-releasejob appropriately downloads all artifacts and usessoftprops/action-gh-release@v2to publish a draft with notes and attachments. This meets the PR objectives.
chlins
force-pushed
the
chore/fix-static-build
branch
4 times, most recently
from
016ae8c to
060c398
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/release-v2.yaml (3)
60-72:⚠️ Potential issueUse macOS-compatible CPU core detection
make -j$(nproc)will fail on macOS, sincenprocisn’t available. Replace it with:run: | - make -j$(nproc) && \ + make -j"$(sysctl -n hw.logicalcpu)" && \Alternatively, factor this into a shared script (
hack/build-libgit2.sh) for DRY and cross-platform consistency.
16-18: 💡 Verification agent❓ Verification inconclusive
Invalid runner label for ARM64 Linux
The labelubuntu-24.04-armis not recognized by GitHub-hosted runners (actionlint flags this). For ARM64 on Ubuntu, you can useubuntu-24.04(the runner’s architecture is inferred from the matrix) orubuntu-latest. If this is intended to target a self-hosted runner, declare the label in youractionlint.yml.
Invalid runner label:
ubuntu-24.04-arm
The labelubuntu-24.04-armisn’t provided by GitHub-hosted runners (actionlint will flag it).
- For cross-compiling to ARM64 you can keep your
goos/goarchmatrix and useor simply- runner: ubuntu-24.04-arm + runner: ubuntu-24.04ubuntu-latest.- If you actually meant to target a self-hosted ARM64 runner, you must declare the
ubuntu-24.04-armlabel in youractionlint.yml..github/workflows/release-v2.yaml (lines 16–18)
- goos: linux goarch: arm64 - runner: ubuntu-24.04-arm + runner: ubuntu-24.04🧰 Tools
🪛 actionlint (1.7.4)
18-18: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
43-59: 🛠️ Refactor suggestionIsolate libgit2 build in a temporary directory and restore working directory
To avoid polluting the workspace and ensure subsequent steps run from the expected location, capture the original directory, use amktemp -dbuild directory, and return viacd. For example:run: | - sudo apt-get update - sudo apt-get install -y pkg-config - sudo apt update && \ - sudo DEBIAN_FRONTEND=noninteractive apt install -y build-essential cmake pkg-config libssl-dev libssh2-1-dev zlib1g-dev libhttp-parser-dev python3 wget tar git && \ - wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz && \ - tar -xzf libgit2-v1.5.1.tar.gz && \ - cd libgit2-1.5.1 && \ - mkdir build && \ - cd build && \ - cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF && \ - make -j$(nproc) && \ - sudo make install && \ - sudo ldconfig + ORIGINAL_DIR=$PWD + BUILD_DIR=$(mktemp -d /tmp/libgit2_build.XXXX) + cd "$BUILD_DIR" + sudo apt-get update + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y build-essential cmake pkg-config libssl-dev libssh2-1-dev zlib1g-dev libhttp-parser-dev python3 wget tar git + wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz + tar -xzf libgit2-v1.5.1.tar.gz + cd libgit2-1.5.1 && mkdir build && cd build + cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF + make -j"$(nproc)" + sudo make install + sudo ldconfig + cd "$ORIGINAL_DIR"
🧹 Nitpick comments (1)
.github/workflows/release-v2.yaml (1)
120-122: Add non-interactive flag when installingnfpm
Ensure the CI job doesn’t block on prompts:- sudo apt install nfpm + sudo apt-get update && sudo apt-get install -y nfpmConsider pinning
nfpmto a specific version or installing viago installto avoid upstream package changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release-v2.yaml(1 hunks).github/workflows/release.yml(2 hunks).goreleaser.yml(1 hunks)hack/nfpm.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .goreleaser.yml
- .github/workflows/release.yml
- hack/nfpm.yaml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-v2.yaml
18-18: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: provenance / upload-assets
chlins
force-pushed
the
chore/fix-static-build
branch
3 times, most recently
from
a955af3 to
f18f170
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/release.yaml (2)
46-49: Reduce redundant apt commands
You installpkg-configtwice and mixapt/apt-get. Consolidate into a single update/install invocation for clarity and reduced run time.Example:
- sudo apt-get update - sudo apt-get install -y pkg-config - sudo apt update && \ - sudo DEBIAN_FRONTEND=noninteractive apt install -y build-essential cmake pkg-config ... + sudo apt-get update && \ + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y build-essential cmake pkg-config ...
89-93: Consider removing commented tmate block
If you no longer need an interactive debugging session, deleting or archiving these commented lines will declutter the workflow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release.yaml(1 hunks).github/workflows/release.yml(0 hunks).goreleaser.yml(0 hunks)hack/nfpm.yaml(1 hunks)
💤 Files with no reviewable changes (2)
- .goreleaser.yml
- .github/workflows/release.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- hack/nfpm.yaml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yaml
18-18: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🔇 Additional comments (16)
.github/workflows/release.yaml (16)
1-7: Workflow trigger looks good
The workflow is correctly named "Release" and triggers on tag pushes matchingv*, which aligns with semantic version–based release practices.
13-17: Matrix definition covers all target platforms
Including Linux (amd64, arm64) and macOS (amd64, arm64) ensures comprehensive multi-platform coverage for your release binaries.Also applies to: 19-24
26-26: Parameterizedruns-onusage is correct
Usingruns-on: ${{ matrix.runner }}cleanly decouples the runner selection from the job definition.
29-33: Checkout step is properly configured
actions/checkout@v4withfetch-depth: 0ensures the full Git history (including tags) is available for embedding version metadata.
34-38: Go setup is correctly defined
actions/setup-go@v5with Go 1.24 matches your project requirements.
39-41: Module download is concise
go mod downloadreadies your dependencies before building.
43-45: Linux CGO setup step is well-scoped
The conditionalif: matrix.goos == 'linux'properly gates CGO dependency installation on Linux runners.
50-59: Linux libgit2 build process is sound
Downloading, configuring (static), building, and installing libgit2 followed byldconfigcorrectly prepares your static CGO dependencies.
60-63: macOS CGO setup start is correct
The Homebrew installs and source fetch for libgit2 are properly gated behind the macOS matrix condition.
73-88: Linux build step is well-defined
Cross-compiling withCGO_ENABLED=1, staticlibgit2tags, and embedding version metadata vialdflagscorrectly produces your Linux binaries.
94-98: macOS build step setup is correct
SettingCGO_ENABLED=1,GOOS,GOARCH, andCGO_LDFLAGSprepares the macOS environment for static linkage.
102-108: macOS build invocation is solid
The Go build command correctly sets tags and version flags, outputting the binary to thebuild/directory.
110-115: Archive creation is well-constructed
IncludingLICENSEandREADME.mdalongside the binary in your tarball ensures end users receive necessary documentation.
116-128: Deb/RPM packaging via nfpm is correctly integrated
Using yourhack/nfpm.yamlto generate.deband.rpmpackages aligns with modern Go release workflows.
129-134: Artifact upload configuration is correct
actions/upload-artifact@v4withpath: dist/will capture all generated archives and packages for the next job.
135-161: Release creation job is properly sequenced
Downloading artifacts, generating checksums, and drafting the GitHub release withsoftprops/action-gh-release@v2covers all necessary steps.
chlins
force-pushed
the
chore/fix-static-build
branch
from
f18f170 to
44202f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/release.yaml (3)
16-18: Invalid runner label:ubuntu-24.04-arm
The labelubuntu-24.04-armis not a supported GitHub-hosted runner. Replace with a valid label (e.g.,ubuntu-latest) or configure a self-hosted ARM64 runner.🧰 Tools
🪛 actionlint (1.7.4)
18-18: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
69-71: Use platform-appropriate CPU count on macOS
The commandmake -j$(nproc)will fail on macOS sincenprocisn’t available. Switch to:- make -j$(nproc) + make -j$(sysctl -n hw.ncpu)
99-100: Incorrect rpath for libgit2 on macOS
Your-rpathis pointing at the build directory rather than the installed libs. Update to:- CGO_LDFLAGS: "-lgit2 -lz -liconv -Wl,-rpath,/Users/runner/work/modctl/modctl/libgit2-1.5.1/build" + CGO_LDFLAGS: "-lgit2 -lz -liconv -Wl,-rpath,/usr/local/lib"
🧹 Nitpick comments (1)
.github/workflows/release.yaml (1)
39-41: Consider caching Go modules to speed up builds
You can reduce CI time by caching your Go dependencies:- uses: actions/cache@v3 with: path: | ~/.cache/go-build ~/go/pkg/mod key: ${{ runner.os }}-go-mod-${{ hashFiles('**/go.sum') }} restore-keys: | ${{ runner.os }}-go-mod-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release.yaml(1 hunks).github/workflows/release.yml(0 hunks).goreleaser.yml(0 hunks)hack/nfpm.yaml(1 hunks)
💤 Files with no reviewable changes (2)
- .goreleaser.yml
- .github/workflows/release.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- hack/nfpm.yaml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yaml
18-18: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🔇 Additional comments (2)
.github/workflows/release.yaml (2)
159-161: Verify yourRELEASE_TOKENsecret
You’re usingsecrets.RELEASE_TOKENto publish the release. Ensure this secret is defined in the repo settings with the appropriatereposcope so the draft release step can authenticate.
60-72:Details
❌ Incorrect review comment
Isolate macOS libgit2 build and restore working directory
Likewise on macOS, wrap the libgit2 steps in a dedicated build directory and ensure you return to the initial path. You could do:ORIGINAL_DIR=$(pwd) TMP_BUILD="/tmp/libgit2_build_${{ github.run_id }}" mkdir -p "$TMP_BUILD" cd "$TMP_BUILD" # … download, cmake, make, sudo make install … cd "$ORIGINAL_DIR"
No need to restore working directory in GitHub Actions steps
Everyrunstep in GitHub Actions starts in the workspace root, so anycdyou do only affects the remainder of that single step—it won’t carry over to the next step. You can still isolate the libgit2 build to avoid clutter, but you don’t have to manuallycdback at the end.Suggested adjustment:
- name: Install CGO dependencies for macOS if: matrix.goos == 'darwin' run: | BUILD_DIR="${RUNNER_TEMP}/libgit2_${{ github.run_id }}" mkdir -p "$BUILD_DIR" && pushd "$BUILD_DIR" wget https://github.com/libgit2/libgit2/archive/refs/tags/v1.5.1.tar.gz -O libgit2-v1.5.1.tar.gz tar -xzf libgit2-v1.5.1.tar.gz pushd libgit2-1.5.1 && mkdir build && pushd build cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF make -j$(nproc) && sudo make install popd; popd; popd
- Uses the runner’s built-in
$RUNNER_TEMPfor a clean build areapushd/popdkeeps the step’s commands organized without worrying about persistingcdLikely an incorrect or invalid review comment.
chlins
force-pushed
the
chore/fix-static-build
branch
from
44202f5 to
9f16bb6
Compare
Signed-off-by: chlins <chlins.zhang@gmail.com>
chlins
force-pushed
the
chore/fix-static-build
branch
from
9f16bb6 to
dd50bcc
Compare
3 participants
This pull request updates the
release.ymlworkflow to enhance the build process forlibgit2. The changes introduce a temporary build directory for better organization and ensure the script returns to the original directory after the build completes.Enhancements to the build process:
.github/workflows/release.yml: Added steps to create and navigate to a temporary build directory (/tmp/libgit2_build_$) before downloading and buildinglibgit2. This helps keep the build process isolated and organized..github/workflows/release.yml: Included a command to return to the original working directory ($ORIGINAL_DIR) after installinglibgit2, ensuring the workflow continues from the correct location.Summary by CodeRabbit