[libcxx-commits] [PATCH] D154246: [libc++] Add check for building with picolibc

[Michael Platings via Phabricator via libcxx-commits]

michaelplatings added a comment.

In D154246#4467715 <https://reviews.llvm.org/D154246#4467715>, @DavidSpickett wrote:

> Let me know when it makes sense for me to try this locally and I'll get that done. Once the containers are updated you'll be able to get a green build here before landing.

It should work locally already. Please try this:

  CXX=clang++ CC=clang libcxx/utils/ci/run-buildbot armv6m-picolibc

However you won't get a green build. You should get an error:

  libcxx/src/filesystem/posix_compat.h:42:11: fatal error: 'sys/statvfs.h' file not found
     42 | # include <sys/statvfs.h>
        |           ^~~~~~~~~~~~~~~

But that's the point of adding this builder - to let libc++ developers know in advance when a change like D152382 <https://reviews.llvm.org/D152382> is going to break things for the embedded folks. If you rebase the change on top of df0348377492fb96a3118e020156ffb0fe9500b7 then it should work.



================
Comment at: libcxx/utils/ci/run-buildbot:606
 ;;
+armv6m-picolibc)
+    clean
----------------
philnik wrote:
> I would encourage you to also enable clang-tidy, since this tests a configuration with custom code paths, but that's not a must if it's too hard to do.
I gave ENABLE_CLANG_TIDY=On a go but wasn't able to get it working, so yes I'd like to categorise it as "too hard to do" for now :)


================
Comment at: libcxx/utils/ci/run-buildbot:608
+    clean
+    ${MONOREPO_ROOT}/libcxx/utils/ci/build-picolibc.sh \
+        --build-dir "${BUILD_DIR}" \
----------------
DavidSpickett wrote:
> DavidSpickett wrote:
> > Mordante wrote:
> > > Why do we need to build picolib every CI run instead of storing it in the base image?
> > > If that is really needed we should add some documentation why it's needed.
> > Assuming we can find a way to use this script directly in Linaro's container builds, it's not a problem. That ensures anyone can reproduce it easily.
> > 
> > The justification for picking a fixed version should be documented somewhere even if it's just that it's whatever the current version was at the time.
> The other problem doing this is how do we place the library in a place that would also be appropriate for anyone reproducing this without the container itself. I guess people could set an env var for the linker, but it's one more difference they have to account for.
> 
> Building it each time keeps it isolated, at the cost of compile time each time.
Yes I'd like to get this working in a relatively environment-independent state at first. Once that's stabilised we can put picolibc into the relevant container(s) and that will speed things up by ~16 seconds. The only additional dependencies needed are python3-venv & python3-pip (for meson) which I //think// are already included in the Linaro containers.


================
Comment at: libcxx/utils/ci/run-buildbot:615-616
+          -DCMAKE_CXX_FLAGS="${flags}" \
+          -DCMAKE_C_COMPILER=clang \
+          -DCMAKE_CXX_COMPILER=clang++ \
+          -DLIBCXX_ENABLE_WERROR=NO
----------------
Mordante wrote:
> philnik wrote:
> > These should be set through the environment variable and not be hard-coded.
> +1
> Also do we need to C compiler at all?
Yes the C compiler is needed for libunwind.


================
Comment at: libcxx/utils/ci/run-buildbot:617
+          -DCMAKE_CXX_COMPILER=clang++ \
+          -DLIBCXX_ENABLE_WERROR=NO
+
----------------
philnik wrote:
> Why?
Changed this to disable the specific warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154246