Skip to content

maint: setupext.py for freetype had a Catch case for missing ft2build.h #12321

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

Merged
merged 1 commit into from
Sep 29, 2018
Merged

Conversation

zerothi
Copy link
Contributor

@zerothi zerothi commented Sep 28, 2018

The error was that the path was constructed manually with '\'. This is now fixed
to use os.path.join

A very minor change in setupext.py

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

The error was that the path was constructed manually with '\'. This is now fixed
to use os.path.join

Signed-off-by: Nick Papior <nickpapior@gmail.com>
@@ -906,7 +906,7 @@ def check(self):
try:
check_include_file(get_include_dirs(), 'ft2build.h', 'freetype')
except CheckFailed:
check_include_file(get_include_dirs(), 'freetype2\\ft2build.h', 'freetype')
check_include_file(get_include_dirs(), os.path.join('freetype2', 'ft2build.h'), 'freetype')
Copy link
Member

Choose a reason for hiding this comment

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

This block should only be executed if sys.platform is windows, but I guess the change is reasonable. Does it fix a known bug?

Copy link
Contributor Author

@zerothi zerothi Sep 28, 2018

Choose a reason for hiding this comment

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

No, it may also be executed IFF you don't have ft2build.h installed in /usr/include. I.e. if you install freetype in a non-default location and you want to link it using setup.cfg and basedirlist, then you have problems because freetype installs the headers in the freetype2/ directory

@anntzer anntzer added the Build label Sep 28, 2018
@jklymak
Copy link
Member

jklymak commented Sep 28, 2018

Where does this need backporting to? Can an approver milestone?

@tacaswell tacaswell modified the milestones: v3.0.x, v2.2.4 Sep 29, 2018
@tacaswell
Copy link
Member

Backporting all the way to 2.2.x as this fixes a build issue.

@tacaswell tacaswell merged commit 6ebc395 into matplotlib:master Sep 29, 2018
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 29, 2018
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 29, 2018
tacaswell added a commit that referenced this pull request Sep 29, 2018
…321-on-v2.2.x

Backport PR #12321 on branch v2.2.x (maint: setupext.py for freetype had a Catch case for missing ft2build.h)
QuLogic added a commit that referenced this pull request Sep 30, 2018
…321-on-v3.0.x

Backport PR #12321 on branch v3.0.x (maint: setupext.py for freetype had a Catch case for missing ft2build.h)
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.

5 participants