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

zlib: add version 1.2.12 #9980

Merged
merged 3 commits into from
Mar 31, 2022
Merged

zlib: add version 1.2.12 #9980

merged 3 commits into from
Mar 31, 2022

Conversation

toge
Copy link
Contributor

@toge toge commented Mar 28, 2022

Specify library name and version: zlib/1.2.12

This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!


  • 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.

@toge toge changed the title Zlib 1.2.12 zlib: add version 1.2.12 Mar 28, 2022
@conan-center-bot

This comment has been minimized.

@ghost
Copy link

ghost commented Mar 28, 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.

intelligide
intelligide previously approved these changes Mar 28, 2022
@toge toge marked this pull request as draft March 28, 2022 23:08
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@toge toge closed this Mar 30, 2022
@toge toge reopened this Mar 30, 2022
#endif

-#if defined(_WIN32) || defined(__CYGWIN__)
+#if defined(_WIN32)
Copy link
Contributor

@a4z a4z Mar 30, 2022

Choose a reason for hiding this comment

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

I don't think that is wrong, and I hope _WIN32 also exists when using mingw to cross-compile to windows (from Linux).

But all the sudden the file will have a different checksum for the old version in the conan_imports_manifest.txt .

Is it a good idea to change it like that, why not just do exactly the same, and use #if defined(_WIN32) || defined(__MINGW32__) ?

@a4z
Copy link
Contributor

a4z commented Mar 30, 2022

This update fixes a security issue,
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-25032

Given the amount on packages that depend on zlib, it would be nice if that PR would go through, but it fails now the second time and is already some days here, and I can not see why it fails so I do not know how to help.

Any idea how the situation could be improved, can you please help me to help @uilianries or @jgsogo, or anyone else ?

@SpaceIm
Copy link
Contributor

SpaceIm commented Mar 30, 2022

This update fixes a security issue, https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-25032

Given the amount on packages that depend on zlib, it would be nice if that PR would go through, but it fails now the second time and is already some days here, and I can not see why it fails so I do not know how to help.

Any idea how the situation could be improved, can you please help me to help @uilianries or @jgsogo, or anyone else ?

Almost all PR fail in Windows agents since yesterday, I don't think it's specific to this PR.

@conan-center-bot
Copy link
Collaborator

All green in build 6 (0fef838317177e0d80a64cc831e987530092663d):

  • zlib/1.2.11@:
    All packages built successfully! (All logs)

  • zlib/1.2.12@:
    All packages built successfully! (All logs)

@toge toge marked this pull request as ready for review March 31, 2022 06:21
@a4z
Copy link
Contributor

a4z commented Mar 31, 2022

Thanks Spaclm for the explaination!

Still would like to know if it's a good idea to change header for the old version for no reason, or if people think it does not matter when the checksums in the imports manifest change

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

LGTM

@toge
Copy link
Contributor Author

toge commented Mar 31, 2022

@a4z
Thanks for your comment.
As @SpaceIm already explained, I decided to keep the patch for 1.2.11.

As you know, the patched zlib/1.2.11 is already widely used in conan recipes.
I do not want to modify the patched code.

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.

6 participants