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

Search pkgconf system libs/cflags #6138

Merged
merged 2 commits into from
May 2, 2022

Conversation

jameshilliard
Copy link
Contributor

We need to search the system paths as well from pkg-config for some packages to be found properly.

@radarhere radarhere changed the title Search pkg-config system libs/cflags. Search pkg-config system libs/cflags Mar 16, 2022
@radarhere
Copy link
Member

some packages

Could you elaborate on this a bit? It's potentially helpful to have this information to refer back to.

@jameshilliard
Copy link
Contributor Author

Could you elaborate on this a bit?

Zlib and anything that has headers in the normal system libs/include folders seems to not get their headers picked up without this change.

I think this issue is mostly going to be something people hit when cross compiling since the prefix based system libs/include folder would probably work in most of the usual cases(searching in sys.prefix will not work when cross compiling since it points to the host toolchain prefix rather than the target toolchain sysroot prefix):

Pillow/setup.py

Lines 494 to 495 in 0cf072d

_add_directory(library_dirs, os.path.join(sys.prefix, "lib"))
_add_directory(include_dirs, os.path.join(sys.prefix, "include"))

When cross compiling we try and make sure pkg-config points to the correct target sysroot however.

@jameshilliard
Copy link
Contributor Author

@radarhere FYI there's some additional background/discussion around this issue here.

@jameshilliard
Copy link
Contributor Author

@radarhere Tests should be passing now, I've re-based on main with #6229.

@radarhere
Copy link
Member

Yes, the tests will pass now, but the failure revealed a problem - on main, libtiff is detected. On this branch, it is not.

@jameshilliard
Copy link
Contributor Author

On this branch, it is not.

Weird, on ubuntu 20.04 with libtiff-dev installed it seems to get detected fine for me.

@jameshilliard
Copy link
Contributor Author

FYI there's a repo setting you can change so that you don't have to manually approve most workflows in pull requests:
Screen Shot 2022-04-22 at 12 17 36 PM

@radarhere
Copy link
Member

Ok, I stopped _pkg_config from catching exceptions, and it turned out there was an error occurring - "Unknown option --keep-system-libs".

If I try on my macOS machine,

$ pkg-config --keep-system-libs
Unknown option --keep-system-libs
$ pkg-config --keep-system-cflags
Unknown option --keep-system-cflags

So I guess those options are only available in certain environments. What is surprising though is that you say the flag works for you on your copy of Ubuntu 20.04, but it doesn't in GitHub Actions. Any ideas why that might be?

Perhaps a solution would be to try --keep-system-libs first, and then to try again without it if it doesn't work? jameshilliard#1

@jameshilliard
Copy link
Contributor Author

jameshilliard commented Apr 24, 2022

What is surprising though is that you say the flag works for you on your copy of Ubuntu 20.04, but it doesn't in GitHub Actions. Any ideas why that might be?

Yeah, not sure why that is happening.

$ pkg-config --version
1.6.3

Is the version in my ubuntu system.

Perhaps a solution would be to try --keep-system-libs first, and then to try again without it if it doesn't work?

That seems to work for me, merged to my branch here.

@radarhere
Copy link
Member

The version number is was helpful, thanks - I think you are using pkgconf, not pkg-config.

https://pkgs.org/download/pkg-config
pkgconf

There is apparently a "compatibility interface" that means you can run it as pkg-config, hence the confusion.

@radarhere radarhere changed the title Search pkg-config system libs/cflags Search pkgconf system libs/cflags Apr 24, 2022
@jameshilliard
Copy link
Contributor Author

Ah, yeah, guess that explains why the fallback is needed for some systems.

jameshilliard and others added 2 commits April 26, 2022 11:12
We need to search the system paths as well from pkg-config for
some packages to be found properly.
@jameshilliard
Copy link
Contributor Author

Rebased on python-pillow:main.

@jameshilliard
Copy link
Contributor Author

@radarhere This good to merge or anything else I should do?

@radarhere radarhere merged commit 0a30e43 into python-pillow:main May 2, 2022
@jameshilliard jameshilliard deleted the fix-pkg-config branch May 2, 2022 11:23
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.

None yet

2 participants