Enhance Release CI Pipeline with ARM64 and Package Builds#19
Conversation
- Add `ado-linux-arm64` cross-compilation target using `gcc-aarch64-linux-gnu` - Install `fpm` to build `.deb` and `.rpm` packages for AMD64 and ARM64 on Linux - Include generated packages in the GitHub Release alongside binary artifacts - Add architecture specifications to the build matrix to feed into `fpm`
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughWalkthroughGitHub Actions release workflow extended to build, package, and release artifacts for both amd64 and arm64 architectures. Build job adds arm64 cross-compilation, DEB/RPM packaging for Linux, and artifact staging under ChangesMulti-architecture release workflow
Possibly related PRs
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Review Summary
This PR enhances the release pipeline with ARM64 support and package generation, but there are critical logic errors that will cause the workflow to fail.
Critical Issues Found:
- Binary name mismatch (lines 43, 63, 71): The workflow attempts to copy and package a file named
docwhen the build producesado - Missing build verification: No check to ensure the binary was successfully created after compilation
- Artifact upload path: Changed to include entire
dist/directory which may capture unintended files
These issues will cause the release workflow to fail or produce incorrect release artifacts. Please address these before merging.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| - name: Prepare Artifact | ||
| run: | | ||
| mkdir -p dist | ||
| cp doc dist/${{ matrix.artifact_name }} |
There was a problem hiding this comment.
🛑 Logic Error: Binary file name mismatch. The build step produces an executable named ado (not doc), but this copies a file named doc. This will fail at runtime if doc doesn't exist or will package the wrong file.
| cp doc dist/${{ matrix.artifact_name }} | |
| cp ado dist/${{ matrix.artifact_name }} |
| # Package DEB | ||
| fpm -s dir -t deb -n ado -v $VERSION -a ${{ matrix.arch }} \ | ||
| --description "Ado - A Minimal Programming Language" \ | ||
| --url "https://github.com/${{ github.repository }}" \ | ||
| --maintainer "Ado Contributors" \ | ||
| --prefix /usr/local/bin \ | ||
| doc=/usr/local/bin/ado | ||
|
|
||
| # Package RPM | ||
| fpm -s dir -t rpm -n ado -v $VERSION -a ${{ matrix.arch }} \ | ||
| --description "Ado - A Minimal Programming Language" \ | ||
| --url "https://github.com/${{ github.repository }}" \ | ||
| --maintainer "Ado Contributors" \ | ||
| --prefix /usr/local/bin \ | ||
| doc=/usr/local/bin/ado |
There was a problem hiding this comment.
🛑 Logic Error: Package mapping error. The fpm command attempts to package a file named doc which doesn't match the built binary name ado. This will create packages with incorrect or missing binaries.
| # Package DEB | |
| fpm -s dir -t deb -n ado -v $VERSION -a ${{ matrix.arch }} \ | |
| --description "Ado - A Minimal Programming Language" \ | |
| --url "https://github.com/${{ github.repository }}" \ | |
| --maintainer "Ado Contributors" \ | |
| --prefix /usr/local/bin \ | |
| doc=/usr/local/bin/ado | |
| # Package RPM | |
| fpm -s dir -t rpm -n ado -v $VERSION -a ${{ matrix.arch }} \ | |
| --description "Ado - A Minimal Programming Language" \ | |
| --url "https://github.com/${{ github.repository }}" \ | |
| --maintainer "Ado Contributors" \ | |
| --prefix /usr/local/bin \ | |
| doc=/usr/local/bin/ado | |
| # Package DEB | |
| fpm -s dir -t deb -n ado -v $VERSION -a ${{ matrix.arch }} \ | |
| --description "Ado - A Minimal Programming Language" \ | |
| --url " github.repository }}" \ | |
| --maintainer "Ado Contributors" \ | |
| --prefix /usr/local/bin \ | |
| ado=/usr/local/bin/ado | |
| # Package RPM | |
| fpm -s dir -t rpm -n ado -v $VERSION -a ${{ matrix.arch }} \ | |
| --description "Ado - A Minimal Programming Language" \ | |
| --url " github.repository }}" \ | |
| --maintainer "Ado Contributors" \ | |
| --prefix /usr/local/bin \ | |
| ado=/usr/local/bin/ado |
| - name: Build | ||
| run: make | ||
| run: | | ||
| if [ "${{ matrix.artifact_name }}" = "ado-linux-arm64" ]; then | ||
| make CC=aarch64-linux-gnu-gcc | ||
| else | ||
| make | ||
| fi |
There was a problem hiding this comment.
🛑 Logic Error: Build output name inconsistency. The macOS build will produce a binary named ado, but line 43 attempts to copy a file named doc. For the ARM64 Linux build with cross-compilation, the output binary name should also be verified to be ado.
| - name: Build | |
| run: make | |
| run: | | |
| if [ "${{ matrix.artifact_name }}" = "ado-linux-arm64" ]; then | |
| make CC=aarch64-linux-gnu-gcc | |
| else | |
| make | |
| fi | |
| - name: Build | |
| run: | | |
| if [ "${{ matrix.artifact_name }}" = "ado-linux-arm64" ]; then | |
| make CC=aarch64-linux-gnu-gcc | |
| else | |
| make | |
| fi | |
| # Verify the binary was created | |
| test -f ado || { echo "Build failed: ado binary not found"; exit 1; } |
| with: | ||
| name: ${{ matrix.artifact_name }} | ||
| path: dist/${{ matrix.artifact_name }} | ||
| path: dist/ |
There was a problem hiding this comment.
Wildcard pattern in artifact upload path may include unintended files. The change from dist/${{ matrix.artifact_name }} to dist/ means all files in dist/ will be uploaded, which could include build artifacts from previous jobs or other temporary files.
| path: dist/ | |
| path: dist/* |
There was a problem hiding this comment.
Pull request overview
This PR expands the GitHub Actions release workflow to build and publish additional Linux artifacts, including an ARM64 binary and Linux packages, alongside the existing release artifacts.
Changes:
- Add an Ubuntu ARM64 target by cross-compiling with
aarch64-linux-gnu-gcc. - Generate
.deband.rpmpackages on Linux viafpmand include them in the uploaded artifacts. - Attach the generated packages to the GitHub Release.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fpm -s dir -t deb -n ado -v $VERSION -a ${{ matrix.arch }} \ | ||
| --description "Ado - A Minimal Programming Language" \ | ||
| --url "https://github.com/${{ github.repository }}" \ | ||
| --maintainer "Ado Contributors" \ | ||
| --prefix /usr/local/bin \ | ||
| doc=/usr/local/bin/ado | ||
|
|
||
| # Package RPM | ||
| fpm -s dir -t rpm -n ado -v $VERSION -a ${{ matrix.arch }} \ | ||
| --description "Ado - A Minimal Programming Language" \ |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/release.yml (3)
9-9: 💤 Low valueConsider scoping permissions and pinning actions (static analysis).
zizmor flags this workflow for relying on default permissions and for unpinned action references (
actions/checkout@v4,actions/upload-artifact@v4,softprops/action-gh-release@v1). Since onlycreate-releaseneedscontents: write, you can add a top-levelpermissions: contents: readto restrict the rest of the workflow, and pin third-party actions to a commit SHA to follow standard supply-chain hardening guidance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml at line 9, The workflow job build-release relies on default permissions and unpinned actions; add a top-level permissions block (permissions: contents: read) to limit rights for all jobs and then override to permissions: contents: write only for the specific job or step that creates the release (the create-release step/job), and replace the unpinned action references actions/checkout@v4, actions/upload-artifact@v4, and softprops/action-gh-release@v1 with fully pinned references using their commit SHAs (e.g., actions/checkout@<commit-sha>, actions/upload-artifact@<commit-sha>, softprops/action-gh-release@<commit-sha>) to harden the supply chain.
99-106: 💤 Low valueGlob upload assumes both arm64 deb and rpm exist.
softprops/action-gh-release@v1will simply warn if a glob matches nothing, so a missing arm64 package won't fail the release — but it will silently publish an incomplete set of assets. Given the packaging now spans two architectures, consider gating thecreate-releasejob on a sanity check (e.g., listing required files in the download step) so partial-release regressions are caught at CI time rather than by users discovering missing.rpm/.debassets after publish.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 99 - 106, The release job currently relies on softprops/action-gh-release@v1 with globs (e.g., artifacts/ado-linux-arm64/*.deb and *.rpm) that may match nothing and silently publish incomplete assets; add a pre-flight sanity check step (before the create-release/upload step that uses softprops/action-gh-release@v1) that explicitly verifies required artifact files exist (for both architectures and package types) and fails the workflow if any are missing — reference the create-release job/step and the file globs like artifacts/ado-linux-arm64/*.deb, artifacts/ado-linux-arm64/*.rpm, artifacts/ado-linux-amd64/*.deb, artifacts/ado-linux-amd64/*.rpm and ensure the check enumerates and asserts presence of each expected file so incomplete releases are prevented.
73-73: ⚡ Quick winAvoid silently swallowing packaging errors.
mv *.deb *.rpm dist/ 2>/dev/null || truewill hide a failedmv(e.g., when fpm produced no package because of a prior silent error). At minimum, fail fast and only tolerate the "no rpm produced" case explicitly.🛠️ Suggested change
- mv *.deb *.rpm dist/ 2>/dev/null || true + shopt -s nullglob + pkgs=( *.deb *.rpm ) + if [ ${`#pkgs`[@]} -eq 0 ]; then + echo "::error::fpm produced no packages"; exit 1 + fi + mv "${pkgs[@]}" dist/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml at line 73, Replace the silent-move pattern `mv *.deb *.rpm dist/ 2>/dev/null || true` with an explicit existence check and only tolerate the “no packages” case: enable shell nullglob or expand the globs into an array (e.g., files=(*.deb *.rpm)), test if the array is non-empty, and call `mv` only when files exist; if the array is empty, emit a clear message and continue, otherwise let `mv` fail so real errors are surfaced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 28-38: Add an output-architecture sanity check in the release
workflow after the Build step and before packaging/uploading: after invoking
make with CC (matrix.artifact_name and CC usage in the Makefile), run a `file`
(or equivalent) on dist/${{ matrix.artifact_name }} and assert its reported
architecture matches the expected matrix.artifact_name (e.g., contains "ARM
aarch64" for ado-linux-arm64); if it does not, fail the job so a host-built
binary is not published. Reference the workflow's matrix.artifact_name, the
Makefile's reliance on CC, and the produced artifact path dist/${{
matrix.artifact_name }} when adding this guard.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Line 9: The workflow job build-release relies on default permissions and
unpinned actions; add a top-level permissions block (permissions: contents:
read) to limit rights for all jobs and then override to permissions: contents:
write only for the specific job or step that creates the release (the
create-release step/job), and replace the unpinned action references
actions/checkout@v4, actions/upload-artifact@v4, and
softprops/action-gh-release@v1 with fully pinned references using their commit
SHAs (e.g., actions/checkout@<commit-sha>, actions/upload-artifact@<commit-sha>,
softprops/action-gh-release@<commit-sha>) to harden the supply chain.
- Around line 99-106: The release job currently relies on
softprops/action-gh-release@v1 with globs (e.g., artifacts/ado-linux-arm64/*.deb
and *.rpm) that may match nothing and silently publish incomplete assets; add a
pre-flight sanity check step (before the create-release/upload step that uses
softprops/action-gh-release@v1) that explicitly verifies required artifact files
exist (for both architectures and package types) and fails the workflow if any
are missing — reference the create-release job/step and the file globs like
artifacts/ado-linux-arm64/*.deb, artifacts/ado-linux-arm64/*.rpm,
artifacts/ado-linux-amd64/*.deb, artifacts/ado-linux-amd64/*.rpm and ensure the
check enumerates and asserts presence of each expected file so incomplete
releases are prevented.
- Line 73: Replace the silent-move pattern `mv *.deb *.rpm dist/ 2>/dev/null ||
true` with an explicit existence check and only tolerate the “no packages” case:
enable shell nullglob or expand the globs into an array (e.g., files=(*.deb
*.rpm)), test if the array is non-empty, and call `mv` only when files exist; if
the array is empty, emit a clear message and continue, otherwise let `mv` fail
so real errors are surfaced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 783bdfb8-77da-4583-b800-239a46767a04
📒 Files selected for processing (1)
.github/workflows/release.yml
| - name: Install cross-compiler | ||
| if: matrix.artifact_name == 'ado-linux-arm64' | ||
| run: sudo apt-get update && sudo apt-get install -y gcc-aarch64-linux-gnu | ||
|
|
||
| - name: Build | ||
| run: make | ||
| run: | | ||
| if [ "${{ matrix.artifact_name }}" = "ado-linux-arm64" ]; then | ||
| make CC=aarch64-linux-gnu-gcc | ||
| else | ||
| make | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect Makefile for cross-compile friendliness: CC usage, hardcoded toolchain, host-vs-target separation.
fd -t f -i '^(GNUmakefile|[Mm]akefile)$' --max-depth 3 --exec sh -c 'echo "=== {} ==="; cat "{}"'Repository: clpi/dopl
Length of output: 641
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== release.yml ==="
sed -n '1,220p' .github/workflows/release.yml
echo
echo "=== Makefile (full) ==="
cat -n MakefileRepository: clpi/dopl
Length of output: 5006
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== release.yml ==="
sed -n '1,220p' .github/workflows/release.yml
echo
echo "=== Makefile (full) ==="
cat -n MakefileRepository: clpi/dopl
Length of output: 5006
Cross-compile here is likely OK with CC, but add an output-arch sanity check before publishing.
- The
Makefilebuilds thedocbinary using only$(CC) $(CFLAGS) ... $(LDFLAGS)and doesn’t referenceAR/LD/STRIP/host-tool execution; the release workflow also doesn’t runmake test, so it won’t execute a host-generated binary. - The workflow still doesn’t validate the produced
dist/${{ matrix.artifact_name }}binary; add a quickfile dist/${{ matrix.artifact_name }}step (or equivalent) before packaging/uploading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yml around lines 28 - 38, Add an
output-architecture sanity check in the release workflow after the Build step
and before packaging/uploading: after invoking make with CC
(matrix.artifact_name and CC usage in the Makefile), run a `file` (or
equivalent) on dist/${{ matrix.artifact_name }} and assert its reported
architecture matches the expected matrix.artifact_name (e.g., contains "ARM
aarch64" for ado-linux-arm64); if it does not, fail the job so a host-built
binary is not published. Reference the workflow's matrix.artifact_name, the
Makefile's reliance on CC, and the produced artifact path dist/${{
matrix.artifact_name }} when adding this guard.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa1075226f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --prefix /usr/local/bin \ | ||
| doc=/usr/local/bin/ado |
There was a problem hiding this comment.
Remove double-prefixing in fpm install path
The new packaging commands combine --prefix /usr/local/bin with an absolute destination mapping doc=/usr/local/bin/ado. For fpm -s dir, destination mappings are still prefixed, so this produces a packaged path like /usr/local/bin/usr/local/bin/ado instead of /usr/local/bin/ado, making the released .deb/.rpm install the binary in the wrong location. This breaks the primary executable path for package consumers.
Useful? React with 👍 / 👎.
This PR updates the GitHub Actions release workflow to be more comprehensive and robust. It adds an ARM64 Linux target via cross-compilation and leverages
fpmto automatically generate.deband.rpmpackages for deployment to standard Linux package managers. All generated packages are now properly included as part of the GitHub Release.PR created automatically by Jules for task 2992588744520766854 started by @clpi
Summary by CodeRabbit