-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
CI: install cygwin to D:/cygwin for better installation performance #14425
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
Conversation
Comparing the last few runs shows:
so this saves about ~5min @jon-turney fyi If the shell wrapper is not wanted I can just replace the path everywhere instead, ymmv. Or I could just use |
.github/workflows/cygwin.yml
Outdated
with: | ||
# should use 'pip3 cache dir' to discover this path | ||
path: C:\cygwin\home\runneradmin\.cache\pip | ||
path: ${{ matrix.INSTALL_DIR }}\home\runneradmin\.cache\pip |
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.
Looking at the comment above this line, this was intended to connect to the pip existing cache location as indicated by pip3 cache dir
command.
Also using ${{ github.workspace }}
should always be preferable, instead of hard-coding D:\ drive which inevitably will break one day.
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.
are you suggesting I move the cygwin install action in front of the cache restore, run pip cach dir, store it as an output and re-use it here?
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.
I would suggest using PIP_CACHE_DIR
and set it to ${{ github.workspace }}/.cache/pip
. Or use pip --cache-dir
because I think cygwin shell doesn't inherit environment .
As for cygwin installation path, I would think it should be fixed upstream. But here I would suggest using install-dir: ${{ github.workspace }}/cygwin
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.
cygwin install action in front of the cache restore, run pip cache dir, store it as an output and re-use it here?
Yeah, that's probably what it should do, but the voodoo to capture the output from a step escaped me at the time I wrote it...
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.
As for cygwin installation path, I would think it should be fixed upstream
Coincidentally, someone is working on this change cygwin/cygwin-install-action#27
.github/workflows/cygwin.yml
Outdated
vala | ||
zlib-devel | ||
- name: Create cygwin shell wrapper and add to PATH |
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.
If I'm reading this correctly, this is just for the purposes of putting the path and options which we use to invoke bash in several steps in one place.
So a comment to that effect around here would be nice?
I've (finally got around to) tagging Seems that you still need the shell wrapper as apparently you can't use an action's output in |
FYI: The D: drive will soon no longer be available on Windows 2025 images. actions/runner-images#12416 |
Should this be closed then? |
The D drive (which is where the GH workspace is by default) is faster compared to the C drive, and extracting packages there is a few minutes faster. Use a drive relative path, so the installation path is always on the same drive as the workspace (D: now), assuming that the workspace is always on the good drive. GH will drop D: shortly, see actions/runner-images#12416. This will still work then, but if it turns out to be slower we could switch back to 2022 which is still supported for 3 years.
ab25b75
to
1124609
Compare
Here is a simpler version, using the fact that the action working directory is always on the "best" drive, which I have learned in the meantime. If 2025 turns out to be slower after the D: change we could still move to 2022 (supported for 3 more years), since cygwin is mostly self contained and shouldn't care. Feel free to close, or give it another look :) |
OK, something can't deal with drive relative paths here, so I give up. |
@lazka: Have you though about using |
The D drive (which is where the GH workspace is by default) is faster compared to the C drive, and extracting packages there is a few minutes faster.
Since the installation location is used in several places, try to specify it in one place as a matrix variable. Since variables can't be used in "shell:" create a cygwin wrapper script and add it to PATH. This also removes the duplicated shell command lines while at it.