[lld] Fixes for unitests in standalone builds #83670
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Inline and update the rule for adding unittest directory, to use the modern approach of checking for `llvm_gtest` target rather than using the `llvm_add_unittests()` function. The latter used a check for a header file from the third-party tree that is no longer necessary nor used in standalone builds. This change also eliminates the last use of this function, making it possible to remove it. As a side effect, LLD_UNITTESTS_ADDED is no longer defined. However, it is not used anywhere.
Remove the obsolete `llvm_add_unittests()` CMake function. It is no longer used anywhere.
Add `LLDUnitTests` target to the test dependencies independently of whether a standalone or non-standalone build is performed. This target is defined unconditionally, and without it unittests would be skipped silently. It was probably accidentally added under the `NOT LLD_BUILT_STANDALONE` condition.
|
@llvm/pr-subscribers-lld Author: Michał Górny (mgorny) Changes
Full diff: https://github.com/llvm/llvm-project/pull/83670.diff 3 Files Affected:
diff --git a/lld/CMakeLists.txt b/lld/CMakeLists.txt
index 595c286abd91a6..cd8ba627306edb 100644
--- a/lld/CMakeLists.txt
+++ b/lld/CMakeLists.txt
@@ -192,7 +192,9 @@ add_subdirectory(tools/lld)
if (LLVM_INCLUDE_TESTS)
add_custom_target(LLDUnitTests)
- llvm_add_unittests(LLD_UNITTESTS_ADDED)
+ if (TARGET llvm_gtest)
+ add_subdirectory(unittests)
+ endif()
add_subdirectory(test)
endif()
diff --git a/lld/test/CMakeLists.txt b/lld/test/CMakeLists.txt
index 558da2b4421a2c..bb6164f19dcef3 100644
--- a/lld/test/CMakeLists.txt
+++ b/lld/test/CMakeLists.txt
@@ -38,11 +38,10 @@ configure_lit_site_cfg(
"LLD_SOURCE_DIR"
)
-set(LLD_TEST_DEPS lld)
+set(LLD_TEST_DEPS lld LLDUnitTests)
if (NOT LLD_BUILT_STANDALONE)
list(APPEND LLD_TEST_DEPS
FileCheck
- LLDUnitTests
count
dsymutil
llc
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 0f1734a64ee6e6..828de4bd9940d6 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -2509,13 +2509,3 @@ function(setup_host_tool tool_name setting_name exe_var_name target_var_name)
add_custom_target(${target_var_name} DEPENDS ${exe_name})
endif()
endfunction()
-
-# Adds the unittests folder if gtest is available.
-function(llvm_add_unittests tests_added)
- if (EXISTS ${LLVM_THIRD_PARTY_DIR}/unittest/googletest/include/gtest/gtest.h)
- add_subdirectory(unittests)
- set(${tests_added} ON PARENT_SCOPE)
- else()
- message(WARNING "gtest not found, unittests will not be available")
- endif()
-endfunction()
|
aganea
approved these changes
Mar 2, 2024
MaskRay
approved these changes
Mar 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
llvm_add_unittests()CMake function with an explicit check forTARGET llvm_gtest. This is more consistent with the rest of the code, and it makes it possible to avoid checking outthird-partytree.LLDUnitTeststest dependency to standalone builds. It is defined unconditionally, and actually necessary to ensure that unit tests will be built.