Skip to content
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

Fix zlib packaging on latest conan not renaming binary appropriately #8789

Closed
wants to merge 1 commit into from

Conversation

Sil3ntStorm
Copy link
Contributor

Specify library name and version: zlib/1.2.11

Update the recipe to rename files properly from the utterly stupid names produced by zlib build scripts.
Fixes #8788


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@CLAassistant
Copy link

CLAassistant commented Jan 10, 2022

CLA assistant check
All committers have signed the CLA.

@conan-center-bot
Copy link
Collaborator

Sorry, the build is only launched for Access Request users. You can request access writing in this issue.

project(cmake_wrapper)

cmake_policy(SET CMP0091 NEW)
Copy link
Contributor

@SpaceIm SpaceIm Jan 10, 2022

Choose a reason for hiding this comment

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

Suggested change
cmake_policy(SET CMP0091 NEW)

useless, conan_basic_setup() takes care of vc runtime regardless of the policy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without it the compiler will complain about an undefined /static command line argument and iirc not produce the correct output.

@SpaceIm
Copy link
Contributor

SpaceIm commented Jan 10, 2022

It's a fix for compiler=msvc in profile actually.

@@ -1,6 +1,7 @@
cmake_minimum_required(VERSION 2.8)
cmake_minimum_required(VERSION 3.15)
Copy link
Contributor

@SpaceIm SpaceIm Jan 10, 2022

Choose a reason for hiding this comment

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

Suggested change
cmake_minimum_required(VERSION 3.15)
cmake_minimum_required(VERSION 2.8)

2.8 is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not, due to the cmake_policy requirement, which was added in 3.15 (according to a quick Google search)

Copy link
Contributor

Choose a reason for hiding this comment

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

I know but conan_basic_setup() should take care of vc runtime regardless of CMP0091 policy. If you bump to 3.15, you don't need to force CMP0091 to NEW anyway because it would be the default behavior.

Copy link
Contributor

@SpaceIm SpaceIm Jan 10, 2022

Choose a reason for hiding this comment

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

Well, maybe it's a conan issue.
@memsharded @lasote @czoido, is new msvc compiler type compatible with old CMake helper + conan_basic_setup()? Does it properly define CONAN_LINK_RUNTIME so that conan_set_vs_runtime() can work as expected?

Copy link
Contributor

Choose a reason for hiding this comment

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

is new msvc compiler type compatible with old CMake helper + conan_basic_setup()?

I would say it was fixed at Conan 1.43.2 (https://github.com/conan-io/conan/pull/10195/files) At this time we are running 1.43.1 here. (soon will be an upgrade to 1.43.3 with some backports from 1.44.1)

@Sil3ntStorm
Copy link
Contributor Author

It's a fix for compiler=msvc in profile actually.

That's what conan generates.

conan profile new --detect temp
conan profile show temp
[settings]
os=Windows
os_build=Windows
arch=x86_64
arch_build=x86_64
compiler=msvc
compiler.version=193
build_type=Release
[options]
[conf]
[build_requires]
[env]

@ghost
Copy link

ghost commented Jan 24, 2022

I detected other pull requests that are modifying zlib/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@stale
Copy link

stale bot commented Feb 23, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 23, 2022
@SSE4
Copy link
Contributor

SSE4 commented Mar 15, 2022

@FSil3ntStorm there are conflicts, please rebase

@stale stale bot removed the stale label Mar 15, 2022
@ghost ghost mentioned this pull request Mar 16, 2022
4 tasks
@ghost ghost mentioned this pull request Mar 28, 2022
4 tasks
@ghost ghost mentioned this pull request Apr 11, 2022
4 tasks
@stale
Copy link

stale bot commented Jun 12, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Jul 13, 2022

This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions.

@stale stale bot closed this Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[package] zlib/1.2.11: Files not correctly renamed with latest conan version
6 participants