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

Rename PyInit syms to avoid clashes #183

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jayvdb
Copy link
Contributor

@jayvdb jayvdb commented Nov 8, 2019

Built extensions in packages often have common names
like speedups, utils, _objects, cpython, etc. which
reside inside the package namespace.
The compiled extensions each have a PyInit_<module>
which needs to be renamed to PyInit_<pkg>_<module>
to avoid clashes when combined into a static binary.

Fixes #169

@jayvdb
Copy link
Contributor Author

jayvdb commented Nov 8, 2019

Please be ruthless in the review; I haven't done much rust so I expect lots of ways this could be improved.

At least I think the symbol rename can be avoided if the full name doesnt contain ., which would improve performance a bit.

Also I could detect clashes first before enabling symbol renaming, avoiding unnecessary work - that is at least sensible for built executables, but maybe isnt desirable if building a library.

@jayvdb
Copy link
Contributor Author

jayvdb commented Nov 9, 2019

Alwsys rewriting objects breaks psutil ...

  = note: /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(psutil._psutil_posix.1.o): in function `psutil_pid_exists':
          /tmp/pip-install-camb2s52/psutil/psutil/_psutil_posix.c:54: multiple definition of `psutil_pid_exists'; /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(psutil._psutil_linux.1.o):/tmp/pip-install-camb2s52/psutil/psutil/_psutil_posix.c:54: first defined here
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(psutil._psutil_posix.1.o): in function `psutil_raise_for_pid':
          /tmp/pip-install-camb2s52/psutil/psutil/_psutil_posix.c:115: multiple definition of `psutil_raise_for_pid'; /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(psutil._psutil_linux.1.o):/tmp/pip-install-camb2s52/psutil/psutil/_psutil_posix.c:115: first defined here
          collect2: error: ld returned 1 exit status

and gevent

  = note: /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(gevent.__imap.0.o): in function `__Pyx_modinit_type_init_code':
          /tmp/pip-install-0la_pko2/gevent/src/gevent/_imap.c:6590: undefined reference to `__pyx_wrapperbase_6gevent_6__imap_13IMapUnordered___init__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(gevent.__imap.0.o):(.debug_info+0x55): undefined reference to `__pyx_wrapperbase_6gevent_6__imap_13IMapUnordered___init__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(gevent._greenlet.0.o): in function `__Pyx_modinit_type_init_code':
          /tmp/pip-install-0la_pko2/gevent/src/gevent/greenlet.c:18401: undefined reference to `__pyx_wrapperbase_6gevent_9_greenlet_8Greenlet___init__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(gevent._greenlet.0.o):(.debug_info+0x55): undefined reference to `__pyx_wrapperbase_6gevent_9_greenlet_8Greenlet___init__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(gevent._queue.0.o): in function `__Pyx_modinit_type_init_code':
          /tmp/pip-install-0la_pko2/gevent/src/gevent/queue.c:16637: undefined reference to `__pyx_wrapperbase_6gevent_6_queue_5Queue_20__len__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /tmp/pip-install-0la_pko2/gevent/src/gevent/queue.c:16702: undefined reference to `__pyx_wrapperbase_6gevent_6_queue_13JoinableQueue___init__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(gevent._queue.0.o):(.debug_info+0x55): undefined reference to `__pyx_wrapperbase_6gevent_6_queue_5Queue_20__len__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/psutil/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-223e3bd0c3e1b5db.rlib(gevent._queue.0.o):(.debug_info+0xe3c): undefined reference to `__pyx_wrapperbase_6gevent_6_queue_13JoinableQueue___init__'

A bug in goblin or object?

Looks like we need to only employ this when necessary, to avoid breakages.

@jayvdb jayvdb force-pushed the static-PyInit-clash branch from efe8df0 to 8b20a7b Compare November 9, 2019 09:17
@jayvdb
Copy link
Contributor Author

jayvdb commented Nov 9, 2019

The logic now avoids rewriting psutil, and it is now ok, but gevent is still failing. Need a more aggressive conflict detector, to avoid rewriting gevent built objects, and/or dig into how to get the gevent sym renames to work.

@jayvdb
Copy link
Contributor Author

jayvdb commented Nov 14, 2019

Further avoidance of sym renaming done.
Now the only time that syms are renamed is when they will conflict.
gevent still fails because its _queue conflict with the stdlib _queue, and the object fiddling fails because of gimli-rs/object#149 . I've raised gevent/gevent#1480 about that.

  = note: /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/builtext/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-61edb0164b82ba09.rlib(gevent._queue.0.o): in function `__Pyx_modinit_type_init_code':
          /tmp/pip-install-j06ct3pk/gevent/src/gevent/queue.c:16637: undefined reference to `__pyx_wrapperbase_6gevent_6_queue_5Queue_20__len__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /tmp/pip-install-j06ct3pk/gevent/src/gevent/queue.c:16702: undefined reference to `__pyx_wrapperbase_6gevent_6_queue_13JoinableQueue___init__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/builtext/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-61edb0164b82ba09.rlib(gevent._queue.0.o):(.debug_info+0x55): undefined reference to `__pyx_wrapperbase_6gevent_6_queue_5Queue_20__len__'
          /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /home/jayvdb/pyox/builtext/build/target/x86_64-unknown-linux-gnu/debug/deps/libpyembed-61edb0164b82ba09.rlib(gevent._queue.0.o):(.debug_info+0xe3c): undefined reference to `__pyx_wrapperbase_6gevent_6_queue_13JoinableQueue___init__'

Also note I ran into tobgu/pyrsistent#182 , meaning only one of pygit2 & pyrsistent are possible in any project, and probably evdev but I havent confirmed that.

@jayvdb jayvdb force-pushed the static-PyInit-clash branch from 231ca63 to d66cc29 Compare November 15, 2019 06:29
@indygreg
Copy link
Owner

Thanks for this patch. I'm probably going to wait on this review until my next serious day of hacking on PyOxidizer because there's a lot to think about here...

I'll try to get to your other pull requests tonight though...

@jayvdb
Copy link
Contributor Author

jayvdb commented Nov 21, 2019

I disabled the object mach-o mangling and now macOS links correctly, but then import simplejson fails on some rust versions with

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "simplejson", line 134, in <module>
  File "simplejson.decoder", line 8, in <module>
  File "simplejson.scanner", line 11, in <module>
  File "simplejson.scanner", line 7, in _import_c_make_scanner
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xae in position 6387: invalid start byte

object is still failing to parse the object files, which have magic 0x00 0x00 0xff 0xff

@jayvdb
Copy link
Contributor Author

jayvdb commented Nov 21, 2019

All versions of Rust on macOS 10.13 pass.

This causes the .obj files to be unparsable by editbin, dumpbin,
nm, objcopy, goblin and object.

Related to indygreg#169
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shared lib builder drops package directory
2 participants