fix pkg-config handling to make cross-compiling work #11218
Conversation
|
hopefully travis can provide test coverage here as i couldn't get pytest to pass on master (even w/out my changes). i was able to validate basic build+install via pip though. |
|
Thanks for re-opening! Sorry about the original closure. Please do ping if no one reviews in a week or so... |
|
Can someone who understand the build process better than me take a look at this? |
| self.has_pkgconfig = shutil.which(self.pkg_config) is not None | ||
| if not self.has_pkgconfig: | ||
| print("IMPORTANT WARNING:\n" | ||
| " pkg-config is not installed.\n" | ||
| " matplotlib may not be able to find some of its dependencies") | ||
|
|
||
| def set_pkgconfig_path(self): |
There was a problem hiding this comment.
Why was this removed?
Note that (unchecked...) this may be needed to make things work in a conda env which does install the libraries in the environment but where pkg-config is "outside" the env (because sysconfig.get_config_var("LIBDIR") will correctly point inside the env in that case).
There was a problem hiding this comment.
i explained in the commit message. did you see that ?
There was a problem hiding this comment.
OK, but in that case I would like someone (perhaps me, just not now) to check whether building Matplotlib as follows still works: in a conda env, freetype and libpng installed from conda, pkg-config not installed from conda but available outside of conda (e.g. from the distro). Also, what gets linked (the conda libs or the distro libs) in the shared objects in that case?
I'm not saying this won't work or that this absolutely needs to work, but if it used to and doesn't anymore, then one should make sure that the docs clearly state that a conda pkg-config is needed in that setup.
There was a problem hiding this comment.
Personally, I usually don't read the commit messages. We usually include important info in the PR body above and/or the code itself. (EDIT:) We usually try and keep the commit messages quite minimal. Thanks!
There was a problem hiding this comment.
(I don't think that "trying to keep commit messages minimal" is a good advice (quite the contrary...). But yes, it's good to also good to duplicate all the info in the thread.)
There was a problem hiding this comment.
sorry, but i'm not familiar with conda (assuming you're referring to https://conda.io/), so i really have no idea how to test it
the goal here isn't to make pkg-config required, but to make it so that if the package supports pkg-config, we only use that info
There was a problem hiding this comment.
Please put details in the commit messages! Those are what help you in 5 years while checking git blame 😉
|
I agree than making sure this does not break building in virtual environments (either venv or conda) is critical. Is there a way to identify things are being cross complied and bail in only that case? This needs a re-base onto current master. |
The current code always sets PKG_CONFIG_PATH to build paths in / which breaks cross-compiling -- things like /usr/lib are for the build system (e.g. x86) and not for the target (e.g. arm). Since we're adding paths that are already the default for pkg-config, there's no point in trying to be smart here. Just punt the code. This basically reverts commit 101beb9.
…able If we have pkg-config, then there's no need to hardcode the system -I/-L paths as the pkg-config files will give us all the info we need.
|
Things actually appear to work fine in the setup I described, although it's totally unclear to me why they do (I expected they wouldn't; I guess I don't understand enough about shared object resolution...). |
|
What is the status of this one? |
This series cleans up the pkg-config logic so that cross-compile works properly. There's still some weirdness with the use of freetype-config/libpng-config, but I've ignored that as it only impacts the display and not the actual config/code generation.