Add package caching #6
Conversation
To speed up Cygwin installations, allow users to store the downloaded packages using GitHub's caching, rather than needing the packages to be downloaded from the Cygwin mirrors on every run. This speeds up the installation when there's a good cache hit, at the expense of adding some additional complexity, and using the limited cache space (although with 10GB assigned to every repository on GitHub, I can't imagine most folk are getting close to the limit!). To avoid unnecessary cache churn, the new steps check if the cache has actually meaningfully changed before storing a new cache. This uses b2sum for speed, since we're just checking for unexpected changes in the cache, and we don't need cryptographic security.
README.md
Outdated
| | site | http://mirrors.kernel.org/sourceware/cygwin/ | Mirror site to install from | ||
| | check-sig | true | Whether to check the setup.ini signature | ||
| | add-to-path | true | Whether to add Cygwin's `/bin` directory to the system `PATH` | ||
| | package-cache-key | '' | The key to use for caching downloaded packages. |
There was a problem hiding this comment.
What's the reasoning behind making this a string, rather than a boolean? I think that the cache-keys are scoped to the repository, so the only possible collision is with other cache action uses in the same workflow?
It would be nice to be able to make this on by default in future, without the user having to choose a key.
There was a problem hiding this comment.
The store of caches is per-repository, and the actual restore behaviour is scoped even more tightly than that, with restrictions about what caches are allowed to be restored on what branch. But yes, this is something I debated; I went with letting the user decide mostly so they wouldn't be surprised by caches appearing with unexpected names.
Having been playing around with this a bit more, though, I think I'm going to suggest a different approach: hard-code the cache key name, but make the behaviour configurable with a cache-behaviour setting or similar, that takes values of disabled (get current cache-free behaviour), enabled (full new behaviour), or saveonly or restoreonly to do the obvious things. The latter are useful when – as with a lot of my cygport jobs – you need to run the action multiple times in the repository.
(As we discussed in #5, the ideal solution might be to swap from a composite action to a JS one, since that would allow post-run steps, which could be responsible for saving the cache after it has been used several times, but that's obviously a much bigger job!)
|
Thanks! This is awesome! I vagely recall trying to do this initially but running into some problem, so thanks for adding this!
Some numbers showing that restoring the cache is actually typically faster than fetching the packages again would help give me confidence that this is a good change.
Unfortunately, setup allows it's package cache to grow without bound. Something which has needed fixing for literal decades... I don't think that's blocking for this PR, since, yes, the likelhood of hitting the cache limit is small, but something you should be aware of.
Presumably there's some analysis been done that all the commands used are safe because they are in 'base' installation, but a comment to that effect might be worthwhile. |
np! This is about my fifth attempt at getting it working without getting stuck and giving up in disgust…
Yep, very fair. I have some rough anecdata that says it helps, but I've not really stress tested it to prove it makes a difference for installations that are big enough for it to be substantial. I should be able to have a go at that shortly :)
I had been thinking about that. GitHub evicts caches that aren't used for 7+ days, but if a repo were getting rebuilds more regularly than that, the cache would just keep getting larger and larger. I think that's a good reason for it to not be default behaviour for this action (yet), and I should probably note it explicitly in the documentation, but I think it mostly means that anyone using this action might need to manually flush the cache every so often, to force a new smaller cache to be generated.
They're all provided as part of the Git Bash / Git for Windows installation on the GitHub Windows runners. I've not actually checked how this works for self-hosted runners, and I could believe it's not guaranteed to work there. I'll add some notes to that effect… |
Add an option to restore a cache without creating a new one, or to create a cache without restoring an old one. This is useful in the scenario where the action is called multiple times in a run, as it allows the cache to only be restored on the first call, and for a new cache to only be written on the last call. This option replaces the `package-cache-key` option; as discussed in cygwin#6 that option isn't very useful. Additionally, add some more scopes to the cache name. Without this change, a second attempt to create a cache in the same action run (e.g. because one step installs some packages, then another step installs some more) will fail because of the cache name collision. Also update the README and tests, although the tests are only getting a very quick update for this function, as I'm going to do a much more comprehensive test update separately.
me-and
force-pushed
the
cache
branch
3 times, most recently
from
c9be7a5 to
79832e3
Compare
This should hopefully show how much of a difference, if any, caching packages makes. There's no automated comparison here, the test just runs one install without a cache, then a second install with a cache, then a human needs to look at what difference it makes.
Add an option to restore a cache without creating a new one, or to create a cache without restoring an old one. This is useful in the scenario where the action is called multiple times in a run, as it allows the cache to only be restored on the first call, and for a new cache to only be written on the last call. This option replaces the `package-cache-key` option; as discussed in cygwin#6 that option isn't very useful. Additionally, add some more scopes to the cache name. Without this change, a second attempt to create a cache in the same action run (e.g. because one step installs some packages, then another step installs some more) will fail because of the cache name collision. Also update the README and tests, although the tests are only getting a very quick update for this function, as I'm going to do a much more comprehensive test update separately.
Add an option to restore a cache without creating a new one, or to create a cache without restoring an old one. This is useful in the scenario where the action is called multiple times in a run, as it allows the cache to only be restored on the first call, and for a new cache to only be written on the last call. This option replaces the `package-cache-key` option; as discussed in cygwin#6 that option isn't very useful. Additionally, add some more scopes to the cache name. Without this change, a second attempt to create a cache in the same action run (e.g. because one step installs some packages, then another step installs some more) will fail because of the cache name collision. Also update the README and tests, although the tests are only getting a very quick update for this function, as I'm going to do a much more comprehensive test update separately.
Add an option to restore a cache without creating a new one, or to create a cache without restoring an old one. This is useful in the scenario where the action is called multiple times in a run, as it allows the cache to only be restored on the first call, and for a new cache to only be written on the last call. This option replaces the `package-cache-key` option; as discussed in cygwin#6 that option isn't very useful. Additionally, add some more scopes to the cache name. Without this change, a second attempt to create a cache in the same action run (e.g. because one step installs some packages, then another step installs some more) will fail because of the cache name collision. Also update the README and tests, although the tests are only getting a very quick update for this function, as I'm going to do a much more comprehensive test update separately.
Add an option to restore a cache without creating a new one, or to create a cache without restoring an old one. This is useful in the scenario where the action is called multiple times in a run, as it allows the cache to only be restored on the first call, and for a new cache to only be written on the last call. This option replaces the `package-cache-key` option; as discussed in cygwin#6 that option isn't very useful. Additionally, add some more scopes to the cache name. Without this change, a second attempt to create a cache in the same action run (e.g. because one step installs some packages, then another step installs some more) will fail because of the cache name collision. Also update the README and tests, although the tests are only getting a very quick update for this function, as I'm going to do a much more comprehensive test update separately.
Aha! I hadn't noticed that all this happens before we modify that PATH, so it'll never be using cygwin tools to do these things, it's assuming they are already provided by something in the VM image. |
|
On the performance topic: initial numbers seem to show using a cache saves very roughly 4 minutes off a Cygwin installation that would otherwise take about 10 minutes in total. Restoring a cache of a few hundred MB seems to take only a few seconds, which clearly beats the download time from the Cygwin mirrors. Currently the hold-up is the reason I raised #5: if Cygwin is in the PATH, the caching code will still try to use the Git for Windows |
I could live with "if you want to use this action multiple time in the same workflow, you must use package-cache: disabled or add-path: false"? |
|
I'm hoping it might be something that can get fixed fairly soon; it looks like the problem was added relatively recently, in actions/cache@9b0be58822, and based on actions/toolkit#1311 I'm not the only person to be seeing similar problems. Another option – which I've not tested properly but is next on my idea list – is to try just using the parent of that commit in actions/cache. I created a simple test case to demonstrate the issue at https://github.com/me-and/repro-cygwin-cache-woe, and it looks like the parent commit might let the caching work as currently designed, but without this issue. Otherwise, I agree: just documenting it as a limitation is probably going to be the best option for now! |
| ------- | ||
|
|
||
| If you're likely to do regular builds, you might want to store the packages | ||
| locally rather than needing to download them from the Cygwin mirrors on every |
There was a problem hiding this comment.
I'm not sure "locally" is the best word to use here.
|
FYI: I came across a js version of cygwin-install-action at https://github.com/secondlife/setup-cygwin and it includes a feature for caching. I have not looked further yet. @jon-turney @me-and I also found a 3-year old (unmaintained?) cygwin-install-action repo with you two as contributors: |
I don't know about you, but I love duplicated effort.
I don't know what actions-marketplace-validations is, but their org page https://github.com/actions-marketplace-validations says it's some kind of archive and "do not use anything here", so idk what I can do about that... |
While I don't know if you can see the other repo, you should have access to see some metrics about how much people use cygwin-install-action: |
|
I came across this while thinking about adding caching to my own workflows using actions/cache. I was going to ask for 2 things:
|
|
@jeremyd2019 Somewhat related data point: I found that for my use, I use actions/cache for the cygwin installation and set an expiration of 1 month on the cache. For my use, that's sufficient to keep the installation under control, and I don't have to cache the package cache directory. If I ran my CI more frequently or on larger package (llvm), I might consider adding ccache, too. It works for me to have a reasonable time frame for cache expiration, combined with the ability to manually modify the https://github.com/lighttpd/lighttpd1.4/blob/master/.github/workflows/ci.yml#L235 Edit: my CI also uses |
|
wow, I would not expect using actions/cache directly on the cygwin install would work - it just zips files, so you'd lose acls/permissions and symlinks. I could probably just add |
As I wrote above: "Unfortunately, setup allows it's package cache to grow without bound. Something which has needed fixing for literal decades..." :sad_face: |
|
I implemented caching in my workflow today, unfortunately the github expression function jeremyd2019/cygwin-rust-bootstrap@74a16fd Before the cygwin-install-action: - uses: actions/cache/restore@v4
with:
key: cygwin-packages-
restore-keys: |
cygwin-packages-
path: cygwin-packages
- run: IF EXIST cygwin-packages move /y cygwin-packages D:\
shell: cmdAfter the cygwin-install-action: - name: prepare cache
run: |
if (Test-Path `${{ steps.cygwin.outputs.package-cache }}\file`* ) {
rm -r -fo `${{ steps.cygwin.outputs.package-cache }}\file`*
}
move "${{ steps.cygwin.outputs.package-cache }}" .
shell: pwsh
- uses: actions/cache/save@v4
with:
key: cygwin-packages-${{ hashFiles('cygwin-packages/**', '!cygwin-packages/**/setup.ini') }}
path: cygwin-packages
- name: post cache
run: move /Y cygwin-packages "${{ steps.cygwin.outputs.package-cache }}"
shell: cmdI had to resort to pwsh because I was getting errors with cmd and After looking at what this PR does, I'm thinking now I should not include setup.ini in the hash, but insetad other files. |
|
I made a wrapper composite action around this one, adding caching the way I was thinking about. I don't know if that'd be helpful to anyone, or make it easier to consider. https://github.com/jeremyd2019/caching-cygwin-install-action/blob/master/action.yml |
I added a simple cache cleaning mechanism to my wrapper action. It compares a list of files "Extracted from" from /var/log/setup.log and removes any files from the package cache not named setup.ini and not in that list. Based on the assumption that the action will be making a fresh Cygwin install on each run, this should remove any packages not actually needed for that install. |
To speed up Cygwin installations, allow users to store the downloaded packages using GitHub's caching, rather than needing the packages to be downloaded from the Cygwin mirrors on every run.
This speeds up the installation when there's a good cache hit, at the expense of adding some additional complexity, and using the limited cache space (although with 10GB assigned to every repository on GitHub, I can't imagine most folk are getting close to the limit!).
To avoid unnecessary cache churn, the new steps check if the cache has actually meaningfully changed before storing a new cache. This uses b2sum for speed, since we're just checking for unexpected changes in the cache, and we don't need cryptographic security.
I was originally working on this for my own builds, e.g. at https://github.com/me-and/Cygwin-inih/blob/fb7534276feeb2a5217df5013c679a370ee6013e/.github/workflows/ci.yml, but it occurred to me that this might be more useful here than in my own repos.