Add CI for Linux#25
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds a Linux GitHub Actions workflow to build and test the project with Clang 20 and CMake 4.3.0, and inserts a missing ChangesLinux CI and Build Infrastructure
Sequence Diagram(s)sequenceDiagram
participant GitHubEvent as GitHub Event
participant Checkout as Checkout (actions/checkout)
participant Toolchain as Toolchain Setup (Clang/CMake/System Deps)
participant Configure as CMake Configure
participant Build as Ninja Build
participant Test as ctest
participant Artifact as Upload Artifact
GitHubEvent->>Checkout: push / pull_request triggers
Checkout->>Toolchain: fetch repo + submodules
Toolchain->>Toolchain: install Clang 20, download CMake 4.3.0, apt deps
Toolchain->>Configure: expose clang/clang++, set RPATH, run cmake --preset debug
Configure->>Build: cmake --build (Ninja) -j$(nproc)
Build->>Test: ctest --output-on-failure -j$(nproc)
Test->>Artifact: upload build/debug/ as linux_debug
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/linux_build.yml:
- Around line 19-24: Replace mutable tag refs with pinned commit SHAs for each
GitHub Action used: change actions/checkout@v4, egor-tensin/setup-clang@v1,
actions/cache@v4, and actions/upload-artifact@v4 to their corresponding full
commit SHAs; keep the human-friendly tag in a trailing comment (e.g., "# v4")
for maintainability. Update the workflow entries for the actions named
actions/checkout, egor-tensin/setup-clang, actions/cache, and
actions/upload-artifact to use the SHA strings instead of tag refs and add the
version tag comment immediately after each SHA.
- Line 3: The workflow contains an unsupported top-level key "description";
remove this key and its value from the workflow root so the file only uses valid
top-level keys (e.g., name, on, permissions, env, defaults, jobs, concurrency,
run-name); simply delete the line `description: Build Draconic Engine on Linux
using Clang 20 and CMake with Ninja` so the workflow validates against the
GitHub Actions schema.
- Around line 30-32: The workflow currently uses wget/tar/echo to download and
add CMake but lacks integrity checks; update the block that runs wget, tar, and
echo to also download or embed the official SHA256, verify the downloaded
cmake-4.3.0-linux-x86_64.tar.gz with sha256sum (or shasum -a 256) and fail the
job on mismatch before extracting; specifically add a variable or file
containing the expected checksum for cmake-4.3.0-linux-x86_64.tar.gz, run
sha256sum -c (or compare computed hash) against that expected value and exit
non‑zero if verification fails, then only proceed to tar -xzf and echo the bin
path to $GITHUB_PATH on success.
- Around line 14-22: The workflow currently leaves default permissions and
persists git credentials; update the build-linux job to enforce least-privilege
and disable persisted credentials: add an explicit permissions map (e.g.,
permissions: contents: read, id-token: write if needed or set other scopes to
none) at the job or workflow level and set the actions/checkout@v4 step option
persist-credentials: false so no token is stored in the checked-out repo; modify
the "build-linux" job and the "uses: actions/checkout@v4" step accordingly.
🪄 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: 5aa5bd30-8b5a-4ae8-beda-2593b7cbc264
📒 Files selected for processing (2)
.github/workflows/linux_build.ymlengine/native/core/math/constants.cppm
OldDev78
left a comment
OldDev78
left a comment
There was a problem hiding this comment.
The presets are there to streamline CI build, so it should be able to rely on them.
| cmake --preset debug \ | ||
| -B build/debug \ | ||
| -DCMAKE_C_COMPILER=clang \ | ||
| -DCMAKE_CXX_COMPILER=clang++ \ | ||
| -DCMAKE_CXX_FLAGS="-stdlib=libc++" \ | ||
| -DCMAKE_EXE_LINKER_FLAGS="-stdlib=libc++" \ | ||
| -DCMAKE_BUILD_WITH_INSTALL_RPATH=TRUE \ | ||
| -DCMAKE_INSTALL_RPATH="\$ORIGIN:\$ORIGIN/engine/native" |
There was a problem hiding this comment.
CI needs to compile using only the presets. This is a requirement.
Overriding stdlib was only needed when using import std. This is no longer the case.
The CMAKE_BUILD_WITH_INSTALL_RPATH and CMAKE_INSTALL_RPATH are of no use for us at this moment.
There was a problem hiding this comment.
The
CMAKE_BUILD_WITH_INSTALL_RPATHandCMAKE_INSTALL_RPATHare of no use for us at this moment.
They are of use to us since otherwise the executable isn't able to find the libraries.
|
|
||
| - name: Compile Target Binary | ||
| run: | | ||
| cmake --build build/debug -j$(nproc) |
There was a problem hiding this comment.
Usually we run the release build on CI.
But we could have CI test both. What do you think @mcdubhghlas @Arctis-Fireblight ?
There was a problem hiding this comment.
I use debug since it allows me to get error messages or output.
2 participants
Note
Contributed by 2LazyDevs.
Summary by CodeRabbit