-
Notifications
You must be signed in to change notification settings - Fork 22
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
Fix the Python config #127
base: master
Are you sure you want to change the base?
Conversation
The previous code relied on flags set at Python configuration time rather than install time. The current flags seem to work better with non-standard Python installations
Thanks- let me think about this... |
I tested with several python 2.7 and 3.X manual python builds - can you elaborate on which python versions and what build flags it is not working with? In particular, the output of python-config.py and how it differs from your lines would be helpful. |
Here is a sample sequence of commands:
$ /nix/store/ccmyi36pqr600f2bq7nivbahnczkfqf3-python-2.7.11/bin/python /home/bnikolic/oss/swift-t/turbine/code/scripts/python-config.py --include-dir
/nix/store/ccmyi36pqr600f2bq7nivbahnczkfqf3-python-2.7.11/include/python2.7
$ cp -r /nix/store/ccmyi36pqr600f2bq7nivbahnczkfqf3-python-2.7.11/ ~/temp/pythontempinst
$ ~/temp/pythontempinst/bin/python /home/bnikolic/oss/swift-t/turbine/code/scripts/python-config.py --include-dir
/nix/store/ccmyi36pqr600f2bq7nivbahnczkfqf3-python-2.7.11/include/python2.7
$ ~/temp/pythontempinst/bin/python -m sysconfig | grep platinclude | perl -lne 'print $1 if /"(.+?)"/'
/home/bnikolic/temp/pythontempinst/include/python2.7
I.e., after a whole Python installation has been moved to different
position in filesystem "python-config.py --include-dir" gives the path
to the original include directory but the "-m sysconfig " approach gives
the correct new include directory.
This is an issue as we use a very cumbersome third party application
that is distributed as a tar ball with its own Python and all dependent
libraries. With the patch I can link against it.
|
I think the best solution to this issue is allow manually specifying the individual paths to override what python-config.py returns. The platinclude used by the patch has a different purpose than include - it's for platform specific header files. In most cases it is the same as include, but may not always be. I have no idea why it is somehow based on the moved location while include is not. It could be an idiosyncrasy and/or bug of the third part application, or specific to the way sysconfig works in certain versions of python. It's definitely not clearly documented anywhere, so this behavior could change. Rather than trying to support each exotic case that comes up, I think we should just provide a manual python setup that can support anything. |
Hmm actually it looks like |
Note that the official |
@bnikolic sorry for the delay on this. Do you still have access to the application with it's own python? I'd be interested to see the out put of ldd on the python executable. I'm trying to understand how relocatable pythons are typically built, in my testing so far you have to pass the right LDFLAGS rpath to make it work without setting LD_LIBRARY_PATH - do you need to set LD_LIBRARY_PATH for the turbine build to work? For reference, I configured python like this:
getting the escaping right was tricky. A static rpath, e.g. /opt/python/2.7.15/lib, should work too, but would break when relocated, hence the |
The application is the CASA radio astronomy package. If you have the
time to look into how it is built you can get it here:
https://casa.nrao.edu/casa_obtaining.shtml
I can have a look into this a bit more but probably not for a few days
On Tue, Oct 23, 2018, at 11:07 PM, Bryce Allen wrote:
@bnikolic[1] sorry for the delay on this. Do you still have access to
the application with it's own python? I'd be interested to see the out
put of ldd on the python executable. I'm trying to understand how
relocatable pythons are typically built, in my testing so far you have
to pass the right LDFLAGS rpath to make it work without setting
LD_LIBRARY_PATH - do you need to set LD_LIBRARY_PATH for the turbine
build to work?> For reference, I configured python like this:
./configure --prefix=/opt/python/2.7.15 --enable-shared LDFLAGS=-Wl,-
rpath,\'\$\$ORIGIN/../lib\'
> getting the escaping right was tricky. A static rpath, e.g.
/opt/python/2.7.15/lib, should work too, but would break when
relocated, hence the $ORIGIN hack with the lovely multi level
escaping.> — You are receiving this because you were mentioned. Reply to this
email directly, view it on GitHub[2], or mute the thread[3].
|
It uses the RUNPATH relative to ORIGIN technique:
2018-10-24 10:40:12 bnikolic@BNT7810:~ $ objdump -x /data/p/casa-release-5.3.0-
143.el7/lib/casa/bin/python | grep RUNPATH RUNPATH $ORIGIN/../..
On Wed, Oct 24, 2018, at 8:06 AM, Bojan Nikolic wrote:
The application is the CASA radio astronomy package. If you have the
time to look into how it is built you can get it here:>
https://casa.nrao.edu/casa_obtaining.shtml
I can have a look into this a bit more but probably not for a few days>
On Tue, Oct 23, 2018, at 11:07 PM, Bryce Allen wrote:
> @bnikolic[1] sorry for the delay on this. Do you still have access to
> the application with it's own python? I'd be interested to see the
> out put of ldd on the python executable. I'm trying to understand how
> relocatable pythons are typically built, in my testing so far you
> have to pass the right LDFLAGS rpath to make it work without setting
> LD_LIBRARY_PATH - do you need to set LD_LIBRARY_PATH for the turbine
> build to work?>> For reference, I configured python like this:
> ./configure --prefix=/opt/python/2.7.15 --enable-shared LDFLAGS=-Wl,-
> rpath,\'\$\$ORIGIN/../lib\'
>>> getting the escaping right was tricky. A static rpath, e.g.
> /opt/python/2.7.15/lib, should work too, but would break when
> relocated, hence the $ORIGIN hack with the lovely multi level
> escaping.>> — You are receiving this because you were mentioned. Reply to this
> email directly, view it on GitHub[2], or mute the thread[3].>
|
Thanks, adding
|
I can confirm this builds correctly now (and runs) |
Great thanks for checking @bnikolic , we'll go ahead and merge it. |
The previous code relied on flags set at Python configuration time
rather than install time. The current flags seem to work better with
non-standard Python installations
Check with your python builds before merging