Re: [xz-devel] Compatibility between CMake config file and FindLibLZMA.cmake

2021-02-14 Thread Markus Rickert
ly and change the filename 
via set_target_properties() and the OUTPUT_NAME property:

https://gitlab.gnome.org/GNOME/libxml2/-/blob/cbe1212db6e22fa92c33242c3ce089476585f872/CMakeLists.txt#L489-497

Or you can keep the liblzma target name and set the EXPORT_NAME target 
property to LibLZMA:

https://github.com/roboticslibrary/zlib/blob/da865819a4f29045f66e5e0024f7082d25630928/CMakeLists.txt#L191

Best regards,

Markus Rickert


smime.p7s
Description: S/MIME cryptographic signature


Re: [xz-devel] Compatibility between CMake config file and FindLibLZMA.cmake

2021-01-31 Thread Markus Rickert

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


[xz-devel] Compatibility between CMake config file and FindLibLZMA.cmake

2021-01-23 Thread Markus Rickert

Hello,

would it be possible to improve the compatibility between the CMake 
config file included in liblzma and the FindLibLZMA.cmake module 
included in CMake?


The CMake config file currently only defines a liblzma::liblzma target, 
while the CMake module defines a LibLZMA::LibLZMA target. On systems 
with case-insensitive file systems, using 
CMAKE_FIND_PACKAGE_PREFER_CONFIG (see [1]) to prefer config files over 
modules results in an error because of this. A find_package(LibLZMA) 
call now also uses the config file, which however does not define the 
expected LibLZMA::LibLZMA target.


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]. This would support using both 
find_package(LibLZMA) and find_package(liblzma), while still preferring 
the CMake module by default in the first case.


Best regards,

Markus Rickert

[1] 
https://cmake.org/cmake/help/v3.19/variable/CMAKE_FIND_PACKAGE_PREFER_CONFIG.html
[2] 
https://cmake.org/cmake/help/v3.19/command/find_package.html#full-signature-and-config-mode


smime.p7s
Description: S/MIME cryptographic signature


Re: [xz-devel] [PATCH] CMake: Add missing include directory when building the shared library

2020-06-04 Thread Markus Rickert

Great to see this being tested. Thanks!


Including CMake build files in the repository is very much appreciated 
for building on different platforms and especially on Windows. Thank you 
for that.



The CMakeLists.txt doesn't create config.h. All #defines are set as
compiler options. The config.h files under windows/vs* aren't meant to
be used with CMake. Instead, common_w32res.rc shouldn't read config.h
when it isn't present.

The following patch works with the GNU Autotools based build. Does it
work with CMake + VS2019? It requires that the #defines used for
building C files are passed to the resource compiler too. (If they
aren't, it should give an error.) After a successful build you can
right-click liblzma.dll -> Properties -> Details to see if the info
from the resource file is present.


This way also works, I just tested your patch on my system with VS2019. 
The DLL includes the following info, looks good to me:


 File description: liblzma data compression library
 File version: 5.3.1.0
 Product name: XZ Utils <https://tukaani.org/xz/>
 Product version: 5.3.1alpha

Best regards,

Markus Rickert


smime.p7s
Description: S/MIME cryptographic signature


[xz-devel] [PATCH] CMake: Add missing include directory when building the shared library

2020-06-02 Thread Markus Rickert
When using CMake to build a shared library with Visual Studio 2019, I get
an error that the file config.h could not be found:

cmake -G "Visual Studio 16 2019" -A x64 -DBUILD_SHARED_LIBS=ON ..\xz
cmake --build . --config Release

xz\src\common\common_w32res.rc(9): fatal error RC1015: cannot open include file 
'config.h'. [xz-build\liblzma.vcxproj]

The file common_w32res.rc has an include for config.h in line 9, but the
corresponding include directory is missing in the build files. The patch
fixes this by adding the windows/2019 directory to the list of include
directories.

The config.h files in vs2013, vs2017, and vs2019 all seem to be identical,
so any folder will do.

With this modification, the shared library build is successful:

Creating library xz-build/Release/liblzma.lib and object 
xz-build/Release/liblzma.exp
liblzma.vcxproj -> xz-build\Release\liblzma.dll

---
 CMakeLists.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d3aa627..b005f45 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -403,6 +403,7 @@ if(WIN32)
 if(BUILD_SHARED_LIBS)
 # Add the Windows resource file for liblzma.dll.
 target_sources(liblzma PRIVATE src/liblzma/liblzma_w32res.rc)
+target_include_directories(liblzma PRIVATE windows/vs2019)
 
 # Export the public API symbols with __declspec(dllexport).
 target_compile_definitions(liblzma PRIVATE DLL_EXPORT)
-- 
2.26.2.windows.1



smime.p7s
Description: S/MIME cryptographic signature