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

Lookup ld.so.cache instead of hardcoding search paths #3245

Merged
merged 3 commits into from Dec 29, 2018

Conversation

pslacerda
Copy link
Contributor

Fixes #3244

Changes proposed in this pull request:

  • Search library dirs in /etc/ld.so.cache instead of hardcoded assumptions;
  • The _find_strings_in_binary() parses the cache and extract directories.

It worked on my Fedora 28 workstation, but with little or even no modification it can works on FreeBSD, NetBSD and Android. Darwin architecture also has the dyld command that is similar to ldconfig on Linux.

@pslacerda
Copy link
Contributor Author

The build fails only at Alpine Linux docker image because Alpline's ldconfig (http://www.musl-libc.org/) don't has a cache. Related to docker-library/python#111.

Note that ctypes.util.find_library() first try ldconfig then gcc and ld if the former fails.

@pslacerda pslacerda force-pushed the fix/3244 branch 2 times, most recently from fa231ef to 9eadf54 Compare July 9, 2018 09:24
@pslacerda
Copy link
Contributor Author

Sorry for the noise on the last comment.

If ctypes.util returned a list of DLLs instead of picking only the required, it would be the more general solution to find where DLLs are located.

If Pillow has it dependencies installed then it can be build, right? So worth the effort to adapt ctypes.util and pack it in setup.py?

https://github.com/python/cpython/blob/master/Lib/ctypes/util.py

@pslacerda
Copy link
Contributor Author

pslacerda commented Jul 13, 2018

I just searched a bit about ldconfig and its cache.

The cache always has the same filename, and I don't expect it on a directory other than /etc, except probably /var like BSD (but it don't use glibc). And the cache format is pretty stable, apparently there is currently two formats and probably the old one nobody cares anymore.

@pslacerda pslacerda force-pushed the fix/3244 branch 7 times, most recently from 40377a9 to e76c994 Compare July 13, 2018 13:07
@pslacerda pslacerda changed the title Fix build on Docker containers Lookup ld.so.cache instead of hardcode search paths Sep 18, 2018
@radarhere radarhere changed the title Lookup ld.so.cache instead of hardcode search paths Lookup ld.so.cache instead of hardcoding search paths Sep 19, 2018
@pslacerda
Copy link
Contributor Author

Any thoughts on this? I had a hard time trying to improve it.

setup.py Outdated Show resolved Hide resolved
@hugovk
Copy link
Member

hugovk commented Oct 30, 2018

I'm not so familiar with this area so would prefer someone else to review it.

Copy link
Contributor Author

@pslacerda pslacerda left a comment

Choose a reason for hiding this comment

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

Let me know if you need some information to review.

setup.py Outdated Show resolved Hide resolved
@hugovk hugovk merged commit 5ae6f16 into python-pillow:master Dec 29, 2018
@pslacerda
Copy link
Contributor Author

Very cool that upstream developers can commit into pull requests.

Happy New year!

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

4 participants