fix(libkrun-ci): use patches in cache key - #910
Conversation
📝 WalkthroughWalkthroughThe macOS libkrun setup action now hashes patch contents and includes that digest in the generated prefix and GitHub Actions cache key, alongside the pinned commit and build-script hash. Changeslibkrun cache invalidation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
| # file emits `<hash> <path>`; sort pins a stable order, then a final | ||
| # digest folds them into one value. No patches => digest of empty input, | ||
| # still deterministic. | ||
| patches_hash="$(find vendor/libkrun/patches -name '*.patch' -type f -exec shasum -a 256 {} + 2>/dev/null | sort | shasum -a 256 | cut -d' ' -f1)" |
There was a problem hiding this comment.
🟠 High setup-libkrun-macos/action.yml:34
When all carried patches are removed, Git deletes the empty vendor/libkrun/patches directory, so find exits nonzero. Because GitHub Actions' default shell uses -e -o pipefail, the patches_hash assignment aborts the step instead of producing the documented "digest of empty input." The build fails on the exact case this change is supposed to support. Consider guarding the directory, e.g. patches_hash="$([ -d vendor/libkrun/patches ] && find vendor/libkrun/patches -name '*.patch' -type f -exec shasum -a 256 {} + | sort | shasum -a 256 | cut -d' ' -f1 || echo -n | shasum -a 256 | cut -d' ' -f1)".
| patches_hash="$(find vendor/libkrun/patches -name '*.patch' -type f -exec shasum -a 256 {} + 2>/dev/null | sort | shasum -a 256 | cut -d' ' -f1)" | |
| patches_hash="$([ -d vendor/libkrun/patches ] && find vendor/libkrun/patches -name '*.patch' -type f -exec shasum -a 256 {} + | sort | shasum -a 256 | cut -d' ' -f1 || echo -n | shasum -a 256 | cut -d' ' -f1)" |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @.github/actions/setup-libkrun-macos/action.yml around line 34:
When all carried patches are removed, Git deletes the empty `vendor/libkrun/patches` directory, so `find` exits nonzero. Because GitHub Actions' default shell uses `-e -o pipefail`, the `patches_hash` assignment aborts the step instead of producing the documented "digest of empty input." The build fails on the exact case this change is supposed to support. Consider guarding the directory, e.g. `patches_hash="$([ -d vendor/libkrun/patches ] && find vendor/libkrun/patches -name '*.patch' -type f -exec shasum -a 256 {} + | sort | shasum -a 256 | cut -d' ' -f1 || echo -n | shasum -a 256 | cut -d' ' -f1)"`.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/actions/setup-libkrun-macos/action.yml:
- Line 34: Update the patches_hash computation to invoke locale-independent
sorting by setting LC_ALL=C on the sort command. Preserve the existing file
hashing and final digest pipeline while ensuring identical patch sets produce
the same hash across runner locales.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76a34d99-dd4b-483a-9732-476f3714a6d7
📒 Files selected for processing (1)
.github/actions/setup-libkrun-macos/action.yml
| # file emits `<hash> <path>`; sort pins a stable order, then a final | ||
| # digest folds them into one value. No patches => digest of empty input, | ||
| # still deterministic. | ||
| patches_hash="$(find vendor/libkrun/patches -name '*.patch' -type f -exec shasum -a 256 {} + 2>/dev/null | sort | shasum -a 256 | cut -d' ' -f1)" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Pin the sorting locale for a stable digest.
sort is locale-sensitive, and its ordering feeds the final SHA-256. Different runner locales can therefore produce different patches_hash values for identical patches, causing avoidable cache misses and divergent prefixes. Use LC_ALL=C sort.
🤖 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/actions/setup-libkrun-macos/action.yml at line 34, Update the
patches_hash computation to invoke locale-independent sorting by setting
LC_ALL=C on the sort command. Preserve the existing file hashing and final
digest pipeline while ensuring identical patch sets produce the same hash across
runner locales.
Include patch files in libkrun macOS cache key to trigger rebuilds on patch changes
The action.yml composite action now hashes all
*.patchfiles undervendor/libkrun/patchesand incorporates the resultingpatches_hashinto both the cache key and the persistent prefix path. Previously, adding, removing, or modifying patches would not invalidate the cache, causing stale builds to be used.📊 Macroscope summarized ca6120f. 1 file reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.
Summary by CodeRabbit