Mark old dune releases as not compatible with windows

[raphael-proust]

fixes #25483

[mseri]

I agree with @emillon
As a compromise I think we can wait a few days and then merge this

[emillon]

Yes. Here is my original comment:

I don't understand the reasoning. This is something important to fix, but this is not a regression or anything. This is something that has never worked. The old versions of dune are working well for a bunch of windows users. Why force them to upgrade? The solver will pick the new version anyway.

In addition, dune started vendoring spawn in 3.0.0, so the older versions might not be affected.

[dra27]

In #25474 (comment):

Well there was never any Windows users using opam-repository as opam-repository is not currently available on Windows. People currently use forks that are not currently updated.

There are users at the moment who have been advised (for some time) to use ocaml/opam-repository with the sunset branch of ocaml-opam/opam-repository-mingw. So there are users at the moment who have been installing versions of dune after 3.5.0 (the last version in opam-repository-mingw) who may suddenly get CI failures.

I think there's an easy compromise, though - instead of os != "win32", if it's os != "win32" | os-distribution = "cygwinports" then the packages remain installable when using OCaml for Windows. I agree it is a good idea not to have (new) users of opam 2.2.0 hitting this.

I also agree with @emillon that this affects Dune 3.0.0+ and 1.x and 2.x should therefore be left alone.

[raphael-proust]

I've pushed a commit to undo the changes on 1.* and 2.* versions.

@dra27 could you edit the wikipage https://github.com/ocaml/opam-repository/wiki/Depexts-os-distribution---os-family-values adding a table at the end for the different values of os and os-distribution that can appear on windows? Alternatively, if you just reply here with a list of the values and a brief explanation I can make the table.

[dra27]

I can do, though do we want to expand the entire table to include the os value as well? The Cygwin part becomes very confusion here, as it's both an OS in its own right but also an os-distribution for native Windows.

• os-family is windows for both native Windows and Cygwin (i.e. Sys.win32 || Sys.cygwin ⇒ os-family = windows)
• os is win32 for a native Windows build of opam (which includes OCaml for Windows).

For os-distribution, we then have:

Distribution	os	os-distribution	os-family	Notes
Native1 Windows	win32	cygwin	windows	Depexts + Unix tools provided by Cygwin
Native Windows	win32	msys22	windows	Depexts + Unix tools provided by MSYS2
Native Windows	win32	cygwinports	windows	Legacy "OCaml for Windows" (opam 2.0.10 only)
Cygwin	cygwin	cygwin	windows	Cygwin's own opam distribution

Footnotes

1. "Native" here refers to programs able to run outside of Cygwin/MSYS2, principally from the Command Prompt or Windows PowerShell. ↩

2. At the moment, this is "tier 2" from opam's perspective - Cygwin is the primary supported mechanism for 2.2.0, but we're paying a lot of attention to MSYS2 partly because that's what Diskuv uses now and also because we expect we'll likely switch to recommending it across the board in future. ↩

[benmandrew]

I don't know if it's useful for the PR but the CI should finish this sometime Friday morning (CET). It generated an extraordinary number of jobs!

[mseri]

Thanks. I think we can release resources and merge, now one week has passed

[MSoegtropIMC]

@mseri @raphael-proust : this PR breaks Coq Platform CI:

https://github.com/coq/platform/actions/runs/8352401423/job/22862430293#step:5:1683

There are a few 1000 users which have successfully used

dune.3.7.0
dune-configurator.3.7.0

on Windows (cygwin based MinGW cross build) and it is the pinned version of dune in Coq Platform 8.17~2023.08 - which for sure makes good use of dune, so that this is a tough test.

Please undo this change at least for version 3.7.0 asap.

[dra27]

I think there's an easy compromise, though - instead of os != "win32", if it's os != "win32" | os-distribution = "cygwinports" then the packages remain installable when using OCaml for Windows. I agree it is a good idea not to have (new) users of opam 2.2.0 hitting this.

The Coq platform uses OCaml for Windows, and this suggestion would have completely avoided this breakage.

[mseri]

That was my oversight, I thought the PR was modified also to include that (but in any case I had no clue if this would have made a difference for coq or not).

