[libcxx-commits] [libcxx] 5d79664 - [take 2] [libc++] Include <__config_site> from <__config>
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Oct 21 07:41:09 PDT 2020
Author: Louis Dionne
Date: 2020-10-21T10:40:33-04:00
New Revision: 5d796645d6c8cadeb003715c33e231a8ba05b6de
URL: https://github.com/llvm/llvm-project/commit/5d796645d6c8cadeb003715c33e231a8ba05b6de
DIFF: https://github.com/llvm/llvm-project/commit/5d796645d6c8cadeb003715c33e231a8ba05b6de.diff
LOG: [take 2] [libc++] Include <__config_site> from <__config>
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.
It does change the workflow for testing header-only changes to libc++.
Previously, we would run `lit` against the headers in libcxx/include.
After this patch, we run it against a fake installation root of the
headers (containing a proper <__config_site> header). This makes use
closer to testing what we actually install, which is good, however it
does mean that we have to update that root before testing header changes.
Thus, we now need to run `ninja check-cxx-deps` before running `lit` by
hand.
This commit was originally applied in 1e46d1aa3 and reverted in eb60c487
because it broke the libc++abi and libunwind test suites. This has now
been fixed.
Differential Revision: https://reviews.llvm.org/D89041
Added:
Modified:
libcxx/CMakeLists.txt
libcxx/docs/TestingLibcxx.rst
libcxx/include/CMakeLists.txt
libcxx/include/__config
libcxx/test/configs/legacy.cfg.in
libcxx/utils/libcxx/test/config.py
libcxxabi/CMakeLists.txt
libcxxabi/src/CMakeLists.txt
libcxxabi/test/libcxxabi/test/config.py
libcxxabi/test/lit.site.cfg.in
libunwind/test/libunwind/test/config.py
Removed:
################################################################################
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 015a359bfb48..bdfecf6c0c60 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -31,6 +31,7 @@ set(CMAKE_MODULE_PATH
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 (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR OR LIBCXX_STANDALONE_BUILD)
project(libcxx CXX C)
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index d42becfd5c3d..ec017e23b147 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -26,8 +26,8 @@ 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
@@ -37,6 +37,12 @@ whether the required libraries have been built, you can use the
$ <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
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 7c97db41bb73..a8d6f74ea38f 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -2,6 +2,7 @@ set(files
__bit_reference
__bsd_locale_defaults.h
__bsd_locale_fallbacks.h
+ __config
__errc
__debug
__functional_03
@@ -184,62 +185,28 @@ if(LIBCXX_INSTALL_SUPPORT_HEADERS)
)
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_
diff erent ${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_
diff erent ${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)
@@ -255,11 +222,10 @@ if (LIBCXX_INSTALL_HEADERS)
)
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)
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 8d2ab2e51b11..d99e972004b8 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -10,6 +10,8 @@
#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
diff --git a/libcxx/test/configs/legacy.cfg.in b/libcxx/test/configs/legacy.cfg.in
index f0a4e8a73e09..148ac3fc5a89 100644
--- a/libcxx/test/configs/legacy.cfg.in
+++ b/libcxx/test/configs/legacy.cfg.in
@@ -3,6 +3,7 @@
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@"
diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index 0d21aa17afd2..c5050d1b6b9a 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -331,7 +331,6 @@ def configure_default_compile_flags(self):
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 += [
@@ -348,16 +347,12 @@ def configure_compile_flags_header_includes(self):
'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',
@@ -365,16 +360,6 @@ def configure_compile_flags_header_includes(self):
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()
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index 3f37dbf4cd36..f0593bc2a076 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -138,10 +138,6 @@ if (NOT LIBCXXABI_ENABLE_SHARED AND NOT LIBCXXABI_ENABLE_STATIC)
message(FATAL_ERROR "libc++abi must be built as either a shared or static library.")
endif()
-set(LIBCXXABI_LIBCXX_INCLUDES "${LIBCXXABI_LIBCXX_PATH}/include" CACHE PATH
- "Specify path to libc++ includes.")
-message(STATUS "Libc++abi will be using libc++ includes from ${LIBCXXABI_LIBCXX_INCLUDES}")
-
option(LIBCXXABI_HERMETIC_STATIC_LIBRARY
"Do not export any symbols from the static library." OFF)
diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
index e9e454082a05..8d4d84c225e2 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -55,8 +55,6 @@ if (MSVC_IDE OR XCODE)
endif()
endif()
-include_directories("${LIBCXXABI_LIBCXX_INCLUDES}")
-
if (LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
add_definitions(-DHAVE___CXA_THREAD_ATEXIT_IMPL)
endif()
@@ -168,7 +166,7 @@ if (LIBCXXABI_ENABLE_SHARED)
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()
@@ -233,7 +231,7 @@ 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()
diff --git a/libcxxabi/test/libcxxabi/test/config.py b/libcxxabi/test/libcxxabi/test/config.py
index 45fb0f5d7afc..4d44d88ccc36 100644
--- a/libcxxabi/test/libcxxabi/test/config.py
+++ b/libcxxabi/test/libcxxabi/test/config.py
@@ -56,10 +56,9 @@ def configure_compile_flags(self):
super(Configuration, self).configure_compile_flags()
def configure_compile_flags_header_includes(self):
- self.configure_config_site_header()
cxx_headers = self.get_lit_conf(
'cxx_headers',
- os.path.join(self.libcxx_src_root, '/include'))
+ os.path.join(self.libcxx_obj_root, 'include', 'c++', 'v1'))
if cxx_headers == '':
self.lit_config.note('using the systems c++ headers')
else:
diff --git a/libcxxabi/test/lit.site.cfg.in b/libcxxabi/test/lit.site.cfg.in
index 87f955e32161..a6d43c6e2da1 100644
--- a/libcxxabi/test/lit.site.cfg.in
+++ b/libcxxabi/test/lit.site.cfg.in
@@ -9,7 +9,6 @@ config.libcxxabi_src_root = "@LIBCXXABI_SOURCE_DIR@"
config.libcxxabi_obj_root = "@LIBCXXABI_BINARY_DIR@"
config.abi_library_path = "@LIBCXXABI_LIBRARY_DIR@"
config.libcxx_src_root = "@LIBCXXABI_LIBCXX_PATH@"
-config.cxx_headers = "@LIBCXXABI_LIBCXX_INCLUDES@"
config.libunwind_headers = "@LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL@"
config.cxx_library_root = "@LIBCXXABI_LIBCXX_LIBRARY_PATH@"
config.llvm_unwinder = @LIBCXXABI_USE_LLVM_UNWINDER@
diff --git a/libunwind/test/libunwind/test/config.py b/libunwind/test/libunwind/test/config.py
index 977f9a0fb3f9..183c48f6a801 100644
--- a/libunwind/test/libunwind/test/config.py
+++ b/libunwind/test/libunwind/test/config.py
@@ -49,8 +49,6 @@ def configure_compile_flags(self):
super(Configuration, self).configure_compile_flags()
def configure_compile_flags_header_includes(self):
- self.configure_config_site_header()
-
libunwind_headers = self.get_lit_conf(
'libunwind_headers',
os.path.join(self.libunwind_src_root, 'include'))
More information about the libcxx-commits
mailing list