TL;DR: add a buildbot builder to test a combination of LLVM & picolibc
Currently there’s newlib-specific code scattered around the libcxx sources. However as I understand it there are no automated checks to ensure the code continues to work.
A potential synergy is that I’m interested in getting LLVM Embedded Toolchain for Arm [1] testing into buildbot, similar to Fuchsia. LLVM Embedded Toolchain for Arm is essentially LLVM + picolibc. Picolibc is derived from Newlib. LLVM Embedded Toolchain for Arm is still fairly young but it’s starting to prove useful to parts of the LLVM community [2].
I believe having LLVM Embedded Toolchain for Arm tested in buildbot would give greater certainty that the newlib-specific parts of libcxx continue to work.
I propose that initally a builder would be attached to the staging buildmaster. This would run for a few months to gather information on what kinds of issues it picks up. If it picks up issues that would be of interest to LLVM contributors then I’d raise another RFC to move it to main. If not then the builder could remain on staging indefinitely, which is permitted according to [3]. The hardware to run the builder would be provided by Linaro.
I’ve personally found the BuildKite pre-commit checks to be highly beneficial so I’d like to explore adding checks there as well, but that’s out of scope for this proposal.
newlib/xlocale.h contains newlib-specific code. I recently fixed it to compile in the LIBCXX_ENABLE_WIDE_CHARACTERS=FALSE configuration.
An option is set in fstream - this seems to work but interestingly git blame shows the last change to it was titled “Move checks for newlib to actually work” suggesting it was broken for some time before that.
What I’ve seen suggests to me that the newlib-specific code does tend to get accidentally broken from time to time, but that there are users who care enough to contribute fixes.
I’ve compared the proposal to the Fuchsia builder. However there is a key difference: the Fuchsia builder downloads the Fuchsia SDK (including the C library) whereas for LLVM Embedded Toolchain for Arm the C library is built from source. This difference means that unlike Fuchsia we can’t just use llvm/CMakeLists.txt with a special cache file.
The LLVM Embedded Toolchain for Arm build process is essentially:
Build LLVM tools like clang
Build picolibc
Build runtimes
It sounds easy, but there’s too much detail in there to ask developers to do the steps manually. Therefore LLVM Embedded Toolchain for Arm provides a CMakeLists.txt that wraps up the steps, while still making the usual LLVM build targets like check-all available as usual. The proposed change is to use this new CMakeLists.txt in buildbot. I believe this will make it easier for developers to reproduce any issues that the builder identifies. But if there’s a more LLVM-native approach then I’m open to it.
Thanks for working on this, I also recently tried to use picolibc with libc++ and ran into various issues. I had some local patches that I’ve now submitted to Phabricator. It would be really nice to have a supported picolibc/newlib configuration that is CI tested.
Another thing that would be useful is if the libc++ CMake build system could automatically detect whether threads are supported, etc. so you don’t have to set all the LIBCXX_ENABLE_* variables when configuring. Alternatively, checking in a CMake cache for picolibc could make configuration simpler.
Will you builder also run the libc++ testsuite? If so what architecture combinations do you intend to test at runtime (I assume on QEMU?).
I can’t comment on the buildbot part, since I don’t know much about that. It sounds to me like a large part of what you want to test is the libc++ integration. For that part the way to achieve it would be a runner in the libc++ pre-commit CI, as you and @DavidSpickett already alluded to. That would make it a supported configuration and avoids commits breaking the configuration in the first place.
Yes thank you for that. I’ll remove the workaround from LLVM Embedded Toolchain for Arm once that lands.
Yes that’s the plan, once we’ve got the tests set up. The set of library variants we currently build are here. Whether we will test all of them depends how long it takes - we may just test a subset of the variants fully, and run a smaller set of tests on the rest.
Yes that’s a big part of it, and yes pre-commit CI would be a good follow-on step after we get some experience from the staging builds.
I think this is a great plan, but all the CI in libc++ is pre-commit. We don’t have any post-commit build bots anymore and don’t want to add any. If you want to make Newlib/Picolibc supported by libc++, the way to go (and the only way to go really) is to add a BuildKite runner. Fortunately, it’s not harder than adding a BuildBot runner (in fact it’s probably easier).
You can reach out to me on Discord and I’ll get you set up with a BuildKite Agent key and I’ll help you get your stuff set up.
I think I would, because we don’t want to be supporting two different systems. If the BuildBot build starts failing, for example, we don’t want to be required to fix it.
Note that BuildKite does have post-commit checks as well, so there’s effectively no added coverage by adding a BuildBot job.
We should be clear here that this is correct if that BuildBot is only looking at newlib+libcxx.
If that BuildBot was building the whole of “LLVM Embedded Toolchain for Arm”, there would be extra things tested. That bot would consume an already validated newlib+libcxx, and in theory, only turn up issues in the rest of the toolchain.
(and if it did find an issue with newlib+libcxx, the Buildkite bot should be improved to find it, then it can be fixed in the usual way)
So we have 2 things:
Buildkite bot(s) for libcxx to check newlib+libcxx
A potential “LLVM Embedded Toolchain for Arm” Buildbot
Yeah, that would be fine. But I’m on record saying that as far as libc++ is concerned, we will only provide support for and notice issues coming from the BuildKite bot (but we’ll do that diligently).