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

Please come up with an alternative to 'available: os != "win32"' for Windows packages failing CI #26089

Open
jonahbeckford opened this issue Jun 14, 2024 · 2 comments

Comments

@jonahbeckford
Copy link
Contributor

Regarding @kit-ty-kate 's https://discuss.ocaml.org/t/ann-your-opam-repository-prs-are-now-tested-on-windows/14781 post.

TLDR: Using available: os != "win32" disables a package for all Windows users, including existing DkML users. Since the Windows CI using GitHub Actions does not test DkML, it is inevitable that a package that works on DkML but fails Windows CI will be disabled (which disables it on DkML).

It is impossible to know in advance what will break. Of course, there will be large differences in the support for C dependencies, MSVC/GCC and Unix shellisms between DkML and opam 2.2 so those packages will be first to break ... an inexhaustive start would be the transitive dependencies of the package overlays at https://github.com/diskuv/diskuv-opam-repository/tree/main.

The Windows CI at https://github.com/ocaml/opam-repository/blob/master/.github/workflows/windows.yml looks like it is testing only GCC. So, even without considering DkML, the broad hammer of os != "win32" really means disable all Windows users who don't compile with GCC. That can't be right.

The way forward is simple ... only disable the systems that were tested.

Two practical implementations of that:

  1. Test DkML with https://github.com/diskuv/dkml-workflows ... it is straightforward to test DkML in CI.
  2. (Better) Use something more fine-grained than os != win32. I can't remember how to express it (I thought @dra27 had a thread how to do it) ... but add in selectors for gcc versus msvc, and Cygwin versus MSYS2.
@dra27
Copy link
Member

dra27 commented Jun 22, 2024

Yes, indeed; the mechanisms were listed in #25861:

  • available: os != "win32" is the sledgehammer: no Windows support at all
  • conflicts: "host-system-msvc": this package works with the mingw-w64 ports, but doesn't work with the Visual Studio ports
  • conflicts: "host-arch-x86_32": this package doesn't work on 32-bit Intel
  • depends: "host-system-mingw": this package only works with the mingw-w64 ports.

I expect that conflicting host-system-mingw is the least dispruptive thing to do at the moment, and that also reflects what CI is actually testing here.

Another thing we could try, unless it becomes too disruptive, is that existing packages should not be constrained away from Windows in other PRs - i.e. let's open an issue or a separate PR first where those concerned can be pinged both to triage and ack the change?

@mseri
Copy link
Member

mseri commented Jun 25, 2024

Another thing we could try, unless it becomes too disruptive, is that existing packages should not be constrained away from Windows in other PRs - i.e. let's open an issue or a separate PR first where those concerned can be pinged both to triage and ack the change?

I like this idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants