-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add package caching #6
Draft
me-and
wants to merge
4
commits into
cygwin:master
Choose a base branch
from
me-and:cache
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ Parameters | |
| 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. | ||
|
||
Line endings | ||
------------ | ||
|
@@ -85,6 +86,22 @@ those executables directly in a `run:` in your workflow. Execute them via | |
Alternatively, putting e.g. `CYGWIN=winsymlinks:native` into the workflow's | ||
environment works, since setup now honours that. | ||
|
||
Caching | ||
------- | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure "locally" is the best word to use here. |
||
build. Set `package-cache-key` to some string (e.g. `cygwin-package-cache`), | ||
and the action will use [GitHub's dependency caching][0] to store downloaded | ||
package files between runs. | ||
|
||
[0]: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows | ||
|
||
This has the effect of speeding up the run of the installation itself, at the | ||
expense of taking slightly longer before and after the installation to check | ||
and potentially update the cache. The installer will still check for updated | ||
packages, and will download new packages if the cached ones are out of date | ||
|
||
Mirrors and signatures | ||
---------------------- | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 ofdisabled
(get current cache-free behaviour),enabled
(full new behaviour), orsaveonly
orrestoreonly
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!)