Improve libpython detection

[ctrueden]

As discussed in #392, conda environments on Linux using conda-forge Python have an unfortunate quirk where LDLIBRARY may point to a non-existent static libpython3.x.a library, rather than the actually present libpython3.x.so shared library. This patch improves the get_libpython() function of commands.python to handle this case, across both MULTIARCH and non-MULTIARCH locations.

$ python
Python 3.8.13 | packaged by conda-forge | (default, Mar 25 2022, 06:04:18) 
[GCC 10.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from distutils import sysconfig
>>> sysconfig.get_config_var('LDLIBRARY')
'libpython3.8.a'
>>> from commands.python import get_libpython
>>> get_libpython()
'/home/curtis/miniconda3/envs/jeptest/lib/libpython3.8.so'

It also updates setup.py to use the get_libpython function, rather than recapitulating similar logic.

Closes #392.

Two questions:

1. I noticed that setup.py was using import sysconfig but commands/python.py uses from distutils import sysconfig. Looking at the respective docstrings, these two modules do seem distinct, and a quick web search did not help me understand how they differ, or which one is more recommended to use. So a consequence of this patch is that setup.py now transitively leans on distutils.sysconfig instead of sysconfig, although their behavior appears identical regarding the get_config_var function. Does this matter at all?

2. I could not figure out how to install jep from source to test this change. I tried python setup.py build install from a clean working copy, as indicated in the README, but it does not copy over the JAR file: Edit: I figured out that python setup.py install_lib was also necessary. Should this be added to the README?

   What goes wrong with a developer install

   $ jep
   Error: Could not find or load main class jep.Run
   $ cat $(which jep) | grep jar
   cp="/home/curtis/miniconda3/envs/jeptest/lib/python3.8/site-packages/jep/jep-4.1.0.jar"
   $ ls /home/curtis/miniconda3/envs/jeptest/lib/python3.8/site-packages/jep/
   ls: cannot access '/home/curtis/miniconda3/envs/jeptest/lib/python3.8/site-packages/jep/': No such file or directory
   $ ls build/java/*.jar
   build/java/jep-4.1.0.jar          build/java/jep-4.1.0-test.jar
   build/java/jep-4.1.0-sources.jar  build/java/jep-4.1.0-test-sources.jar
   

   Although a subsequent python setup.py test does pass (230 tests, 26 skipped).

   What am I doing wrong?

[bsteffensmeier]

I think both your questions may be related #354. I think miniconda is using setuptools instead of distutils(can you confirm this?) and setuptools is putting jep in an egg instead of where we expect it. Since distutils is deprecated I think sysconfig would be better than distutils.sysconfig but since we have other work to do to move off distutils it doesn't really matter which you use here.

On a python install using distutils I do not need to do install_lib, so I am not sure if it needs to be in the documentation. We would need to test it in more environment and make sure it works. I would really like to get some resolution for #354 before releasing jep 4.1 so we might not care what distutils does since you are targeting 4.1 and we will likely need to update many things for that transition.

We do not have a firm release schedule but I am not planning to release jep 4.1 before October. If you need this in sooner we could consider putting this in 4.0.4, let me know if that matters to you.

[ctrueden]

@bsteffensmeier PR updated to use the basename of the detected path from get_libpython(). A question remains about what to do if that detected path is the multiarch one, since that nuance would be lost when setting PYTHON_LDLIBRARY. Maybe this is fine though? I don't know enough about how this stuff works to have any opinion.

Also updated the setup.py logic to fall back to LDLIBRARY again if get_libpython() fails to find anything real.

We do not have a firm release schedule but I am not planning to release jep 4.1 before October.

On my side, I think an October release would be OK, although of course releasing earlier helps the Linux portion of our user community to try out this functionality more easily. I don't want to make extra work for you, but I'm happy to rebase this PR against dev_4.0 if releasing 4.0.4 would be a simple matter for you.

[ctrueden]

miniconda is using setuptools instead of distutils(can you confirm this?)

I guess so, since this is the output I see:

$ python setup.py build install install_lib
numpy not found, running without numpy support
running build
running setup_java
running build_java
running build_jar
running build_py
running build_ext
running build_scripts
/home/curtis/miniconda3/envs/jeptest/lib/python3.8/site-packages/setuptools/command/install.py:34: SetuptoolsDeprecationWarning: setup.py install is deprecated. Use build and pip and other standards-based tools.
  warnings.warn(
running install
/home/curtis/miniconda3/envs/jeptest/lib/python3.8/site-packages/setuptools/command/easy_install.py:144: EasyInstallDeprecationWarning: easy_install command is deprecated. Use build and pip and other standards-based tools.
  warnings.warn(
running bdist_egg
...

Which looks a lot like setuptools to me. 😆

The setup.py script contains the line from distutils.core import setup, Extension and then a direct call to setup(...) in the main block, but I guess setuptools now adopted distutils and I definitely see the _distutils_hack in my environment, so, yeah.

[bsteffensmeier]

Ah, right. But then... how does MULTIARCH support work?

PYTHON_LDLIBRARY is used to reload a library that is already loaded. It's my understanding that at that point the path to the library is irrelevant and as long as the basename matches it will be reloaded so what you have is correct.

Also, do you think get_libpython should call get_python_lib_dir instead of going straight for LIBDIR?

I am fine either way. It looks like get_python_lib_dir is dead code now and only special on windows while this code is only relevant on linux so it really makes no difference right now.

On my side, I think an October release would be OK, although of course releasing earlier helps the Linux portion of our user community to try out this functionality more easily. I don't want to make extra work for you, but I'm happy to rebase this PR against dev_4.0 if releasing 4.0.4 would be a simple matter for you.

Let's leave it in dev_4.1. If anything else comes up for a new 4.0 release I will definitely cherry pick it back but otherwise I would rather wait for 4.1.

The python code looks good to me but I do not understand the change to the README. I do not see any difference in running setup.py with or without install_lib, as far as I can tell install always runs the install_lib task. The jep script has problems for me in miniconda because of setuptools putting the libraries in an egg. From my brief research I am thinking the readme should probably recommend running pip install . for a source install. See https://stackoverflow.com/questions/6301003/stopping-setup-py-from-installing-as-egg

[ctrueden]

Thanks @bsteffensmeier. I removed the README change for now, until I have time to investigate in more detail why it was necessary for me. If I figure anything out related to that, I'll open a new issue or PR for it.