install libpython.* in lib dir

[gst]

This correctly fix #1015 in my situation.

Have to copy the library prior to trying to execute the copied python binary from the virtualenv directory.

Reminder: I have python installations with python compiled with --with-shared and LDFLAGS='-rpath,$ORIGIN/../lib'

[gst]

Hi,

I enhanced the PR:

• only copy the shared lib when I detect the used python was compiled with -rpath with $ORIGIN in LDFLAGS, this correctly works :)
• but add an option to allow to do it in case detection is no good
• add a test for this

thx.

[gst]

@pfmoore : don't you know someone who can review & approve the PR ?

[pfmoore]

@gst I don't use Unix, so I'm not willing to merge this - I don't know anything about the issue it's trying to address, or whether this is the right way to do so. Sorry.

[gst]

that's why I ask if you know someone who can review ;)

[Ekultek]

I don't see why this wouldn't work.

[matrixise]

maybe @dstufft could review your PR ?

[gst]

@pfmoore @dstufft I'm volonteer to join the PyPA in order to help as mutch as I can ..

don't know what's / where is the entry point or howto for that..
can you hint me ?
thx.

[Erotemic]

Hi, I just ran into an issue where I compiled a version of python with shared libraries and I want to put it into a virtual environment for testing. This PR seems to be targeted at this use case.

I built my custom python2.7 (with debugging symbols and pymalloc disabled) like so:

cd ~/code
git clone https://github.com/python/cpython.git cpython-27
deactivate_venv

REPO_DIR=~/code/cpython-27
cd $REPO_DIR
git checkout v2.7.12

PYINSTALL="$REPO_DIR/install-py27-debug"

./configure \
    --prefix="$PYINSTALL" \
    --with-ensurepip=install \
    --without-pymalloc --with-pydebug --with-valgrind \
    --enable-shared --enable-unicode=ucs4 
make -j5
make install
make altinstall

I merged this PR into a local test branch.

cd ~/code
git clone https://github.com/pypa/virtualenv.git
cd virtualenv
git remote add gst https://github.com/gst/virtualenv.git
git checkout -b test
git merge gst/fix_for_shared_lib_and_rpath_origin

Then I installed it with my new python (however, to even get my python to work correctly I need to setup a the LD_LIBRARY_PATH, otherwise things will try to call system python libraries)

# NOTE THIS WILL BREAK IF YOU RUN IT HERE
# $PY_EXE -c "import ctypes"  

export PATH=$PYINSTALL/bin:$PATH
export LD_LIBRARY_PATH=$PYINSTALL/lib:$LD_LIBRARY_PATH
export C_INCLUDE_PATH=$PYINSTALL/include:$C_INCLUDE_PATH
export CPATH=$PYINSTALL/include:$CPATH

# WORKS now that the env is right
$PY_EXE -c "import ctypes"   

$PY_EXE -m pip install pip setuptools -U
cd ~/code/virtualenv
$PY_EXE -m pip install -e .

I then opened a new terminal (starting with a fresh set of environment variables) and activated my virtual env. However, there was an issue.

VENV_DIR="$HOME/code/cpython-27/venvs/venv2-debug"
source $VENV_DIR/bin/activate

# The environment is not set correctly in actiavte, so ctypes breaks again
python -c "import ctypes"

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/joncrall/code/cpython-27/install-py27-debug/lib/python2.7/ctypes/__init__.py", line 7, in <module>
    from _ctypes import Union, Structure, Array
ImportError: /home/joncrall/code/cpython-27/venvs/venv2-debug/lib/python2.7/lib-dynload/_ctypes.so: undefined symbol: _Py_RefTotal

However, with a few modifications to the activate script, things started to work correctly. Specifically I had to set LD_LIBRARY_PATH and C_INCLUDE_PATH environment variables in the activate script via this patch:

--- activate	2017-09-14 09:21:06.680270683 -0400
+++ activate.new	2017-09-14 09:21:02.272244500 -0400
@@ -8,8 +8,12 @@
     # ! [ -z ${VAR+_} ] returns true if VAR is declared at all
     if ! [ -z "${_OLD_VIRTUAL_PATH+_}" ] ; then
         PATH="$_OLD_VIRTUAL_PATH"
+        C_INCLUDE_PATH="$_OLD_C_INCLUDE_PATH"
+        VIRTUAL_LD_LIBRARY_PATH="$_OLD_VIRTUAL_LD_LIBRARY_PATH"
         export PATH
         unset _OLD_VIRTUAL_PATH
