Re: [xz-devel] Compatibility between CMake config file and FindLibLZMA.cmake
I'm afraid I didn't I fully understand the exact situation when this problem occurs. I trust that it's real and that you tested it, thus I committed this change. Thanks! I assume it has no significant downsides compared to the ALIAS method. Thanks, I just tested this and it also works for this use case now. There is no downside compared to the ALIAS method. Can something go wrong in the opposite direction: CMakeLists.txt recommends that one uses "find_package(liblzma 5.2.5 REQUIRED CONFIG)" to ensure that FindLibLZMA isn't used. If one does so and then something else in the project uses FindLibLZMA anyway, does the interface library (or previously the alias) cause a problem? Or is this situation unlikely to happen? When you specify "CONFIG" in find_package(), CMake always looks for the config file and does not look for a module. The only available target definitions are therefore the ones in the config file, with the interface library providing backward compatibility with the CMake module and its LibLZMA::LibLZMA target. In the CMake world, perhaps it could matter when two things are able to use liblzma but only one of them needs encoding support. Then those variables could help to keep the build working. However, it wouldn't suprise me if most packages don't check those variables and simply assume that all features are available if liblzma is found (which I think is reasonable behavior). I didn't try to verify this though so I may be wrong. So far I haven't checked these variables when adding liblzma as a dependency. Having a matching variable in the config file if the library can be configured with certain options enabled or disabled can however be quite helpful depending on the use case. For this, CMake provides an option() command and the respective variable can be added to the config file. This can be done too, although first it should be decided if full compatibility with FindLibLZMA is desirable. I guess FindLibLZMA won't see major changes so maintaining compatibility wouldn't need frequent changes in the liblzma config file. Having access to the version information in CMake variables is also quite useful, e.g., if features change. Otherwise these have to be extracted from header files or via library functions. FindLibLZMA also sets LIBLZMA_LIBRARIES and LIBLZMA_INCLUDE_DIRS. Is it > OK to not care about these in context of FindLibLZMA compatibility? The LIBLZMA_LIBRARIES and LIBLZMA_INCLUDE_DIRS are mostly for backward compatibility if targets are not used and contain absolute paths to the libraries and headers, including dependencies. The LibLZMA::LibLZMA target is included in FindLibLZMA.cmake starting with CMake 3.14. This information can be extracted from the target via get_target_property(), but the property names depend on the platform and build types, e.g.: https://gitlab.gnome.org/GNOME/libxml2/-/blob/cbe1212db6e22fa92c33242c3ce089476585f872/libxml2-config.cmake.cmake.in#L33-58 https://gitlab.gnome.org/GNOME/libxml2/-/blob/cbe1212db6e22fa92c33242c3ce089476585f872/libxml2-config.cmake.cmake.in#L79 I was thinking if the naming should have been such that it doesn't overlap or conflict with FindLibLZMA module at all. However, that would mean that if one thing depends on, for example, xz_liblzma::xz_liblzma and another thing on LibLZMA::LibLZMA, then two different targets would refer to the same library and so the compiler and linker flags would be duplicated. I'm not sure if that could become a problem. As a user I think it would be nice to be able to use the config file instead of the module without major changes, otherwise this always has to be considered when trying to support multiple platforms and versions. Keeping LibLZMA::LibLZMA would therefore be the easiest option, but I can also understand if you prefer to correct the name. The new interface library definition (LibLZMA::LibLZMA -> liblzma::liblzma) already helps a lot in supporting both. It will take some time for all platforms and package managers to include the config file. For this, it would probably also have to be generated when building via autotools. I have always written liblzma in lower case so changing the primary target name to LibLZMA::LibLZMA would look a bit funny to me. Of course, I'm fine with it still if it means that things work better overall. LibXml2 and LibXslt have similar issues. In the end I kept the mixed case of the module names for compatibility reasons. The FindLibLZMA.cmake module could probably also be updated to include the lowercase target name with a deprecation message. If I change the main add_library(liblzma ) to add_library(LibLZMA ) then the filename will be LibLZMA.something too. That isn't good because then one cannot replace a CMake-built shared liblzma with an Autotools-built one on operating systems where file and library names are case sensitive. You can either change the target name directly and change the filename
Re: [xz-devel] Compatibility between CMake config file and FindLibLZMA.cmake
On 2021-01-31 Markus Rickert wrote:
> I noticed however, that there is still an issue when building against
> a library that was built against liblzma due to using ALIAS (e.g.,
> testproject -> LibXml2 -> LibLZMA):
> CMake will resolve the alias to include "liblzma::liblzma" instead of
> "LibLZMA::LibLZMA" in the LibXml2 link dependencies. When another
> library does not set CMAKE_FIND_PACKAGE_PREFER_CONFIG, it will
> however still use FindLibLZMA and is then not able to resolve the
> "liblzma::liblzma" dependency.
>
> With the following alternative (that is also used in the CMake
> exported target files), LibXml2 will continue to use
> "LibLZMA::LibLZMA" in its dependencies when linking against this
> target:
> add_library(LibLZMA::LibLZMA INTERFACE IMPORTED)
> set_target_properties(LibLZMA::LibLZMA PROPERTIES
> INTERFACE_LINK_LIBRARIES liblzma::liblzma)
I'm afraid I didn't I fully understand the exact situation when this
problem occurs. I trust that it's real and that you tested it, thus
I committed this change. Thanks! I assume it has no significant
downsides compared to the ALIAS method.
Can something go wrong in the opposite direction: CMakeLists.txt
recommends that one uses "find_package(liblzma 5.2.5 REQUIRED CONFIG)"
to ensure that FindLibLZMA isn't used. If one does so and then
something else in the project uses FindLibLZMA anyway, does the
interface library (or previously the alias) cause a problem? Or is this
situation unlikely to happen?
> >- FindLibLZMA sets a few CMake cache variables that the config
> > file doesn't, for example, LIBLZMA_HAS_EASY_ENCODER. I have no
> > idea if there are packages that care about this.
>
> This seems to be due to the autotools options for enabling/disabling
> certain encoders and decoders. If there is no equivalent CMake option
> for modifying this you could add variables to the config files that
> are always set to ON:
> set(LIBLZMA_HAS_AUTO_DECODER ON)
> set(LIBLZMA_HAS_EASY_ENCODER ON)
> set(LIBLZMA_HAS_LZMA_PRESET ON)
Autotools options being the reason sounds reasonable, although all
normal builds of liblzma always have all features enabled. However, I
might add CMake options to disable encoder or decoder because there are
use cases where encoder support isn't needed. Then those compatibility
variables would need to be set conditionally too.
If liblzma is built with some features disabled and then something
breaks, I'm not sure if I should care. With Autotools-based builds
no help is provided: if one wants to disable features to reduce the
library size, one must be careful to do it without breaking anything.
In the CMake world, perhaps it could matter when two things are able to
use liblzma but only one of them needs encoding support. Then those
variables could help to keep the build working. However, it wouldn't
suprise me if most packages don't check those variables and simply
assume that all features are available if liblzma is found (which I
think is reasonable behavior). I didn't try to verify this though so I
may be wrong.
> The entries for LIBLZMA_VERSION_MAJOR etc. can also be easily added
> to the file:
> set(LIBLZMA_VERSION_MAJOR ${PROJECT_VERSION_MAJOR})
> set(LIBLZMA_VERSION_MINOR ${PROJECT_VERSION_MINOR})
> set(LIBLZMA_VERSION_PATCH ${PROJECT_VERSION_PATCH})
> set(LIBLZMA_VERSION_STRING \"${PROJECT_VERSION}\")
This can be done too, although first it should be decided if full
compatibility with FindLibLZMA is desirable. I guess FindLibLZMA won't
see major changes so maintaining compatibility wouldn't need frequent
changes in the liblzma config file.
FindLibLZMA also sets LIBLZMA_LIBRARIES and LIBLZMA_INCLUDE_DIRS. Is it
OK to not care about these in context of FindLibLZMA compatibility?
> > Perhaps there are other details affecting compatiblity. I just
> > wonder how big mistake it was to use liblzma::liblzma in the config
> > file. I guess it's too late to change it now.
>
> By using the same principle as above, you should still be able to
> change this if you prefer. In that case, liblzma::liblzma can be an
> alias for LibLZMA::LibLZMA in the config file.
I was thinking if the naming should have been such that it doesn't
overlap or conflict with FindLibLZMA module at all. However, that would
mean that if one thing depends on, for example, xz_liblzma::xz_liblzma
and another thing on LibLZMA::LibLZMA, then two different targets would
refer to the same library and so the compiler and linker flags would be
duplicated. I'm not sure if that could become a problem.
I have always written liblzma in lower case so changing the primary
target name to LibLZMA::LibLZMA would look a bit funny to me. Of
course, I'm fine with it still if it means that things work better
overall.
> I think the CMake build files also were not yet included in any
> official release.
CMakeLists.txt and friends were included in XZ Utils 5.2.5 (with the
bug that shared library doesn't build on Windows). It's
Re: [xz-devel] Compatibility between CMake config file and FindLibLZMA.cmake
I have committed both of your suggestions (hopefully correctly). Thanks!
Thanks, I just tested this and it works correctly when building directly
against liblzma via CMake, both with config and module mode.
I noticed however, that there is still an issue when building against a
library that was built against liblzma due to using ALIAS (e.g.,
testproject -> LibXml2 -> LibLZMA):
CMake will resolve the alias to include "liblzma::liblzma" instead of
"LibLZMA::LibLZMA" in the LibXml2 link dependencies. When another
library does not set CMAKE_FIND_PACKAGE_PREFER_CONFIG, it will however
still use FindLibLZMA and is then not able to resolve the
"liblzma::liblzma" dependency.
With the following alternative (that is also used in the CMake exported
target files), LibXml2 will continue to use "LibLZMA::LibLZMA" in its
dependencies when linking against this target:
add_library(LibLZMA::LibLZMA INTERFACE IMPORTED)
set_target_properties(LibLZMA::LibLZMA PROPERTIES
INTERFACE_LINK_LIBRARIES liblzma::liblzma)
- FindLibLZMA doesn't #define LZMA_API_STATIC when building against
static liblzma. LZMA_API_STATIC omits __declspec(dllimport) from
liblzma function declarations on Windows.
The FindLibLZMA module cannot easily determine if a detected library was
built as shared or static library (there is an open issue for this [1])
without looking at header or config files. autotools seems to build both
at the same time and FindLibLZMA will link the shared version if it is
available.
CMake builds either static or shared via BUILD_SHARED_LIBS, but includes
the correct compiler definition in the exported target.
- FindLibLZMA sets a few CMake cache variables that the config file
doesn't, for example, LIBLZMA_HAS_EASY_ENCODER. I have no idea if
there are packages that care about this.
This seems to be due to the autotools options for enabling/disabling
certain encoders and decoders. If there is no equivalent CMake option
for modifying this you could add variables to the config files that are
always set to ON:
set(LIBLZMA_HAS_AUTO_DECODER ON)
set(LIBLZMA_HAS_EASY_ENCODER ON)
set(LIBLZMA_HAS_LZMA_PRESET ON)
The entries for LIBLZMA_VERSION_MAJOR etc. can also be easily added to
the file:
set(LIBLZMA_VERSION_MAJOR ${PROJECT_VERSION_MAJOR})
set(LIBLZMA_VERSION_MINOR ${PROJECT_VERSION_MINOR})
set(LIBLZMA_VERSION_PATCH ${PROJECT_VERSION_PATCH})
set(LIBLZMA_VERSION_STRING \"${PROJECT_VERSION}\")
- The config file has find_dependency(Threads) while FindLibLZMA
doesn't. This can affect the linker flags.
Other find modules have similar issues. FindLibXml2 for instance also
does not link against dependencies such as Iconv or LibLZMA. For
link-only dependencies this issue only shows up in static builds. The
config file should still include the correct dependencies.
Perhaps there are other details affecting compatiblity. I just wonder
how big mistake it was to use liblzma::liblzma in the config file. I
guess it's too late to change it now.
By using the same principle as above, you should still be able to change
this if you prefer. In that case, liblzma::liblzma can be an alias for
LibLZMA::LibLZMA in the config file. I think the CMake build files also
were not yet included in any official release.
You can add an alias for target "liblzma" to target "LibLZMA" in the
CMakeLists.txt file (after the target definition in add_library, line
193) for users that embed the xz project as a subdirectory:
add_library(LibLZMA::LibLZMA ALIAS LibLZMA)
add_library(liblzma ALIAS LibLZMA::LibLZMA)
add_library(liblzma::liblzma ALIAS LibLZMA::LibLZMA)
With CMake >3.17, you can further set a deprecation message for the old
target name (works only on INTERFACE IMPORTED, not on ALIAS):
if(NOT CMAKE_VERSION VERSION_LESS 3.17)
set_property(TARGET liblzma::liblzma PROPERTY DEPRECATION "Deprecated
target. Please use LibLZMA::LibLZMA instead.")
endif()
Best regards,
Markus Rickert
[1] https://gitlab.kitware.com/cmake/cmake/-/issues/18564
smime.p7s
Description: S/MIME cryptographic signature
Re: [xz-devel] Compatibility between CMake config file and FindLibLZMA.cmake
On 2021-01-23 Markus Rickert wrote: > This could be solved by adding an alias to the config file: > add_library(LibLZMA::LibLZMA ALIAS liblzma::liblzma) > > An additional improvement would be to enable this on case-sensitive > file systems as well. For this, the config file would need to be > renamed from liblzmaConfig.cmake to liblzma-config.cmake (and the > version file to liblzma-config-version.cmake), see [2]. I have committed both of your suggestions (hopefully correctly). Thanks! Some extra thoughts: There are some differences between FindLibLZMA and the config file: - FindLibLZMA doesn't #define LZMA_API_STATIC when building against static liblzma. LZMA_API_STATIC omits __declspec(dllimport) from liblzma function declarations on Windows. - FindLibLZMA sets a few CMake cache variables that the config file doesn't, for example, LIBLZMA_HAS_EASY_ENCODER. I have no idea if there are packages that care about this. - The config file has find_dependency(Threads) while FindLibLZMA doesn't. This can affect the linker flags. Perhaps there are other details affecting compatiblity. I just wonder how big mistake it was to use liblzma::liblzma in the config file. I guess it's too late to change it now. -- Lasse Collin | IRC: Larhzu @ IRCnet & Freenode
