[libc++] Stop copying headers to the build directory #115380
Conversation
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-libcxx Author: Alexander Richardson (arichardson) ChangesThis was needed before #115077 Full diff: https://github.com/llvm/llvm-project/pull/115380.diff 1 Files Affected:
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index d699135774ee0b..04596fccdfc923 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -414,15 +414,16 @@ set(LIBCXX_INSTALL_MODULES_DIR "share/libc++/v1" CACHE STRING
set(LIBCXX_SHARED_OUTPUT_NAME "c++" CACHE STRING "Output name for the shared libc++ runtime library.")
set(LIBCXX_STATIC_OUTPUT_NAME "c++" CACHE STRING "Output name for the static libc++ runtime library.")
+set(LIBCXX_GENERATED_INCLUDE_DIR "${LIBCXX_BINARY_DIR}/include/c++/v1")
+set(LIBCXX_GENERATED_MODULE_DIR "${LIBCXX_BINARY_DIR}/modules/c++/v1")
+
if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
set(LIBCXX_TARGET_SUBDIR ${LLVM_DEFAULT_TARGET_TRIPLE})
if(LIBCXX_LIBDIR_SUBDIR)
string(APPEND LIBCXX_TARGET_SUBDIR /${LIBCXX_LIBDIR_SUBDIR})
endif()
set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LIBCXX_TARGET_SUBDIR})
- set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
- set(LIBCXX_GENERATED_MODULE_DIR "${LLVM_BINARY_DIR}/modules/c++/v1")
- set(LIBCXX_GENERATED_INCLUDE_TARGET_DIR "${LLVM_BINARY_DIR}/include/${LIBCXX_TARGET_SUBDIR}/c++/v1")
+ set(LIBCXX_GENERATED_INCLUDE_TARGET_DIR "${LIBCXX_BINARY_DIR}/include/${LIBCXX_TARGET_SUBDIR}/c++/v1")
set(LIBCXX_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LIBCXX_TARGET_SUBDIR} CACHE STRING
"Path where built libc++ libraries should be installed.")
set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR "${CMAKE_INSTALL_INCLUDEDIR}/${LIBCXX_TARGET_SUBDIR}/c++/v1" CACHE STRING
@@ -431,12 +432,8 @@ if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
else()
if(LLVM_LIBRARY_OUTPUT_INTDIR)
set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
- set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
- set(LIBCXX_GENERATED_MODULE_DIR "${LLVM_BINARY_DIR}/modules/c++/v1")
else()
set(LIBCXX_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXX_LIBDIR_SUFFIX})
- set(LIBCXX_GENERATED_INCLUDE_DIR "${CMAKE_BINARY_DIR}/include/c++/v1")
- set(LIBCXX_GENERATED_MODULE_DIR "${CMAKE_BINARY_DIR}/modules/c++/v1")
endif()
set(LIBCXX_GENERATED_INCLUDE_TARGET_DIR "${LIBCXX_GENERATED_INCLUDE_DIR}")
set(LIBCXX_INSTALL_LIBRARY_DIR lib${LIBCXX_LIBDIR_SUFFIX} CACHE STRING
|
arichardson
changed the base branch from
main
to
users/arichardson/spr/main.libc-stop-copying-headers-to-the-build-directory
|
Thanks for the review - will merge once #115387 has landed and CI is happy. |
|
It looks like the lldb test suite depends on these files being copied as well, so I've updated it to depend on the runtimes-test-depends target to ensure the libc++ headers have been installed to the fake prefix in the build dir first. Hardcoding these paths is unfortunate but I couldn't see an obviously better solution. This dates back to https://reviews.llvm.org/D133973 |
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
ldionne
force-pushed
the
users/arichardson/spr/libc-stop-copying-headers-to-the-build-directory
branch
from
a232598 to
d1d0ff5
Compare
ldionne
requested review from
DeinAlptraum,
daniel-grumberg,
QuietMisdreavus,
cyndyishida,
aaupov,
maksfb,
rafaelauler,
ayermolo,
dcci,
lanza and
bcardosolopes
as code owners
ldionne
removed request for
daniel-grumberg,
cyndyishida,
ayermolo and
DeinAlptraum
|
A bunch of CI jobs were triggered because of my faulty rebase, but the necessary jobs seems to have passed. What's still pending are some release jobs which normally don't run on PRs, and the documentation job which fails for an unrelated reason ( |
|
LLDB, please ping us if you encounter problems after we merge this patch. |
ldionne
deleted the
users/arichardson/spr/libc-stop-copying-headers-to-the-build-directory
branch
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/9056 Here is the relevant piece of the build log for the reference |
|
|
We are seeing a failures in our Fuchsia toolchain builders and I suspect this CL is the root cause. |
This reverts commit 428c876. Breaks sanitizer build: https://lab.llvm.org/buildbot/#/builders/51/builds/9056
|
@arichardson this was reverted due to sanitizer build failures: https://lab.llvm.org/buildbot/#/builders/51/builds/9056 |
|
I realize there are some issues with this and while it's a nice cleanup, I don't have the time right now to deal with the potential fallout (build directory can't be used as a cross-compiler out of the box). |
|
@kstoimenov @vitalybuka Could you folks show us how the sanitizer builds you run are different from the ones we already run in our CI pipeline? It would be important to align both since we often run into issues with your post-commit CI jobs failing but our pre-commit CI not catching it. |
We have many of them:
These must be very close to what libc++ have in CI. The most important part of this build is instrumented libc++ used for 2nd stage clang build. But it's very slow. I don't expect we can afford this on pre-commit CI.
|
|
IIUC, the build that encountered issues in this patch (in the comment above) is a flavour of (2), right? That seems surprising to me though since we also use the same flags when configuring libc++: https://github.com/llvm/llvm-project/blob/main/libcxx/CMakeLists.txt#L596 So the difference would be that you folks are building libc++ instrumented via the bootstrapping build instead of the "runtimes" build that we use for the rest of libc++ CI. I also fail to understand why our own bootstrapping build pre-commit CI didn't trip this wire. |
I think the problem is that some of the buildbots use the compiler from the stage1 build directory as the compiler for the stage2 build. This happens to pick up the correct libc++ if the headers are in the build directory, but without the symlinks it would need to be installed first to another directory and then use the stage1 install as the compiler for stage 2. |
it's no.1 |
|
It looks like https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh tries to run |
Very likely. I can take a look, unless you are already there. |
#123104 should hopefully fix it. |
|
Sanitizer build has been fixed but I know this will also break the Fuchsia build bots if we reland it (see #115379 (comment)) - not sure if there is any plan to update those builders to use installed toolchains for the multi-stage build. |
Have you seen LLDB failures? #115380 (comment) |
This was needed before #115077
since the compiler-rt test build made assumptions about the build
layout of libc++ and libc++abi, but now they link against a local
installation of these libraries so we no longer need this workaround.