build: update package workflow#307
Conversation
|
Warning Rate limit exceeded@16bit-ykiko has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a new matrix "package" CI workflow, updates CI macOS Homebrew handling, consolidates xmake build steps, renames three prebuilt-LLVM artifact filenames to "releasedbg" variants, moves packaging into xmake.lua Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as "Push / PR"
participant GH as "GitHub Actions"
participant Runner as "Runner (win / linux / macos)"
rect rgba(230,245,255,0.35)
Dev->>GH: trigger package.yml or xmake.yml
GH->>Runner: start matrix job
end
rect rgba(245,255,235,0.25)
Runner->>Runner: checkout repo\nsetup xmake v3.0.4 + cache\napply HOMEBREW_NO_AUTO_UPDATE on macOS
Runner->>Runner: OS-specific preflight\n(Linux: disk/swap/apt & alternatives)\n(macOS: brew llvm + PATH)
end
Runner->>Runner: xmake configure + build (toolchain-aware)
Runner->>Runner: xmake on_package -> assemble build dir\ncreate symbol archive, copy binary, docs, lib/clang\ncreate final archive (.zip or .tar.gz)
alt push event
Runner->>GH: upload release artifacts (per-OS naming, overwrite)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/package.yml (1)
46-46: Improve safety of apt remove command.The
apt removecommand uses|| trueto ignore failures, but a safer approach would be to check if packages are installed before attempting removal, or useapt-get remove --purgewith proper error handling.Consider this alternative:
- sudo apt remove -y llvm-16-linker-tools llvm-16 llvm-17-linker-tools llvm-17 llvm-18-linker-tools llvm-18 || true + sudo dpkg -l | grep -E 'llvm-(16|17|18)' && sudo apt-get remove --purge -y llvm-16-linker-tools llvm-16 llvm-17-linker-tools llvm-17 llvm-18-linker-tools llvm-18 || echo "No conflicting LLVM versions found"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/check-format.yml(0 hunks).github/workflows/package.yml(4 hunks)config/prebuilt-llvm.json(3 hunks)xmake.lua(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/check-format.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: package (ubuntu-24.04, clice.tar.xz, clice-x86_64-linux-gnu.tar.xz, clang-20)
- GitHub Check: build (macos-15, Debug, clang, clang++)
- GitHub Check: build (windows-2025, RelWithDebInfo, clang, clang++)
- GitHub Check: build (ubuntu-24.04, Debug, clang-20, clang++-20)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (macos-15, releasedbg)
- GitHub Check: build (ubuntu-24.04, releasedbg)
- GitHub Check: build (windows-2025, releasedbg)
- GitHub Check: build (macos-15, debug)
🔇 Additional comments (6)
.github/workflows/package.yml (5)
1-1: LGTM: Workflow renamed and path filter updated.The workflow has been appropriately renamed from "release" to "package" with the corresponding path filter update.
Also applies to: 14-14
19-19: Good addition: fail-fast disabled for matrix builds.Setting
fail-fast: falseallows all platform builds to complete even if one fails, which is helpful for identifying platform-specific issues.
59-73: Swap configuration is comprehensive but verbose.The swap setup with detailed status logging is thorough, which is good for debugging. The 16GB swap file should help with memory-intensive LLVM builds.
102-103: Verify macOS PATH configuration persists correctly.The PATH export for macOS LLVM and LLD should work within the bash shell, but please verify that the xmake config command correctly picks up the LLVM toolchain from this PATH.
Consider adding a verification step to confirm the PATH is set correctly:
export PATH="/opt/homebrew/opt/llvm@20/bin:/opt/homebrew/opt/lld@20/bin:$PATH" + echo "PATH=$PATH" + which clang + clang --version xmake config --yes --enable_test=n --dev=n --release=y --toolchain=${{ matrix.toolchain }} --sdk=/opt/homebrew/opt/llvm@20
86-86: xmake version 3.0.4 is valid and stable.xmake 3.0.4 is the current stable release, published in October 2025, and is available in Fedora, Alpine, and the xmake Ubuntu PPA. The pinned version is appropriate for the workflow.
config/prebuilt-llvm.json (1)
14-14: Artifacts exist and hashes have been updated. The threereleasedbg-ltoartifacts are present in the GitHub release, and the config file contains SHA256 hashes that differ from the non-LTO variants—indicating they were updated when filenames changed. However, verification of whether these hashes match the actual file contents requires downloading the artifacts, which cannot be completed in this environment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
xmake.lua (1)
311-312: Critical: Artifact format mismatch persists - workflow file not updated.The packaging format was standardized to
.zipfor all platforms, but the workflow file.github/workflows/package.ymlstill expects platform-specific formats:
- Windows:
clice.7z(line 23)- Linux:
clice.tar.xz(line 28)- macOS:
clice.tar.xz(line 33)This mismatch will cause the "Upload binaries to release" step to fail.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/package.yml(4 hunks)xmake.lua(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (windows-2025, RelWithDebInfo, clang, clang++)
- GitHub Check: build (macos-15, Debug, clang, clang++)
- GitHub Check: build (ubuntu-24.04, Debug, clang-20, clang++-20)
- GitHub Check: build (macos-15, releasedbg)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (windows-2025, releasedbg)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: package (ubuntu-24.04, clice.tar.xz, clice-x86_64-linux-gnu.tar.xz, clang-20)
- GitHub Check: build (ubuntu-24.04, releasedbg)
🔇 Additional comments (7)
.github/workflows/package.yml (7)
1-1: LGTM: Workflow rename is consistent.The workflow name and self-referential path filter have been updated consistently from "release" to "package".
Also applies to: 14-14
19-19: LGTM: Fail-fast disabled for better multi-platform diagnostics.Disabling fail-fast allows all platform builds to complete independently, providing better visibility into platform-specific issues.
40-42: LGTM: Linux disk space management.The disk space cleanup step is properly conditioned for Linux runners and helps prevent out-of-disk issues during builds.
62-76: LGTM: Linux dependencies properly consolidated.The Linux dependency setup consolidates the previous separate steps and is properly guarded with the OS condition.
77-80: LGTM: MacOS dependencies setup.The macOS dependency installation is properly conditioned and installs the required LLVM and LLD versions.
96-108: LGTM: Configure and package steps properly consolidated.The consolidation of configure and package steps is logical. The OS-specific handling is appropriate, particularly the macOS PATH adjustment for Homebrew-installed LLVM and the platform-specific SDK configuration.
88-94: LGTM: Xmake version pinned to latest stable release with caching enabled.xmake v3.0.4 is the latest stable release, confirming the version pin is both current and reliable. Pinning to a specific version (instead of
branch@master) improves reproducibility and CI stability. The package and build cache configuration with OS-specific keys will enhance CI performance while maintaining proper build isolation.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
xmake.lua (1)
311-317: Critical: Artifact format mismatch still unresolved.This is the same critical issue previously flagged: the packaging format produces
.zip(Windows) and.tar.gz(Linux/macOS) archives, but the workflow at.github/workflows/package.ymlstill expects platform-specific formats (.7zfor Windows at line 23,.tar.xzfor Linux/macOS at lines 28 and 33). This mismatch will cause the "Upload binaries to release" step to fail.
🧹 Nitpick comments (3)
xmake.lua (2)
322-322: Verify removal of installfiles directive.The
add_installfilesforclice.tomlhas been commented out and replaced with manual copying at line 347 withinon_package. While this appears intentional, ensure that the new approach handles all edge cases that the previous installfiles mechanism covered.
324-352: Consider adding error handling for critical operations.The
on_packagefunction usesos.tryrmfor cleanup (lines 328, 335, 344), which silently ignores failures. While this is acceptable for cleanup operations, consider whether critical operations like file copying (lines 341, 346, 347, 350) should have explicit error handling to catch packaging failures early..github/workflows/package.yml (1)
40-81: LGTM: OS-specific preflight steps with proper conditionals.The Linux disk management and macOS dependency installation steps are correctly guarded with
runner.osconditionals. The swap file increase issue from the previous review has been properly addressed.Consider adding error checking after the macOS brew install step to verify LLVM and LLD are available before proceeding, though the current approach should work in most cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/package.yml(4 hunks)xmake.lua(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (windows-2025, releasedbg)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-24.04, releasedbg)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (macos-15, releasedbg)
- GitHub Check: build (windows-2025, RelWithDebInfo, clang, clang++)
- GitHub Check: build (macos-15, Debug, clang, clang++)
- GitHub Check: build (ubuntu-24.04, Debug, clang-20, clang++-20)
- GitHub Check: package (macos-15, clice.tar.xz, clice-arm64-macos-darwin.tar.xz, clang)
- GitHub Check: package (ubuntu-24.04, clice.tar.xz, clice-x86_64-linux-gnu.tar.xz, clang-20)
- GitHub Check: package (windows-2025, clice.7z, clice-x64-windows-msvc.7z, clang-cl)
🔇 Additional comments (3)
.github/workflows/package.yml (3)
1-19: LGTM: Workflow rename and matrix configuration.The rename from "release" to "package" is clear, and adding
fail-fast: falseensures all platform builds complete independently, which is appropriate for release packaging.
97-109: Configure and package consolidation looks good.The unified "Configure and Package" step appropriately handles OS-specific differences. The macOS PATH export (line 105) correctly references the LLVM@20 and LLD@20 installations from line 81.
Note: The artifact format mismatch between this workflow (expecting
.7z,.tar.xz) and xmake.lua (producing.zip,.tar.gz) is already flagged in the xmake.lua review.
86-95: No issues found — xmake version pinning and cache configuration verified.xmake includes the XPack packaging plugin with on_package APIs, confirming that xmake 3.0.4 supports the required features. The version pinning and cache options are appropriate for reproducibility and build performance.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/package.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (macos-15, releasedbg)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (windows-2025, releasedbg)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (ubuntu-24.04, releasedbg)
- GitHub Check: package (ubuntu-24.04, clice.tar.xz, clice-x86_64-linux-gnu.tar.xz, clang-20)
- GitHub Check: package (macos-15, clice.tar.xz, clice-arm64-macos-darwin.tar.xz, clang)
- GitHub Check: package (windows-2025, clice.7z, clice-x64-windows-msvc.7z, clang-cl)
- GitHub Check: build (macos-15, Debug, clang, clang++)
- GitHub Check: build (windows-2025, RelWithDebInfo, clang, clang++)
- GitHub Check: build (ubuntu-24.04, Debug, clang-20, clang++-20)
🔇 Additional comments (2)
.github/workflows/package.yml (2)
78-81: No issues found — macOS workflow configuration is correct.GitHub Actions macos-15 runners use Apple Silicon, and Homebrew installs LLVM to /opt/homebrew/opt/llvm/bin on Apple Silicon systems. The workflow's use of
/opt/homebrew/opt/llvm@20/binand/opt/homebrew/opt/lld@20/binis appropriate for this runner architecture.
111-118: The code is correct as written. The action accepts github.ref (e.g. refs/tags/v1.2.3) and automatically strips the refs/tags/ prefix, so usingtag: ${{ github.ref }}on tag push events will work properly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
xmake.lua (2)
323-323: Remove or document the commented-out line.Since
docs/clice.tomlis now copied explicitly inon_package(line 348), either remove this commented line or add a brief comment explaining why it was replaced (e.g.,-- now handled in on_package).- -- add_installfiles(path.join(os.projectdir(), "docs/clice.toml"))
329-345: Optional: Simplify redundant directory cleanup.The build directory is removed and recreated at line 329, then removed again at line 345. While this works correctly (the symbol archive is created from line 342 before the second cleanup), you could simplify by creating the directory once and managing contents more explicitly.
Consider this flow:
-- Create once os.tryrm(build_dir) os.mkdir(build_dir) -- Archive symbol os.vcp(target:symbolfile(), build_dir) clice_archive(path.join(package:outputdir(), "clice-symbol" .. package:extension())) -- Clear for main package os.tryrm(path.join(build_dir, "*")) -- or just overwrite with main files -- Proceed with main package
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/cmake.yml(1 hunks).github/workflows/package.yml(1 hunks).github/workflows/xmake.yml(2 hunks)xmake.lua(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/package.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (ubuntu-24.04, Debug, clang-20, clang++-20)
- GitHub Check: build (windows-2025, RelWithDebInfo, clang, clang++)
- GitHub Check: build (macos-15, Debug, clang, clang++)
- GitHub Check: package (ubuntu-24.04, clice.tar.xz, clice-x86_64-linux-gnu.tar.xz, clice-symbol.tar.xz, clice-x8...
- GitHub Check: build (ubuntu-24.04, releasedbg)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (macos-15, releasedbg)
- GitHub Check: build (windows-2025, releasedbg)
🔇 Additional comments (5)
.github/workflows/cmake.yml (1)
67-68: LGTM: Prevents unnecessary Homebrew updates during CI.Disabling auto-updates speeds up the build and makes it more deterministic by avoiding unexpected version changes mid-run.
.github/workflows/xmake.yml (2)
52-53: LGTM: Prevents unnecessary Homebrew updates during CI.Consistent with the same change in
cmake.yml.
71-83: LGTM: macOS PATH setup ensures LLVM tools are discoverable.The PATH export on line 79 correctly places LLVM 20 and LLD 20 in scope before the xmake config call, matching the pattern already used in the test step (lines 92-93).
xmake.lua (2)
321-321: LGTM: Stripping symbols reduces release package size.Using
set_strip("all")is appropriate since debug symbols are packaged separately (lines 341-343).
341-343: Symbol archive upload confirmed—no action needed.The workflow file verifies that symbol artifacts are uploaded:
.github/workflows/package.ymldefinessymbol_artifact_nameandsymbol_assert_namefor each OS (Windows, Linux, macOS) and uses them in the upload step (lines 134–135). The xmake.lua lines 341–343 correctly package the symbols, which are then distributed with each release.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
xmake.lua (1)
325-325: Consider removing commented-out code.The
add_installfilescall is now replaced by manual copying in theon_packagefunction (line 350). Consider removing this commented line to keep the code clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
xmake.lua(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (macos-15, releasedbg)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (windows-2025, releasedbg)
- GitHub Check: build (ubuntu-24.04, releasedbg)
- GitHub Check: build (ubuntu-24.04, Debug, clang-20, clang++-20)
- GitHub Check: build (windows-2025, RelWithDebInfo, clang, clang++)
- GitHub Check: build (macos-15, Debug, clang, clang++)
- GitHub Check: package (windows-2025, clice.zip, clice-x64-windows-msvc.zip, clice-symbol.zip, clice-x64-windows...
- GitHub Check: package (macos-15, clice.tar.xz, clice-arm64-macos-darwin.tar.xz, clice-symbol.tar.xz, clice-arm6...
- GitHub Check: package (ubuntu-24.04, clice.tar.xz, clice-x86_64-linux-gnu.tar.xz, clice-symbol.tar.xz, clice-x8...
🔇 Additional comments (2)
xmake.lua (2)
119-121: LGTM!Adding strip configuration for release builds is appropriate and will reduce the final binary size.
327-355: Well-structured packaging implementation.The
on_packageimplementation cleanly separates the symbol archive from the main distribution package and properly assembles all required components (binary, docs, LLVM resources). The helper functionclice_archiveprovides good abstraction for the archiving logic.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
xmake.lua (1)
327-327: Remove commented-out code.The
add_installfilesfunctionality has been moved to theon_packagehook (line 352). The commented line serves no purpose and should be removed entirely for clarity.Apply this diff:
- -- add_installfiles(path.join(os.projectdir(), "docs/clice.toml")) -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
xmake.lua(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (macos-15, releasedbg)
- GitHub Check: build (ubuntu-24.04, releasedbg)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (windows-2025, releasedbg)
- GitHub Check: build (macos-15, debug)
- GitHub Check: package (macos-15, clice.tar.xz, clice-arm64-macos-darwin.tar.xz, clice-symbol.tar.xz, clice-arm6...
- GitHub Check: package (ubuntu-24.04, clice.tar.xz, clice-x86_64-linux-gnu.tar.xz, clice-symbol.tar.xz, clice-x8...
- GitHub Check: package (windows-2025, clice.zip, clice-x64-windows-msvc.zip, clice-symbol.zip, clice-x64-windows...
- GitHub Check: build (macos-15, Debug, clang, clang++)
- GitHub Check: build (ubuntu-24.04, Debug, clang-20, clang++-20)
- GitHub Check: build (windows-2025, RelWithDebInfo, clang, clang++)
🔇 Additional comments (2)
xmake.lua (2)
123-127: LGTM: macOS workaround properly implemented.The dsymutil toolset override addresses the known xmake issue #7029 and is correctly scoped to macOS only.
329-357: LGTM: Packaging logic is well-structured.The
on_packageimplementation correctly:
- Separates symbol and main archives for independent distribution
- Creates standard directory structure (
bin/,lib/clang/)- Includes runtime dependencies (LLVM clang resources)
- Uses best compression for optimal artifact size
- Properly cleans the build directory between archive creations
The flow is logical and the implementation handles cross-platform packaging appropriately.
Summary by CodeRabbit
New Features
Chores
Packaging
Bug Fixes