Skip to content

When linking against libpng/zlib on Windows, use upstream lib names. #13084

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 24, 2019
Merged

When linking against libpng/zlib on Windows, use upstream lib names. #13084

merged 1 commit into from
Jun 24, 2019

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jan 2, 2019

See changelog for details.

Should fix the symptoms (failure to link png.lib) of #13016 (@rth, can you confirm?).

Implements part of #9693; the static/dynamic switch proposed there could be added later.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@tacaswell tacaswell added this to the v3.1 milestone Jan 2, 2019
tacaswell
tacaswell approved these changes Jan 2, 2019
View reviewed changes
Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

Modulo not messing up @cgohlke

@tacaswell
Copy link
Member

We should also check if this will have an impact on conda-forge

@anntzer anntzer force-pushed the use-upstream-libpng-zlib-windows-names branch 2 times, most recently from 74b8a16 to aba8901 Compare January 2, 2019 19:07
@anntzer
Copy link
Contributor Author

anntzer commented Jan 2, 2019

Likely also needed for #13077 unless one wants to also write the manual renaming for azure.

@anntzer anntzer mentioned this pull request Jan 2, 2019
Merged
6 tasks
@tacaswell
Copy link
Member

I'm going to close this as it has been folded into #13077

@tacaswell tacaswell closed this Jan 3, 2019
@tacaswell tacaswell removed this from the v3.1 milestone Jan 3, 2019
@anntzer anntzer deleted the use-upstream-libpng-zlib-windows-names branch January 3, 2019 10:52
@anntzer anntzer mentioned this pull request Jun 4, 2019
Closed
6 tasks
@anntzer anntzer restored the use-upstream-libpng-zlib-windows-names branch June 4, 2019 16:28
@anntzer anntzer reopened this Jun 4, 2019
@anntzer anntzer force-pushed the use-upstream-libpng-zlib-windows-names branch from aba8901 to d6aeade Compare June 4, 2019 16:32
@anntzer
Copy link
Contributor Author

anntzer commented Jun 4, 2019

Reopening as #13077 doesn't appear close to being merged and the nonstandard names are still confusing (#14355 (comment)).

@timhoffm
Copy link
Member

timhoffm commented Jun 4, 2019

Not yet merging because the second approval was long ago. @tacaswell is your approval still valid?

@anntzer anntzer requested a review from tacaswell June 20, 2019 13:36
@tacaswell
Copy link
Member

This may conflict with the patches conda applies: https://github.com/conda-forge/matplotlib-feedstock/blob/master/recipe/setupext.patch

tacaswell
tacaswell approved these changes Jun 24, 2019
View reviewed changes
Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

still 👍 but would like to give @cgohlke a change to veto this if it is going to cause him problems.

@tacaswell tacaswell added this to the v3.2.0 milestone Jun 24, 2019
@cgohlke
Copy link
Contributor

cgohlke commented Jun 24, 2019

LGTM

@tacaswell tacaswell merged commit b88d6b5 into matplotlib:master Jun 24, 2019
@anntzer anntzer deleted the use-upstream-libpng-zlib-windows-names branch June 24, 2019 23:49
@anntzer anntzer mentioned this pull request Sep 3, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

@timhoffm timhoffm timhoffm approved these changes

Assignees
No one assigned
Labels
Build OS: Microsoft
Projects
None yet
Milestone
v3.2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@anntzer @tacaswell @timhoffm @cgohlke