[libc++] Include <__config_site> from <__config>

[ldionne]

Summary

Prior to this patch, we would generate a fancy <config> header by
concatenating <config_site> and <__config>. This complexifies the
build system and also increases the difference between what's tested
and what's actually installed.

This patch removes that complexity and instead simply installs <config_site>
alongside the libc++ headers. <config_site> is then included by <__config>,
which is much simpler.

Doing this also opens the door to having different <__config_site> headers
depending on the target, which was impossible before.

[ldionne]

Note that the next logical step will be to run the tests against a fake complete installation of libc++, which will make us even closer to testing exactly what's shipped.

[phosek]

libcxx/CMakeLists.txt

403

Do we still need this? Could we copy ABI files directly into LIBCXX_GENERATED_INCLUDE_DIR?

[ldionne]

libcxx/CMakeLists.txt

403

I wanted to tackle that as a separate issue after investigating.

I actually don't see why we need to copy them at all, since the libc++abi headers don't need a generation step.

[phosek]

LGTM

libcxx/CMakeLists.txt

403

OK, I was going to suggest having only a single variable called LIBCXX_INCLUDE_DIR for simplicity if we don't need this anymore, but that can be done in follow up changes.

[phosek]

@ldionne do you plan to land this change?

[ldionne]

Rebase onto master

[ldionne]

In D89041#2343125, @phosek wrote:

@ldionne do you plan to land this change?

Yes, thanks for the heads up. I was waiting a bit cause this changes the workflow when testing libc++ header only changes, however overall I think it's going to be a lot better and more robust. I'm committing this.

[ye-luo]

Fails at make install.

CMake Error at projects/libcxx/include/cmake_install.cmake:753 (file):

file INSTALL cannot find
"/scratch/opt/llvm-clang/build_mirror_offload_nightly/projects/libcxx/__config_site":
No such file or directory.

[ldionne]

In D89041#2345002, @ye-luo wrote:

Fails at make install.

CMake Error at projects/libcxx/include/cmake_install.cmake:753 (file):

file INSTALL cannot find
"/scratch/opt/llvm-clang/build_mirror_offload_nightly/projects/libcxx/__config_site":
No such file or directory.

This change is surprisingly tricky to land. Thanks for the heads up.

Should be fixed in

commit b5aa67446e01bd277727b05710a42e69ac41e74b
Author: Louis Dionne <[e-mail]>
Date:   Wed Oct 21 12:53:24 2020 -0400

    [libc++] Fix the installation of libc++ headers since the __config_site change

[jhuber6]

In D89041#2345055, @ldionne wrote:

In D89041#2345002, @ye-luo wrote:

Fails at make install.

CMake Error at projects/libcxx/include/cmake_install.cmake:753 (file):

file INSTALL cannot find
"/scratch/opt/llvm-clang/build_mirror_offload_nightly/projects/libcxx/__config_site":
No such file or directory.

This change is surprisingly tricky to land. Thanks for the heads up.

Should be fixed in

commit b5aa67446e01bd277727b05710a42e69ac41e74b
Author: Louis Dionne <[e-mail]>
Date:   Wed Oct 21 12:53:24 2020 -0400

    [libc++] Fix the installation of libc++ headers since the __config_site change

I'm still getting build errors, from an existing build that worked earlier today, links against libcxx installed previously.

/home2/3n4/clang/bin/../include/c++/v1/__config:13:10: fatal error: '__config_site' file not found
#include <__config_site>

When I try a fresh build using an install script,

In file included from /home2/3n4/llvm/trunk/llvm-project/compiler-rt/lib/fuzzer/FuzzerCrossOver.cpp:11:
/home2/3n4/llvm/trunk/llvm-project/compiler-rt/lib/fuzzer/FuzzerDefs.h:14:10: fatal error: cassert: No such file or directory
 #include <cassert>

[ye-luo]

In D89041#2345115, @jhuber6 wrote:

In D89041#2345055, @ldionne wrote:

In D89041#2345002, @ye-luo wrote:

Fails at make install.

CMake Error at projects/libcxx/include/cmake_install.cmake:753 (file):

file INSTALL cannot find
"/scratch/opt/llvm-clang/build_mirror_offload_nightly/projects/libcxx/__config_site":
No such file or directory.

This change is surprisingly tricky to land. Thanks for the heads up.

Should be fixed in

commit b5aa67446e01bd277727b05710a42e69ac41e74b
Author: Louis Dionne <[e-mail]>
Date:   Wed Oct 21 12:53:24 2020 -0400

    [libc++] Fix the installation of libc++ headers since the __config_site change

I'm still getting build errors, from an existing build that worked earlier today, links against libcxx installed previously.

/home2/3n4/clang/bin/../include/c++/v1/__config:13:10: fatal error: '__config_site' file not found
#include <__config_site>