@dra27 I can make the change still for the affected versions, the merge was very recent so it could prevent further breakages. What do you think? The change is ready here: https://github.com/mseri/opam-repository/pull/new/relax-dune

[mseri]

@jonahbeckford is this creating issues with DKML (which uses dune 3.12.1)?
Should I relax the requirements on that dune version, with something like #25529 or you can use the new fixed 3.14.2 version in dkml?

[jonahbeckford]

is this creating issues with DKML (which uses dune 3.12.1)?

Yes. DkML started using unpatched Dune at 3.8.3. Changes to dune in opam-repository can now break DkML users. And the availability conditions os != "win32" | os-distribution = "cygwinports" excludes DkML.

with something like #25529 or you can use the new fixed 3.14.2 version in dkml?

I cannot force people to upgrade to 3.14.2.

From the user perspective: It is a terrible UX and I have no way of contacting the users to notify them.

From the resource perspective: cutting a new version of DkML is very expensive for me ... it takes a week of my time and at least a month of soak testing to ensure the package + versions work well together for the users. That expense will continue until the day that opam CI gates each new package version against MSVC, just like today if an opam package version fails on Linux opam CI that package version is not permitted in the opam repository.

From a technical perspective: The "most important" packages (~100) are pinned in DkML to minimize my support burden, to mitigate the ecosystem wide lack of testing for native Windows, and because I'm opinionated that everything should be versioned by default. This thread is close to convincing me that I should have been versioning the opam repository as well.

So ... can we please roll back soon?

Emergency mitigation: Although I am having serious difficulty figuring out what happened and why ... and I'm concerned by that Discuss #windows-support thread ... if a rollback can't occur on the opam repository than please someone do the equivalent of a git revert PR in https://github.com/diskuv/diskuv-opam-repository/tree/main/packages. After that I can retag it for DkML 2.10. Every DkML switch has diskuv-opam-repository at a higher rank so this would be invisible for users.

[mseri]

@jonahbeckford I have sent a PR on the fly, while we rediscuss this issue

[dra27]

@jonahbeckford - which bit of the #windows-support thread on Discord are you referring to? (sorry, I don't use Discord very frequently, and I'm struggling to find the context).

On the CI front, as I noted in #25536 (comment), I think the lack of CI here (ocaml/opam-repository) is being over-blown for this issue. For good or ill, these packages were intentionally disabled in this PR - CI would simply have told us that they'd been disabled. The error has been one of human logic.

Having already pointed out one error in the logic behind "Well there was never any Windows users using opam-repository as opam-repository is not currently available on Windows.", we now have two. Given that Dune 3.x survived for quite some time before we discovered a user with a non-ASCII character in their user ID, rather than repeatedly patching these packages, I'm rather on the side of completely reverting this. If we get continuing reports of problems, we could re-apply the post-messages filter in #25529 to 3.0.x-3.14.0 incl., but I think it's quite clear that trying to patch these older packages has caused more problems than it's solved.

[emillon]

Given that Dune 3.x survived for quite some time before we discovered a user with a non-ASCII character in their user ID, rather than repeatedly patching these packages, I'm rather on the side of completely reverting this.

Agree. This change breaks dune for users that were unaffected, and affected users would install the latest version by default anyway.

[mseri]

I am not on my laptop until monday, @raphael-proust could you revert this PR and instead add just a message on those dune versions like

post-messages: [
  "Please note that this version of dune does not support unicode characters on Windows and may behave unexpectedly, consider updating to version 3.14.2 or greater" {os = "win32" & os-distribution="cygwinports"}
]

[mseri]

Then we can merge the revert as soon as the linter is green

[emillon]

Sorry to nitpick, but I don't think that "does not support unicode characters on Windows" is an accurate issue of the problem. It crashes when the environment contains something other than ASCII. You could point at ocaml/dune#10180, but really I find post-messages are pretty noisy. We are not listing all the open issues for a given dune release and I don't think that this one is special enough to be listed on every install.

[mseri]

I am fine with rewording it, but it is listed only on windows and only on the possibly affected installations. At least we warn the users, I think it is better than pretending it is fine and already informs them that weird crashes may be solved just by upgrading