+        unset _OLD_C_INCLUDE_PATH
+        unset _OLD_VIRTUAL_LD_LIBRARY_PATH
     fi
     if ! [ -z "${_OLD_VIRTUAL_PYTHONHOME+_}" ] ; then
         PYTHONHOME="$_OLD_VIRTUAL_PYTHONHOME"
@@ -44,8 +48,16 @@
 export VIRTUAL_ENV
 
 _OLD_VIRTUAL_PATH="$PATH"
+_OLD_C_INCLUDE_PATH="$C_INCLUDE_PATH"
+_OLD_VIRTUAL_LD_LIBRARY_PATH="$LD_LIBRARY_PATH"
+
 PATH="$VIRTUAL_ENV/bin:$PATH"
+C_INCLUDE_PATH="$C_INCLUDE_PATH/include:$C_INCLUDE_PATH"
+LD_LIBRARY_PATH="$VIRTUAL_ENV/lib:$LD_LIBRARY_PATH"
+
 export PATH
+export C_INCLUDE_PATH
+export LD_LIBRARY_PATH
 
 # unset PYTHONHOME if set
 if ! [ -z "${PYTHONHOME+_}" ] ; then

Following the patch it works fine

(venv2-debug) joncrall@calculex:~/code/cpython-27/venvs/venv2-debug/include$ python -c "import ctypes"
[24205 refs]

Note, the in the example I outlined, the C_INCLUDE_PATH variable was unused, but I found that it is required (the CPATH var should also work) for CMake to find the correct python headers in the virtualenv if you are building a C++ library against Python.

Hope this helps, and I hope this patch can be merged in soon!

[markeganfuller]

We've hit the same issue on our HPC Cluster as we use environment modules to handle different Python versions.

Without the PR and @Erotemic 's patch the necessary module needs to be loaded before sourcing the virtualenv, with the PR and patch you can just source the virtualenv and it works.

For any one else wanting to test heres a quick list of the commands:

git clone  https://github.com/pypa/virtualenv.git
cd virtualenv
git remote add gst https://github.com/gst/virtualenv.git
git fetch gst
git merge --no-edit gst/fix_for_shared_lib_and_rpath_origin
patch virtualenv_embedded/activate.sh activate.patch                                                                   
python bin/rebuild-script.py
python setup.py install

I did have to tweak the patch slightly to apply but its essentially the same.

We're really hoping this PR + patch get merged, as we don't want to maintain custom patches,
I think this PR will also need patches to the other activate scripts (fish, csh, etc).

[owainkenwayucl]

Just a note to say we also have this problem at UCL with our clusters where we also use environment modules.

There's actually a worse case than the interpreter dying without being able to find the appropriate .so, namely that it finds the one from the ancient RHEL 7 version of python (2.7.5) and then sort of works but breaks in really exciting ways.

[pradyunsg]

There's a bunch of other not-needed changes here but yeah, this seems to be fine to me.

[gst]

@Erotemic

FYI: if you would configure your python like this:

./configure \
    --prefix="$PYINSTALL" \
    --with-ensurepip=install \
    --without-pymalloc --with-pydebug --with-valgrind \
    --enable-shared --enable-unicode=ucs4 \
    'LDFLAGS=-Wl,-rpath=\$$ORIGIN/../lib'

that would normally solve your problem without the need of using LD_LIBRARY_PATH in the activate script (which I believe is slightly better),
that's what I'm using and that results in a python binary which will look relatively to its own path to find the shared lib (which by default should be installed in lib subdir of --prefix)

on the other hand I'm not against adding your patch to this PR.. but normally rpath solution is just fine on Linux at least.

[btovar]

Any news on this pr? We also needed it for a cluster installation.

[gaborbernat]

Closing this due to inactivity.

[gst]

@gaborbernat

Inactivity ok but I don't see what was missing or, actually, if there was any open question / request targeted against the PR change itself ; that's why I'm dubitative :/

on the other side.. I'm not anymore impacted by the source problem, and now know how to quickly fix it if I would be again impacted, so I care quite less but it looks like at least few other guys are experiencing the same (kind of) problem and that it was (at least partially) solving. Thus I guess other people will be too impacted still in the future.

Anyway, happy new year ;)

[gaborbernat]

can you rebase on master, add changelog and tests to validate the changes then?

[gst]

Re-open if you wish.

[vesche]

Is there any reason not to re-open and merge this? I also need this feature. @gaborbernat @pradyunsg

[gaborbernat]

No reason other than someone needs to fix the merge conflicts, polish the code, add some more tests. Feel freeo tackle it if you can.