-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[cmake] Add support for statically linking libxml2 #166867
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
base: main
Are you sure you want to change the base?
[cmake] Add support for statically linking libxml2 #166867
Conversation
|
@llvm/pr-subscribers-lldb Author: Keith Smiley (keith) ChangesDynamically depending on libxml2 results in various annoyances across With this, enabled by default for releases, we don't sacrifice any macOS ignores this setting since libxml2 is part of the OS and stable This mirrors what we do for zstd Fixes #113696 Full diff: https://github.com/llvm/llvm-project/pull/166867.diff 6 Files Affected:
diff --git a/clang/cmake/caches/Release.cmake b/clang/cmake/caches/Release.cmake
index 25f7970119d07..45343c66dfb7f 100644
--- a/clang/cmake/caches/Release.cmake
+++ b/clang/cmake/caches/Release.cmake
@@ -171,6 +171,7 @@ set_final_stage_var(CPACK_GENERATOR "TXZ" STRING)
set_final_stage_var(CPACK_ARCHIVE_THREADS "0" STRING)
set_final_stage_var(LLVM_USE_STATIC_ZSTD "ON" BOOL)
+set_final_stage_var(LLVM_USE_STATIC_LIBXML2 "ON" BOOL)
if (LLVM_RELEASE_ENABLE_LTO)
set_final_stage_var(LLVM_ENABLE_FATLTO "ON" BOOL)
set_final_stage_var(CPACK_PRE_BUILD_SCRIPTS "${CMAKE_CURRENT_LIST_DIR}/release_cpack_pre_build_strip_lto.cmake" STRING)
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index c9e8afe48fcde..4054db77bed5a 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -152,7 +152,7 @@ set(EXTRA_LIBS)
if (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
list(APPEND EXTRA_LIBS kvm)
endif()
-if (LLDB_ENABLE_LIBXML2)
+if (LLDB_ENABLE_LIBXML2 AND NOT LLVM_USE_STATIC_LIBXML2)
list(APPEND EXTRA_LIBS LibXml2::LibXml2)
endif()
if (HAVE_LIBDL)
@@ -190,3 +190,8 @@ add_lldb_library(lldbHost NO_PLUGIN_DEPENDENCIES
${LLDB_LIBEDIT_LIBS}
)
+if (LLDB_ENABLE_LIBXML2 AND LLVM_USE_STATIC_LIBXML2)
+ target_link_libraries(lldbHost PRIVATE ${LIBXML2_LIBRARIES})
+ target_include_directories(lldbHost PRIVATE ${LIBXML2_INCLUDE_DIRS})
+ add_dependencies(lldbHost libxml2)
+endif()
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index f192cd05b5a34..45ed2ca0e92d0 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -605,6 +605,8 @@ set(LLVM_TARGET_ARCH "host"
set(LLVM_ENABLE_LIBXML2 "ON" CACHE STRING "Use libxml2 if available. Can be ON, OFF, or FORCE_ON")
+set(LLVM_USE_STATIC_LIBXML2 "OFF" CACHE BOOL "Use static version of libxml2. Can be ON, or OFF")
+
option(LLVM_ENABLE_LIBEDIT "Use libedit if available." ON)
option(LLVM_ENABLE_LIBPFM "Use libpfm for performance counters if available." ON)
diff --git a/llvm/cmake/config-ix.cmake b/llvm/cmake/config-ix.cmake
index ed2bfa6df68f4..d374226214fe7 100644
--- a/llvm/cmake/config-ix.cmake
+++ b/llvm/cmake/config-ix.cmake
@@ -210,22 +210,31 @@ if(LLVM_ENABLE_ZSTD)
endif()
if(LLVM_ENABLE_LIBXML2)
- if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON)
- find_package(LibXml2 REQUIRED)
- elseif(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
- find_package(LibXml2)
+ if(APPLE)
+ set(LLVM_USE_STATIC_LIBXML2 OFF)
endif()
- if(LibXml2_FOUND)
- # Check if libxml2 we found is usable; for example, we may have found a 32-bit
- # library on a 64-bit system which would result in a link-time failure.
- cmake_push_check_state()
- list(APPEND CMAKE_REQUIRED_INCLUDES ${LIBXML2_INCLUDE_DIRS})
- list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBXML2_LIBRARIES})
- list(APPEND CMAKE_REQUIRED_DEFINITIONS ${LIBXML2_DEFINITIONS})
- check_symbol_exists(xmlReadMemory libxml/xmlreader.h HAVE_LIBXML2)
- cmake_pop_check_state()
- if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON AND NOT HAVE_LIBXML2)
- message(FATAL_ERROR "Failed to configure libxml2")
+
+ if(LLVM_USE_STATIC_LIBXML2)
+ include(LibXml2)
+ set(HAVE_LIBXML2 1)
+ else()
+ if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON)
+ find_package(LibXml2 REQUIRED)
+ elseif(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
+ find_package(LibXml2)
+ endif()
+ if(LibXml2_FOUND)
+ # Check if libxml2 we found is usable; for example, we may have found a 32-bit
+ # library on a 64-bit system which would result in a link-time failure.
+ cmake_push_check_state()
+ list(APPEND CMAKE_REQUIRED_INCLUDES ${LIBXML2_INCLUDE_DIRS})
+ list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBXML2_LIBRARIES})
+ list(APPEND CMAKE_REQUIRED_DEFINITIONS ${LIBXML2_DEFINITIONS})
+ check_symbol_exists(xmlReadMemory libxml/xmlreader.h HAVE_LIBXML2)
+ cmake_pop_check_state()
+ if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON AND NOT HAVE_LIBXML2)
+ message(FATAL_ERROR "Failed to configure libxml2")
+ endif()
endif()
endif()
set(LLVM_ENABLE_LIBXML2 "${HAVE_LIBXML2}")
diff --git a/llvm/cmake/modules/LibXml2.cmake b/llvm/cmake/modules/LibXml2.cmake
new file mode 100644
index 0000000000000..1d967d70e81b9
--- /dev/null
+++ b/llvm/cmake/modules/LibXml2.cmake
@@ -0,0 +1,46 @@
+include(ExternalProject)
+
+if (NOT LIBXML2_PREFIX)
+ set (LIBXML2_PREFIX libxml2)
+endif()
+
+set(LIBXML2_PATH ${CMAKE_CURRENT_BINARY_DIR}/${LIBXML2_PREFIX}/src/${LIBXML2_PREFIX})
+set(LIBXML2_LIB_PATH ${LIBXML2_PATH}-build/libxml2.a)
+
+ExternalProject_Add(${LIBXML2_PREFIX}
+ PREFIX ${LIBXML2_PREFIX}
+ GIT_REPOSITORY https://github.com/GNOME/libxml2.git
+ GIT_TAG v2.15.1
+ CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
+ -DBUILD_SHARED_LIBS=OFF
+ -DLIBXML2_WITH_PYTHON=OFF
+ -DLIBXML2_WITH_PROGRAMS=OFF
+ -DLIBXML2_WITH_TESTS=OFF
+ -DLIBXML2_WITH_LZMA=OFF
+ -DLIBXML2_WITH_ZLIB=OFF
+ CMAKE_CACHE_ARGS -DCMAKE_C_COMPILER:FILEPATH=${CMAKE_C_COMPILER}
+ -DCMAKE_CXX_COMPILER:FILEPATH=${CMAKE_CXX_COMPILER}
+ -DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=ON
+ BUILD_BYPRODUCTS ${LIBXML2_LIB_PATH}
+ UPDATE_COMMAND ""
+ INSTALL_COMMAND ""
+ )
+
+set(LIBXML2_INCLUDE_DIRS ${LIBXML2_PATH}/include ${LIBXML2_PATH}-build)
+set(LIBXML2_LIBRARIES ${LIBXML2_LIB_PATH})
+set(LIBXML2_DEFINITIONS "")
+
+if(NOT TARGET LibXml2::LibXml2)
+ add_library(LibXml2_static STATIC IMPORTED GLOBAL)
+ set_target_properties(LibXml2_static PROPERTIES
+ IMPORTED_LOCATION ${LIBXML2_LIB_PATH}
+ )
+
+ add_library(LibXml2_LibXml2 INTERFACE)
+ target_link_libraries(LibXml2_LibXml2 INTERFACE LibXml2_static)
+ target_include_directories(LibXml2_LibXml2 INTERFACE ${LIBXML2_INCLUDE_DIRS})
+
+ add_dependencies(LibXml2_LibXml2 ${LIBXML2_PREFIX})
+
+ add_library(LibXml2::LibXml2 ALIAS LibXml2_LibXml2)
+endif()
diff --git a/llvm/lib/WindowsManifest/CMakeLists.txt b/llvm/lib/WindowsManifest/CMakeLists.txt
index 910132a4c7dec..0eef3a52ec1a3 100644
--- a/llvm/lib/WindowsManifest/CMakeLists.txt
+++ b/llvm/lib/WindowsManifest/CMakeLists.txt
@@ -1,6 +1,6 @@
include(GetLibraryName)
-if(LLVM_ENABLE_LIBXML2)
+if(LLVM_ENABLE_LIBXML2 AND NOT LLVM_USE_STATIC_LIBXML2)
set(imported_libs LibXml2::LibXml2)
endif()
@@ -18,17 +18,28 @@ add_llvm_component_library(LLVMWindowsManifest
Support
)
+if(LLVM_ENABLE_LIBXML2 AND LLVM_USE_STATIC_LIBXML2)
+ target_link_libraries(LLVMWindowsManifest PRIVATE ${LIBXML2_LIBRARIES})
+ target_include_directories(LLVMWindowsManifest PRIVATE ${LIBXML2_INCLUDE_DIRS})
+ add_dependencies(LLVMWindowsManifest libxml2)
+endif()
+
# This block is only needed for llvm-config. When we deprecate llvm-config and
# move to using CMake export, this block can be removed.
if(LLVM_ENABLE_LIBXML2)
- # CMAKE_BUILD_TYPE is only meaningful to single-configuration generators.
- if(CMAKE_BUILD_TYPE)
- string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
- get_property(libxml2_library TARGET LibXml2::LibXml2 PROPERTY LOCATION_${build_type})
- endif()
- if(NOT libxml2_library)
- get_property(libxml2_library TARGET LibXml2::LibXml2 PROPERTY LOCATION)
+ if(LLVM_USE_STATIC_LIBXML2)
+ # When using static libxml2 built via ExternalProject, use the library path directly
+ get_library_name(${LIBXML2_LIBRARIES} libxml2_library)
+ else()
+ # CMAKE_BUILD_TYPE is only meaningful to single-configuration generators.
+ if(CMAKE_BUILD_TYPE)
+ string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+ get_property(libxml2_library TARGET LibXml2::LibXml2 PROPERTY LOCATION_${build_type})
+ endif()
+ if(NOT libxml2_library)
+ get_property(libxml2_library TARGET LibXml2::LibXml2 PROPERTY LOCATION)
+ endif()
+ get_library_name(${libxml2_library} libxml2_library)
endif()
- get_library_name(${libxml2_library} libxml2_library)
set_property(TARGET LLVMWindowsManifest PROPERTY LLVM_SYSTEM_LIBS ${libxml2_library})
endif()
|
|
@llvm/pr-subscribers-clang Author: Keith Smiley (keith) ChangesDynamically depending on libxml2 results in various annoyances across With this, enabled by default for releases, we don't sacrifice any macOS ignores this setting since libxml2 is part of the OS and stable This mirrors what we do for zstd Fixes #113696 Full diff: https://github.com/llvm/llvm-project/pull/166867.diff 6 Files Affected:
diff --git a/clang/cmake/caches/Release.cmake b/clang/cmake/caches/Release.cmake
index 25f7970119d07..45343c66dfb7f 100644
--- a/clang/cmake/caches/Release.cmake
+++ b/clang/cmake/caches/Release.cmake
@@ -171,6 +171,7 @@ set_final_stage_var(CPACK_GENERATOR "TXZ" STRING)
set_final_stage_var(CPACK_ARCHIVE_THREADS "0" STRING)
set_final_stage_var(LLVM_USE_STATIC_ZSTD "ON" BOOL)
+set_final_stage_var(LLVM_USE_STATIC_LIBXML2 "ON" BOOL)
if (LLVM_RELEASE_ENABLE_LTO)
set_final_stage_var(LLVM_ENABLE_FATLTO "ON" BOOL)
set_final_stage_var(CPACK_PRE_BUILD_SCRIPTS "${CMAKE_CURRENT_LIST_DIR}/release_cpack_pre_build_strip_lto.cmake" STRING)
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index c9e8afe48fcde..4054db77bed5a 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -152,7 +152,7 @@ set(EXTRA_LIBS)
if (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
list(APPEND EXTRA_LIBS kvm)
endif()
-if (LLDB_ENABLE_LIBXML2)
+if (LLDB_ENABLE_LIBXML2 AND NOT LLVM_USE_STATIC_LIBXML2)
list(APPEND EXTRA_LIBS LibXml2::LibXml2)
endif()
if (HAVE_LIBDL)
@@ -190,3 +190,8 @@ add_lldb_library(lldbHost NO_PLUGIN_DEPENDENCIES
${LLDB_LIBEDIT_LIBS}
)
+if (LLDB_ENABLE_LIBXML2 AND LLVM_USE_STATIC_LIBXML2)
+ target_link_libraries(lldbHost PRIVATE ${LIBXML2_LIBRARIES})
+ target_include_directories(lldbHost PRIVATE ${LIBXML2_INCLUDE_DIRS})
+ add_dependencies(lldbHost libxml2)
+endif()
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index f192cd05b5a34..45ed2ca0e92d0 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -605,6 +605,8 @@ set(LLVM_TARGET_ARCH "host"
set(LLVM_ENABLE_LIBXML2 "ON" CACHE STRING "Use libxml2 if available. Can be ON, OFF, or FORCE_ON")
+set(LLVM_USE_STATIC_LIBXML2 "OFF" CACHE BOOL "Use static version of libxml2. Can be ON, or OFF")
+
option(LLVM_ENABLE_LIBEDIT "Use libedit if available." ON)
option(LLVM_ENABLE_LIBPFM "Use libpfm for performance counters if available." ON)
diff --git a/llvm/cmake/config-ix.cmake b/llvm/cmake/config-ix.cmake
index ed2bfa6df68f4..d374226214fe7 100644
--- a/llvm/cmake/config-ix.cmake
+++ b/llvm/cmake/config-ix.cmake
@@ -210,22 +210,31 @@ if(LLVM_ENABLE_ZSTD)
endif()
if(LLVM_ENABLE_LIBXML2)
- if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON)
- find_package(LibXml2 REQUIRED)
- elseif(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
- find_package(LibXml2)
+ if(APPLE)
+ set(LLVM_USE_STATIC_LIBXML2 OFF)
endif()
- if(LibXml2_FOUND)
- # Check if libxml2 we found is usable; for example, we may have found a 32-bit
- # library on a 64-bit system which would result in a link-time failure.
- cmake_push_check_state()
- list(APPEND CMAKE_REQUIRED_INCLUDES ${LIBXML2_INCLUDE_DIRS})
- list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBXML2_LIBRARIES})
- list(APPEND CMAKE_REQUIRED_DEFINITIONS ${LIBXML2_DEFINITIONS})
- check_symbol_exists(xmlReadMemory libxml/xmlreader.h HAVE_LIBXML2)
- cmake_pop_check_state()
- if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON AND NOT HAVE_LIBXML2)
- message(FATAL_ERROR "Failed to configure libxml2")
+
+ if(LLVM_USE_STATIC_LIBXML2)
+ include(LibXml2)
+ set(HAVE_LIBXML2 1)
+ else()
+ if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON)
+ find_package(LibXml2 REQUIRED)
+ elseif(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
+ find_package(LibXml2)
+ endif()
+ if(LibXml2_FOUND)
+ # Check if libxml2 we found is usable; for example, we may have found a 32-bit
+ # library on a 64-bit system which would result in a link-time failure.
+ cmake_push_check_state()
+ list(APPEND CMAKE_REQUIRED_INCLUDES ${LIBXML2_INCLUDE_DIRS})
+ list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBXML2_LIBRARIES})
+ list(APPEND CMAKE_REQUIRED_DEFINITIONS ${LIBXML2_DEFINITIONS})
+ check_symbol_exists(xmlReadMemory libxml/xmlreader.h HAVE_LIBXML2)
+ cmake_pop_check_state()
+ if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON AND NOT HAVE_LIBXML2)
+ message(FATAL_ERROR "Failed to configure libxml2")
+ endif()
endif()
endif()
set(LLVM_ENABLE_LIBXML2 "${HAVE_LIBXML2}")
diff --git a/llvm/cmake/modules/LibXml2.cmake b/llvm/cmake/modules/LibXml2.cmake
new file mode 100644
index 0000000000000..1d967d70e81b9
--- /dev/null
+++ b/llvm/cmake/modules/LibXml2.cmake
@@ -0,0 +1,46 @@
+include(ExternalProject)
+
+if (NOT LIBXML2_PREFIX)
+ set (LIBXML2_PREFIX libxml2)
+endif()
+
+set(LIBXML2_PATH ${CMAKE_CURRENT_BINARY_DIR}/${LIBXML2_PREFIX}/src/${LIBXML2_PREFIX})
+set(LIBXML2_LIB_PATH ${LIBXML2_PATH}-build/libxml2.a)
+
+ExternalProject_Add(${LIBXML2_PREFIX}
+ PREFIX ${LIBXML2_PREFIX}
+ GIT_REPOSITORY https://github.com/GNOME/libxml2.git
+ GIT_TAG v2.15.1
+ CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
+ -DBUILD_SHARED_LIBS=OFF
+ -DLIBXML2_WITH_PYTHON=OFF
+ -DLIBXML2_WITH_PROGRAMS=OFF
+ -DLIBXML2_WITH_TESTS=OFF
+ -DLIBXML2_WITH_LZMA=OFF
+ -DLIBXML2_WITH_ZLIB=OFF
+ CMAKE_CACHE_ARGS -DCMAKE_C_COMPILER:FILEPATH=${CMAKE_C_COMPILER}
+ -DCMAKE_CXX_COMPILER:FILEPATH=${CMAKE_CXX_COMPILER}
+ -DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=ON
+ BUILD_BYPRODUCTS ${LIBXML2_LIB_PATH}
+ UPDATE_COMMAND ""
+ INSTALL_COMMAND ""
+ )
+
+set(LIBXML2_INCLUDE_DIRS ${LIBXML2_PATH}/include ${LIBXML2_PATH}-build)
+set(LIBXML2_LIBRARIES ${LIBXML2_LIB_PATH})
+set(LIBXML2_DEFINITIONS "")
+
+if(NOT TARGET LibXml2::LibXml2)
+ add_library(LibXml2_static STATIC IMPORTED GLOBAL)
+ set_target_properties(LibXml2_static PROPERTIES
+ IMPORTED_LOCATION ${LIBXML2_LIB_PATH}
+ )
+
+ add_library(LibXml2_LibXml2 INTERFACE)
+ target_link_libraries(LibXml2_LibXml2 INTERFACE LibXml2_static)
+ target_include_directories(LibXml2_LibXml2 INTERFACE ${LIBXML2_INCLUDE_DIRS})
+
+ add_dependencies(LibXml2_LibXml2 ${LIBXML2_PREFIX})
+
+ add_library(LibXml2::LibXml2 ALIAS LibXml2_LibXml2)
+endif()
diff --git a/llvm/lib/WindowsManifest/CMakeLists.txt b/llvm/lib/WindowsManifest/CMakeLists.txt
index 910132a4c7dec..0eef3a52ec1a3 100644
--- a/llvm/lib/WindowsManifest/CMakeLists.txt
+++ b/llvm/lib/WindowsManifest/CMakeLists.txt
@@ -1,6 +1,6 @@
include(GetLibraryName)
-if(LLVM_ENABLE_LIBXML2)
+if(LLVM_ENABLE_LIBXML2 AND NOT LLVM_USE_STATIC_LIBXML2)
set(imported_libs LibXml2::LibXml2)
endif()
@@ -18,17 +18,28 @@ add_llvm_component_library(LLVMWindowsManifest
Support
)
+if(LLVM_ENABLE_LIBXML2 AND LLVM_USE_STATIC_LIBXML2)
+ target_link_libraries(LLVMWindowsManifest PRIVATE ${LIBXML2_LIBRARIES})
+ target_include_directories(LLVMWindowsManifest PRIVATE ${LIBXML2_INCLUDE_DIRS})
+ add_dependencies(LLVMWindowsManifest libxml2)
+endif()
+
# This block is only needed for llvm-config. When we deprecate llvm-config and
# move to using CMake export, this block can be removed.
if(LLVM_ENABLE_LIBXML2)
- # CMAKE_BUILD_TYPE is only meaningful to single-configuration generators.
- if(CMAKE_BUILD_TYPE)
- string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
- get_property(libxml2_library TARGET LibXml2::LibXml2 PROPERTY LOCATION_${build_type})
- endif()
- if(NOT libxml2_library)
- get_property(libxml2_library TARGET LibXml2::LibXml2 PROPERTY LOCATION)
+ if(LLVM_USE_STATIC_LIBXML2)
+ # When using static libxml2 built via ExternalProject, use the library path directly
+ get_library_name(${LIBXML2_LIBRARIES} libxml2_library)
+ else()
+ # CMAKE_BUILD_TYPE is only meaningful to single-configuration generators.
+ if(CMAKE_BUILD_TYPE)
+ string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+ get_property(libxml2_library TARGET LibXml2::LibXml2 PROPERTY LOCATION_${build_type})
+ endif()
+ if(NOT libxml2_library)
+ get_property(libxml2_library TARGET LibXml2::LibXml2 PROPERTY LOCATION)
+ endif()
+ get_library_name(${libxml2_library} libxml2_library)
endif()
- get_library_name(${libxml2_library} libxml2_library)
set_property(TARGET LLVMWindowsManifest PROPERTY LLVM_SYSTEM_LIBS ${libxml2_library})
endif()
|
|
cc @tstellar since this is similar to the zstd setting and I changed it in the release cmake config here |
a3c4858 to
20afe5d
Compare
tstellar
left a comment
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.
Thank you for doing this.
|
failure here is on this change, looking into a fix |
Dynamically depending on libxml2 results in various annoyances across different linux distros for release artifacts. Specifically on fedora and nixos the library has a different name than on debian, and on arch-linux they tried to remove the old name entirely. With this, enabled by default for releases, we don't sacrifice any behavior changes, but no longer have these issues. For lld the binary size impact is <1mb macOS ignores this setting since libxml2 is part of the OS and stable enough. This mirrors what we do for zstd Fixes llvm#113696 Fixes llvm#138225 Fixes https://discourse.llvm.org/t/official-builds-without-libxml2-and-libtinfo/58169
20afe5d to
04e8ca2
Compare
| if (LLDB_ENABLE_LIBXML2 AND LLVM_USE_STATIC_LIBXML2) | ||
| target_link_libraries(lldbHost PRIVATE ${LIBXML2_LIBRARIES}) | ||
| target_include_directories(lldbHost PUBLIC ${LIBXML2_INCLUDE_DIRS}) | ||
| add_dependencies(lldbHost libxml2) | ||
| endif() |
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.
The goal of using the interface libraries (like LibXml2::LibXml2) is to simplify our CMake code where we don't need to manually update the list include directories and link libraries and can instead depend on a single target, but this negates that benefit, even worse now we have to both depend on the interface target and manually update the list include directories and link libraries everywhere where we depend on libxml2.
| ExternalProject_Add(${LIBXML2_PREFIX} | ||
| PREFIX ${LIBXML2_PREFIX} | ||
| GIT_REPOSITORY https://github.com/GNOME/libxml2.git | ||
| GIT_TAG v2.15.1 | ||
| CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} | ||
| -DBUILD_SHARED_LIBS=OFF | ||
| -DLIBXML2_WITH_PYTHON=OFF | ||
| -DLIBXML2_WITH_PROGRAMS=OFF | ||
| -DLIBXML2_WITH_TESTS=OFF | ||
| -DLIBXML2_WITH_LZMA=OFF | ||
| -DLIBXML2_WITH_ZLIB=OFF | ||
| CMAKE_CACHE_ARGS -DCMAKE_C_COMPILER:FILEPATH=${CMAKE_C_COMPILER} | ||
| -DCMAKE_CXX_COMPILER:FILEPATH=${CMAKE_CXX_COMPILER} | ||
| -DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=ON | ||
| BUILD_BYPRODUCTS ${LIBXML2_LIB_PATH} | ||
| UPDATE_COMMAND "" | ||
| INSTALL_COMMAND "" | ||
| ) |
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.
I don't want the build system to automatically clone and build a dependency without my approval (and what's worse, control this behavior by a flag such as LLVM_USE_STATIC_LIBXML2). This won't work on machines don't allow internet access during build stage (our bots actually fall into that category, and they statically link libxml2 already). I may also want to reuse an existing build of libxml2 or configure it in a specific way (for example enabling the zlib support or enabling LTO) which this implementation doesn't let me.
petrhosek
left a comment
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.
I'm supportive of the idea but not the implementation. I think the implementation should do exactly what the description says, that is follow the model we used for zstd: it should provide a custom FindLibXml2 which finds (but doesn't build) both shared and static version of libxml2 and defines the corresponding interface targets we can depend on.
Dynamically depending on libxml2 results in various annoyances across
different linux distros for release artifacts. Specifically on fedora
and nixos the library has a different name than on debian, and on
arch-linux they tried to remove the old name entirely.
With this, enabled by default for releases, we don't sacrifice any
behavior changes, but no longer have these issues. For lld the binary
size impact is <1mb
macOS ignores this setting since libxml2 is part of the OS and stable
enough.
This mirrors what we do for zstd
Fixes #113696
Fixes #138225
Fixes https://discourse.llvm.org/t/official-builds-without-libxml2-and-libtinfo/58169