complete refactoring of python installation

[wolfv]

@Tobias-Fischer @traversaro I've completely changed the way we'd install Python packages now. This looks a lot more like what's going on in the debian packages, but I am not sure if the change is good. I think I should test that further.

[Tobias-Fischer]

Looks good to me! I like trial & error, should I simply bump the build number and let's have another go at it?

[wolfv]

Maybe let me try the Linux stuff locally first ... I'm scared that these changes break things

[Tobias-Fischer]

Linux seemed fine, didn't it? Maybe let's just change Windows (which is broken anyhow).

[wolfv]

Hm, I just tried these changes fro linux and they resulted in an error in one of the ament packages.

[wolfv]

I just tried the other way around by using pip install . on Windows as well .. let's see how that works :)

[wolfv]

So now I went the other way -- using pip on Windows and Unix ... that will probably also fix the problems with PYTHONPATH ... let's see!

[wolfv]

@traversaro do you know where ros2 run ... expects the scripts? With this change the regular site-packages folder on Windows is used %ROOT_PREFIX%\Lib\site-packages as opposed to the custom one that ROS was using before.

I am not sure which bin directory this will use though ... maybe one that is again not discovered by ros2 run node... although the install-scripts stuff would hopefully work properly.

[traversaro]

@traversaro do you know where ros2 run ... expects the scripts?

ros2 run pkg_name ... looks for scripts in %CONDA_PREFIX%\Library\lib\pkg_name, see https://github.com/ros2/ros2cli/blob/0.16.1/ros2pkg/ros2pkg/api/__init__.py#L44 and RoboStack/ros-galactic#74 (comment) .

[Tobias-Fischer]

