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

Add libdl on Unix-like systems. #10310

Merged
merged 1 commit into from Jan 26, 2018

Conversation

Projects
None yet
3 participants
@QuLogic
Copy link
Member

commented Jan 24, 2018

PR Summary

Not linking with libdl causes errors when -z defs is in the linker flags.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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
@anntzer

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2018

We (and every python extension module) are already linking against libpython, which itself loads libdl. If that's not sufficient to you I'd argue you need to raise the issue upstream (with distutils)...

@QuLogic

This comment has been minimized.

Copy link
Member Author

commented Jan 25, 2018

This is not a general addition; we are calling dl* functions in the TkAgg extension, thus this is making the linkage explicit.

@anntzer

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2018

Why is this different? Does -z defs not look into transitively loaded libraries? Should it not? (Perhaps it should indeed not, I just don't know)

@QuLogic

This comment has been minimized.

Copy link
Member Author

commented Jan 25, 2018

Here is what the build looks like with -z defs:

g++ -pthread -shared -Wl,-z,relro -Wl,-z,defs -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -g -Wl,-z,relro -Wl,-z,defs -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -g -flto -fuse-linker-plugin -ffat-lto-objects -flto-partition=none build/temp.linux-x86_64-3.6/src/py_converters.o build/temp.linux-x86_64-3.6/src/_tkagg.o -L/usr/local/lib -L/usr/local/lib64 -L/usr/lib -L/usr/lib64 -L/usr/lib64 -lpython3.6m -o build/lib.linux-x86_64-3.6/matplotlib/backends/_tkagg.cpython-36m-x86_64-linux-gnu.so
build/temp.linux-x86_64-3.6/src/_tkagg.o: In function `_dfunc(void*, char const*)':
/builddir/build/BUILD/matplotlib-2.1.2/src/_tkagg.cpp:371: undefined reference to `dlerror'
/builddir/build/BUILD/matplotlib-2.1.2/src/_tkagg.cpp:372: undefined reference to `dlsym'
/builddir/build/BUILD/matplotlib-2.1.2/src/_tkagg.cpp:374: undefined reference to `dlerror'
build/temp.linux-x86_64-3.6/src/_tkagg.o: In function `load_tkinter_funcs()':
/builddir/build/BUILD/matplotlib-2.1.2/src/_tkagg.cpp:409: undefined reference to `dlopen'
/builddir/build/BUILD/matplotlib-2.1.2/src/_tkagg.cpp:433: undefined reference to `dlopen'
/builddir/build/BUILD/matplotlib-2.1.2/src/_tkagg.cpp:441: undefined reference to `dlclose'
collect2: error: ld returned 1 exit status
error: command 'g++' failed with exit status 1
@anntzer

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2018

OK, looks like that's a fedora thing: https://fedoraproject.org/wiki/UnderstandingDSOLinkChange

@QuLogic

This comment has been minimized.

Copy link
Member Author

commented Jan 25, 2018

The default for building shared libraries is to ignore undefined things. From the man page:

The default behaviour is to report errors for any undefined symbols referenced in shared libraries if the linker is being used to create an executable, but to allow them if the linker is being used to create a shared library.

But as you've also found, transitive deps are not searched for, and I believe the --copy-dt-needed-entries was switched quite a while back to disable it. (That page is from Fedora 13, which is almost 7 years ago.)

setupext.py Outdated
@@ -1493,6 +1493,8 @@ def add_flags(self, ext):
if sys.platform == 'win32':
# PSAPI library needed for finding Tcl / Tk at run time
ext.libraries.extend(['psapi'])
elif sys.platform != 'darwin':

This comment has been minimized.

Copy link
@anntzer

anntzer Jan 25, 2018

Contributor

seems to be a slightly obfuscated way to say "startswith('linux')"...

This comment has been minimized.

Copy link
@QuLogic

QuLogic Jan 25, 2018

Author Member

I had assumed it was needed on BSD, but I think that was wrong now.

This comment has been minimized.

Copy link
@QuLogic

QuLogic Jan 25, 2018

Author Member

Fixed.

@QuLogic QuLogic force-pushed the QuLogic:libdl branch from 77bae7e to 1aecf4c Jan 25, 2018

Add libdl on Linux systems.
Not linking with libdl causes errors when -z defs is in the linker
flags.

@QuLogic QuLogic force-pushed the QuLogic:libdl branch from 1aecf4c to 972cfc6 Jan 25, 2018

@anntzer anntzer merged commit 4e140c8 into matplotlib:master Jan 26, 2018

8 checks passed

ci/circleci: docs-python27 Your tests passed on CircleCI!
Details
ci/circleci: docs-python35 Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 61085e1...972cfc6
Details
codecov/project/library 63.77% (target 50%)
Details
codecov/project/tests 98.82% remains the same compared to 61085e1
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details

@QuLogic QuLogic deleted the QuLogic:libdl branch Jan 26, 2018

@QuLogic QuLogic added this to the v2.2 milestone Jan 26, 2018

@QuLogic QuLogic modified the milestones: needs sorting, v2.2.0 Feb 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.