cache-homebrew-prefix: restore git state after prefix cache restore#799
cache-homebrew-prefix: restore git state after prefix cache restore#799
Conversation
The `actions/cache/restore` step restores the entire Homebrew prefix, including `.git`. When the cached snapshot comes from a previous CI run on a different branch (typically `main`), this silently resets the repository to that branch, discarding the current checkout (e.g. a PR merge commit). Save the git HEAD sha before the cache restore and re-checkout it afterwards if the cache restore changed it. This ensures callers always run against the code they checked out.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Use env: block instead of ${{ }} interpolation in the run: script
to satisfy CodeQL's code injection check (GitHub Advanced Security).
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical issue where restoring the Homebrew prefix cache inadvertently resets the Homebrew repository's git state to a previously cached branch (typically main), causing PR workflows to test the wrong code and triggering "uncommitted modifications" errors.
Changes:
- Add step to save the Homebrew repository's git HEAD SHA before cache restoration
- Add step to restore the git state after cache restoration if it was changed
- The fix ensures PR workflows test the correct code by re-checking out the original commit after cache restore
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Wouldn't it make more sense to just cache the downloads? Because this will make it simpler and actually cache the thing we really want to cache? Especially since in a pr targeting homebrew/brew, can have unexpected issues. Also caching files that have been changes in the pr might be wasted cache storage. (just look at the size of the current prefix caches) |
No. It is significantly slower. If we can exclude the repository let's do that but this fixes the bug so is definitely better than the status quo. |
Sorry, I actually noticed, that I made it sound as criticism related to the pr, but no, I do agree this pr is important and be urged. And I should have opened an issue, which in retrospect would have been better. I apologise. |
If rev-parse HEAD fails (e.g. corrupt .git), silently skip saving the SHA so the restore step is a no-op and the action behaves as it did before this PR.
3 participants
Summary
The
cache-homebrew-prefixaction caches the entire Homebrew prefix directory (including.git) viaactions/cache/restore. When the cached snapshot was saved during a previous CI run on a different branch (typicallymain), restoring it silently resets the Homebrew repository to that branch — discarding the current checkout (e.g. a PR's merge commit).This causes two problems for consumers:
mainsnapshot instead of the PR.brew doctor(viabrew test-bot --only-setup) to fail with:Fix
Save the git
HEADsha before the cache restore and re-checkout it afterwards if the restore changed it. The new steps are a no-op when the cache doesn't affect.git(theifcondition and sha comparison both guard against unnecessary work).Instance of the failure
See this failing build from Homebrew/brew PR #21603 for a specific example.
Replaces the workaround in Homebrew/brew#21604 which applied the same fix inline in
tests.yml.