lcm-proj / lcm

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

Python builds for python2 by default. #431

Closed judfs closed 1 year ago

judfs commented 1 year ago

For a system that has both python2 and python3, the lcm build (cmake ..) defaults to python which probably resolves to python2 which is probably not the desired target.

If there is no python or if python is python3 then this probably broken case occurs:

~~ Edit: Ignore this, came from bad testing

-- Found PythonInterp: /usr/bin/python (found version "3.10.6") 
-- Found PythonLibs: /usr/lib/x86_64-linux-gnu/libpython2.7.so (found version "2.7.18") 
-- Found Python: /usr/bin/python 

~~

I expect python2 to be targeted only if it is the only python or if it is manually specified.

If fully dropping python2 is on the roadmap, then the temporary workaround is to mention relevant cmake flags in the build instructions.

ihilt commented 1 year ago

I was able to reproduce this in a minimal ubuntu focal container (but not jammy) with the following packages installed:

openjdk-11-jdk cmake build-essential python2-dev python3 python-is-python3 libglib2.0-dev git ninja-build

Is this how the environment in question is setup? If so, can you comment on the use case for having the python2-dev matched with python3?

judfs commented 1 year ago

I have no use case for python2-dev + python3. I believe this to be an erroneous cmake resolution.

Just going to take some notes here...

docker run --entrypoint /bin/bash -it ubuntu:22.04

apt update
apt install wget unzip cmake build-essential python2-dev python3-dev libglib2.0-dev
wget https://github.com/lcm-proj/lcm/archive/refs/heads/dev-release-1.5.zip
unzip dev-release-1.5.zip 
cd lcm-dev-release-1.5/
mkdir build1 build2
cd build1
cmake ..
-- Found PythonInterp: /usr/bin/python3.10 (found version "3.10.6") 
-- Found PythonLibs: /usr/lib/x86_64-linux-gnu/libpython3.10.so (found version "3.10.6") 
-- Found Python: /usr/bin/python3.10 
cd ../build2
apt install python-is-python3
cmake ..
-- Found PythonInterp: /usr/bin/python (found version "3.10.6") 
-- Found PythonLibs: /usr/lib/x86_64-linux-gnu/libpython3.10.so (found version "3.10.6") 
-- Found Python: /usr/bin/python 

Ok those both work. I think last time I tried faking python-is-python2 with update-alternatives, but that is a bit out there. Let's actually try 20.04.

docker run --entrypoint /bin/bash -it ubuntu:20.04

apt update
apt install wget unzip cmake build-essential python2-dev python3-dev libglib2.0-dev
wget https://github.com/lcm-proj/lcm/archive/refs/heads/dev-release-1.5.zip
unzip dev-release-1.5.zip 
cd lcm-dev-release-1.5/
mkdir build1 build2 build3
cd build1
cmake ..
-- Found PythonInterp: /usr/bin/python3.8 (found version "3.8.10") 
-- Found PythonLibs: /usr/lib/x86_64-linux-gnu/libpython3.8.so (found version "3.8.10") 
-- Found Python: /usr/bin/python3.8  
apt install  python-is-python2
cd ../build2
cmake ..
-- Found PythonInterp: /usr/bin/python (found version "2.7.18") 
-- Found PythonLibs: /usr/lib/x86_64-linux-gnu/libpython2.7.so (found version "2.7.18") 
-- Found Python: /usr/bin/python 
apt install  python-is-python3
cd ../build3
cmake ..
-- Found PythonInterp: /usr/bin/python (found version "3.8.10") 
-- Found PythonLibs: /usr/lib/x86_64-linux-gnu/libpython3.8.so (found version "3.8.10") 
-- Found Python: /usr/bin/python  

Ok these all look reasonable. I guess the mismatch was caused by me being dumb and trying to simulate python-is-python2 with update-alternatives. So ignore that.

judfs commented 1 year ago

I still consider, with the combination of python2-dev python3-dev python-is-python2, cmake .. defaulting to python aka python2 to be an issue.

nosracd commented 1 year ago

I still consider, with the combination of python2-dev python3-dev python-is-python2, cmake .. defaulting to python aka python2 to be an issue.

@judfs Does #433 change the behavior to what you would expect?

judfs commented 1 year ago

This does work as I expect.

Ubuntu 18.04 LTS (standard support ends June 2023) has CMake 3.10.2 from apt. [I have systems still on it, but actual apps build/run with docker so having lcm on the 18.04 host is just for the convenience of lcm-spy].

On your branch, I was able to run cmake .. on 18.04 after

From /usr/share/cmake-3.16/Modules on 20.04, copying

./FindPython
./FindPython/Support.cmake
./SelectLibraryConfigurations.cmake
./FindPackageMessage.cmake
./FindPackageHandleStandardArgs.cmake
./FindPython.cmake

and commenting 2 CMP0094 lines from FindPython/Support.cmake. That file demands at least cmake 3.7.

Also had to comment out your version check in lcm-python.

Looks like similar cmake backporting was done before for lcm. Not sure if this is worth it.

nosracd commented 1 year ago

Ubuntu 18.04 LTS (standard support ends June 2023) has CMake 3.10.2 from apt. [I have systems still on it, but actual apps build/run with docker so having lcm on the 18.04 host is just for the convenience of lcm-spy].

It's unfortunate that #433 would eliminate 18.04 users who prefer to install cmake via apt, but couldn't they still install a more recent version of cmake via pip?

judfs commented 1 year ago

Probably? There's also PPAs or building from source. Even without back porting the files, I do not think requiring cmake 3.12 is a problem. Just clearly document it.