Run rosdep init and update in build_and_test devel tasks

[peci1]

Init and update rosdep database before running tests in devel tasks.

Having an environment with non-initialized rosdep database is (IMO) invalid, as the commands to init and update rosdep are present in every tutorial to installing ROS, even if no packages are intended to be built on the host system.

Fixes a part of #923 (and e.g. running catkin_lint directly from a CMake test target as reported in fkie/catkin_lint#108).

[peci1]

@cottsay You had a comment #923 (comment) against this kind of fix, but I did not quite understand it. Could you check whether it still applies to this PR?

[cottsay]

Could you check whether it still applies to this PR?

I would say that the comment still applies. A populated rosdep cache isn't something you can declare a dependency on from a deb or RPM package, so it isn't something we should pollute the "pristine" build environment with. There is also a fair amount of ambiguity about how "fresh" the cache might be.

For at least the past decade there have been environment variables for controlling the rosdep cache location and sources list file location. I think it would be reasonable that if your test required a populated rosdep cache, a test fixture should be made to ensure that the data is available. It could initialize and download the data into a temporary directory for the test to use without modifying the user's directory or system's /etc/... directories at all. It's up to you if you want to use a populated user-level cache, but it's hard to tell if the cache is up-to-date. Definitely an efficiency vs correctness tradeoff there.

the commands to init and update rosdep are present in every tutorial to installing ROS

I disagree. At the very least, rosdep is not used on Windows at all.

What is catkin_lint doing with the rosdep cache? Is it something we could reasonably just skip? Given that the package is building on the buildfarm to begin with, could we assume that the dependencies are all valid and satisfied?

[peci1]

A populated rosdep cache isn't something you can declare a dependency on from a deb or RPM package, so it isn't something we should pollute the "pristine" build environment with.

I only suggest adding it for tests.

There is also a fair amount of ambiguity about how "fresh" the cache might be.

The tests Docker image is built right before running, so it would always be fresh.

For at least the past decade there have been environment variables for controlling the rosdep cache location and sources list file location.

Not true. That would need ros-infrastructure/rosdep#908 getting merged.

I think it would be reasonable that if your test required a populated rosdep cache, a test fixture should be made to ensure that the data is available. It could initialize and download the data into a temporary directory for the test to use without modifying the user's directory or system's /etc/... directories at all. It's up to you if you want to use a populated user-level cache, but it's hard to tell if the cache is up-to-date. Definitely an efficiency vs correctness tradeoff there.

That would make the tests dependent on Internet connection. I don't think it's necessary.

What is catkin_lint doing with the rosdep cache? Is it something we could reasonably just skip? Given that the package is building on the buildfarm to begin with, could we assume that the dependencies are all valid and satisfied?

The catkin_lint functionality is indeed duplicating a bit the other checks the buildfarm does to ensure dependencies exist. The catkin_lint test basically reads package.xml and ensures that all packages referenced there can be resolved via rosdep. This is why it needs the rosdep cache. However, it seems weird to me to explicitly detect running on the buildfarm and turning off the test. I'd consider it more logical if the test could just run everywhere (even if it duplicates the checks done by buildfarm).

In my view, when you run the test locally, it just uses your global rosdep cache as you have it (thus not requiring internet connection), and on the buildfarm, it should use a cache downloaded when building the docker image (thus also not requiring internet connection when actually running the tests). And if you want to be super-sure it uses a clean rosdep cache, with ros-infrastructure/rosdep#908 you could just create a temporary directory, download the cache in it and set ROSDEP_CACHE_PATH for the whole build&test. This would allow building with a really clean and fresh cache while being sure you do not interfere with anything (as the cache would get downloaded to a temporary folder).

I disagree. At the very least, rosdep is not used on Windows at all.

I hope you're joking :-D

[cottsay]

For at least the past decade there have been environment variables for controlling the rosdep cache location and sources list file location.

Not true. That would need ros-infrastructure/rosdep#908 getting merged.

This pytest code works in both Python 2 and Python 3 and uses only rosdep public API. I believe this approach has been possible since about 2014.

https://gist.github.com/cottsay/549deca7a6664cebbfc3cf42a3421f6e

I disagree. At the very least, rosdep is not used on Windows at all.

I hope you're joking :-D

👎

[peci1]

Hmm, and would it also move the meta.cache dir?

[cottsay]

Hmm, and would it also move the meta.cache dir?

This approach redirects all of the content in ~/.ros, so yes. In that example, when run on a completely virgin system, the "temporary" rosdep cache contains the following:

/tmp/pytest-of-cottsay/pytest-14/ros_home0/
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/a4b3c8956ab7a3e6d293ba33b57fac42799e69e6.pickle
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/b4931581ca7c5741f7ee4ac9767a1174838296f7.pickle
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/7cfbeed211a3f9a6507c86cebaeedb3e3c4387f8.pickle
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/825fae71ab9ea366e0b22e8dbe1e2948232cb2e8.pickle
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/b67f64890f6d5269c51a68744711f115a8f207ae.pickle
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/293e47ffef56c54851128bd819e049251c816702.pickle
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/5cdc2467bc336758a462e787cebf9878a5cf2805.pickle
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/6fbb8434ee1cd44cd9d6761573ef582820b8828f.pickle
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/10be71ecf140657927f11aa700ec034e8f4759d7.pickle
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/sources.cache/index
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/meta.cache/
/tmp/pytest-of-cottsay/pytest-14/ros_home0/rosdep/meta.cache/5c57306d8ea9d8eec9db440c6e9937664e0159b5.pickle

If there was already a usable rosdep cache on the system, that example code will no-op.

[peci1]

Great. So should it be sufficient to prefix both lines added in this PR by ROS_HOME=<some dir> and creating the directory? I'm not sure how the dockerfile is (re)used. Do you know if using mktemp -d would be okay or if Docker layer caching would interfere in a bad way?

[cottsay]

I created that example to demonstrate that no changes to any of the tooling are required for a test to use data from the rosdep DB and/or rosdistro index. If a package's tests require internet resources, it need not rely on the user to acquire and cache them for the test to run - the test can acquire the resources itself.

If you were to take the example code I linked, remove the demo test_rosdep() function, and rename the file to conftest.py and drop it in the root of a package's pytest directory, then any test that needed a cache of the rosdep db or rosdistro index would only need to be updated to use the ensure_rosdep fixture. As long as the system invoking the test had either a preexisting cache of the rosdep db or an internet connection, the test should be unblocked.

You could re-use this same workflow in GTest or even create a wrapper script for invoking a subprocess the needs rosdep. All without touching the sources of rosdep or ros_buildfarm at all, and with the added benefit of breaking the test's environmental dependency on the host system's configuration.

[peci1]

I don't think that reusing the global cache (however old it is) can be called breaking the dependency on the host environment. This can actually make things much worse if it's too out-of-date.

Looking in more detail to the proposed test fixture and the overall approach using ROS_HOME, it seems to me it wouldn't work generally, because the tested code could also want to use ROS_HOME for other purposes. And when it gets redirected into a temporary directory, the tested code would fail if it prepares the content of ROS_HOME in a section where it doesn't know about this redirection. That's why I propose the ROSDEP_CACHE_PATH variable.

Try to look at it from the maintenance point of view - 2 or 3 lines of code here could completely resolve the issue for all downstream packages. Your suggestion basically throws the ball to the downstream packages which would need to implement complicated setups just for the sake of satisfying ROS buildfarm.

I still think populated rosdep cache is something a ROS package can count with on relevant platforms.