I'm pretty sure I tried pip install on windows before and it didn't do the right job :(

[wolfv]

So everything looked fine with the pip install -- packages were building fine. Due to an issue with rclutils and the windows sdk on that particular machine, it didn't continue further. But I then built demo_nodes_py alone, and it ended up in this layout:

- Lib\demo_nodes_py\add_two_ints_client.exe
- Lib\demo_nodes_py\add_two_ints_client_async.exe
- Lib\demo_nodes_py\add_two_ints_server.exe
- Lib\demo_nodes_py\demo_listener.exe
- Lib\demo_nodes_py\demo_talker.exe
- Lib\demo_nodes_py\listener_qos.exe
- Lib\demo_nodes_py\listener_serialized.exe
- Lib\demo_nodes_py\talker_qos.exe
- Lib\site-packages\demo_nodes_py-0.14.3.dist-info\INSTALLER
- Lib\site-packages\demo_nodes_py-0.14.3.dist-info\METADATA
- Lib\site-packages\demo_nodes_py-0.14.3.dist-info\RECORD
- Lib\site-packages\demo_nodes_py-0.14.3.dist-info\REQUESTED
- Lib\site-packages\demo_nodes_py-0.14.3.dist-info\WHEEL
- Lib\site-packages\demo_nodes_py-0.14.3.dist-info\direct_url.json
- Lib\site-packages\demo_nodes_py-0.14.3.dist-info\entry_points.txt
- Lib\site-packages\demo_nodes_py-0.14.3.dist-info\top_level.txt
- Lib\site-packages\demo_nodes_py-0.14.3.dist-info\zip-safe
- Lib\site-packages\demo_nodes_py\__init__.py
- Lib\site-packages\demo_nodes_py\__pycache__\__init__.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\services\__init__.py
- Lib\site-packages\demo_nodes_py\services\__pycache__\__init__.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\services\__pycache__\add_two_ints_client.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\services\__pycache__\add_two_ints_client_async.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\services\__pycache__\add_two_ints_server.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\services\add_two_ints_client.py
- Lib\site-packages\demo_nodes_py\services\add_two_ints_client_async.py
- Lib\site-packages\demo_nodes_py\services\add_two_ints_server.py
- Lib\site-packages\demo_nodes_py\topics\__init__.py
- Lib\site-packages\demo_nodes_py\topics\__pycache__\__init__.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\topics\__pycache__\listener.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\topics\__pycache__\listener_qos.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\topics\__pycache__\listener_serialized.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\topics\__pycache__\talker.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\topics\__pycache__\talker_qos.cpython-38.pyc
- Lib\site-packages\demo_nodes_py\topics\listener.py
- Lib\site-packages\demo_nodes_py\topics\listener_qos.py
- Lib\site-packages\demo_nodes_py\topics\listener_serialized.py
- Lib\site-packages\demo_nodes_py\topics\talker.py
- Lib\site-packages\demo_nodes_py\topics\talker_qos.py
- share\ament_index\resource_index\packages\demo_nodes_py
- share\demo_nodes_py\package.xml

I just tried and ros2 run demo_nodes_py demo_talker gives "Package demo_nodes_py not found". However, starting python and donig import demo_nodes_py works fine.

Starting the exe directly does not work either because for some reason the interpreter path seems to be hard coded

Fatal error in launcher: Unable to create process using '"C:\Users\robostack\mambaforge\conda-bld\ros_1643409833506\_h_env\python.exe"  "C:\Users\robostack\mambaforge\envs\r2demopy\Lib\demo_nodes_py\demo_talker.exe" ': The system cannot find the file specified.

Our previous exe works either through ros2 run or directly, so not sure where this particular problem comes from (maybe setuptools >60?)

Idk we have a couple of options to tackle this:

• improve https://github.com/ament/ament_index/blob/5ce2eba781801154d93c6fd7d6f66b23e4a7182d/ament_index_python/ament_index_python/packages.py#L39 to deal with the slightly different paths for python on Windows
• create some logic dependening on the existence of the script_install stuff

[wolfv]

Hmm, looks like setuptools <60 didn't change anythign there wrt to the hard coded python path.

[wolfv]

Switching from %PYTHON% -m pip install . --no-deps -vvv to pip install . --no-deps -vvv also doesn't change anything wrt to the hard coded Python path in the .exe file. I don't know where this comes from now :/

[wolfv]

Maybe we could also figure out how to replace $base from setup.cfg with Library\lib instaead of Lib...

[wolfv]

Okay, I've looked into this problem with the pip installation a little bit more, I still don't completely get why it happens but:

the python setup.py install version creates .exe and ...-script.py files and that works well. The ...-script.py file contains the path to the python interpreter and that is properly replaced at installation time.
The pip install ... method only creates .exe files and that doesn't work.

I will try to think more on the best solution.

[Tobias-Fischer]

pip has both prefix and root arguments. There's also --install-option which are forwarded to setup.py. Maybe those arguments help?

[Tobias-Fischer]

Actually I just saw that at the end of the day https://github.com/ament/ament_index/blob/5ce2eba781801154d93c6fd7d6f66b23e4a7182d/ament_index_python/ament_index_python/packages.py#L39 looks at AMENT_PREFIX_PATH which we set in https://github.com/RoboStack/vinca/blob/master/vinca/templates/activate.bat.in - how about simply removing /Library from there and stick with the install path that pip defaults to? This is what we do on Linux

[wolfv]

I have two logs here, one from pip install and one from python setup.py install and it looks like the pip install creates a wheel and does the (wrong) .exe https://gist.github.com/wolfv/f725a25734836990e9ed1daeaf14d2cb

I still don't properly understand what is going on wrt to the .exe files. Going to try to figure it out further...

The other problem we have is that with the pip install the .../share folder is in the wrong location (at %PREFIX%\share instead of %PREFIX\Library\share).

Maybe the better idea is to keep the python setup.py install version and add special handling when we do find a install-scripts in a setup.cfg.

[Tobias-Fischer]

How about going back to 7a0b122 for windows and keep pip on Linux?

[wolfv]

Yeah, @Tobias-Fischer I think that's what we need to do. I am still really curious to understand what exactly is going on, but that probably has to wait :)

[Tobias-Fischer]

Should we merge it in @wolfv @traversaro?

[Tobias-Fischer]

Hi @wolfv @traversaro - did you have a chance to test whether this works okay in Windows? If so, I'd be happy to trigger the Galactic rebuild.

[Tobias-Fischer]

Merging, as current way is definitely broken, so it can only be an improvement.