ros-infrastructure / ros_buildfarm

ROS buildfarm based on Docker
Apache License 2.0
81 stars 96 forks source link

ROS_ROOT not available when running devel job tests? #923

Open peci1 opened 2 years ago

peci1 commented 2 years ago

I've got this build failing: https://build.ros.org/job/Ndev__rosmsg_cpp__ubuntu_focal_amd64/1/consoleFull .

The error is:

21:30:07 [ RUN      ] CppMsgDef.getMD5Text
21:30:08 the rosdep view is empty: call 'sudo rosdep init' and 'rosdep update'
21:30:08 Traceback (most recent call last):
21:30:08   File "/opt/ros/noetic/lib/python3/dist-packages/roslib/gentools.py", line 327, in get_dependencies
21:30:08     _add_msgs_depends(rospack, spec, deps, package)
21:30:08   File "/opt/ros/noetic/lib/python3/dist-packages/roslib/gentools.py", line 109, in _add_msgs_depends
21:30:08     key, depspec = roslib.msgs.load_by_type(t, package_context)
21:30:08   File "/opt/ros/noetic/lib/python3/dist-packages/roslib/msgs.py", line 599, in load_by_type
21:30:08     m_f = msg_file(pkg, basetype)
21:30:08   File "/opt/ros/noetic/lib/python3/dist-packages/roslib/msgs.py", line 450, in msg_file
21:30:08     return roslib.packages.resource_file(package, 'msg', type_+EXT)
21:30:08   File "/opt/ros/noetic/lib/python3/dist-packages/roslib/packages.py", line 281, in resource_file
21:30:08     d = get_pkg_subdir(package, subdir, False)
21:30:08   File "/opt/ros/noetic/lib/python3/dist-packages/roslib/packages.py", line 263, in get_pkg_subdir
21:30:08     pkg_dir = get_pkg_dir(package, required, ros_root=env[ROS_ROOT])
21:30:08   File "/usr/lib/python3.8/os.py", line 675, in __getitem__
21:30:08     raise KeyError(key) from None
21:30:08 KeyError: 'ROS_ROOT'

The code that is tested there calls roscpp RosPack() to find paths to already installed system packages (geometry_msgs). Is there a way to ensure that rosdep init && rosdep update is called in the container before running the tests?

cottsay commented 2 years ago

Is there a way to ensure that rosdep init && rosdep update is called in the container before running the tests?

Not at the moment, no. The dominant pattern regarding dependencies in the various jobs of ros_buildfarm employs minimalism as a mechanism to promote portability. Put another way, If a dependency can't be described by a package.xml, it shouldn't be expected or required.

I would suggest making your test automatically skip if the rosdep cache hasn't been created.

nuclearsandwich commented 2 years ago

This is a situation that potentially be helped by ROSDEP_ETC_DIR https://github.com/ros-infrastructure/rosdep/pull/583 to allow the entire rosdep init and update process to be isolated from the host system and thus possible to add to the test harness rather than making assumptions about the test environment.

peci1 commented 2 years ago

Okay, the rosdep init error was a false one. The root problem really is that ROS_ROOT is missing during the build. If I manually set ROS_ROOT variable before executing the test, I don't see the rospack error anymore and the test succeeds.

So, the question is whether ROS_ROOT should not be one of the things that should be available during the build (or at least test phase)... I found it is sourced in catkin env hook /opt/ros/melodic/etc/catkin/profile.d/10.rosbuild.sh installed by rosbuild package.

peci1 commented 1 year ago

catkin_lint has an actual problem with the fact that buildfarm doesn't run rosdep update before run_tests: https://github.com/fkie/catkin_lint/issues/108 .

So the code that correctly disables catkin_lint on ROS buildfarm became pretty ugly:

if(CATKIN_ENABLE_TESTING)
  find_package(roslint REQUIRED)

  # catkin_lint - checks validity of package.xml and CMakeLists.txt
  # ROS buildfarm calls this without any environment and with empty rosdep cache,
  # so we have problems reading the list of packages from env
  # see https://github.com/ros-infrastructure/ros_buildfarm/issues/923
  if(DEFINED ENV{ROS_HOME})
    #catkin_lint: ignore_once env_var
    set(ROS_HOME "$ENV{ROS_HOME}")
  else()
    #catkin_lint: ignore_once env_var
    set(ROS_HOME "$ENV{HOME}/.ros")
  endif()

  #catkin_lint: ignore_once env_var
  if(DEFINED ENV{ROS_ROOT} AND EXISTS "${ROS_HOME}/rosdep/sources.cache")
    roslint_custom(catkin_lint "-W2" .)
    roslint_add_test()
  endif()
endif()
peci1 commented 1 year ago

@cottsay Using ROSDEP_SOURCE_PATH environment variable and https://github.com/ros-infrastructure/rosdep/pull/908, it would be possible to init and update a cache that doesn't affect the host system - e.g. in /tmp or in the build space. With these changes, would it be possible to add the cache init and update to the run_tests step, and set the environment variables, so that all subsequent uses of rosdep (either via CLI or Python API) would access the custom cache (except those called with sudo, but I hope it's not possible to do stuff as root from run_tests).

ros-discourse commented 6 months ago

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros1-now-is-a-great-time-to-add-catkin-lint-to-your-packages/36521/1