This PR disable cache save for pull requests in GitHub Actions.
Otherwise, multiple pull requests fill GitHub Actions cache quota shortly.
See a discussion here.
NOTE for the maintainers with “owner” permissions.
This PR needs the actions/cache/restore@* and actions/cache/save@* acrions to be explicitly allowed in the repository’s Actions permissions.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | MarcoFalke |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
68+ - name: Save Ccache cache
69+ uses: actions/cache/save@v3
70+ if: github.event_name != 'pull_request'
71+ with:
72+ path: ${{ env.CCACHE_DIR }}
73+ key: ${{ github.job }}-ccache-${{ github.run_id }}
Shouldn’t the key be just the task name? There should be no reason to bloat the cache directory with entries that won’t ever be used again.
0 key: ${{ github.job }}-ccache-${{ github.run_id }} # https://github.com/actions/cache/blob/main/tips-and-workarounds.md#update-a-cache
58 with:
59 path: ${{ env.CCACHE_DIR }}
60- key: ${{ github.job }}-ccache-cache-${{ github.run_id }}
61- restore-keys: ${{ github.job }}-ccache-cache
62+ key: ${{ github.job }}-ccache-${{ github.run_id }}
63+ restore-keys: ${{ github.job }}-ccache-
… is there a reason to use different values?
Yes, it is. github.run_id is unique for every run. That is why the restore-keys input contains prefixes that are expected to match. The latest matched cache will be restored.
65 - name: CI script
66 run: ./ci/test_run_all.sh
67+
68+ - name: Save Ccache cache
69+ uses: actions/cache/save@v3
70+ if: github.event_name != 'pull_request'
master, to make sure that storage is constant, even if there was an intermittent push to a backport branch?
In the main repository, caches for backport branches will be evicted after 7 days of non-using them.
However, the current logic seems useful for fork repos, no?
I was thinking that (assuming a cache size per push of $n GB, and a total limit of 10GB), 10/n pushes to a backport branch will evict the main cache. For example, for n=5, 2 pushes are enough.
Though, it is only a cache, so I guess it doesn’t matter.
However, the current logic seems useful for fork repos, no?
If you are worried about hard-coding master, I think, if possible, using the GitHub default branch name should achieve the same.
However, the current logic seems useful for fork repos, no?
I mean, topic branches in contributors’ fork repos.
I mean, topic branches in contributors’ fork repos.
The cache key isn’t included the branch name, so I don’t think this will work.
push event. So the updated push will reuse the cache.
Can anyone with a fork clobber our cache?
No.
Can someone elaborate on why branches in forks of this repository are something we need to consider when deciding on our caching stratergy for this repository?
I use CI in my personal repo before pushing my branch into the pull request. And I want to use CI with some efficiency.
On a contributor’s topic branch in their personal repo, the workflow will be triggered by a
pushevent. So the updated push will reuse the cache.
No? I think it will use the latest cache, which may be from a different branch, because the cache key isn’t included the branch name, so I don’t think this will work.
Edit: Maybe it does: https://github.com/actions/cache/blob/main/tips-and-workarounds.md#force-deletion-of-caches-overriding-default-cache-eviction-policy
On a contributor’s topic branch in their personal repo, the workflow will be triggered by a
pushevent. So the updated push will reuse the cache.No? I think it will use the latest cache, which may be from a different branch, because the cache key isn’t included the branch name, so I don’t think this will work.
https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#example-using-multiple-restore-keys: a feature branch is considered before the default branch.
master branch, maybe that is fine?
I guess as an alternative, the ghcr can be used to store a ccache? This would require a bit more code on our side, and maybe a few more permissions to be enabled?
For now, only two jobs, namely native macOS and Windows, are planned to run on GitHub Actions. It should work fine at this scale, no?
Though, if the write-permissions are limited to the
masterbranch, maybe that is fine?
I’m not sure about how to make “the write-permissions are limited to the master branch”. I’ll do my research though.
the ghcr can be used to store a ccache?
Anyway, it won’t work with non-Linux runners out-of-the-box.
Looks like CI is red, but it is not shown here?
Looks like CI is red, but it is not shown here?
I’m kindly asking you to update Actions permissions according to this PR description.
I’m kindly asking you to update Actions permissions according to this PR description.
Should be available now.
Otherwise, multiple pull requests fill GitHub Actions cache quota
shortly.