-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb] Enforce that only the LLDB API unit tests can link liblldb #162384
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
Enforce that only specific tests can link liblldb. All the other unit tests statically link the private libraries. Linking both the static libraries and liblldb results in duplicated symbols. Fixes llvm#162378
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesEnforce that only specific tests can link liblldb. All the other unit tests statically link the private libraries. Linking both the static libraries and liblldb results in duplicated symbols. Fixes #162378 Full diff: https://github.com/llvm/llvm-project/pull/162384.diff 3 Files Affected:
diff --git a/lldb/unittests/API/CMakeLists.txt b/lldb/unittests/API/CMakeLists.txt
index 06ac49244176c..ea140a23af605 100644
--- a/lldb/unittests/API/CMakeLists.txt
+++ b/lldb/unittests/API/CMakeLists.txt
@@ -3,6 +3,8 @@ add_lldb_unittest(APITests
SBLineEntryTest.cpp
SBMutexTest.cpp
+ SBAPITEST
+
LINK_LIBS
liblldb
)
diff --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt
index 4c5267ae25b74..194dd425430e2 100644
--- a/lldb/unittests/CMakeLists.txt
+++ b/lldb/unittests/CMakeLists.txt
@@ -12,7 +12,7 @@ endif()
function(add_lldb_unittest test_name)
cmake_parse_arguments(ARG
- ""
+ "SBAPITEST"
""
"LINK_LIBS;LINK_COMPONENTS"
${ARGN})
@@ -21,6 +21,10 @@ function(add_lldb_unittest test_name)
message(FATAL_ERROR "Unit test name must end with 'Tests' for lit to find it.")
endif()
+ if ("liblldb" IN_LIST ARG_LINK_LIBS AND NOT ARG_SBAPITEST)
+ message(FATAL_ERROR "The ${test_name} are not allowed to link liblldb.")
+ endif()
+
list(APPEND LLVM_LINK_COMPONENTS ${ARG_LINK_COMPONENTS})
add_unittest(LLDBUnitTests
diff --git a/lldb/unittests/DAP/CMakeLists.txt b/lldb/unittests/DAP/CMakeLists.txt
index 716159b454231..a08414c30e6cd 100644
--- a/lldb/unittests/DAP/CMakeLists.txt
+++ b/lldb/unittests/DAP/CMakeLists.txt
@@ -12,6 +12,8 @@ add_lldb_unittest(DAPTests
TestBase.cpp
VariablesTest.cpp
+ SBAPITEST
+
LINK_COMPONENTS
Support
LINK_LIBS
|
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.
Great idea. I wish CMake had a mechanism to enforce these kinds of linking constraints.
We're seeing this after this patch: - Performing Test CXX_SUPPORTS_NO_DOCUMENTATION_DEPRECATED_SYNC - Success
CMake Error at /b/s/w/ir/x/w/llvm-llvm-project/lldb/unittests/CMakeLists.txt:25 (message):
The LLDBBreakpointTests are not allowed to link liblldb.
Call Stack (most recent call first):
/b/s/w/ir/x/w/llvm-llvm-project/lldb/unittests/Breakpoint/CMakeLists.txt:1 (add_lldb_unittest)
Seems related to this change. would you mind taking a look? Bot: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/lldb-linux-x64/b8701546007217978577/overview |
Looks like Jim forgot to unstage these additions in #162370. I'm removing them. |
Pre-commit CI is also broken with the same error. |
Fixed by 1395d43 |
…vm#162384) Enforce that only specific tests can link liblldb. All the other unit tests statically link the private libraries. Linking both the static libraries and liblldb results in duplicated symbols. Fixes llvm#162378 (cherry picked from commit 02572c6)
…vm#162384) Enforce that only specific tests can link liblldb. All the other unit tests statically link the private libraries. Linking both the static libraries and liblldb results in duplicated symbols. Fixes llvm#162378
Enforce that only specific tests can link liblldb. All the other unit tests statically link the private libraries. Linking both the static libraries and liblldb results in duplicated symbols.
Fixes #162378