[libcxx-commits] [libcxx] [llvm] [libcxx] [ci] Test mingw environments with msvcrt.dll, too (PR #115783)

[Martin Storsjö via libcxx-commits]

mstorsjo wrote:

> > It's a legacy configuration indeed, and it's not as functional as UCRT. Many users do move away from it - however I don't see being able to entirely drop it within any foreseeable future either. (Note, my toolchain distribution is "upstream-only", I don't carry any downstream patches on anything, as I make sure to work entirely upstream.)
> 
> That sometimes makes sense, and sometimes it doesn't. I think it makes sense for mainstream configurations to be supported upstream, but folks must also be OK with maintaining a bit of downstream diff for non-mainstream configurations. We do that at Apple as well -- we have many downstream configurations that we don't try to contribute upstream to avoid creating a support burden upstream. It's all about finding a balance.
> 
> I'm not claiming `msvcrt.dll` falls in that category, but this tradeoff is something that does exist.

Sure, that's understandable.

In this case, I do hope that it can be taken into account, that while I'm one toolchain vendor that builds it in this configuration, I believe that there are others that also build it in the same configuration, so keeping it buildable without requiring downstream patches is valuable. And in the case of my toolchain, as a policy, I don't do downstream patches.

> > If we keep it as a build-only CI configuration, it's quite cheap wrt CPU time on the CI. If we want to, we can also add running of tests with it; it requires adding a handful of extra `XFAIL`s but nothing really terrible actually (I have patches set up for that on top of this, if we want to go that way).
> 
> I had actually not noticed that we were only building the runtimes, not testing them. I don't think that's sufficient -- we want to also run the tests, otherwise this clearly isn't worth supporting upstream.

Ok, that's reasonable - and actually running tests obviously is even more valuable. I do have a couple of patches on top of this that makes the tests run in this configuration - it requires a handful of extra `XFAIL`s. I have patches for that lined up as well, that I can post if it makes any difference in judging the situation.

On the other side, if you decide you don't want to take this configuration in, I would still hope that we can keep this build configuration working through refactorings like https://github.com/llvm/llvm-project/pull/115752; cherrypicking this commit into the branch would add the test coverage at least while working on that code while it can be dropped when that's done. (Refactorings like that one is one of the very few cases within libcxx where the distinction of this configuration makes any difference at all.) Alternatively, I can of course also test the PR (when it otherwise seems to work) and let you know what tweaks it may require to keep things working.

https://github.com/llvm/llvm-project/pull/115783