When I try a fresh build using an install script,

In file included from /home2/3n4/llvm/trunk/llvm-project/compiler-rt/lib/fuzzer/FuzzerCrossOver.cpp:11:
/home2/3n4/llvm/trunk/llvm-project/compiler-rt/lib/fuzzer/FuzzerDefs.h:14:10: fatal error: cassert: No such file or directory
 #include <cassert>

I had to delete the build folder and rebuild from clean. Reusing existing build folder doesn't work for me as well.

[ldionne]

In D89041#2345115, @jhuber6 wrote:

When I try a fresh build using an install script,

In file included from /home2/3n4/llvm/trunk/llvm-project/compiler-rt/lib/fuzzer/FuzzerCrossOver.cpp:11:
/home2/3n4/llvm/trunk/llvm-project/compiler-rt/lib/fuzzer/FuzzerDefs.h:14:10: fatal error: cassert: No such file or directory
 #include <cassert>

How are you building? This does not appear to be one of the usual setups, cause the builds are green on my side.

[jhuber6]

In D89041#2345124, @ldionne wrote:

In D89041#2345115, @jhuber6 wrote:

When I try a fresh build using an install script,

In file included from /home2/3n4/llvm/trunk/llvm-project/compiler-rt/lib/fuzzer/FuzzerCrossOver.cpp:11:
/home2/3n4/llvm/trunk/llvm-project/compiler-rt/lib/fuzzer/FuzzerDefs.h:14:10: fatal error: cassert: No such file or directory
 #include <cassert>

How are you building? This does not appear to be one of the usual setups, cause the builds are green on my side.

Yeah, I think that one might've been a compiler-rt failure that broke some other time since I haven't built from scratch in awhile. I'll let you know if my build finishes without compiler-rt.

[jhuber6]

In D89041#2345160, @jhuber6 wrote:

Yeah, I think that one might've been a compiler-rt failure that broke some other time since I haven't built from scratch in awhile. I'll let you know if my build finishes without compiler-rt.

Yeah, it works now, thanks.

[mstorsjo]

This seems to have broken my builds of libcxxabi+libcxx. I'm building them standalone (i.e. configuring each of libunwind, libcxxabi and libcxx individually with cmake), and I've used to set LIBCXXABI_LIBCXX_INCLUDES to point at the libcxx include dir, but that cmake option is removed now. I presume the cxx-headers dependency is supposed to take care of that, but that only works when building them all with one cmake invocation (part of full llvm build, or part of runtimes build)?

Could we keep LIBCXXABI_LIBCXX_INCLUDES (in order not to break existing scripts), or what's the best way forward here?

[vitalybuka]

In D89041#2345252, @jhuber6 wrote:

In D89041#2345160, @jhuber6 wrote:

Yeah, I think that one might've been a compiler-rt failure that broke some other time since I haven't built from scratch in awhile. I'll let you know if my build finishes without compiler-rt.

Yeah, it works now, thanks.

Removing the dir does not help for my build:

cmake -GNinja ../../llvm-project/llvm -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra;compiler-rt;lld;libcxx;libcxxabi" -DLLVM_ENABLE_LLD=ON -DCMAKE_C_COMPILER=/usr/local/google/home/vitalybuka/src/clang/bin/clang -DCMAKE_CXX_COMPILER=/usr/local/google/home/vitalybuka/src/clang/bin/clang++ -DLLVM_BINUTILS_INCDIR=/usr/include -DCMAKE_BUILD_TYPE=Release
ninja check-all

[6891/7170] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64-with-call.o
FAILED: projects/compiler-rt/lib/msan/tests/MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64-with-call.o 
cd /usr/local/google/home/vitalybuka/src/llvm.git/out/z/projects/compiler-rt/lib/msan/tests && /usr/local/google/home/vitalybuka/src/llvm.git/out/z/./bin/clang -nostdinc++ -isystem /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/../libcxx/include -Wno-covered-switch-default -DGTEST_NO_LLVM_SUPPORT=1 -DGTEST_HAS_RTTI=0 -I/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/include -I/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest -I/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/include -I/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib -I/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/msan -g -O2 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -Wno-deprecated-declarations -Wno-unused-variable -Wno-zero-length-array -Wno-uninitialized -Werror=sign-compare -Wno-gnu-zero-variadic-macro-arguments -fsanitize=memory -fsanitize-memory-track-origins -mllvm -msan-keep-going=1 -mllvm -msan-instrumentation-with-call-threshold=0 -m64 -c -o MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64-with-call.o /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/msan/tests/msan_test.cpp
In file included from /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/msan/tests/msan_test.cpp:15:
In file included from /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/lib/msan/tests/msan_test_config.h:17:
In file included from /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:54:
In file included from /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/../libcxx/include/limits:104:
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/llvm/../libcxx/include/__config:13:10: fatal error: '__config_site' file not found

it's on cd4a4ae97a7ccfa381e06936cd0981cb7d978ec1

[ldionne]

In D89041#2345765, @vitalybuka wrote:

Removing the dir does not help for my build:

cmake -GNinja ../../llvm-project/llvm -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra;compiler-rt;lld;libcxx;libcxxabi" -DLLVM_ENABLE_LLD=ON -DCMAKE_C_COMPILER=/usr/local/google/home/vitalybuka/src/clang/bin/clang -DCMAKE_CXX_COMPILER=/usr/local/google/home/vitalybuka/src/clang/bin/clang++ -DLLVM_BINUTILS_INCDIR=/usr/include -DCMAKE_BUILD_TYPE=Release
ninja check-all

[...]

it's on cd4a4ae97a7ccfa381e06936cd0981cb7d978ec1

I think it's because of

set(MSAN_UNITTEST_COMMON_CFLAGS
  -nostdinc++
  -isystem ${COMPILER_RT_LIBCXX_PATH}/include

in compiler-rt/lib/msan/tests/CMakeLists.txt. Other compiler-rt tests don't pass any custom libc++ path, and they rely on the fact that they can find it alongside Clang in the build directory (which, funnily enough, is exactly what 69c2087283cf7b17ca75f69daebf4ffc158b754a fixed. Possible fix in https://reviews.llvm.org/D89915, I pinged you there.

In D89041#2345632, @mstorsjo wrote:

This seems to have broken my builds of libcxxabi+libcxx. I'm building them standalone (i.e. configuring each of libunwind, libcxxabi and libcxx individually with cmake), and I've used to set LIBCXXABI_LIBCXX_INCLUDES to point at the libcxx include dir, but that cmake option is removed now. I presume the cxx-headers dependency is supposed to take care of that, but that only works when building them all with one cmake invocation (part of full llvm build, or part of runtimes build)?

Could we keep LIBCXXABI_LIBCXX_INCLUDES (in order not to break existing scripts), or what's the best way forward here?

I've started looking at this one -- I'll have to resume tomorrow.

This change really showcases everything worse about libc++'s build system today:

1. There are many users of libc++, and there's unfortunately no single standardized way of taking a dependency on libc++ at the CMake level. As a result, some projects expect it to be magically placed in some location, other projects expect to be able to include the libc++ headers in the source tree (which used to work, but has always been incorrect cause you were skipping the <__config_site>, which could have up to ABI breaking implications), others manually add an include path to a location where the libc++ headers have been installed properly, and others link against the cxx-headers target in CMake. Only the two last ones are intended to be supported.
2. There's too many ways of building libc++. There's the Standalone build, the runtimes build and the monorepo build. Not all of them are tested automatically. We ordinarily only test the standard monorepo build in our test bots, hence the break discovered by @mstorsjo
3. Even within the libc++ and libc++abi build itself, there are violations of that. Also, we don't run all targets all the time, so for example this change broke the cxx-benchmarks target cause it didn't run by default when I did the tests. Running it all the time is too slow, so it's excluded by default.

I'll pick this up tomorrow morning first thing, but I'll be looking to standardize a single way of building libc++ that doesn't involve building all the rest of LLVM (so it should satisfy the standalone build users requirements).

[ldionne]

At least, I'm glad I made this change because it shows us a bunch of places that had incorrect assumptions :-)

[phosek]

@ldionne I've started doing a related cleanup of compiler-rt in D88922 but I had to revert it because it broke various builders that use custom flags and I'm still working through those issues.

[aeubanks]

Is the test-libc++abi error in http://lab.llvm.org:8011/#/builders/60/builds/202 related to this?

llvm-lit.py: c:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-bins\libcxxabi\test\lit.site.cfg:59: note: Using configuration variant: libcxxabi
llvm-lit.py: C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxx\utils\libcxx\test\target_info.py:194: note: inferred target_info as: 'libcxx.test.target_info.LinuxRemoteTI'
llvm-lit.py: C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxx\utils\libcxx\test\config.py:643: note: Setting target_triple to armv7-linux-gnueabihf
llvm-lit.py: C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxxabi\test\libcxxabi\test\config.py:67: fatal: cxx_headers='C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-bins\include\c++\v1' is not a directory.

[ldionne]

In D89041#2348597, @aeubanks wrote:

Is the test-libc++abi error in http://lab.llvm.org:8011/#/builders/60/builds/202 related to this?

llvm-lit.py: c:\buildbot\as-builder-1\x-armv7l\build\runtimes\runtimes-bins\libcxxabi\test\lit.site.cfg:59: note: Using configuration variant: libcxxabi
llvm-lit.py: C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxx\utils\libcxx\test\target_info.py:194: note: inferred target_info as: 'libcxx.test.target_info.LinuxRemoteTI'
llvm-lit.py: C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxx\utils\libcxx\test\config.py:643: note: Setting target_triple to armv7-linux-gnueabihf
llvm-lit.py: C:\buildbot\as-builder-1\x-armv7l\llvm-project\libcxxabi\test\libcxxabi\test\config.py:67: fatal: cxx_headers='C:/buildbot/as-builder-1/x-armv7l/build/runtimes/runtimes-bins\include\c++\v1' is not a directory.

Yes, almost certainly. It's on my radar, but I'm not entirely sure how that build is setup, since it's hidden behind the runtimes build. I do suspect it's the same failure that @mstorsjo described above, since the runtimes build is effectively using a Standalone build for libc++abi.

@mstorsjo Can you please confirm whether 529ac33197f6408952ae995075ac5e2dc5287e81 resolves your issues?

[ldionne]

In D89041#2348643, @ldionne wrote:

@mstorsjo Can you please confirm whether 529ac33197f6408952ae995075ac5e2dc5287e81 resolves your issues?

Also, please note that you'll need to pass the path to where the libc++ headers are *installed*, not just libcxx/include. If that's what you did previously, then this change is nicely preventing you from using mis-configured libc++ headers.

[stefanp]

This is still causing a buildbot failure on a Power PC sanitizer bot:
http://lab.llvm.org:8011/#/builders/18

Did do a complete clean of the build directory but that has not fixed the issue.
Same error as other users:

In file included from /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/lib/msan/tests/msan_loadable.cpp:15:
In file included from /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/llvm/../libcxx/include/stdlib.h:87:
/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/llvm/../libcxx/include/__config:13:10: fatal error: '__config_site' file not found
In file included from /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/lib/msan/tests/msan_loadable.cpp:15:
In file included from /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/llvm/../libcxx/include/stdlib.h:87:
/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/llvm/../libcxx/include/__config:13:10:#include <__config_site> 
fatal error         ^~~~~~~~~~~~~~~: 
'__config_site' file not found
#include <__config_site>

The cmake command does not specify the path for CXX

cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_PARALLEL_LINK_JOBS=10 '-DLLVM_ENABLE_PROJECTS='\''clang;compiler-rt'\''' '-DLLVM_ENABLE_PROJECTS='\''clang;compiler-rt;libcxx;libcxxabi'\''' /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/llvm

[ldionne]

In D89041#2348774, @stefanp wrote:

This is still causing a buildbot failure on a Power PC sanitizer bot:
http://lab.llvm.org:8011/#/builders/18

This is the same issue as the one reported by @vitalybuka and for which I proposed a fix at https://reviews.llvm.org/D89915.

[mstorsjo]

In D89041#2348645, @ldionne wrote:

In D89041#2348643, @ldionne wrote:

@mstorsjo Can you please confirm whether 529ac33197f6408952ae995075ac5e2dc5287e81 resolves your issues?

Also, please note that you'll need to pass the path to where the libc++ headers are *installed*, not just libcxx/include. If that's what you did previously, then this change is nicely preventing you from using mis-configured libc++ headers.

That's indeed what I've been doing so far. When building things entirely from scratch, I can't build libcxx before I've built libcxxabi, and now libcxxabi can't make do with the out-of-the-box headers from libcxx either. And I'm not alone in using such a bootstrap procedure, https://reviews.llvm.org/D89518 also tries to document how to do such a setup.

But maybe it's possible to run cmake on libcxx even though libcxxabi isn't built yet? In that case, I'd have to first configure libcxx (even though it can't be built yet due to missing libcxxabi) just to get proper "installed" libcxx headers, so that I can build libcxxabi, so I then can build libcxx. I'll see if I can make that work.

[mstorsjo]

In D89041#2349100, @mstorsjo wrote:

In D89041#2348645, @ldionne wrote:

In D89041#2348643, @ldionne wrote:

@mstorsjo Can you please confirm whether 529ac33197f6408952ae995075ac5e2dc5287e81 resolves your issues?

Also, please note that you'll need to pass the path to where the libc++ headers are *installed*, not just libcxx/include. If that's what you did previously, then this change is nicely preventing you from using mis-configured libc++ headers.

That's indeed what I've been doing so far. When building things entirely from scratch, I can't build libcxx before I've built libcxxabi, and now libcxxabi can't make do with the out-of-the-box headers from libcxx either. And I'm not alone in using such a bootstrap procedure, https://reviews.llvm.org/D89518 also tries to document how to do such a setup.

But maybe it's possible to run cmake on libcxx even though libcxxabi isn't built yet? In that case, I'd have to first configure libcxx (even though it can't be built yet due to missing libcxxabi) just to get proper "installed" libcxx headers, so that I can build libcxxabi, so I then can build libcxx. I'll see if I can make that work.

Yes, that does indeed seem to work; I first configure libcxx and build the generate-cxx-headers target, then configure libcxxabi, pointing it at the libcxx build dir's <build>/include/c++/v1 directory and build libcxxabi, then finally build libcxx. (I can also build the install-cxx-headers target and point libcxxabi at <installprefix>/include/c++/v1.) As a bonus, this also seems to work the same with the 11 stable release. (If possible, I want to be able to build both the latest stable release and the current master with the same setup.)

I'll have a look at your unified standalone setup as well - it sounds like it could clean up a lot of issues like these and also get rid of a number of sharp edges and reliance on kinda internal details in how I build libunwind+libcxxabi+libcxx together right now.

[mstorsjo]

Another effect of this change, is that if I'm working on libcxx, have a previously fully built workdir, and e.g. edit a header, and rerun ninja, it will only do one build task (copy the updated header into the build dir's include dir). If I rerun ninja again, it will actually rebuild the sources against the newly updated header. So at the ninja level, it's missing a dependency that implies that the copy header phase actually updates files that ninja already tracks the modification date for.

[ldionne]

In D89041#2349186, @mstorsjo wrote:

Yes, that does indeed seem to work; I first configure libcxx and build the generate-cxx-headers target, then configure libcxxabi, pointing it at the libcxx build dir's <build>/include/c++/v1 directory and build libcxxabi, then finally build libcxx. (I can also build the install-cxx-headers target and point libcxxabi at <installprefix>/include/c++/v1.) As a bonus, this also seems to work the same with the 11 stable release. (If possible, I want to be able to build both the latest stable release and the current master with the same setup.)

I'll have a look at your unified standalone setup as well - it sounds like it could clean up a lot of issues like these and also get rid of a number of sharp edges and reliance on kinda internal details in how I build libunwind+libcxxabi+libcxx together right now.

Yes, this +10000. At this point, it's 100% clear to me we need to drop support for the classic standalone builds -- they are just too tricky to maintain, and that's not good for anyone. FWIW, here's the setup I use for testing standalone builds locally:

rm -rf "${BUILD_DIR}/libcxx" "${BUILD_DIR}/libcxxabi" "${INSTALL_DIR}"
mkdir -p "${BUILD_DIR}/libcxx" "${BUILD_DIR}/libcxxabi" "${INSTALL_DIR}"

(cd "${BUILD_DIR}/libcxx" &&
    cmake "${LIBCXX_ROOT}" -GNinja \
                           -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
                           -DLLVM_PATH="${LLVM_ROOT}" \
                           -DLIBCXX_INCLUDE_TESTS=ON \
                           -DLIBCXX_CXX_ABI=libcxxabi \
                           -DLIBCXX_CXX_ABI_INCLUDE_PATHS="${LIBCXXABI_ROOT}/include")

(cd "${BUILD_DIR}/libcxxabi" &&
    cmake "${LIBCXXABI_ROOT}" -GNinja \
                              -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
                              -DLLVM_PATH="${LLVM_ROOT}" \
                              -DLIBCXXABI_INCLUDE_TESTS=ON \
                              -DLIBCXXABI_LIBCXX_PATH="${LIBCXX_ROOT}" \
                              -DLIBCXXABI_LIBCXX_INCLUDES="${BUILD_DIR}/libcxx/include/c++/v1")

Notice how I'm passing the build directory's include/c++/v1 path, not libcxx/include. This is still a hack cause that isn't technically the "installed" version of libc++, it's the build-internal one, but whatever for now.

In D89041#2349675, @mstorsjo wrote:

Another effect of this change, is that if I'm working on libcxx, have a previously fully built workdir, and e.g. edit a header, and rerun ninja, it will only do one build task (copy the updated header into the build dir's include dir). If I rerun ninja again, it will actually rebuild the sources against the newly updated header. So at the ninja level, it's missing a dependency that implies that the copy header phase actually updates files that ninja already tracks the modification date for.

Yeah, I'm seeing that too. Using a minimal setup that's exactly like ours, I see that CMake is doing the right thing, though. This is weird -- it's either a fine subtlety in our setup, or a CMake bug.

I've decided to revert this change. We're still seeing failures in the runtimes build that I'm unable to reproduce locally, and we need to get the bots green again. It's been two days of scrambling around and trying to fix everything, but it's getting out of control. Now that we know how much ripple effect this change has, I'll split the change up into small bits and check it back in piecemeal.

@thakis ^ for your GN build

libcxx/CMakeLists.txt

 endif ()
 
 #===============================================================================
 # Configure System
 #===============================================================================
 
 set(LIBCXX_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
 set(LIBCXX_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
 set(LIBCXX_BINARY_INCLUDE_DIR "${LIBCXX_BINARY_DIR}/include/c++build")
+set(LIBCXX_GENERATED_INCLUDE_DIR "${LIBCXX_BINARY_DIR}/include/c++/v1")
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
   set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
   set(LIBCXX_HEADER_DIR ${LLVM_BINARY_DIR})
   set(LIBCXX_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
   if(LIBCXX_LIBDIR_SUBDIR)
     string(APPEND LIBCXX_LIBRARY_DIR /${LIBCXX_LIBDIR_SUBDIR})
     string(APPEND LIBCXX_INSTALL_LIBRARY_DIR /${LIBCXX_LIBDIR_SUBDIR})

libcxx/docs/TestingLibcxx.rst

 
 .. _LIT Command Guide: https://llvm.org/docs/CommandGuide/lit.html
 
 Usage
 -----
 
 After building libc++, you can run parts of the libc++ test suite by simply
 running ``llvm-lit`` on a specified test or directory. If you're unsure
-whether the required libraries have been built, you can use the
-`check-cxx-deps` target. For example:
+whether the required targets have been built, you can use the `check-cxx-deps`
+target to build them. For example:
 
 .. code-block:: bash
 
   $ cd <monorepo-root>
   $ make -C <build> check-cxx-deps # If you want to make sure the targets get rebuilt
   $ <build>/bin/llvm-lit -sv libcxx/test/std/re # Run all of the std::regex tests
   $ <build>/bin/llvm-lit -sv libcxx/test/std/depr/depr.c.headers/stdlib_h.pass.cpp # Run a single test
   $ <build>/bin/llvm-lit -sv libcxx/test/std/atomics libcxx/test/std/threads # Test std::thread and std::atomic
 
+In the default configuration, the tests are built against headers that form a
+fake installation root of libc++. This installation root has to be updated when
+changes are made to the headers, so you should re-run the `check-cxx-deps` target
+before running the tests manually with `lit` when you make any sort of change,
+including to the headers.
+
 Sometimes you'll want to change the way LIT is running the tests. Custom options
 can be specified using the `--param=<name>=<val>` flag. The most common option
 you'll want to change is the standard dialect (ie -std=c++XX). By default the
 test suite will select the newest C++ dialect supported by the compiler and use
 that. However if you want to manually specify the option like so:
 
 .. code-block:: bash
 

libcxx/include/CMakeLists.txt

 set(files
   __bit_reference
   __bsd_locale_defaults.h
   __bsd_locale_fallbacks.h
+  __config
   __errc
   __debug
   __functional_03
   __functional_base
   __functional_base_03
   __hash_table
   __libcpp_version
   __locale
set(files
     support/win32/limits_msvc_win32.h
     support/win32/locale_win32.h
     support/xlocale/__nop_locale_mgmt.h
     support/xlocale/__posix_l_fallback.h
     support/xlocale/__strtonum_fallback.h
     )
 endif()
 
-configure_file("__config_site.in"
-               "${LIBCXX_BINARY_DIR}/__config_site"
-               @ONLY)
-
-# Generate a custom __config header. The new header is created
-# by prepending __config_site to the current __config header.
-add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config
-  COMMAND ${Python3_EXECUTABLE} ${LIBCXX_SOURCE_DIR}/utils/cat_files.py
-    ${LIBCXX_BINARY_DIR}/__config_site
-    ${LIBCXX_SOURCE_DIR}/include/__config
-    -o ${LIBCXX_BINARY_DIR}/__generated_config
-  DEPENDS ${LIBCXX_SOURCE_DIR}/include/__config
-          ${LIBCXX_BINARY_DIR}/__config_site
-)
-# Add a target that executes the generation commands.
-add_custom_target(cxx-generated-config ALL
-  DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config)
-
 if(LIBCXX_HEADER_DIR)
-  set(output_dir ${LIBCXX_HEADER_DIR}/include/c++/v1)
+  configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/__config_site" @ONLY)
 
-  set(out_files)
+  set(_all_includes "${LIBCXX_GENERATED_INCLUDE_DIR}/__config_site")
   foreach(f ${files})
-    set(src ${CMAKE_CURRENT_SOURCE_DIR}/${f})
-    set(dst ${output_dir}/${f})
+    set(src "${CMAKE_CURRENT_SOURCE_DIR}/${f}")
+    set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/${f}")
     add_custom_command(OUTPUT ${dst}
       DEPENDS ${src}
       COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
       COMMENT "Copying CXX header ${f}")
-    list(APPEND out_files ${dst})
+    list(APPEND _all_includes "${dst}")
   endforeach()
-
-  # Copy the generated header as __config into build directory.
-  set(src ${LIBCXX_BINARY_DIR}/__generated_config)
-  set(dst ${output_dir}/__config)
-  add_custom_command(OUTPUT ${dst}
-      DEPENDS ${src} cxx-generated-config
-      COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
-      COMMENT "Copying CXX __config")
-  list(APPEND out_files ${dst})
-  add_custom_target(generate-cxx-headers DEPENDS ${out_files})
+  add_custom_target(generate-cxx-headers DEPENDS ${_all_includes})
 
   add_library(cxx-headers INTERFACE)
   add_dependencies(cxx-headers generate-cxx-headers ${LIBCXX_CXX_ABI_HEADER_TARGET})
   # TODO: Use target_include_directories once we figure out why that breaks the runtimes build
   if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")
-    target_compile_options(cxx-headers INTERFACE /I "${output_dir}")
-  else()
-    target_compile_options(cxx-headers INTERFACE -I "${output_dir}")
-  endif()
-
-  # Make sure the generated __config_site header is included when we build the library.
-  if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")
-    target_compile_options(cxx-headers INTERFACE /FI "${LIBCXX_BINARY_DIR}/__config_site")
+    target_compile_options(cxx-headers INTERFACE /I "${LIBCXX_GENERATED_INCLUDE_DIR}")
   else()
-    target_compile_options(cxx-headers INTERFACE -include "${LIBCXX_BINARY_DIR}/__config_site")
+    target_compile_options(cxx-headers INTERFACE -I "${LIBCXX_GENERATED_INCLUDE_DIR}")
   endif()
 else()
   add_library(cxx-headers INTERFACE)
 endif()
 
 if (LIBCXX_INSTALL_HEADERS)
   foreach(file ${files})
     get_filename_component(dir ${file} DIRECTORY)
     install(FILES ${file}
       DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dir}
       COMPONENT cxx-headers
       PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
     )
   endforeach()
 
-  # Install the generated header as __config.
-  install(FILES ${LIBCXX_BINARY_DIR}/__generated_config
+  # Install the generated __config_site.
+  install(FILES ${LIBCXX_BINARY_DIR}/__config_site
     DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1
     PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-    RENAME __config
     COMPONENT cxx-headers)
 
   if (NOT CMAKE_CONFIGURATION_TYPES)
     add_custom_target(install-cxx-headers
                       DEPENDS cxx-headers cxx-generated-config
                       COMMAND "${CMAKE_COMMAND}"
                               -DCMAKE_INSTALL_COMPONENT=cxx-headers
                               -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
     # Stripping is a no-op for headers
     add_custom_target(install-cxx-headers-stripped DEPENDS install-cxx-headers)
   endif()
 endif()

libcxx/include/__config

 // -*- C++ -*-
 //===--------------------------- __config ---------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP_CONFIG
 #define _LIBCPP_CONFIG
 
+#include <__config_site>
+
 #if defined(_MSC_VER) && !defined(__clang__)
 #  if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #    define _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
 #  endif
 #endif
 
 #ifndef _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
 #pragma GCC system_header

libcxx/test/configs/legacy.cfg.in

 @AUTO_GEN_COMMENT@
 
 import os
 import site
 
+config.cxx_headers              = "@LIBCXX_GENERATED_INCLUDE_DIR@"
 config.cxx_under_test           = "@CMAKE_CXX_COMPILER@"
 config.project_obj_root         = "@CMAKE_BINARY_DIR@"
 config.libcxx_src_root          = "@LIBCXX_SOURCE_DIR@"
 config.libcxx_obj_root          = "@LIBCXX_BINARY_DIR@"
 config.cxx_library_root         = "@LIBCXX_LIBRARY_DIR@"
 config.enable_exceptions        = @LIBCXX_ENABLE_EXCEPTIONS@
 config.enable_debug_tests       = @LIBCXX_ENABLE_DEBUG_MODE@
 config.enable_experimental      = @LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY@

libcxx/utils/libcxx/test/config.py

def configure_default_compile_flags(self):
         if pstl_src_root is not None and pstl_obj_root is not None:
             self.cxx.compile_flags += ['-I' + os.path.join(pstl_src_root, 'include')]
             self.cxx.compile_flags += ['-I' + os.path.join(pstl_obj_root, 'generated_headers')]
             self.cxx.compile_flags += ['-I' + os.path.join(pstl_src_root, 'test')]
             self.config.available_features.add('parallel-algorithms')
 
     def configure_compile_flags_header_includes(self):
         support_path = os.path.join(self.libcxx_src_root, 'test', 'support')
-        self.configure_config_site_header()
         if self.cxx_stdlib_under_test != 'libstdc++' and \
            not self.target_info.is_windows():
             self.cxx.compile_flags += [
                 '-include', os.path.join(support_path, 'nasty_macros.h')]
         if self.cxx_stdlib_under_test == 'msvc':
             self.cxx.compile_flags += [
                 '-include', os.path.join(support_path,
                                          'msvc_stdlib_force_include.h')]
             pass
         if self.target_info.is_windows() and self.debug_build and \
                 self.cxx_stdlib_under_test != 'msvc':
             self.cxx.compile_flags += [
                 '-include', os.path.join(support_path,
                                          'set_windows_crt_report_mode.h')
             ]
         cxx_headers = self.get_lit_conf('cxx_headers')
-        if cxx_headers == '' or (cxx_headers is None
-                                 and self.cxx_stdlib_under_test != 'libc++'):
+        if cxx_headers is None and self.cxx_stdlib_under_test != 'libc++':
             self.lit_config.note('using the system cxx headers')
             return
         self.cxx.compile_flags += ['-nostdinc++']
-        if cxx_headers is None:
-            cxx_headers = os.path.join(self.libcxx_src_root, 'include')
         if not os.path.isdir(cxx_headers):
-            self.lit_config.fatal("cxx_headers='%s' is not a directory."
-                                  % cxx_headers)
+            self.lit_config.fatal("cxx_headers='{}' is not a directory.".format(cxx_headers))
         self.cxx.compile_flags += ['-I' + cxx_headers]
         if self.libcxx_obj_root is not None:
             cxxabi_headers = os.path.join(self.libcxx_obj_root, 'include',
                                           'c++build')
             if os.path.isdir(cxxabi_headers):
                 self.cxx.compile_flags += ['-I' + cxxabi_headers]
 
-    def configure_config_site_header(self):
-        # Check for a possible __config_site in the build directory. We
-        # use this if it exists.
-        if self.libcxx_obj_root is None:
-            return
-        config_site_header = os.path.join(self.libcxx_obj_root, '__config_site')
-        if not os.path.isfile(config_site_header):
-            return
-        self.cxx.compile_flags += ['-include', config_site_header]
-
     def configure_link_flags(self):
         # Configure library path
         self.configure_link_flags_cxx_library_path()
         self.configure_link_flags_abi_library_path()
 
         # Configure libraries
         if self.cxx_stdlib_under_test == 'libc++':
             self.cxx.link_flags += ['-nodefaultlibs']

libcxxabi/src/CMakeLists.txt

 if (MSVC_IDE OR XCODE)
   # Force them all into the headers dir on MSVC, otherwise they end up at
   # project scope because they don't have extensions.
   if (MSVC_IDE)
     source_group("Header Files" FILES ${LIBCXXABI_HEADERS})
   endif()
 endif()
 
-include_directories("${LIBCXXABI_LIBCXX_INCLUDES}")
-
 if (LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
   add_definitions(-DHAVE___CXA_THREAD_ATEXIT_IMPL)
 endif()
 
 if (LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST)
   add_definitions(-D_LIBCXXABI_FORGIVING_DYNAMIC_CAST)
 endif()
 

 endif()
 
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
   if(COMMAND llvm_setup_rpath)
     llvm_setup_rpath(cxxabi_shared)
   endif()
-  target_link_libraries(cxxabi_shared PRIVATE ${LIBCXXABI_SHARED_LIBRARIES} ${LIBCXXABI_LIBRARIES})
+  target_link_libraries(cxxabi_shared PRIVATE cxx-headers ${LIBCXXABI_SHARED_LIBRARIES} ${LIBCXXABI_LIBRARIES})
   if (TARGET pstl::ParallelSTL)
     target_link_libraries(cxxabi_shared PUBLIC pstl::ParallelSTL)
   endif()
   set_target_properties(cxxabi_shared
                         PROPERTIES
                           CXX_EXTENSIONS
                             OFF
                           CXX_STANDARD
if (LIBCXXABI_ENABLE_EXCEPTIONS)
       endif()
     endif()
   endif()
 endif()
 
 # Build the static library.
 if (LIBCXXABI_ENABLE_STATIC)
   add_library(cxxabi_static STATIC ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
-  target_link_libraries(cxxabi_static PRIVATE ${LIBCXXABI_STATIC_LIBRARIES} ${LIBCXXABI_LIBRARIES})
+  target_link_libraries(cxxabi_static PRIVATE cxx-headers ${LIBCXXABI_STATIC_LIBRARIES} ${LIBCXXABI_LIBRARIES})
   if (TARGET pstl::ParallelSTL)
     target_link_libraries(cxxabi_static PUBLIC pstl::ParallelSTL)
   endif()
   set_target_properties(cxxabi_static
                         PROPERTIES
                           CXX_EXTENSIONS
                             OFF
                           CXX_STANDARD