[CMake] Fix DynamicLibraryTests exports symbol when plugins are enabled #102941
Conversation
This file contains hidden or 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
Created using spr 1.3.5
llvmbot
added
cmake
|
@llvm/pr-subscribers-llvm-support Author: Steven Wu (cachemeifyoucan) ChangesExporting symbols from tools directory never worked with Full diff: https://github.com/llvm/llvm-project/pull/102941.diff 3 Files Affected:
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 3e7e3a965559af..99959ecfae9cba 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -1071,7 +1071,7 @@ macro(add_llvm_executable name)
if (DEFINED LLVM_ENABLE_EXPORTED_SYMBOLS_IN_EXECUTABLES AND
NOT LLVM_ENABLE_EXPORTED_SYMBOLS_IN_EXECUTABLES AND
- NOT ARG_EXPORT_SYMBOLS AND NOT ARG_EXPORT_SYMBOLS_FOR_PLUGINS)
+ NOT ARG_EXPORT_SYMBOLS)
if(LLVM_LINKER_SUPPORTS_NO_EXPORTED_SYMBOLS)
set_property(TARGET ${name} APPEND_STRING PROPERTY
LINK_FLAGS " -Wl,-no_exported_symbols")
diff --git a/llvm/unittests/Support/DynamicLibrary/CMakeLists.txt b/llvm/unittests/Support/DynamicLibrary/CMakeLists.txt
index d8dff1ef4a3f77..e3b44a219edfb7 100644
--- a/llvm/unittests/Support/DynamicLibrary/CMakeLists.txt
+++ b/llvm/unittests/Support/DynamicLibrary/CMakeLists.txt
@@ -18,9 +18,9 @@ set_output_directory(DynamicLibraryLib
add_llvm_unittest(DynamicLibraryTests
DynamicLibraryTest.cpp
- EXPORT_SYMBOLS
)
target_link_libraries(DynamicLibraryTests PRIVATE DynamicLibraryLib)
+set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/DynamicLibraryTests.exports)
function(dynlib_add_module NAME)
add_library(${NAME} MODULE
diff --git a/llvm/unittests/Support/DynamicLibrary/DynamicLibraryTests.exports b/llvm/unittests/Support/DynamicLibrary/DynamicLibraryTests.exports
new file mode 100644
index 00000000000000..a9122211071ee6
--- /dev/null
+++ b/llvm/unittests/Support/DynamicLibrary/DynamicLibraryTests.exports
@@ -0,0 +1 @@
+TestA
|
|
Second attempt to fix: https://lab.llvm.org/buildbot/#/builders/64/builds/643/steps/6/logs/FAIL__LLVM-Unit__DynamicLibraryTests_DynamicLibrar I have no idea how that used to work on AIX builder though. |
Created using spr 1.3.5
|
I tested this change on AIX and |
jakeegan
approved these changes
Aug 13, 2024
cachemeifyoucan
deleted the
users/cachemeifyoucan/spr/cmake-fix-dynamiclibrarytests-exports-symbol-when-plugins-are-enabled
branch
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/164/builds/2011 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/2105 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/25/builds/1626 Here is the relevant piece of the build log for the reference: |
|
I guess sanitizer build needs to export more symbols :( Might need to revert again. Need to think a better solution. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/1549 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/1267 Here is the relevant piece of the build log for the reference: |
|
I reverted. @jakeegan @qiongsiwu I might need some help to figure out why currently it fails on AIX to figure out a fix. Specifically, what is the difference made by #102671 this patch? Does it generate a different export file on AIX and how is it different? That might also help me to understand how the PR mentioned above can break seemingly unrelated tests. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/85/builds/1077 Here is the relevant piece of the build log for the reference: |
|
Reland previous fix as 2596464 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/863 Here is the relevant piece of the build log for the reference: |
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.
Exporting symbols from tools directory never worked with
LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On. When that options is on, onlysymbols from the static library is linked is exported due to the export
symbols computation script. DynamicLibraryTests needs to export a symbol
from the tools/executable directory, so update it to use export list
instead.