Robust CI/CD Pipeline and Artifacts#17
Conversation
- Add Linux ARM64 cross-compilation in `release.yml`. - Introduce `fpm` step to build `.deb` and `.rpm` packages for Linux builds. - Include `.tar.gz` archive generation alongside standalone binaries. - Ensure proper file uploading to the GitHub Release. - Improve `flake.nix` by adding `meta` fields (description, homepage, license) to enhance NixOS indexing.
|
👋 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. |
📝 WalkthroughWalkthroughThis PR extends the release process by refactoring the GitHub Actions workflow to build and package artifacts per architecture using a matrix, conditionally installing cross-compilers and packaging tools, and generating tarballs and OS packages (.deb and .rpm). Flake package metadata is added for Nix discoverability. ChangesRelease and Package Metadata
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 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 |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds robust multi-platform CI/CD capabilities with ARM64 support and package manager integration. I've identified 2 issues that need to be addressed:
Critical:
- FPM packaging command has incorrect syntax that will cause build failures
Minor:
- Duplicate URL variable assignment in Homebrew update job
The Nix flake metadata addition looks good. Please fix the FPM command syntax to ensure successful package creation.
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.
| fpm -s dir -t deb -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin doc=ado | ||
| fpm -s dir -t rpm -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin doc=ado |
There was a problem hiding this comment.
🛑 Logic Error: The FPM command syntax is incorrect. The doc=ado argument maps a source directory named doc to destination ado, but line 55 copies a file (not directory) named doc as the artifact. This will cause FPM packaging to fail because it expects a directory structure.
The correct approach is to specify the actual binary file path with proper destination mapping. FPM expects either a directory structure to package or explicit file paths with --prefix.
| fpm -s dir -t deb -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin doc=ado | |
| fpm -s dir -t rpm -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin doc=ado | |
| fpm -s dir -t deb -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin -C dist ${{ matrix.artifact_name }}=ado | |
| fpm -s dir -t rpm -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin -C dist ${{ matrix.artifact_name }}=ado |
| # Download the source tarball to calculate sha256 | ||
| URL="https://github.com/${{ github.repository }}/archive/refs/tags/${TAG_NAME}.tar.gz" | ||
| curl -sL $URL -o source.tar.gz |
There was a problem hiding this comment.
Remove duplicate URL assignment. The URL variable is already defined on line 125.
| # Download the source tarball to calculate sha256 | |
| URL="https://github.com/${{ github.repository }}/archive/refs/tags/${TAG_NAME}.tar.gz" | |
| curl -sL $URL -o source.tar.gz | |
| # Download the source tarball to calculate sha256 | |
| curl -sL $URL -o source.tar.gz |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a meta block to the flake.nix file, providing project metadata such as description, homepage, and license. The reviewer suggests adding a platforms field to improve CI/CD system identification and removing the redundant empty maintainers list.
| meta = with pkgs.lib; { | ||
| description = "Ado - A Minimal Programming Language"; | ||
| homepage = "https://github.com/ado-lang/ado"; | ||
| license = licenses.mit; | ||
| maintainers = [ ]; | ||
| }; |
There was a problem hiding this comment.
To support the goal of a "Robust CI/CD Pipeline" and "multi-platform artifacts", the meta block should include the platforms field. This allows CI tools and Nix commands to correctly identify compatible systems. Additionally, the empty maintainers list is redundant and should be removed if no maintainers are currently specified.
meta = with pkgs.lib; {
description = "Ado - A Minimal Programming Language";
homepage = "https://github.com/ado-lang/ado";
license = licenses.mit;
platforms = platforms.unix;
};
There was a problem hiding this comment.
Pull request overview
This PR enhances the project’s release automation to produce and publish multi-platform build artifacts (including tarballs and Linux packages), and improves Nix packaging metadata for the ado derivation.
Changes:
- Extend the GitHub Actions release workflow to build Linux
amd64+ cross-compiledarm64, plus macOSarm64artifacts. - Add packaging outputs:
.tar.gzbundles and Linux.deb/.rpmpackages, and attach them to GitHub Releases. - Add a
metablock to the Nix flake derivation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
flake.nix |
Adds Nix package metadata (meta) for the ado derivation. |
.github/workflows/release.yml |
Expands release matrix and produces tarballs and .deb/.rpm artifacts, then publishes them to a GitHub Release and updates the Homebrew formula. |
💡 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 }} --prefix /usr/local/bin doc=ado | ||
| fpm -s dir -t rpm -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin doc=ado |
| URL="https://github.com/${{ github.repository }}/archive/refs/tags/${TAG_NAME}.tar.gz" | ||
|
|
||
| # Download the source tarball to calculate sha256 | ||
| URL="https://github.com/${{ github.repository }}/archive/refs/tags/${TAG_NAME}.tar.gz" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
9-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit least-privilege
permissionstobuild-release.Line 9 defines a job without a
permissionsblock, so token scope falls back to repository defaults. Please scope this job explicitly (typicallycontents: readand the minimum required for artifact upload).🤖 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 9 - 13, Add an explicit permissions block to the build-release job so the checkout and artifact upload use least privilege: inside the build-release job definition add a permissions section that at minimum sets contents: read and artifacts: write (to allow actions/upload-artifact to upload artifacts); ensure the new permissions block is placed alongside runs-on/strategy and not at the workflow root so it only scopes this job.
🤖 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:
- Line 26: Replace all tag-pinned GitHub Action references in the workflow with
their corresponding full commit SHAs: locate each `uses:` entry mentioned (e.g.,
`actions/checkout@v4` and the other `uses:` entries at the commented lines) and
replace the `@<tag>` with the official full commit SHA for that action version;
ensure you update every occurrence (lines noted: 26, 36, 73, 86, 89, 94, 117) so
each `uses:` now points to `owner/repo@<full-commit-sha>` and verify the SHAs
match the releases for those action repositories.
- Around line 67-68: The RPM packaging command is using Debian-style
architecture names via ${ { matrix.arch } }, resulting in incorrect RPM
metadata; update the second fpm invocation (the rpm target) to map amd64→x86_64
and arm64→aarch64 (e.g., conditionally translate ${ { matrix.arch } } for the
rpm -a argument) or simply use fpm's -a native for the rpm build instead of
passing the raw matrix.arch value so RPM packages get correct architecture
names.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 9-13: Add an explicit permissions block to the build-release job
so the checkout and artifact upload use least privilege: inside the
build-release job definition add a permissions section that at minimum sets
contents: read and artifacts: write (to allow actions/upload-artifact to upload
artifacts); ensure the new permissions block is placed alongside
runs-on/strategy and not at the workflow root so it only scopes this job.
🪄 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: 910d21d0-a02e-4a68-a69e-551d64cf5d54
📒 Files selected for processing (2)
.github/workflows/release.ymlflake.nix
| artifact_name: ado-macos-arm64 | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify unpinned action references in this workflow.
rg -nP '^\s*-\s*uses:\s*[^@]+@v[0-9]+' .github/workflows/release.yml
# Expected after fix: no matches (all uses pinned to full commit SHAs).Repository: clpi/dopl
Length of output: 159
🏁 Script executed:
cat -n .github/workflows/release.yml | sed -n '20,40p; 65,80p; 80,90p; 110,120p'Repository: clpi/dopl
Length of output: 1989
🏁 Script executed:
rg -n 'uses:\s*[^@]+@v[0-9]' .github/workflows/release.ymlRepository: clpi/dopl
Length of output: 325
Pin all GitHub Actions to full commit SHAs.
These uses: references are tag-pinned (@v*) instead of SHA-pinned, creating a supply-chain risk.
Apply to lines: 26, 36, 73, 86, 89, 94, 117
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 26-26: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 26-26: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 26, Replace all tag-pinned GitHub
Action references in the workflow with their corresponding full commit SHAs:
locate each `uses:` entry mentioned (e.g., `actions/checkout@v4` and the other
`uses:` entries at the commented lines) and replace the `@<tag>` with the
official full commit SHA for that action version; ensure you update every
occurrence (lines noted: 26, 36, 73, 86, 89, 94, 117) so each `uses:` now points
to `owner/repo@<full-commit-sha>` and verify the SHAs match the releases for
those action repositories.
| fpm -s dir -t deb -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin doc=ado | ||
| fpm -s dir -t rpm -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin doc=ado |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For fpm packaging, what architecture values are valid for -t rpmversus-t deb? Specifically, are amd64andarm64valid for RPM packages, or should they bex86_64andaarch64?
💡 Result:
For fpm, architecture strings for -a are interpreted/normalized differently depending on the target package type (-t). Answer to your question (canonical values): - For -t rpm (RPM / Red Hat naming), use x86_64 and aarch64 (not amd64 / arm64). fpm’s RPM logic targets Red Hat-style architecture naming, and the fpm maintainer notes the Debian vs Red Hat naming split (Debian uses amd64, Red Hat uses x86_64). [1] - For -t deb (Debian naming), use amd64 and arm64 (not x86_64 / aarch64). The same fpm maintainer notes Debian rejects x86_64 as a valid Debian arch value and uses amd64 instead, with fpm converting when building -t deb. [1] - For ARM specifically, Debian uses arm64 whereas the kernel/other tooling often reports aarch64; fpm has special-case conversion for this in Debian packaging contexts. [2] So: amd64/arm64 are Debian names; for RPM packages you should prefer x86_64/aarch64. [1][2] Practical tip: if you want to avoid manual arch-string differences, fpm supports -a native / -a all, which it translates into the correct architecture value for the target package type. [3]
Citations:
- 1: deb package: architecture seemingly ignored when generating package jordansissel/fpm#1762
- 2: Fixed Debian platform tag for aarch64/arm64 jordansissel/fpm#1775
- 3: https://fpm.readthedocs.io/en/v1.14.1/cli-reference.html
🏁 Script executed:
cat -n .github/workflows/release.yml | head -80Repository: clpi/dopl
Length of output: 2784
Fix architecture values for RPM packaging.
The workflow uses Debian-style architecture names (amd64/arm64) via ${{ matrix.arch }} for both deb and rpm targets at lines 67-68. RPM requires Red Hat-style names (x86_64/aarch64). The generated RPM packages will have incorrect architecture metadata.
Map the architecture values conditionally when building RPM:
fpm -s dir -t deb -n ado -v $VERSION -a ${{ matrix.arch }} --prefix /usr/local/bin doc=ado
fpm -s dir -t rpm -n ado -v $VERSION -a $([ "${{ matrix.arch }}" = "amd64" ] && echo "x86_64" || echo "aarch64") --prefix /usr/local/bin doc=ado
Alternatively, use fpm's -a native flag to auto-translate for the target package type.
🤖 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 67 - 68, The RPM packaging
command is using Debian-style architecture names via ${ { matrix.arch } },
resulting in incorrect RPM metadata; update the second fpm invocation (the rpm
target) to map amd64→x86_64 and arm64→aarch64 (e.g., conditionally translate ${
{ matrix.arch } } for the rpm -a argument) or simply use fpm's -a native for the
rpm build instead of passing the raw matrix.arch value so RPM packages get
correct architecture names.
Updates to
release.ymlandflake.nixto robustly build multi-platform artifacts and.deb/.rpmpackages for package managers, and standard tarball deployments.PR created automatically by Jules for task 8766514705031405948 started by @clpi
Summary by CodeRabbit