-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[libcxx] [test] Fix restoring LLVM_DIR and Clang_DIR #132838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
In 664f345, a fix was introduced, attempting to restore LLVM_DIR and Clang_DIR after doing find_package(Clang). However, 6775285 added a return if the clangTidy target wasn't found. If this is hit, we don't restore LLVM_DIR and Clang_DIR, which causes strange effects if CMake is rerun a second time. Move the code for restoring LLVM_DIR and Clang_DIR to directly after the find_package calls, to make sure they are restored, regardless of the find_package outcome.
@llvm/pr-subscribers-libcxx Author: Martin Storsjö (mstorsjo) ChangesIn 664f345, a fix was introduced, attempting to restore LLVM_DIR and Clang_DIR after doing find_package(Clang). However, 6775285 added a return if the clangTidy target wasn't found. If this is hit, we don't restore LLVM_DIR and Clang_DIR, which causes strange effects if CMake is rerun a second time. Move the code for restoring LLVM_DIR and Clang_DIR to directly after the find_package calls, to make sure they are restored, regardless of the find_package outcome. Full diff: https://github.com/llvm/llvm-project/pull/132838.diff 1 Files Affected:
diff --git a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
index f8b523ec0ba93..5797a32974820 100644
--- a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
+++ b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
@@ -8,6 +8,10 @@ set(Clang_DIR_SAVE ${Clang_DIR})
# versions must match. Otherwise there likely will be ODR-violations. This had
# led to crashes and incorrect output of the clang-tidy based checks.
find_package(Clang ${CMAKE_CXX_COMPILER_VERSION})
+
+set(LLVM_DIR "${LLVM_DIR_SAVE}" CACHE PATH "The directory containing a CMake configuration file for LLVM." FORCE)
+set(Clang_DIR "${Clang_DIR_SAVE}" CACHE PATH "The directory containing a CMake configuration file for Clang." FORCE)
+
if(NOT Clang_FOUND)
message(STATUS "Clang-tidy tests are disabled since the "
"Clang development package is unavailable.")
@@ -19,9 +23,6 @@ if(NOT TARGET clangTidy)
return()
endif()
-set(LLVM_DIR "${LLVM_DIR_SAVE}" CACHE PATH "The directory containing a CMake configuration file for LLVM." FORCE)
-set(Clang_DIR "${Clang_DIR_SAVE}" CACHE PATH "The directory containing a CMake configuration file for Clang." FORCE)
-
message(STATUS "Found system-installed LLVM ${LLVM_PACKAGE_VERSION} with headers in ${LLVM_INCLUDE_DIRS}")
set(CMAKE_CXX_STANDARD 20)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM!
/cherry-pick 51bceb4 |
/pull-request #133153 |
In 664f345, a fix was introduced, attempting to restore LLVM_DIR and Clang_DIR after doing find_package(Clang). However, 6775285 added a return if the clangTidy target wasn't found. If this is hit, we don't restore LLVM_DIR and Clang_DIR, which causes strange effects if CMake is rerun a second time. Move the code for restoring LLVM_DIR and Clang_DIR to directly after the find_package calls, to make sure they are restored, regardless of the find_package outcome. (cherry picked from commit 51bceb4)
In 664f345, a fix was introduced, attempting to restore LLVM_DIR and Clang_DIR after doing find_package(Clang).
However, 6775285 added a return if the clangTidy target wasn't found. If this is hit, we don't restore LLVM_DIR and Clang_DIR, which causes strange effects if CMake is rerun a second time.
Move the code for restoring LLVM_DIR and Clang_DIR to directly after the find_package calls, to make sure they are restored, regardless of the find_package outcome.