lcm-proj / lcm

Lightweight Communications and Marshalling
GNU Lesser General Public License v2.1
944 stars 385 forks source link

Update Python paths #406

Closed nosracd closed 1 year ago

nosracd commented 1 year ago

This PR fixes the following warnings, which occur while configuring a build directory:

<string>:2: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
<string>:2: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead

These happen because of a this code block which occurs in three places in the current revision:

execute_process(
  COMMAND "${PYTHON_EXECUTABLE}" -c "if True:
    from distutils import sysconfig as sc
    print(sc.get_python_lib(prefix='', plat_specific=True))"
  OUTPUT_VARIABLE PYTHON_SITE
  OUTPUT_STRIP_TRAILING_WHITESPACE)

On my system, PYTHON_SITE ends up being lib/python3/dist-packages. This variable is used to set the output of Python build artifacts to build/lib/python3/dist-packages and to install Python artifacts to /usr/local/lib/python3/dist-packages/lcm/.

This PR refactors the above Python call to instead be:

execute_process(
  COMMAND "${PYTHON_EXECUTABLE}" -c "if True:
    import sysconfig as sc
    print(sc.get_path('platlib'))"
  OUTPUT_VARIABLE PYTHON_SITE
  OUTPUT_STRIP_TRAILING_WHITESPACE)

Now, on my system, PYTHON_SITE ends up being /usr/local/lib/python3.10/dist-packages. This is still used to set the install path, but is probably not ideal for local build artifacts in the build directory. Instead, I've refactored these to be hard-coded to go to build/python.

wxmerkt commented 1 year ago

Hi, many thanks for fixing long-standing issues and returning the project to be maintained :-)

We noticed a change in behaviour from this commit, particularly the move to using absolute paths now breaks build flows and would require sudo - this can be noticed when comparing the output of the previous and the new way of finding the PYTHON_SITE variable:

from distutils import sysconfig as sc                                   
print("old", sc.get_python_lib(prefix='', plat_specific=True) # outputs: 'lib/python3/dist-packages' on Debian/Ubuntu based systems that do not use Python standard layout

import sysconfig as sc2
print("new", sc2.get_path('platlib')) # outputs: '/usr/lib/python3.8/site-packages' which is an absolute path

This may break the buildflow of other users as well (?)

wxmerkt commented 1 year ago

Note: This had already been reported in #432