Skip to content

Create CMake package when installing#1030

Merged
Cyan4973 merged 1 commit intolz4:devfrom
kostasdizas:cmake
Oct 21, 2021
Merged

Create CMake package when installing#1030
Cyan4973 merged 1 commit intolz4:devfrom
kostasdizas:cmake

Conversation

@kostasdizas
Copy link
Copy Markdown
Contributor

@kostasdizas kostasdizas commented Oct 19, 2021

Made a small change to the CMake build files to create a package so it can be easily included in other projects.

@Cyan4973
Copy link
Copy Markdown
Member

Thanks @kostasdizas ! Looks good to me !

@Cyan4973 Cyan4973 merged commit 0e2743c into lz4:dev Oct 21, 2021
@kostasdizas kostasdizas deleted the cmake branch October 21, 2021 07:35
@mxmlnkn
Copy link
Copy Markdown

mxmlnkn commented Nov 2, 2021

Thank you so much! Exactly what I needed after finding out that the CMake integration of v1.9.3 wasn't working as easily as expected.

INSTALL_DESTINATION ${LZ4_PKG_INSTALLDIR})
export(EXPORT lz4Targets
FILE ${CMAKE_CURRENT_BINARY_DIR}/lz4Targets.cmake
NAMESPACE LZ4::)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the namespace upper case? The config file is called lz4, and exporting a namespace named differently is confusing, as it breaks the common practice used in the wider CMake ecosystem.

This also creates issues with pretty much every project who was already using lz4 in CMake, as people (quite unsurprisingly) use the lowercase lz4 namespace, and package managers like vcpkg do as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants