[libcxx-commits] [PATCH] D140343: [libc++] Update supported system versions

Wed Mar 1 07:56:16 PST 2023

mstorsjo added inline comments.

================
Comment at: libcxx/docs/index.rst:121
+FreeBSD 10+     i386, x86_64, arm         ???-???
+Linux           i386, x86_64, arm, arm64  glibc-2.24+
+Windows         i386, x86_64              MSVCRT-???, MinGW???   Both MSVC and MinGW style environments
----------------
philnik wrote:
> q66 wrote:
> > Mordante wrote:
> > > mstorsjo wrote:
> > > > philnik wrote:
> > > > > thesamesam wrote:
> > > > > > philnik wrote:
> > > > > > > MaskRay wrote:
> > > > > > > > philnik wrote:
> > > > > > > > > This seems to be the oldest glibc version referenced in libc++.
> > > > > > > > CC @dalias @q66 @thesamesam about the supported musl versions.
> > > > > > > We don't have any bots testing musl + libc++, so I wouldn't consider musl to be supported currently. I think it might actually be broken in trunk. Maybe someone can provide a bot to test this configuration?
> > > > > > Does unsupported here mean "we expect help to fix things" or "we'll reject patches" (like we discussed in previous encounters)?
> > > > > > 
> > > > > > I think trunk works fine last time I checked.
> > > > > > 
> > > > > > I'm just not sure what the purpose of this change is with regard to libc. I don't expect the libc++ team to rush to fix musl bugs, just accept patches if we provide them.
> > > > > > 
> > > > > > I'd like to provide a buildbot but I'm not sure which hardware we could do it on right now.
> > > > > IMO it should definitely be "we'll reject patches", since that's the stance we have on any platforms, which a different libc effectively is. I guess I'll have a discussion with Louis about this at some point.
> > > > > As I'm sure I said before, the problem is that we don't know whether the code actually works and we have no way to check. This in turn means that we either don't touch the code, or we break people and then get to know about it a year later. Neither of which is healthy for a project. Another problem is that we don't know whether anybody is still interested in a platform, which means we might drag code around that nobody actually cares about (we removed a lot of that since switching to a stricter compiler and platform support policy).
> > > > > 
> > > > > If you want to provide a builkite bot we're happy to help with setting things up.
> > > > I wholeheartedly disagree that the policy should be "we'll reject patches" for anything outside of this list. There should clearly be more nuance than that to it.
> > > > 
> > > > For example - currently the CI covers some OSes on some architectures. For Linux, this is 4 specific architectures right now. We clearly shouldn't go ahead and add an `#if defined(__linux__) && !(defined(__i386__) || defined(__x86_64__) || defined(__arm__) || defined(__aarch64__)) #error Unsupported configuration, not allowed to use libc++ there!`. I'm sure lots of Linux distros regularly build and use libc++ on other architectures than that. Do we guarantee that it works? No. Does it work in practice? Most probably.
> > > > 
> > > > E.g. for Windows on ARM64, I do build latest libc++ nightly, and run some amounts of tests with it (actual usage tests, not the libc++ testsuite itself - but I do test that one occasionally too), so it's not entirely broken. And if that gets broken, I'll let you know within 24h. It's not worthwhile to add an `#error unsupported` for such configurations, I hope you agree?
> > > > 
> > > > I totally agree that it's problematic to maintain the code when we don't know if it works or not, and if such code gets in the way for refactorings without a reasonable way forward, removing it might be the only option.
> > > > 
> > > > I get that there's a difference between untested variants of support for otherwise supported OSes already in the tree, and adding new code for entirely different OSes. But if that code mainly provides a different OS/libc specific handling of something that is already customized for all targets, I don't see a big issue with it.
> > > > 
> > > > There's a whole range of potential configurations inbetween, with various level of support. For new configurations, maybe aspiring to reach a level where it's included here with CI coverage in the future, it's hard to get there if any patches whatsoever are rejected unless the CI setup is there to begin with.
> > > > 
> > > > I.e., don't let perfect be the enemy of good. If it can provide value to users without causing maintainance harm, I don't think it needs to be rejected out of principle. If it causes maintainance issues, it should of course be reconsidered.
> > > > IMO it should definitely be "we'll reject patches", since that's the stance we have on any platforms, which a different libc effectively is. I guess I'll have a discussion with Louis about this at some point.
> > > 
> > > I'm not aware we (libc++) have that policy. I think it would be good for us to discuss that. Of course without CI it's not always possible to verify the patch fixes what it should, but we can verify it doesn't break an official supported platform.
> > > 
> > > In the past I have accepted patches for platforms we don't officially support. However when we don't officially support a platform, I will not consider that platform when working on patches. For example for formatting I need to support locales. These are far from portable, abbreviations of the day/month in the French locale looks different on MacOS and Linux. The CI will tell me what I need to adjust to fix all supported platforms.
> > > 
> > > I still prefer a CI. For officially supporting a platform I see that as required.
> > > 
> > > 
> > > I still prefer a CI. For officially supporting a platform I see that as required.
> > > 
> > 
> > of course, a CI is always better, and it would be great to have one for musl, however, providing a bot is something small projects can't always afford to do (for instance I'm a maintainer of a Linux distribution that extensively relies on LLVM as its system toolchain, including libc++, and I really have enough pain thrown in my way as things are, and such policy would make me waste even more time on just proving the project's right to exist rather than working on anything more useful)
> > 
> > therefore i would greatly appreciate if the status quo was preserved; right now libc++ works great for me, and I am fine with providing patches for whenever something breaks, but please don't make me maintain separate support downstream, i really have enough to do already :)
> > I wholeheartedly disagree that the policy should be "we'll reject patches" for anything outside of this list. There should clearly be more nuance than that to it.
> > 
> > For example - currently the CI covers some OSes on some architectures. For Linux, this is 4 specific architectures right now. We clearly shouldn't go ahead and add an `#if defined(__linux__) && !(defined(__i386__) || defined(__x86_64__) || defined(__arm__) || defined(__aarch64__)) #error Unsupported configuration, not allowed to use libc++ there!`. I'm sure lots of Linux distros regularly build and use libc++ on other architectures than that. Do we guarantee that it works? No. Does it work in practice? Most probably.
> > 
> > E.g. for Windows on ARM64, I do build latest libc++ nightly, and run some amounts of tests with it (actual usage tests, not the libc++ testsuite itself - but I do test that one occasionally too), so it's not entirely broken. And if that gets broken, I'll let you know within 24h. It's not worthwhile to add an `#error unsupported` for such configurations, I hope you agree?
> I didn't want to say that we should go out of our way to break people. If it came across that way I'm sorry.
> > I totally agree that it's problematic to maintain the code when we don't know if it works or not, and if such code gets in the way for refactorings without a reasonable way forward, removing it might be the only option.
> > 
> > I get that there's a difference between untested variants of support for otherwise supported OSes already in the tree, and adding new code for entirely different OSes. But if that code mainly provides a different OS/libc specific handling of something that is already customized for all targets, I don't see a big issue with it.
> > 
> > There's a whole range of potential configurations inbetween, with various level of support. For new configurations, maybe aspiring to reach a level where it's included here with CI coverage in the future, it's hard to get there if any patches whatsoever are rejected unless the CI setup is there to begin with.
> As I understand it, we accept patches if someone is working on getting a bot up. For example, there were quite a few patches landed for FreeBSD, without a bot currently there. The important part is that there is an effort to get us a bot.
> > I.e., don't let perfect be the enemy of good. If it can provide value to users without causing maintainance harm, I don't think it needs to be rejected out of principle. If it causes maintainance issues, it should of course be reconsidered.
> That is the point where we are right now IMO. I'm not confident to refactor any of the `locale` stuff because we have lots of code that is basically not tested. For musl we even have CMake stuff.
> 
> To be clear, I'd be more than happy to have a minimal bot for musl where everything other than locale support is disabled. That would resolve any maintainability problems.
> 
> > > 
> > > I still prefer a CI. For officially supporting a platform I see that as required.
> > > 
> > 
> > of course, a CI is always better, and it would be great to have one for musl, however, providing a bot is something small projects can't always afford to do (for instance I'm a maintainer of a Linux distribution that extensively relies on LLVM as its system toolchain, including libc++, and I really have enough pain thrown in my way as things are, and such policy would make me waste even more time on just proving the project's right to exist rather than working on anything more useful)
> > 
> > therefore i would greatly appreciate if the status quo was preserved; right now libc++ works great for me, and I am fine with providing patches for whenever something breaks, but please don't make me maintain separate support downstream, i really have enough to do already :)
> 
> I get that it seems easier to have the code upstream, but I'm not sure it actually is. I don't think for you there is a significant difference. Currently, you update libc++ at some point, and might see that something broke. Then you have to create a patch to fix it and upstream that fix. With a downsteam fork, you instead update libc++ at some point and if something broke you fix it downstream. That would result in duplicated work, but that could be resolved by a semi-official musl fork. Then we don't have the problem of having to keep dead code around, but people who want libc++ to work in some specific configuration can still get it without too much work (assuming someone already did the work).
Thanks for the follow up - I guess our views aren’t that far apart after all :-)

Wrt refactoring, anything not covered by CI is quite expressly not supported, so you can essentially disregard it entirely. If it does break, an active downstream will send a patch within a week. If downstream doesn’t follow up, it is of course ripe to remove after some time.

I think there’s still a lot of value to have unofficial ports working in the upstream repo - if it works 90% of the time (especially around releases) it is tremendously useful already, compared to an outright downstream fork.

Plus it gives upstream visibility into what the unofficial ports do.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140343/new/

https://reviews.llvm.org/D140343