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

[ci] Add cygwin bin directories to PATH #26508

Closed
wants to merge 1 commit into from

Conversation

tobil4sk
Copy link
Contributor

@tobil4sk tobil4sk commented Sep 7, 2024

Without these PATH entries, cygwin's binaries never get used. This means that make and bash and other executables are not used from cygwin, but from other preinstalled packages in the github actions runner (e.g. git bash and the preinstalled mingw).

This causes build issues with certain packages, see:
#26501
aantron/luv#162

@avsm avsm requested a review from dra27 September 7, 2024 11:09
@dra27
Copy link
Member

dra27 commented Sep 7, 2024

Something else is going on here - it shouldn’t be necessary to do this in a package build - I’ll endeavour to have a look tomorrow/next week

@tobil4sk
Copy link
Contributor Author

tobil4sk commented Sep 7, 2024

This seems somewhat related to ocaml/opam#6181. To me the ideal solution seems to be for opam env/opam exec to take care of setting the PATH correctly so that it is not necessary to set the PATH manually.

@tobil4sk
Copy link
Contributor Author

tobil4sk commented Dec 12, 2024

Should mingw-w64-shims also add /usr/x86_64-w64-mingw32/bin to PATH maybe? It already adds /usr/x86_64-w64-mingw32/sys-root/mingw/bin: https://github.com/dra27/mingw-w64-shims/blob/3c4a82700d9b03443ad6f2927449f7a8f25a77cf/mingw-w64-shims.opam#L8

The problem is that the luv library builds a native library using autotools, and the configure script expects an ar executable to be available in PATH. Since /usr/x86_64-w64-mingw32/bin isn't present in PATH, the build fails (both in CI and on local systems where the PATH hasn't been configured like this).

Does adding this to mingw-w64-shims sound like a reasonable idea? If so, I could try to send a patch there instead and close this PR.

EDIT: I've gone and opened: dra27/mingw-w64-shims#3

@tobil4sk
Copy link
Contributor Author

tobil4sk commented Feb 3, 2025

It turns out there was actually a deeper issue behind why the /usr/x86_64-w64-mingw32/bin was needed, and resolving that has removed the need for it.

For /bin, the reason it is needed is because luv's dune file is trying to run bash and assumes it can be found in PATH. As a result, issues occur when it is not present or bash from another software package is found first. Since this is a feature provided by dune, it seems to make sense to handle it there: ocaml/dune#11438.

Closing this because, as mentioned, this isn't the correct place to handle this issue.

@tobil4sk tobil4sk closed this Feb 3, 2025
@tobil4sk tobil4sk deleted the ci-fix-PATH branch February 3, 2025 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants