ci: add binary dependency verification for Linux, macOS, and Flatpak#770
ci: add binary dependency verification for Linux, macOS, and Flatpak#770
Conversation
📝 WalkthroughWalkthroughThese changes add new binary dependency verification steps to GitHub Actions workflows. The flatpak workflow verifies the dash-evo-tool binary dependencies using ldd, while the release workflow adds platform-specific verification steps for Linux (ldd) and macOS (otool -L) to ensure binaries are properly linked before release. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Pull request overview
Adds CI post-build verification steps to ensure release artifacts don’t have missing or unintended dynamic library dependencies on non-Windows platforms, complementing the existing Windows PE import checks.
Changes:
- Add
ldd-based dependency verification for Linux release artifacts (fail on missing libs; warn on non-allowlisted libs). - Add
otool -L-based dependency verification for macOS (fail on non-system dylibs; warn on@rpathdeps). - Add a Flatpak binary dependency verification step intended to fail on missing libs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/release.yml | Adds Linux/macOS dependency inspection steps (ldd/otool) to catch missing/non-system dynamic deps in release builds. |
| .github/workflows/flatpak.yml | Adds a dependency verification step for the Flatpak-produced binary after flatpak-builder runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
701d8c2 to
c7683d4
Compare
Add post-build sanity checks to catch missing or unexpected shared library dependencies before release artifacts are published: - Linux: ldd check with allowlist, fail on "not found" - macOS (ARM64 + x86): otool -L check, fail on non-system libs - Flatpak: ldd check inside build-dir, fail on "not found" Complements the Windows PE verification from #769. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c7683d4 to
1bad487
Compare
When all binary dependencies are valid system libraries, `grep -Ev` filters out all lines and returns exit code 1 (no matches). Under `bash -e` (GitHub Actions default), this kills the script before it can report success. Add `|| true` to all grep commands inside command substitutions across Linux and macOS verification steps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
276-295: Extract duplicated macOS dependency check to a shared script to avoid drift.The ARM64 and x86_64 verification blocks are identical. Consider moving this logic into a single script (e.g.,
scripts/verify-macos-deps.sh) and calling it from both jobs to keep policy changes synchronized.Refactor sketch
- - name: Verify macOS binary dependencies - run: | - echo "Checking dynamic library dependencies..." - DEPS=$(otool -L build/dash-evo-tool) - echo "$DEPS" - # Only system libraries (/usr/lib/) and frameworks (/System/Library/) are allowed - UNEXPECTED=$(echo "$DEPS" | tail -n +2 | awk '{print $1}' | grep -Ev "^(/usr/lib/|/System/Library/|@rpath/)" || true) - if [ -n "$UNEXPECTED" ]; then - echo "::error::Binary links non-system libraries:" - echo "$UNEXPECTED" - exit 1 - fi - # Warn on `@rpath` dependencies (acceptable for frameworks but worth noting) - RPATH=$(echo "$DEPS" | tail -n +2 | awk '{print $1}' | grep "^@rpath/" || true) - if [ -n "$RPATH" ]; then - echo "::warning::Binary has `@rpath` dependencies (verify these are bundled):" - echo "$RPATH" - fi - echo "✅ macOS binary dependencies look clean" + - name: Verify macOS binary dependencies + run: bash scripts/verify-macos-deps.sh build/dash-evo-toolAlso applies to: 565-584
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 276 - 295, Extract the duplicated "Verify macOS binary dependencies" step into a single executable script (e.g., scripts/verify-macos-deps.sh) that takes the binary path as an argument (defaulting to build/dash-evo-tool), preserves the otool/awk/grep logic, emits the same ::error:: and ::warning:: messages and exit codes, and is made executable (chmod +x). Then replace both ARM64 and x86_64 verification blocks in the release.yml with a single step that runs this script (passing the binary path), ensuring the workflow still checks out the repo before running the script.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 276-295: Extract the duplicated "Verify macOS binary dependencies"
step into a single executable script (e.g., scripts/verify-macos-deps.sh) that
takes the binary path as an argument (defaulting to build/dash-evo-tool),
preserves the otool/awk/grep logic, emits the same ::error:: and ::warning::
messages and exit codes, and is made executable (chmod +x). Then replace both
ARM64 and x86_64 verification blocks in the release.yml with a single step that
runs this script (passing the binary path), ensuring the workflow still checks
out the repo before running the script.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 71735fe3-4a18-4e4f-a905-d9fbcdc9dee4
📒 Files selected for processing (2)
.github/workflows/flatpak.yml.github/workflows/release.yml
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Trivial fix: adds || true to 5 grep commands to prevent exit code 1 (no matches found) from aborting CI steps under set -e/pipefail. Correct and complete.
Reviewed commit: 0978aee
The ldd check was running against the host runner's libraries, which don't match the Flatpak runtime. Use `flatpak build build-dir` to execute ldd inside the sandbox where libraries resolve against the Flatpak runtime (/usr) and app (/app) directories. Addresses review comment from copilot-pull-request-reviewer on PR #770. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
lddcheck with system library allowlist, fails on "not found" depsotool -Lcheck, fails on non-system libraries, warns on@rpathdepslddcheck insidebuild-dir, fails on "not found" depsComplements the Windows PE verification added in #769. Together, these checks ensure release binaries across all platforms are self-contained and won't crash at runtime due to missing shared libraries.
Test plan
lddverification stepotool -Lverification stepotool -Lverification steplddverification stepDepends on #769 (builds on
fix/windows-static-linkingbranch).🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
Release Notes