Skip to content
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

Allow keeping Cygwin out of PATH #5

Merged
merged 1 commit into from
Dec 29, 2022
Merged

Conversation

me-and
Copy link
Contributor

@me-and me-and commented Dec 24, 2022

In some circumstances, it might be useful to have Cygwin installed, but not have its executables take precedence over pre-installed packages. Add an "add-to-path" option that, if set to "false", will skip the step of adding Cygwin's /bin directory to PATH.

Also update documentation to mention this option, and update the test scripts to check it works as expected; the default behaviour is tested as part of the "install-dir-test", and the new behaviour as part of the main test.

My specific use case is with actions/cache: the only way I've been able to get that to work is by making sure that Cygwin's binaries aren't in the system path. Without that, I get the following error when the action attempts to store something in the cache:

Post job cleanup.
"C:\Program Files\Git\usr\bin\tar.exe" --posix -cf cache.tgz --exclude cache.tgz -P -C D:/a/Cygwin-GitHub-Actions-playground/Cygwin-GitHub-Actions-playground --files-from manifest.txt --force-local -z
/usr/bin/tar: cache.tgz: Cannot write: Broken pipe
/usr/bin/tar: Child returned status 1[2](https://github.com/me-and/Cygwin-GitHub-Actions-playground/actions/runs/3771792811/jobs/6412370832#step:19:2)7
/usr/bin/tar: Error is not recoverable: exiting now
Warning: Failed to save: "C:\Program failed with error: The process 'C:\Program Files\Git\usr\bin\tar.exe' failed with exit code 2

I'm sure there's a more "correct" solution here, but providing the option of not putting Cygwin in the PATH seems like it could be useful more generally, and I've confirmed it solves my immediate problem.

In some circumstances, it might be useful to have Cygwin installed, but
not have its executables take precedence over pre-installed packages.
Add an "add-to-path" option that, if set to "false", will skip the step
of adding Cygwin's /bin directory to PATH.

Also update documentation to mention this option, and update the test
scripts to check it works as expected; the default behaviour is tested
as part of the "install-dir-test", and the new behaviour as part of the
main test.
@jon-turney jon-turney merged commit db47559 into cygwin:master Dec 29, 2022
@jon-turney
Copy link
Member

I'm sure there's a more "correct" solution here,

Yes. This also seems to cause problems with actions/checkout@v3's post-run step when cygwin git is in the path. The workaround I ended up with just doing was 'rm /usr/bin/git' before post-run steps started.

If we could register a post-run step to unwind the path change, that would be best, but I'm not sure if that can be done at all (only adding to the path seems allowed), and it seems that composite actions don't support post-run steps, so the whole thing would have to be re-written as a JS or docker action.

Since this seems generally useful anyhow, I merged it :)

@me-and me-and deleted the preserve-path branch January 9, 2023 22:38
@me-and me-and mentioned this pull request Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants