Page MenuHomePhabricator

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

Authored by ldionne on Oct 8 2020, 6:31 AM.

Details

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.

Diff Detail

Event Timeline

ldionne created this revision.Oct 8 2020, 6:31 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 8 2020, 6:31 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne requested review of this revision.Oct 8 2020, 6:31 AM

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 added inline comments.Oct 9 2020, 3:36 PM
libcxx/CMakeLists.txt
400–401

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

ldionne added inline comments.Oct 13 2020, 6:21 AM
libcxx/CMakeLists.txt
400–401

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 accepted this revision.Oct 13 2020, 3:54 PM

LGTM

libcxx/CMakeLists.txt
400–401

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.

@ldionne do you plan to land this change?

ldionne updated this revision to Diff 299651.Oct 21 2020, 5:45 AM

Rebase onto master

@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.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 21 2020, 5:47 AM
This revision was automatically updated to reflect the committed changes.
ye-luo added a subscriber: ye-luo.Oct 21 2020, 9:44 AM

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.

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 <ldionne@apple.com>
Date:   Wed Oct 21 12:53:24 2020 -0400

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

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 <ldionne@apple.com>
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>

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 <ldionne@apple.com>
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.

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.

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.

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.

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 added a subscriber: vitalybuka.EditedOct 21 2020, 2:58 PM

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

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.

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).

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

@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.

dmajor added a subscriber: dmajor.Oct 22 2020, 8:40 AM
krisb added a subscriber: krisb.Oct 22 2020, 2:18 PM

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.

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?

@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.

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

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 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 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.

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.

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.

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