Skip to content

[lldb] build: cleanup extraneous include paths #117615

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

Merged
merged 1 commit into from
Nov 27, 2024
Merged

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Nov 25, 2024

Clean up some unnecessary include paths. The use of LibXml2::LibXml2
with target_link_libraries on libLLDBHost ensures that the header
search path is properly propagated.

@llvmbot
Copy link
Member

llvmbot commented Nov 25, 2024

@llvm/pr-subscribers-lldb

Author: Saleem Abdulrasool (compnerd)

Changes

The find_package(LibXml2 ...) invocation that we are currently using precludes the use of "CONFIG mode" for libxml2. This is important to allow dependencies to flow through the build with static builds on Windows, which depends on Bcrypt and conditionally on Ws2_32 (in development - current releases are unconditionally dependent on it). If libxml2 is built statically, this dependency would need to be replicated into LLDB's build configuration to ensure that the dependencies are met.

Add a workaround to support CONFIG mode search which avoids this need. Additionally, clean up some unnecessary include paths. The use of LibXml2::LibXml2 with target_link_libraries on libLLDBHost ensures that the header search path is properly propagated.


Full diff: https://github.com/llvm/llvm-project/pull/117615.diff

1 Files Affected:

  • (modified) lldb/cmake/modules/LLDBConfig.cmake (+11-6)
diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake
index 93ccd9c479c2b8..ba3ce6c444f887 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -57,7 +57,17 @@ add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" Curse
 add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in LLDB" LibLZMA LIBLZMA_FOUND)
 add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB" LuaAndSwig LUAANDSWIG_FOUND)
 add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in LLDB" PythonAndSwig PYTHONANDSWIG_FOUND)
-add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8)
+# LibXml2 is required if LLDB_ENABLE_LIBXML2 is ON or AUTO. The LibXml2 package
+# search is not setup properly to support the CONFIG mode search. Furthermore,
+# in CONFIG mode, we cannot specify the version as that is an exact match. As a
+# mitigation, perform a CONFIG mode search for LibXml2 if LLDB_ENABLE_LIBXML2
+# is ON or AUTO, and fallback to the old path if not found.
+if(LLDB_ENABLE_LIBXML2 OR "${LLDB_ENABLE_LIBXML2}" STREQUAL "AUTO")
+  find_package(LibXml2 CONFIG)
+  if(NOT TARGET LibXml2::LibXml2)
+    add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8)
+  endif()
+endif()
 add_optional_dependency(LLDB_ENABLE_FBSDVMCORE "Enable libfbsdvmcore support in LLDB" FBSDVMCore FBSDVMCore_FOUND QUIET)
 
 option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON)
@@ -239,10 +249,6 @@ if (LLDB_ENABLE_LZMA)
   include_directories(${LIBLZMA_INCLUDE_DIRS})
 endif()
 
-if (LLDB_ENABLE_LIBXML2)
-  include_directories(${LIBXML2_INCLUDE_DIR})
-endif()
-
 include_directories(BEFORE
   ${CMAKE_CURRENT_BINARY_DIR}/include
   ${CMAKE_CURRENT_SOURCE_DIR}/include
@@ -283,7 +289,6 @@ if (APPLE)
   find_library(FOUNDATION_LIBRARY Foundation)
   find_library(CORE_FOUNDATION_LIBRARY CoreFoundation)
   find_library(SECURITY_LIBRARY Security)
-  include_directories(${LIBXML2_INCLUDE_DIR})
 endif()
 
 if( WIN32 AND NOT CYGWIN )

Clean up some unnecessary include paths. The use of `LibXml2::LibXml2`
with `target_link_libraries` on `libLLDBHost` ensures that the header
search path is properly propagated.
@compnerd compnerd changed the title build: enable CONFIG mode search for LibXml2 for LLDB build: remove extraneous search paths for LibXml2 Nov 27, 2024
@DavidSpickett DavidSpickett changed the title build: remove extraneous search paths for LibXml2 [lldb] build: remove extraneous search paths for LibXml2 Nov 27, 2024
@DavidSpickett
Copy link
Collaborator

I don't have any objections to this.

Please update the PR description before landing, it will be used as the commit message.

@compnerd compnerd changed the title [lldb] build: remove extraneous search paths for LibXml2 [lldb] build: cleanup extraneous include paths Nov 27, 2024
@compnerd compnerd merged commit 24593f1 into llvm:main Nov 27, 2024
7 checks passed
@compnerd compnerd deleted the lldb-xml2 branch November 27, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants