ament support (ament_virtualenv) about catkin_virtualenv

[paulbovbel]

Well done @max-krichenbauer, although I'm a little jealous I didn't get a chance to work on this. I also still have dreams of using a common python library to handle bundling the virtualenv that both catkin_ and ament_ call into. I'll have to have a poke once we get around to taking a ROS2 migration seriously. Consider opening issues for the pieces you feel are missing from your implementation as a starting point.

[paulbovbel]

I'm very interested in an ament port, but haven't investigated it.

To be useful, I imagine it'll need python bindings exposed by ament_virtualenv to allow it to be used from setup.py in pure python packages, somehow...

Ideally there would be a common library shared by both the catkin and ament versions, which seems possible these days using package.xml version 3 conditionals.

Furthermore, catkin_pkg, catkin.find_in_workspaces and rospkg seem to be not available in an ament build environment either.

I'm fairly sure that ament has equivalent mechanisms for all this functionality, including exporting a requirements.txt manifest - sorry I can't be more help at the moment.

[max-krichenbauer]

@paulbovbel Thank you for your reply!
My experience with the build process is fairly limited, so I don't think I could be of much help.

[max-krichenbauer]

@paulbovbel
So I started working on an ament port of catkin_virtualenv over at https://github.com/esol-community/ament_virtualenv
It's working but I'm not yet confident that it works as intended and would be happy to get some feedback.
The most important topics are:

1. ament distinguishes between cmake packages and python packages (different build types), so the project is split up into ament_virtualenv and a ament_cmake_virtualenv wrapper, following the example of ament_lint packages.
2. In ament there is no devel space, so the destination of files have all been changed. Most importantly, ${CATKIN_PACKAGE_SHARE_DESTINATION} should be ${CMAKE_INSTALL_PREFIX}/share/${PROJECT_NAME}, but I'm not sure if this is acceptable in all cases.
3. I assume Python3 as the default. I think catkin_virtualenv doesn't work for Python3. (For example, the order of parameters when using python -m venv was wrong and import Queue needs to be lower-case). I'm not sure if anything else needs to change that breaks when using Python3.

Please let me know what you think.

Best regards,
Max

[paulbovbel]

Hi @max-krichenbauer, thanks for doing the work, it looks great.

I think you're definitely on the right track, and I'd love to factor out the 'common' python bits into a library that can be shared between catkin_virtualenv and ament_virtualenv. I believe this should be possible given colcon's support for pure python packages, though I'll have to do a few experiments.

Would you be open to combining efforts here, and me pulling your work into this repository?

---

The other outstanding bit I'm interested in, is how to use something like ament_virtualenv inside pure python or ament_python packages - I imagine it should be possible to hook into the logic via setup.py, but I haven't looked at it in detail. Any thoughts?

[max-krichenbauer]

Hi @paulbovbel , thank you for your message.

Yes, I'd be happy to combine our efforts. Since my experience with ROS is very limited, I'm happy for any sort of input or collaboration.

First and foremost I actually have questions about catkin_virtualenv and how it's supposed to be used.
Probably I misunderstand it, but it seems to me that after setting up the Python virtual environment, the part that will actually trigger that the virtual environment is being used is overwriting the NOSETESTS environment variable.
That means that the virtual environment will only be used during nosetests, and it will be used for all subsequents built/tested packages, not just the one that requested the virtual environment.
Is that correct and is that the intended behavior?
If not, could you please explain the venv activation process a bit more?

[paulbovbel]

Happy to! catkin_virtualenv injects the virtualenv into execution in two ways:

1. For running ROS nodes via rosrun, roslaunch, or rostest, catkin_virtualenv hijacks catkin_install_python to install a wrapper script which loads the virtualenv before running the target python script. This wrapper script is different for 'devel' and 'install' mode in catkin - luckily this is not a relevant concept with colcon/ament.

2. In order to make sure that unit tests use the virtualenv as well, as you noticed we override the NOSETESTS variable. 'Modern' tooling builds each ROS package in isolation (catkin-tools for ROS1 workspaces, colcon for ROS1/ROS2 workspaces). This does not hold for catkin_make usage, but the ROS community has mainly moved on from using that tool.

---

Both these mechanisms probably have to be adjusted in the ament/ROS2 world. I'm particularly not a fan of the approach in #2, but it was much more straightforward than hijacking/overriding/extending catkin_add_nosetests.

[max-krichenbauer]

Thanks so much for the explanation! Now I think I finally understand how it needs to work for colcon:
ament_virtualenv would have to provide a wrapper or alternative implementation for distutils.command.build.py and/or distutils.command.install.py,
and the package using ament_virtualenv would adjust it's setup.py to include:

setup(
    cmdclass={
        'build': ament_virtualenv_build,
        'install': ament_virtualenv_install,
    },
    [...]

Then ament_virtualenv_build and/or ament_virtualenv_install can wrangle all Python files to be installed to use the virtual environment, right?

I'll do some experiments and let you know if it works as expected.

[paulbovbel]

That sounds pretty cool. I'm not super familiar with what you can and can't do in a setup.py file, but I learned recently that it shouldn't depend on importing anything, since setup.py invoked before any dependencies are installed.

Does that mean you can get away with something like a:

try:
  from ament_virtualenv import virtualenv_build
except:
  virtualenv_build = distutils.command.build.py

[max-krichenbauer]

Sorry for the delay!

I finally figured out a suitable(?) way to provide the virtual environment:
The user simply needs to add a few lines of code to his setup.py where he overrides the install cmdclass:
https://github.com/esol-community/ament_virtualenv/blob/master/test_ament_virtualenv/setup.py

There he calls an installation script that builds the venv and then goes over the installed python script entry points (in install/my_package/bin) and wraps each of them into a new script that uses the venv to call the original script:
https://github.com/esol-community/ament_virtualenv/blob/07ff430e581168010923db130c54ba959efd6470/ament_virtualenv/ament_virtualenv/install.py#L108

Please let me know what you think.

I think the code needs some simplification and refactoring, and I still have not figured out how to make it work with colcon test.

[max-krichenbauer]

I was finally able to navigate the release process and get ament_virtualenv listed on rosdistro and built on the build farm.

While there are still problems left to solve, I think we should continue that conversation over at https://github.com/esol-community/ament_virtualenv

Thank you so much @paulbovbel for your explanations.