manodeep / Corrfunc

⚡️⚡️⚡️Blazing fast correlation functions on the CPU.
https://corrfunc.readthedocs.io
MIT License
165 stars 50 forks source link

Potential issue working with virtualenv #156

Closed rainwoodman closed 6 years ago

rainwoodman commented 6 years ago

We have a user reporting unable to install nbodykit with pip, crashing at Corrfunc with virtualenv.

    make[1]: Entering directory `/tmp/pip-install-hzbzub__/Corrfunc/theory'
    ../common.mk:372: "PYTHON" is set to /home/dc-pede1/virtualenv_directory/bin/python3.6; using /home/dc-pede1/virtualenv_directory/bin/python3-config as python-config. If this is not correct, please also set "PYTHON_CONFIG_EXE" in "common.mk" to appropriate python-config
    ../common.mk:377: *** python-config (/home/dc-pede1/virtualenv_directory/bin/python3-config) not found. Please set PYTHON_CONFIG_EXE in "common.mk" to appropriate python-config before installing Corrfunc.2.0.0 .  Stop.
    make[1]: Leaving directory `/tmp/pip-install-hzbzub__/Corrfunc/theory'
    make: *** [libs] Error 2

Some brief search lead me to this:

https://github.com/posborne/dbus-python/pull/1

Might be relevant? Perhaps the default make file shall try try a few additional paths. I don't know enough of virtualenv to say exactly what can be the best approach here. cc @Chris-Pedersen ?

manodeep commented 6 years ago

Can you check if either python3-config or python-config are in the PATH?

If so, then the fix is fairly straightforward in these lines of common.mk

Otherwise, an relatively easy work-around would be to define PYTHON_CONFIG_EXE here

Chris-Pedersen commented 6 years ago

Thanks for raising this Yu - no neither of those are in PATH.

lgarrison commented 6 years ago

Setting up a clean Python 3 virtualenv shows me that python-config (but not python3-config) is present in the {virtualenv}/bin directory. So perhaps the fix is as simple as setting the default PYTHON_CONFIG_EXE to be the former instead of the latter in common.mk; the latter can be the fallback if the Python version is 3 and the former doesn't work. As long as two versions of Python aren't installed the same directory, I don't think will cause trouble since we always specify the full path in PYTHON_CONFIG_EXE.

Still, we should check that this will work. @Chris-Pedersen, could you try manually specifying /home/dc-pede1/virtualenv_directory/bin/python-config here in common.mk?

Chris-Pedersen commented 6 years ago

@lgarrison Yep that worked fine, installed now, thanks a lot :)

lgarrison commented 6 years ago

Great, unless anyone sees a problem with the above fix I'll implement it.

lgarrison commented 6 years ago

I found a problem: conda leaves multiple bin directories in $PATH if you activate one env after another. So we can't just let the shell resolve python-config, since it will resolve to a Python 2 version even if a Python 3 env was loaded after Python 2.

The solution is to always use python-config that's in the same directory as python. So what's the portable way to do get the directory of the python executable from a Makefile? This StackOverflow answer suggests that command -v python is the POSIX-compliant way, but do we want to require a POSIX shell? A quick check shows that csh/tcsh doesn't have command -v, for example.

rainwoodman commented 6 years ago

If we are to use the python on the path, and assuming it is properly installed (not a brute-force copy) then what about:

python -c "import sysconfig;print(sysconfig.get_config_var('exec_prefix');"

On Mon, Jun 25, 2018 at 10:57 AM, Lehman Garrison notifications@github.com wrote:

I found a problem: conda leaves multiple bin directories in $PATH if you activate one env after another. So we can't just let the shell resolve python-config, since it will resolve to a Python 2 version even if a Python 3 env was loaded after Python 2.

The solution is to always use python-config that's in the same directory as python. So what's the portable way to do get the directory of the python executable from a Makefile? This StackOverflow https://stackoverflow.com/questions/592620/how-to-check-if-a-program-exists-from-a-bash-script answer suggests that command -v python is the POSIX-compliant way, but do we want to require a POSIX shell? A quick check shows that csh/tcsh doesn't have command -v, for example.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/manodeep/Corrfunc/issues/156#issuecomment-400040349, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIbTIPEbCHi3n4MLjeHFeANu1Nc40s3ks5uASR1gaJpZM4U0ail .

lgarrison commented 6 years ago

I like that, thanks! I've created a PR that implements this fix.