[emillon]

I'll let you decide what's best in the context of opam-repository, but to be fair, wouldn't "did you try to upgrade the package" be the first thing to recommend in any support discussion?

[jonahbeckford]

@jonahbeckford I have sent a PR on the fly, while we rediscuss this issue

Thanks for looking into that. Took too long for me to get to it. For now I'm not going to retag because my muscle memory is telling me that opam doesn't always see a change when a tag's commit changes. I could be making the problem worse by retagging.

The pinning of the opam repository that I put on https://discuss.ocaml.org/t/stuck-installing-dune-on-windows/14395/3?u=jbeckford is safer for now.

---

@jonahbeckford - which bit of the #windows-support thread on Discord are you referring to? (sorry, I don't use Discord very frequently, and I'm struggling to find the context).

On the CI front, as I noted in #25536 (comment), I think the lack of CI here (ocaml/opam-repository) is being over-blown for this issue. For good or ill, these packages were intentionally disabled in this PR - CI would simply have told us that they'd been disabled. The error has been one of human logic.

I couldn't follow the context on Discord very well either so let's skip that.

I wasn't aware of the CI issue you mentioned. Stepping back, the human curated opam repository and the resulting quality is amazing IMHO. But I know I'm not prescient, and I would need something to visually break if I was managing thousands of packages and the equivalent of rm -rf /windows is accidentally slipped in. My knee-jerk reaction is to say "CI", but I don't care about the particular mechanism as long as there is one.

[samoht]

I've reverted these changes here: #25609

The goal is to give us more time to find the appropriate fix and unbreak downstream Windows users.

[mseri]

Thanks a lot @samoht!

[MSoegtropIMC]

Comming back on this a bit late (was on vacation): the proposed fix using os != "win32" | os-distribution = "cygwinports" does not work for Coq Platform. Coq Platform uses Cygwin to build things, but this is a MinGW cross build and OCaml and dune are actually MinGW, so os-distribution = "cygwinports" does not apply to dune.

This is a bit or a weird configuration, but it is what evolved over the years and this is what works. In the end we definitely need a MinGW dune, because we create a cygwin free Windows installer and this needs a MinGW dune. One could discuss if we should have an intermediate cygwin MinGW cross dune, but last time I tried to build OCaml for cygwin, the 64 bit version appeared to be broken - and 32 bit cygwin does hardly exist any more.

[dra27]

@MSoegtropIMC - os-distribution refers to opam, which I believe you're using from "OCaml from Windows". I just kicked off a build of Coq Platform "from sources" and opam var os-distribution is giving me cygwinports?

[MSoegtropIMC]

True. But fact is that when I completely remove the restriction from the dune 3.7.0 package (as it was before this PR) Coq Platform 8.17 CI works - with the currently published opam package for dune.3.7.0 it doesn't. I will do a local build and see.

It might be some strange coincidence that something else broke around the time these changes have been done, but it is not that likely because the newer Coq Platform builds (which use newer dune) work fine. As I said, I will test it locally.

[MSoegtropIMC]

To clarify: the Coq Platform 8.17 build does not work any more since I removed my local patch for dune 3.7.0 which restored the version prior to this PR. It did run fine before this PR and during the few days I used a local patched opam file for dune 3.7.0.

[samoht]

@MSoegtropIMC is the Coq platform still broken? I thought I completely reverted this PR with #25609 , but maybe I forgot something (if yes, it was my mistake -- as there were multiple changes happening after that PR had been merged, I had to revert it manually)

[mseri]

I had also double checked the PR as well as Jonah, and all the packages seemed addressed. Looking at it, 3.7.0 was properly reverted to no bound: https://github.com/ocaml/opam-repository/pull/25609/files#diff-ef6a2ce22378da0735d76b3235cafc19525f58c48976804639d5679e884cc584

[MSoegtropIMC]

Sorry, there was some overlap - Coq Platform Windows CI failed until including April 1st with

unmet availability conditions: os != "win32" | os-distribution != "cygwinports"

and started to work again an April 2nd - so it did already work on April 5th when I reported it. I was picking up analysis on a local build I did a few days before ...

So all is well now. Thanks again!