-
Notifications
You must be signed in to change notification settings - Fork 9
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
CMake update to find liblz4 using pkg-config instead of cmake. #46
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
0a8793c
Add first a pkg-config search, then fallback.
mholtrop 13dca79
Updated way to find and process lz4, thanks to Chris Dilks
mholtrop f0d3287
Still need to include the lz4 dir.
mholtrop ab1fd4f
Minor update correcting bank names.
mholtrop c183ecf
Merge pull request #2 from mholtrop/use_pkgconfig_for_lz4
mholtrop 98d07bd
Try to fix the LZ4 stuff for different behaviors of cmake on differen…
mholtrop d220e0a
Testing on Intel Mac
mholtrop 9f8f4f1
Cleanup. Seems to work in Intel Mac
mholtrop f64b58d
Merge pull request #5 from mholtrop/use_pkgconfig_for_lz4
mholtrop File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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 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 |
---|---|---|
@@ -1,41 +1,15 @@ | ||
# | ||
# Find the LZ4 library and include files and configure target for it. | ||
# | ||
get_filename_component(_DIR "${CMAKE_CURRENT_LIST_FILE}" PATH) | ||
find_path(LZ4_INCLUDE_DIR NAMES lz4.h) | ||
find_library(LZ4_LIBRARY NAMES lz4) | ||
if (LZ4_LIBRARY) | ||
include(CheckCSourceRuns) | ||
set(CMAKE_REQUIRED_INCLUDES ${LZ4_INCLUDE_DIR}) | ||
set(CMAKE_REQUIRED_LIBRARIES ${LZ4_LIBRARY}) | ||
enable_language(C) | ||
check_c_source_runs(" | ||
#include <lz4.h> | ||
int main() { | ||
int good = (LZ4_VERSION_MAJOR > 1) || | ||
((LZ4_VERSION_MAJOR == 1) && (LZ4_VERSION_MINOR >= 8)); | ||
return !good; | ||
}" LZ4_GOOD_VERSION) | ||
set(CMAKE_REQUIRED_INCLUDES) | ||
set(CMAKE_REQUIRED_LIBRARIES) | ||
endif() | ||
|
||
include(FindPackageHandleStandardArgs) | ||
FIND_PACKAGE_HANDLE_STANDARD_ARGS( | ||
LZ4 DEFAULT_MSG | ||
LZ4_LIBRARY LZ4_INCLUDE_DIR LZ4_GOOD_VERSION) | ||
set(ENV{PKG_CONFIG_PATH} "${PKG_CONFIG_PATH}:${CMAKE_INSTALL_PREFIX}/lib/pkgconfig") | ||
find_package(PkgConfig REQUIRED) | ||
pkg_check_modules(LZ4 IMPORTED_TARGET "liblz4") | ||
|
||
if (NOT LZ4_FOUND) | ||
message(STATUS "No system version of LZ4 found.") | ||
else() | ||
if(NOT TARGET LZ4) | ||
add_library(LZ4 INTERFACE IMPORTED) | ||
set_target_properties(LZ4 PROPERTIES | ||
INTERFACE_INCLUDE_DIRECTORIES "${LZ4_INCLUDE_DIR}" | ||
) | ||
endif() | ||
message(STATUS "Found LZ4: ${LZ4_LIBRARY}") | ||
set(LZ4_INCLUDE_DIRS ${LZ4_INCLUDE_DIR}) | ||
set(LZ4_LIBRARIES ${LZ4_LIBRARY}) | ||
add_library(LZ4 INTERFACE IMPORTED) | ||
set_target_properties(LZ4 PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${LZ4_INCLUDE_DIR}") | ||
message(STATUS "Found LZ4: ${LZ4_LIBRARIES}") | ||
mark_as_advanced(LZ4_INCLUDE_DIRS LZ4_INCLUDE_DIR LZ4_LIBRARIES LZ4_LIBRARY) | ||
endif (NOT LZ4_FOUND) |
This file contains 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 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
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.
Is this needed? I think CMake's PkgConfig module will search
CMAKE_PREFIX_PATH
'slib/pkgconfig
subdirectories by default, so you just need to make sure LZ4's top-level installation prefix is inCMAKE_PREFIX_PATH
(and if LZ4 is installed at the system level, then it's not necessary to add the system directories since they will already be searched).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.
Well, "needed" it is not. If lz4 is properly installed it all works. However if the user installs lz4 from the directory included with hipo, then this line avoids the confusion with cmake not finding that install unless you also set the CMAKE_PREFIX_PATH. I would think that "expected behavior" is that cmake just finds the package files, as it would if we used a cmake install instead of pkgconfig. It seems a simple fix to me, but if anyone objects I am fine without it.
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.
Sounds good to me