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

RFC: Add the ability to include extensions & shared libraries in pexs. #61

Closed
wants to merge 1 commit into from

Conversation

mikekap
Copy link
Contributor

@mikekap mikekap commented Apr 6, 2015

Extensions already (sort-of) worked via add_source - this makes sure that when adding a .(so|dll|pyd), we force zip_safe to False.
In addition this adds the ability to include other shared libraries with the pex. This is intended to be used for dependencies of the python extensions (e.g. a cityhash extension will depend on a cityhash library).

The way this works is a bit hacky and only works well on unix-ish platforms. It works by changing LD_LIBRARY_PATH and re-execing the interpreter (so that the loader picks up the path change). If this should ever need to support windows, using something like AddDllDirectory will likely be cleaner.

@wickman
Copy link
Contributor

wickman commented Apr 13, 2015

will take a closer look at this today or tomorrow. thanks for the PR. this is a pretty oft requested feature.

@mikekap mikekap force-pushed the native_libs branch 3 times, most recently from de68c04 to 0073753 Compare April 15, 2015 15:27
Extensions already (sort-of) worked via add_source - this makes sure that when adding a .(so|dll|pyd), we force zip_safe to False.
In addition this adds the ability to include other shared libraries with the pex. This is intended to be used for dependencies of the python extensions (e.g. a cityhash extension will depend on a cityhash library).
@mikekap
Copy link
Contributor Author

mikekap commented May 17, 2015

PTAL - updated to support multi-pex setups.

@rouge8
Copy link
Contributor

rouge8 commented Jul 24, 2015

This is really cool, one comment though. It doesn't handle when --native-library is a symbolic link, either by resolving the link, or with a helpful error message.

With geos on CentOS 6 and this command:

pex -r /src/requirements.txt --native-library /usr/lib64/libgeos_c.so.1 --native-library /usr/lib64/libgeos-3.3.2.so -o /build/env.pex

I get this error:

Traceback (most recent call last):
  File "/usr/bin/pex", line 9, in <module>
    load_entry_point('pex==1.0.0.dev3', 'console_scripts', 'pex')()
  File "/usr/lib/python2.6/site-packages/pex/bin/pex.py", line 537, in main
    return pex.run(args=list(cmdline))
  File "/usr/lib64/python2.6/contextlib.py", line 34, in __exit__
    self.gen.throw(type, value, traceback)
  File "/usr/lib/python2.6/site-packages/pex/tracer.py", line 48, in env_override
    yield
  File "/usr/lib/python2.6/site-packages/pex/bin/pex.py", line 526, in main
    pex_builder.build(tmp_name)
  File "/usr/lib/python2.6/site-packages/pex/pex_builder.py", line 415, in build
    self.freeze()
  File "/usr/lib/python2.6/site-packages/pex/pex_builder.py", line 400, in freeze
    self._prepare_code_hash()
  File "/usr/lib/python2.6/site-packages/pex/pex_builder.py", line 260, in _prepare_code_hash
    self._pex_info.code_hash = CacheHelper.pex_hash(self._chroot.path())
  File "/usr/lib/python2.6/site-packages/pex/util.py", line 150, in pex_hash
    return cls._compute_hash(names, stream_factory)
  File "/usr/lib/python2.6/site-packages/pex/util.py", line 123, in _compute_hash
    with contextlib.closing(stream_factory(name)) as fp:
  File "/usr/lib/python2.6/site-packages/pex/util.py", line 149, in stream_factory
    return open(os.path.join(d, name), 'rb')  # noqa: T802
IOError: [Errno 2] No such file or directory: '/tmp/tmpec0Jym/native-libs/libgeos_c.so.1'

This is because libgeos_c.so.1 is actually a link:

[root@28385b087ca4 /]# ls -l /usr/lib64/libgeos_c.so.1
lrwxrwxrwx 1 root root 18 Jul 23 20:27 /usr/lib64/libgeos_c.so.1 -> libgeos_c.so.1.7.2

@jsirois
Copy link
Member

jsirois commented Oct 7, 2015

Noting that #166 has new thoughts by @wickman on this. I'm keeping these 2 linked but closing this issue to keep the conversation over on #166

@jsirois jsirois closed this Oct 7, 2015
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.

4 participants