llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-libcxx Author: Nick Begg (neek78) <details> <summary>Changes</summary> Fix libc++.modules.json location errors when LIBCXX_INSTALL_LIBRARY_PATH is set to something non-default. Separates out some logic to new cmake script cmake/Modules/GetLibCxxInstallDir.cmake which is shared by both Clang and Libc++ libc++.manifest.json gets installed in LIBCXX_INSTALL_LIBRARY_DIR, which can arbitrarily be set during build (for example, this is the case in Homebrew's install), or through some non-trivial logic in libcxx/CMakeLists.txt. The latter has not even run yet when building Clang, so there's no easy way to know what this value will be reliably inside Driver.cpp right now. Because of this -print-file-name=libc++.modules.json and -print-library-module-manifest-path don't work when LIBCXX_INSTALL_LIBRARY_DIR is set to a non-default value. This is my first bash at fixing this; I've separated the logic for setting LIBCXX_INSTALL_LIBRARY_DIR into a new script, that is used by both Clang and Libc++. LIBCXX_INSTALL_LIBRARY_DIR is now visible to Driver.cpp (via LibCxxDir), which I'd guess will be useful for other things. It's also worth noting that the present logic uses the current directory as a search path, so if there happens to be a file named libc++.modules.json in the current dir, it'll return that rather than the configured version. There's a few other cases where something similar and somewhat unpredictable happens along these lines - this is the case with and without my patch. I couldn't work out how to add a new regression test to check this, as it doesn't seem the required CMake vars are visible in lit - please correct me if I'm wrong. --- Full diff: https://github.com/llvm/llvm-project/pull/176772.diff 8 Files Affected: - (modified) clang/CMakeLists.txt (+7) - (modified) clang/include/clang/Config/config.h.cmake (+3) - (modified) clang/include/clang/Driver/Driver.h (+3) - (modified) clang/include/clang/Options/OptionUtils.h (+4) - (modified) clang/lib/Driver/Driver.cpp (+11) - (modified) clang/lib/Options/OptionUtils.cpp (+16) - (added) cmake/Modules/GetLibCxxInstallDir.cmake (+28) - (modified) libcxx/CMakeLists.txt (+11-4) ``````````diff diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index e4cb1a359620d..149f28e461d0c 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -24,6 +24,7 @@ list(INSERT CMAKE_MODULE_PATH 0 # Must go below project(..) include(GNUInstallDirs) include(GetDarwinLinkerVersion) +include(GetLibCxxInstallDir) if(CLANG_BUILT_STANDALONE) set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to") @@ -906,6 +907,12 @@ endif() set(CLANG_INSTALL_LIBDIR_BASENAME "lib${CLANG_LIBDIR_SUFFIX}") +if(NOT DEFINED LIBCXX_INSTALL_LIBRARY_DIR) + get_libcxx_install_dir(LIBCXX_INSTALL_LIBRARY_DIR) +endif() + +message(STATUS "Clang LIBCXX_INSTALL_LIBRARY_DIR: ${LIBCXX_INSTALL_LIBRARY_DIR}") + configure_file( ${CLANG_SOURCE_DIR}/include/clang/Config/config.h.cmake ${CLANG_BINARY_DIR}/include/clang/Config/config.h) diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 00c352b458c34..d2b6950fb51eb 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -38,6 +38,9 @@ /* Relative directory for resource files */ #define CLANG_RESOURCE_DIR "${CLANG_RESOURCE_DIR}" +/* Directory in which libc++ is installed */ +#cmakedefine LIBCXX_INSTALL_LIBRARY_DIR "${LIBCXX_INSTALL_LIBRARY_DIR}" + /* Directories clang will search for headers */ #define C_INCLUDE_DIRS "${C_INCLUDE_DIRS}" diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index b7bd1bc8aab49..0d723f0d83ac9 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -194,6 +194,9 @@ class Driver { /// User directory for config files. std::string UserConfigDir; + /// directory where Libc++ is installed + std::string LibCxxDir; + /// A prefix directory used to emulate a limited subset of GCC's '-Bprefix' /// functionality. /// FIXME: This type of customization should be removed in favor of the diff --git a/clang/include/clang/Options/OptionUtils.h b/clang/include/clang/Options/OptionUtils.h index 02c9c27554db1..0ee700f31bd8e 100644 --- a/clang/include/clang/Options/OptionUtils.h +++ b/clang/include/clang/Options/OptionUtils.h @@ -77,6 +77,10 @@ std::string GetResourcesPath(StringRef BinaryPath); /// executable), for finding the builtin compiler path. std::string GetResourcesPath(const char *Argv0, void *MainAddr); +/// Get the directory where LibC++ is installed, relative to the +/// compiler binary path \p BinaryPath. +std::string GetLibCxxPath(StringRef BinaryPath); + } // namespace clang #endif // LLVM_CLANG_OPTIONS_OPTIONUTILS_H diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index eb3f9cbea2845..c7f0120d4b7d6 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -216,6 +216,9 @@ Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple, // Compute the path to the resource directory. ResourceDir = GetResourcesPath(ClangExecutable); + + // Compute the path to Libc++'s install directory + LibCxxDir = GetLibCxxPath(ClangExecutable); } void Driver::setDriverMode(StringRef Value) { @@ -6582,6 +6585,14 @@ std::string Driver::GetFilePath(StringRef Name, const ToolChain &TC) const { if (llvm::sys::fs::exists(Twine(R2))) return std::string(R2); + // search Libc++ install dir (if configured) + // this is required for find the .modules.json manifest when libc++ is + // installed in a non-default location + SmallString<128> L(LibCxxDir); + llvm::sys::path::append(L, Name); + if (llvm::sys::fs::exists(Twine(L))) + return std::string(L); + return std::string(Name); } diff --git a/clang/lib/Options/OptionUtils.cpp b/clang/lib/Options/OptionUtils.cpp index e5aefa012f679..2a2795ac7094d 100644 --- a/clang/lib/Options/OptionUtils.cpp +++ b/clang/lib/Options/OptionUtils.cpp @@ -239,6 +239,22 @@ std::string clang::GetResourcesPath(StringRef BinaryPath) { return std::string(P); } +std::string clang::GetLibCxxPath(StringRef BinaryPath) { +#ifdef LIBCXX_INSTALL_LIBRARY_DIR + if (llvm::sys::path::is_absolute(LIBCXX_INSTALL_LIBRARY_DIR)) { + return LIBCXX_INSTALL_LIBRARY_DIR; + } else { + // based on ToolChain::getStdlibPath(), this seems to be the way + // to determine CMAKE_INSTALL_PREFIX. + auto Dir = std::string(llvm::sys::path::parent_path(BinaryPath)); + SmallString<128> C(Dir); + llvm::sys::path::append(C, "..", LIBCXX_INSTALL_LIBRARY_DIR); + return std::string(C); + } +#endif + return ""; +} + std::string clang::GetResourcesPath(const char *Argv0, void *MainAddr) { const std::string ClangExecutable = llvm::sys::fs::getMainExecutable(Argv0, MainAddr); diff --git a/cmake/Modules/GetLibCxxInstallDir.cmake b/cmake/Modules/GetLibCxxInstallDir.cmake new file mode 100644 index 0000000000000..c0dcdfd5b8588 --- /dev/null +++ b/cmake/Modules/GetLibCxxInstallDir.cmake @@ -0,0 +1,28 @@ + +# get libcxx install directory +# +# usage: +# get_libcxx_install_dir(library_dir_var) +# +# determine the path to install libc++ (usually LIBCXX_INSTALL_INCLUDE_TARGET_DIR) +# +# This logic was broken out of libcxx/CMakeLists.txt as both libcxx and clang's +# driver.cpp need to know at this +# + +function(get_libcxx_install_dir lib_dir_var) + set(LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}") + + if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(TARGET_SUBDIR ${LLVM_DEFAULT_TARGET_TRIPLE}) + if(LIBDIR_SUBDIR) + string(APPEND TARGET_SUBDIR /${LIBDIR_SUBDIR}) + endif() + set(ret_dir lib${LLVM_LIBDIR_SUFFIX}/${TARGET_SUBDIR}) + else() + set(ret_dir lib${LIBDIR_SUFFIX}) + endif() + + set(${lib_dir_var} ${ret_dir} PARENT_SCOPE) +endfunction() + diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt index 56b346415c1c6..6ab9c9c74055a 100644 --- a/libcxx/CMakeLists.txt +++ b/libcxx/CMakeLists.txt @@ -421,6 +421,8 @@ 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.") +include(GetLibCxxInstallDir) + if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) set(LIBCXX_TARGET_SUBDIR ${LLVM_DEFAULT_TARGET_TRIPLE}) if(LIBCXX_LIBDIR_SUBDIR) @@ -431,8 +433,6 @@ if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) 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_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 "Path where target-specific libc++ headers should be installed.") unset(LIBCXX_TARGET_SUBDIR) @@ -447,12 +447,19 @@ else() 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 - "Path where built libc++ libraries should be installed.") set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR "${LIBCXX_INSTALL_INCLUDE_DIR}" CACHE STRING "Path where target-specific libc++ headers should be installed.") endif() +if(NOT DEFINED LIBCXX_INSTALL_LIBRARY_DIR) + get_libcxx_install_dir(LIBCXX_INSTALL_LIBRARY_DIR) +endif() + +set(LIBCXX_INSTALL_LIBRARY_DIR "${LIBCXX_INSTALL_LIBRARY_DIR}" CACHE STRING + "Path where built libc++ libraries should be installed.") + +MESSAGE(STATUS "libc++ LIBCXX_INSTALL_LIBRARY_DIR: ${LIBCXX_INSTALL_LIBRARY_DIR}") + set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBCXX_LIBRARY_DIR}) set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBCXX_LIBRARY_DIR}) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBCXX_LIBRARY_DIR}) `````````` </details> https://github.com/llvm/llvm-project/pull/176772 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
