Skip to content

Conversation

@shym
Copy link
Contributor

@shym shym commented Nov 6, 2025

This PR proposes to handle x-ci-accept-failures in Windows CI tests just like in other platforms. (This came up in the context of #28750 where it would have helped silence the expected failures.)

It uses for that the values "msys2" and "cygwin": I wondered whether they should be prefixed by windows (because we use macos-homebrew, but I supposed that it was because homebrew also exists on Linux, IIUC) and/or suffixed by a version (but I didn’t look whether those are versioned).

The second commit is there just to help test the new behaviour and so should be dropped before merging. I hope I covered all the cases with it. In particular it fails intentionally on Cygwin to make sure the failure in one package is not hidden by the other.

(I have little knowledge of powershell syntax so I admit some guesswork has been involved in there…)

shym added 2 commits November 6, 2025 13:01
Handle `"msys2"` and `"cygwin"` values in the `x-ci-accept-failures`
field to ignore installation failures on Windows CI, similarly to what
is done for other platforms
@jmid
Copy link
Member

jmid commented Nov 7, 2025

This is excellent - thanks a ton! 🥳

Thinking a bit a head, one could imagine having a "pure-Cygwin" CI workflow too at some future point? 🤔
At that point, we might regret naming the current one "cygwin" and having added that to a bunch of opam files.
With that in mind, would it make sense to name the two something like "cygwin-mingw" and "msys2-mingw" instead?

Note: I am not married to these name proposals ...and I admit having contributed to the existing workflow's naming mess! 😅

@shym
Copy link
Contributor Author

shym commented Nov 10, 2025

This is excellent

🙏

Thinking a bit a head, one could imagine having a "pure-Cygwin" CI workflow too at some future point? 🤔 At that point, we might regret naming the current one "cygwin" and having added that to a bunch of opam files.

There’s indeed quite some name ambiguities, in particular with Cygwin. And I agree it might be sensible to keep cygwin to stand for the most cygwinish of them (which is when -cygwin is used in the triple; btw, maybe an output of ocamlopt -config would be useful).

With that in mind, would it make sense to name the two something like "cygwin-mingw" and "msys2-mingw" instead?

I thought msys2 was always using mingw but now I see an unprefixed gcc package along with the mingw-prefixed one. Do you know what is this gcc? Do we support it for OCaml?

Once we settle on names, I suppose the best way to use them would be to assign the windows_env variable with them. Or do you think only the x-ci-accept-failures values should be changed?

@jmid
Copy link
Member

jmid commented Nov 10, 2025

I thought msys2 was always using mingw but now I see an unprefixed gcc package along with the mingw-prefixed one. Do you know what is this gcc? Do we support it for OCaml?

I believe https://packages.msys2.org/packages/gcc?variant=x86_64 installs an equivalent of Cygwin's gcc, meaning that executables will depend on msys-2.0.dll, the MSys2-equivalent of cygwin1.dll:
https://stackoverflow.com/questions/37524839/msys2-statically-link-output-binary

That suggests at least two more future CI workflows for 'pure Cygwin' and 'pure MSys2'... 😮
If I am understanding @dra27 correctly #28851 (review) one can also build using MSVC under both Cygwin and MSys2...

Once we settle on names, I suppose the best way to use them would be to assign the windows_env variable with them. Or do you think only the x-ci-accept-failures values should be changed?

I'm complete fine with changing windows_env too - that was the part about me contributing to the naming mess
(insert the old joke about naming is one of the hardest... 😄 )

Polite ping @dra27 and @kit-ty-kate:
How does x-ci-accept-failures: ["cygwin-mingw"] (and "msys2-mingw") sound to you?

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