zlib: Fix shared library on Windows MSVC #25076

alessiosacco · 2024-08-29T07:34:18Z

Summary

Changes to recipe: zlib/1.3.1

Motivation

When option.shared = True and compiler is MSVC, the CMakeLists.txt creates an archive called zlib.lib, but the dll is called zdll.dll.

This configuration is not standard and is not supported by cmakedeps_macro.cmake module when using find_package(ZLIB).

The source patch now creates zlib.lib and zlib.dll.

Details

Tested using Windows, MSVC v142 and v143, debug and release with option.shared both false and true, Debian Linux GCC 10.

Read the contributing guidelines
Checked that this PR is not a duplicate: list of PRs by recipe
Tested locally with at least one configuration using a recent version of Conan

CLAassistant · 2024-08-29T07:34:24Z

All committers have signed the CLA.

When option.shared = True and compiler is MSVC, the CMakeLists.txt creates an archive called zlib.lib, but the dll is called zdll.dll. This configuration is not standard and is not supported by cmakedeps_macro.cmake module when using find_package(ZLIB). The source patch now creates zlib.lib and zlib.dll.

jcar87

Renaming the DLL on Windows will break compatibility with pre-existing binaries and cause widespread breakages so I'm afraid we cannot accept this change.

When we say the name is "not standard" why do we mean by that? There is no prerequisite whatsoever for the lib and DLL files to have the same "name", as far as I know, even if that's typically the expected.

alessiosacco · 2024-08-29T12:40:05Z

The cmakedeps_macro.cmake generated by CMakeDeps, in the function conan_package_library_targets is not able to find the shared library because it expects the name of the implib and the runtime library to be the same, which is not what happens when zlib is compiled under windows using the old conanfile and patch.
The function uses the CMake find_library function with the name argument _LIBRARY_NAME that is extracted from the implib filename in order to search for the runtime library. Since the runtime does not have the same name as the implib a STATUS message is shown during CMake configure: message(STATUS "Cannot locate shared library: ${_LIBRARY_NAME}")
The problem is also reflected in how the target is created, because it will be an UNKNOWN IMPORTED library instead of a SHARED IMPORTED library.
With the current self.cpp_info specified in package_info there is no way to specify that the runtime library has a different name from the implib, see docs.

SpaceIm · 2024-08-29T13:32:21Z

Therefore it's a conan issue (not being able to specify DLL name in package_info() to help CMakeDeps finding DLL, or telling conan whether some lib is shared or not. It's usually not a big issue, unless you want to rely on CMake to do something with these files).
I don't think it's a good idea to change import lib name due to some conan client issue, it should be fixed in conan client.

jcar87 · 2024-09-03T09:53:53Z

Hi @alessiosacco - thanks for providing more details.

I have opened this issue in the Conan repository: conan-io/conan#16926 - as this is general and indeed a limitation.
The "warning" being a warning is correct - CMake only needs the import library .lib - although the logic in CMakeDeps that sets the imported target type to UNKNOWN instead of SHARED as a result of this, may have other side effects (see conan-io/conan#12654)

Are you experiencing any errors or issues other than the warning?

That said, I'm afraid we can't accept this PR as it would cause breakages across the board on Windows when using shared libraries. The Conan model takes into account that if the version hasn't changed and a library is depended on as a shared library, downstream dependees don't need to be rebuilt. If ABI or the shared library names change for the same version in a new recipe revision, existing binaries of dependees that rely on zlib shared would not be rebuilt BUT they would stop working at runtime as the new package does not provide the DLL name expected. A possible workaround would be to simply duplicate the DLLs in the new revision when making this sort of change (provide the old dll name and the new dll name). However we tend to be very conservative here: we should keep the exact names used upstream, unless there's as a very strong and compelling justification. Arguably the zlib recipe should have never renamed the DLL in the existing patch in the same place - would need to check the historical of the recipe to understand why this was done.

alessiosacco · 2024-09-04T18:57:29Z

Hi @jcar87,
I have run into this issue when trying to upgrade the version of zlib that is packaged inside the project I am building. The zlib library was previously prebuilt and copyied to the install folder of the shared library that I am distributing. The prebuilt zlib was installed using CMake install using install(IMPORTED_RUNTIME_ARTIFACTS). CMake requires the targets that are used for this call to declare the IMPORTED_LOCATION_<CONFIG> property and the targets need to be IMPORTED SHARED for this to work in the case of RUNTIME in Windows and LIBRARY in Linux. The libraries that are provided by the cmakedeps_macros module do not satisfy these requirements because the .dll is not declared in the property and the target is IMPORTED UNKNOWN. I will follow the issue you have opened.

jcar87 · 2024-09-05T08:31:09Z

Hi @alessiosacco - thank you for providing additional information, this is very useful. Indeed this would be a limitation as a result of conan-io/conan#16926.

For getting CMake to bring/copy the DLLs you depend on, you may have better luck with the following example, where we can try for CMake to recursively find the DLLs (and it should find them, as it relies on information external to the targets)
https://docs.conan.io/2/reference/tools/cmake/cmaketoolchain.html#conan-runtime-lib-dirs

conan-center-bot · 2024-09-23T13:31:38Z

Conan v1 pipeline ❌

Failure in build 7 (9de4c27588de93363cb328d96b732d56091e02dc):

zlib/1.3.1:
Didn't run or was cancelled before finishing

zlib/1.3:
CI failed to create some packages (All logs)

Logs for packageID 4f1710918aa542fccb5a54d7bd712e4b0750b50d:

[settings]
arch=x86_64
build_type=Debug
compiler=Visual Studio
compiler.runtime=MDd
compiler.version=16
os=Windows
[options]
zlib:shared=True

[...]
zlib/1.3: CMake command: cmake --install "C:\J\workspace\prod-v1\bsr\99778\ecccf\.conan\data\zlib\1.3\_\_\build\4f1710918aa542fccb5a54d7bd712e4b0750b50d\build" --config Debug --prefix "C:/J/workspace/prod-v1/bsr/99778/ecccf/.conan/data/zlib/1.3/_/_/package/4f1710918aa542fccb5a54d7bd712e4b0750b50d"

----Running------
> cmake --install "C:\J\workspace\prod-v1\bsr\99778\ecccf\.conan\data\zlib\1.3\_\_\build\4f1710918aa542fccb5a54d7bd712e4b0750b50d\build" --config Debug --prefix "C:/J/workspace/prod-v1/bsr/99778/ecccf/.conan/data/zlib/1.3/_/_/package/4f1710918aa542fccb5a54d7bd712e4b0750b50d"
-----------------
-- Installing: C:/J/workspace/prod-v1/bsr/99778/ecccf/.conan/data/zlib/1.3/_/_/package/4f1710918aa542fccb5a54d7bd712e4b0750b50d/lib/zdll.lib
-- Installing: C:/J/workspace/prod-v1/bsr/99778/ecccf/.conan/data/zlib/1.3/_/_/package/4f1710918aa542fccb5a54d7bd712e4b0750b50d/bin/zlib1.dll
-- Installing: C:/J/workspace/prod-v1/bsr/99778/ecccf/.conan/data/zlib/1.3/_/_/package/4f1710918aa542fccb5a54d7bd712e4b0750b50d/include/zconf.h
-- Installing: C:/J/workspace/prod-v1/bsr/99778/ecccf/.conan/data/zlib/1.3/_/_/package/4f1710918aa542fccb5a54d7bd712e4b0750b50d/include/zlib.h
[HOOK - conan-center.py] post_package(): [PACKAGE LICENSE (KB-H012)] OK
[HOOK - conan-center.py] post_package(): [DEFAULT PACKAGE LAYOUT (KB-H013)] OK
[HOOK - conan-center.py] post_package(): [MATCHING CONFIGURATION (KB-H014)] OK
[HOOK - conan-center.py] post_package(): [SHARED ARTIFACTS (KB-H015)] OK
[HOOK - conan-center.py] post_package(): [STATIC ARTIFACTS (KB-H074)] OK
[HOOK - conan-center.py] post_package(): [EITHER STATIC OR SHARED OF EACH LIB (KB-H076)] OK
[HOOK - conan-center.py] post_package(): [PC-FILES (KB-H020)] OK
[HOOK - conan-center.py] post_package(): [CMAKE-MODULES-CONFIG-FILES (KB-H016)] OK
[HOOK - conan-center.py] post_package(): [PDB FILES NOT ALLOWED (KB-H017)] OK
[HOOK - conan-center.py] post_package(): [LIBTOOL FILES PRESENCE (KB-H018)] OK
[HOOK - conan-center.py] post_package(): [MS RUNTIME FILES (KB-H021)] OK
[HOOK - conan-center.py] post_package(): [SHORT_PATHS USAGE (KB-H066)] OK
**********************************************************************
** Visual Studio 2019 Developer Command Prompt v16.11.26
** Copyright (c) 2021 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'
[HOOK - conan-center.py] post_package(): [MISSING SYSTEM LIBS (KB-H043)] OK
[HOOK - conan-center.py] post_package(): [APPLE RELOCATABLE SHARED LIBS (KB-H077)] OK
zlib/1.3 package(): Packaged 1 '.dll' file: zlib1.dll
zlib/1.3 package(): Packaged 2 '.h' files: zconf.h, zlib.h
zlib/1.3 package(): Packaged 1 '.lib' file: zdll.lib
zlib/1.3 package(): Packaged 1 file: LICENSE
zlib/1.3: Package '4f1710918aa542fccb5a54d7bd712e4b0750b50d' created
zlib/1.3: Created package revision 32acc2c20f2ed21334aad213104ed3fb
[HOOK - conan-center.py] post_package_info(): [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] OK
[HOOK - conan-center.py] post_package_info(): [INCLUDE PATH DOES NOT EXIST (KB-H071)] OK
CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_POLICY_DEFAULT_CMP0091


WARN: **************************************************
WARN: *** Conan 1 is legacy and on a deprecation path **
WARN: *********** Please upgrade to Conan 2 ************
WARN: **************************************************
zlib/1.3: WARN: Using the new toolchains and generators without specifying a build profile (e.g: -pr:b=default) is discouraged and might cause failures and unexpected behavior
[HOOK - conan-center.py] post_package_info(): ERROR: [LIBRARY DOES NOT EXIST (KB-H054)] Component zlib::zlib library 'zlib' is listed in the recipe, but not found installed at self.cpp_info.libdirs. Make sure you compiled the library correctly. If so, then the library name should probably be fixed. Otherwise, then the component should be removed. (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H054-LIBRARY-DOES-NOT-EXIST) 
ERROR: 
	ConanException: [HOOK - conan-center.py] post_package_info(): Some checks failed running the hook, check the output

zlib/1.2.13:
Didn't run or was cancelled before finishing
zlib/1.2.12:
Didn't run or was cancelled before finishing
zlib/1.2.11:
Didn't run or was cancelled before finishing

^{Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.}

Conan v2 pipeline ❌

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

The v2 pipeline failed. Please, review the errors and note this is required for pull requests to be merged. In case this recipe is still not ported to Conan 2.x, please, ping @conan-io/barbarians on the PR and we will help you.

Failure in build 3 (9de4c27588de93363cb328d96b732d56091e02dc):

zlib/1.3.1:
Didn't run or was cancelled before finishing
zlib/1.3:
Didn't run or was cancelled before finishing
zlib/1.2.13:
Didn't run or was cancelled before finishing
zlib/1.2.12:
Didn't run or was cancelled before finishing

zlib/1.2.11:
CI failed to create some packages (All logs)

Logs for packageID f7dcd1cec4586c154a242a765a1de011300db8e2:

[settings]
arch=x86_64
build_type=Release
compiler=msvc
compiler.cppstd=14
compiler.runtime=dynamic
compiler.runtime_type=Release
compiler.version=193
os=Windows
[options]
*/*:shared=True

[...]
WARN: deprecated:     'cpp_info.names' used in: zlib/1.2.11

======== Testing the package ========
Removing previously existing 'test_package' build folder: C:\J\workspace\prod-v2\bsr\cci-12e2ea26\recipes\zlib\all\test_package\build\msvc-193-x86_64-14-release
zlib/1.2.11 (test package): Test package build: build\msvc-193-x86_64-14-release
zlib/1.2.11 (test package): Test package build folder: C:\J\workspace\prod-v2\bsr\cci-12e2ea26\recipes\zlib\all\test_package\build\msvc-193-x86_64-14-release
zlib/1.2.11 (test package): Writing generators to C:\J\workspace\prod-v2\bsr\cci-12e2ea26\recipes\zlib\all\test_package\build\msvc-193-x86_64-14-release\generators
zlib/1.2.11 (test package): Generator 'CMakeToolchain' calling 'generate()'
zlib/1.2.11 (test package): CMakeToolchain generated: conan_toolchain.cmake
zlib/1.2.11 (test package): CMakeToolchain generated: C:\J\workspace\prod-v2\bsr\cci-12e2ea26\recipes\zlib\all\test_package\build\msvc-193-x86_64-14-release\generators\CMakePresets.json
zlib/1.2.11 (test package): CMakeToolchain generated: C:\J\workspace\prod-v2\bsr\cci-12e2ea26\recipes\zlib\all\test_package\CMakeUserPresets.json
zlib/1.2.11 (test package): Generator 'CMakeDeps' calling 'generate()'
zlib/1.2.11 (test package): CMakeDeps necessary find_package() and targets for your CMakeLists.txt
    find_package(ZLIB)
    target_link_libraries(... ZLIB::ZLIB)
zlib/1.2.11 (test package): Generator 'VirtualRunEnv' calling 'generate()'
zlib/1.2.11 (test package): Generating aggregated env files
zlib/1.2.11 (test package): Generated aggregated env files: ['conanrun.bat', 'conanbuild.bat']

======== Testing the package: Building ========
zlib/1.2.11 (test package): Calling build()
zlib/1.2.11 (test package): Running CMake.configure()
zlib/1.2.11 (test package): RUN: cmake -G "Visual Studio 17 2022" -DCMAKE_TOOLCHAIN_FILE="generators/conan_toolchain.cmake" -DCMAKE_INSTALL_PREFIX="C:/J/workspace/prod-v2/bsr/cci-12e2ea26/recipes/zlib/all/test_package" -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" "C:/J/workspace/prod-v2/bsr/cci-12e2ea26/recipes/zlib/all/test_package"
-- Using Conan toolchain: C:/J/workspace/prod-v2/bsr/cci-12e2ea26/recipes/zlib/all/test_package/build/msvc-193-x86_64-14-release/generators/conan_toolchain.cmake
-- Conan toolchain: CMAKE_GENERATOR_TOOLSET=v143
-- Conan toolchain: Setting CMAKE_MSVC_RUNTIME_LIBRARY=$<$<CONFIG:Release>:MultiThreadedDLL>
-- Conan toolchain: C++ Standard 14 with extensions OFF
-- The C compiler identification is MSVC 19.36.32532.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.36.32532/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Conan: Target declared 'ZLIB::ZLIB'
CMake Error at build/msvc-193-x86_64-14-release/generators/cmakedeps_macros.cmake:67 (message):
  Library 'zlib' not found in package.  If 'zlib' is a system library,
  declare it with 'cpp_info.system_libs' property
Call Stack (most recent call first):
  build/msvc-193-x86_64-14-release/generators/ZLIB-Target-release.cmake:23 (conan_package_library_targets)
  build/msvc-193-x86_64-14-release/generators/ZLIBTargets.cmake:24 (include)
  build/msvc-193-x86_64-14-release/generators/ZLIBConfig.cmake:16 (include)
  CMakeLists.txt:4 (find_package)


-- Configuring incomplete, errors occurred!
See also "C:/J/workspace/prod-v2/bsr/cci-12e2ea26/recipes/zlib/all/test_package/build/msvc-193-x86_64-14-release/CMakeFiles/CMakeOutput.log".

ERROR: zlib/1.2.11 (test package): Error in build() method, line 20
	cmake.configure()
	ConanException: Error 1 while executing

^{Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.}

conan-center-bot added the User-approval pending label Aug 29, 2024

This comment has been minimized.

Sign in to view

alessiosacco force-pushed the zlib-shared branch from 4e17461 to d4cc052 Compare August 29, 2024 07:42

This comment has been minimized.

Sign in to view

jcar87 requested changes Aug 29, 2024

View reviewed changes

jcar87 mentioned this pull request Sep 3, 2024

[bug] CMakeDeps: issues when import library (.lib) and DLL (.dll) have a different filename conan-io/conan#16926

Closed

uilianries mentioned this pull request Sep 9, 2024

[bot] Update authorized users list (2024-09-09) #25188

Merged

conan-center-bot added the Failed label Sep 12, 2024

This comment has been minimized.

Sign in to view

Merge branch 'master' into zlib-shared

9de4c27

AbrilRBS removed the User-approval pending label Sep 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

zlib: Fix shared library on Windows MSVC #25076

zlib: Fix shared library on Windows MSVC #25076

alessiosacco commented Aug 29, 2024

CLAassistant commented Aug 29, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

jcar87 left a comment

alessiosacco commented Aug 29, 2024

SpaceIm commented Aug 29, 2024 •

edited

Loading

jcar87 commented Sep 3, 2024

alessiosacco commented Sep 4, 2024 •

edited

Loading

jcar87 commented Sep 5, 2024

This comment has been minimized.

This comment has been minimized.

conan-center-bot commented Sep 23, 2024

zlib: Fix shared library on Windows MSVC #25076

Are you sure you want to change the base?

zlib: Fix shared library on Windows MSVC #25076

Conversation

alessiosacco commented Aug 29, 2024

Summary

Motivation

Details

CLAassistant commented Aug 29, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

jcar87 left a comment

Choose a reason for hiding this comment

alessiosacco commented Aug 29, 2024

SpaceIm commented Aug 29, 2024 • edited Loading

jcar87 commented Sep 3, 2024

alessiosacco commented Sep 4, 2024 • edited Loading

jcar87 commented Sep 5, 2024

This comment has been minimized.

This comment has been minimized.

conan-center-bot commented Sep 23, 2024

Conan v1 pipeline ❌

Conan v2 pipeline ❌

CLAassistant commented Aug 29, 2024 •

edited

Loading

SpaceIm commented Aug 29, 2024 •

edited

Loading

alessiosacco commented Sep 4, 2024 •

edited

Loading