build(deps): Add task-based installation and cmake find module script for LibLZMA dependency. #1184
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| # Try to find LibLZMA | ||
| # NOTE: The FindLibLZMA.cmake included with CMake has no support for static libraries, so we use our | ||
| # own. | ||
| # | ||
| # Set LibLZMA_USE_STATIC_LIBS=ON to look for static libraries. | ||
| # | ||
| # Once done, this will define: | ||
| # LibLZMA_FOUND - Whether the library was found on the system | ||
| # LibLZMA_INCLUDE_DIR - The library include directories | ||
| # LibLZMA_LIBRARY - The path to the library file | ||
| # LibLZMA_VERSION - The version of the library installed on the system | ||
| # | ||
| # Conventions: | ||
| # - Variables only for use within the script are prefixed with "liblzma_" | ||
| # - Variables that should be externally visible are prefixed with "LibLZMA_" | ||
|
|
||
| set(liblzma_HEADER "lzma.h") | ||
| set(liblzma_LIBNAME "lzma") | ||
| set(liblzma_PKGCONFIG_NAME "liblzma") | ||
|
|
||
| if(DEFINED CLP_CORE_DEPS_DIR) | ||
| set(ENV{liblzma_ORIG_PKG_CONFIG_PATH} "$ENV{PKG_CONFIG_PATH}") | ||
| set(ENV{PKG_CONFIG_PATH} "${CLP_CORE_DEPS_DIR}:$ENV{PKG_CONFIG_PATH}") | ||
| endif() | ||
Bill-hbrhbr marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+21
to
+24
There was a problem hiding this comment. 🛠️ Refactor suggestion Use platform-appropriate PKG_CONFIG_PATH separator and handle empty PATH robustly On Windows, PKG_CONFIG_PATH must use “;” instead of “:”. Also guard for an initially empty PKG_CONFIG_PATH to avoid leading/trailing separators. Apply: -if(DEFINED CLP_CORE_DEPS_DIR)
- set(ENV{liblzma_ORIG_PKG_CONFIG_PATH} "$ENV{PKG_CONFIG_PATH}")
- set(ENV{PKG_CONFIG_PATH} "${CLP_CORE_DEPS_DIR}:$ENV{PKG_CONFIG_PATH}")
-endif()
+if(DEFINED CLP_CORE_DEPS_DIR)
+ set(ENV{liblzma_ORIG_PKG_CONFIG_PATH} "$ENV{PKG_CONFIG_PATH}")
+ if(WIN32)
+ set(_liblzma_pkgconf_sep ";")
+ else()
+ set(_liblzma_pkgconf_sep ":")
+ endif()
+ if("$ENV{PKG_CONFIG_PATH}" STREQUAL "")
+ set(ENV{PKG_CONFIG_PATH} "${CLP_CORE_DEPS_DIR}")
+ else()
+ set(ENV{PKG_CONFIG_PATH} "${CLP_CORE_DEPS_DIR}${_liblzma_pkgconf_sep}$ENV{PKG_CONFIG_PATH}")
+ endif()
+ unset(_liblzma_pkgconf_sep)
+endif()🤖 Prompt for AI Agents |
||
|
|
||
| # Run pkg-config | ||
| find_package(PkgConfig) | ||
| pkg_check_modules(liblzma_PKGCONF QUIET "${liblzma_PKGCONFIG_NAME}") | ||
|
|
||
| # Set include directory | ||
| find_path(LibLZMA_INCLUDE_DIR ${liblzma_HEADER} | ||
| HINTS ${liblzma_PKGCONF_INCLUDEDIR} | ||
| PATH_SUFFIXES include | ||
| ) | ||
|
|
||
|
Comment on lines
+30
to
+35
There was a problem hiding this comment. 🛠️ Refactor suggestion Add CLP_CORE_DEPS_DIR and lib64 to HINTS/PATH_SUFFIXES to reduce reliance on pkg-config If pkg-config doesn’t resolve (or resolves to the “other” variant), discovery may fail. Adding CLP_CORE_DEPS_DIR to HINTS and lib64 to PATH_SUFFIXES makes the module resilient and deterministic within your deps prefix. find_path(LibLZMA_INCLUDE_DIR ${liblzma_HEADER}
- HINTS ${liblzma_PKGCONF_INCLUDEDIR}
+ HINTS ${liblzma_PKGCONF_INCLUDEDIR} ${CLP_CORE_DEPS_DIR}
PATH_SUFFIXES include
)
@@
find_library(LibLZMA_LIBRARY
NAMES "${liblzma_LIBNAME}"
- HINTS ${liblzma_PKGCONF_LIBDIR}
- PATH_SUFFIXES lib
+ HINTS ${liblzma_PKGCONF_LIBDIR} ${CLP_CORE_DEPS_DIR}
+ PATH_SUFFIXES lib lib64
)Also applies to: 43-47 🤖 Prompt for AI Agents |
||
| # Handle static libraries | ||
| if(LibLZMA_USE_STATIC_LIBS) | ||
| set(liblzma_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES}) | ||
| set(CMAKE_FIND_LIBRARY_SUFFIXES "${CMAKE_STATIC_LIBRARY_SUFFIX}") | ||
| endif() | ||
|
|
||
|
Comment on lines
+36
to
+41
There was a problem hiding this comment. 🛠️ Refactor suggestion Prefer static when requested but gracefully fall back to shared Current logic restricts suffixes to static (when LibLZMA_USE_STATIC_LIBS is ON), which will hard-fail if the static variant isn’t installed or the resolved libdir points at the shared install. Align with the project’s pattern: prefer static, then fall back to shared. Option A (single-pass preference via suffix ordering): if(LibLZMA_USE_STATIC_LIBS)
- set(liblzma_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES})
- set(CMAKE_FIND_LIBRARY_SUFFIXES "${CMAKE_STATIC_LIBRARY_SUFFIX}")
+ set(liblzma_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES})
+ # Prefer static by putting it first, but keep originals for fallback.
+ set(CMAKE_FIND_LIBRARY_SUFFIXES "${CMAKE_STATIC_LIBRARY_SUFFIX};${CMAKE_FIND_LIBRARY_SUFFIXES}")
endif()
@@
find_library(LibLZMA_LIBRARY
NAMES "${liblzma_LIBNAME}"
HINTS ${liblzma_PKGCONF_LIBDIR} ${CLP_CORE_DEPS_DIR}
- PATH_SUFFIXES lib
+ PATH_SUFFIXES lib lib64
)
if(LibLZMA_USE_STATIC_LIBS)
# Restore original value of CMAKE_FIND_LIBRARY_SUFFIXES
set(CMAKE_FIND_LIBRARY_SUFFIXES ${liblzma_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES})
unset(liblzma_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES)
endif()Option B (two-pass fallback if you want strict separation):
Also applies to: 43-53 🤖 Prompt for AI Agents |
||
| # Find library | ||
| find_library(LibLZMA_LIBRARY | ||
| NAMES "${liblzma_LIBNAME}" | ||
| HINTS ${liblzma_PKGCONF_LIBDIR} | ||
| PATH_SUFFIXES lib | ||
| ) | ||
|
|
||
| if(LibLZMA_USE_STATIC_LIBS) | ||
| # Restore original value of CMAKE_FIND_LIBRARY_SUFFIXES | ||
| set(CMAKE_FIND_LIBRARY_SUFFIXES ${liblzma_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES}) | ||
| unset(liblzma_ORIG_CMAKE_FIND_LIBRARY_SUFFIXES) | ||
| endif() | ||
|
|
||
| # Set version | ||
| set(LibLZMA_VERSION ${liblzma_PKGCONF_VERSION}) | ||
|
|
||
| include(FindPackageHandleStandardArgs) | ||
| find_package_handle_standard_args(LibLZMA | ||
| REQUIRED_VARS LibLZMA_LIBRARY LibLZMA_INCLUDE_DIR | ||
| VERSION_VAR LibLZMA_VERSION | ||
| ) | ||
|
|
||
| if(NOT TARGET LibLZMA::LibLZMA) | ||
| # Add library to build | ||
| if (LibLZMA_USE_STATIC_LIBS) | ||
| add_library(LibLZMA::LibLZMA STATIC IMPORTED) | ||
| else() | ||
| # NOTE: We use UNKNOWN so that if the user doesn't have the SHARED | ||
| # libraries installed, we can still use the STATIC libraries | ||
| add_library(LibLZMA::LibLZMA UNKNOWN IMPORTED) | ||
| endif() | ||
|
|
||
| # Set include directories for library | ||
| if(LibLZMA_INCLUDE_DIR) | ||
| set_target_properties(LibLZMA::LibLZMA | ||
| PROPERTIES | ||
| INTERFACE_INCLUDE_DIRECTORIES "${LibLZMA_INCLUDE_DIR}" | ||
| ) | ||
| endif() | ||
|
|
||
| # Set location of library | ||
| if(EXISTS "${LibLZMA_LIBRARY}") | ||
| set_target_properties(LibLZMA::LibLZMA | ||
| PROPERTIES | ||
| IMPORTED_LINK_INTERFACE_LANGUAGES "C" | ||
| IMPORTED_LOCATION "${LibLZMA_LIBRARY}" | ||
| ) | ||
| endif() | ||
| endif() | ||
|
|
||
| # Restore original value of PKG_CONFIG_PATH | ||
| if(DEFINED CLP_CORE_DEPS_DIR) | ||
| set(ENV{PKG_CONFIG_PATH} "$ENV{liblzma_ORIG_PKG_CONFIG_PATH}") | ||
| unset(ENV{liblzma_ORIG_PKG_CONFIG_PATH}) | ||
| endif() | ||
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
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
Oops, something went wrong.
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.
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.
🧹 Nitpick (assertive)
Avoid per-variant LibLZMA_ROOT; rely on CLP_CORE_DEPS_DIR and toggle only LibLZMA_USE_STATIC_LIBS.
Per retrieved learnings, our Find modules already search within CLP_CORE_DEPS_DIR and handle static/shared selection via LibLZMA_USE_STATIC_LIBS. The LibLZMA-static_ROOT / LibLZMA-shared_ROOT overrides are unnecessary and add maintenance burden.
Apply this diff:
if(CLP_NEED_LIBLZMA) if(CLP_USE_STATIC_LIBS) - set(LibLZMA_ROOT ${LibLZMA-static_ROOT}) set(LibLZMA_USE_STATIC_LIBS ON) else() - set(LibLZMA_ROOT ${LibLZMA-shared_ROOT}) + # Be explicit to avoid stale cache values from toolchain/user presets. + set(LibLZMA_USE_STATIC_LIBS OFF) endif()📝 Committable suggestion
🤖 Prompt for AI Agents