Fixing the broken build

[crdelsey]

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	NA
Primary OS tested on	Ubuntu
Robotic platform tested on	Turtlebot 3 on Gazebo

---

Description of contribution in a few bullet points

• The build is broken due to Replace node constructor arguments with NodeOptions ros2/rclcpp#622. This PR fixes the build.
• I temporarily changed the upstream gazebo_ros_pkgs repo to point to my fork until Fix build to account for new NodeOptions interface. ros-simulation/gazebo_ros_pkgs#887 is integrated.
• I added the gazebo_ros_pkgs dependencies to the ros2_dependencies.repos files to make it easier to work with them locally. No need to use rosdep to build them. This will help avoid bringing in lots of ros-crystal dependencies in the future if their dependency list every changes as noted in [WIP] Remove unneccessary additional ROS 2 dependencies #558.
• I changed the image_common repo to point to my fork until Update ROS2 branch to account for new NodeOptions interface ros-perception/image_common#120 is integrated.
• I reverted AMCL TransformListener reuses node rather than creating its own #538. It is no longer required now that Add change error to warn for multiple loggers ros2/rcl#384 is integrated. Also by reverting this change we are no longer affected by threading error spews a non-stop stream of messages via printf ros2/geometry2#92

[mhpanah]

LGTM.

[crdelsey]

@SteveMacenski @ruffsl This is still failing the ROS build farm. I guess it must be building against the Crystal branch of ROS2 instead of master.

I don't see a way to tell the build farm to build master instead of a release branch, but we've got coverage of the master branch already with Travis and CircleCI. Maybe we should just get the ROS 2 build farm to build our crystal-devel branch.

Thoughts?

[crdelsey]

@ruffsl Am I correct in thinking we cannot get CircleCI to give a checkmark to this PR? Since it relies on the latest Docker build of master, and master is broken, the docker image won't get updated. As a result Circle CI is always building against an old version of the ROS 2 codebase that doesn't have the new interface?

[ruffsl]

This is still failing the ROS build farm. I guess it must be building against the Crystal branch of ROS2 instead of master.

The build farm from OSRF via Jinkens? I'm not sure which commit it builds against, but I would have thought it'd be master. @nuclearsandwich ?

@ruffsl Am I correct in thinking we cannot get CircleCI to give a checkmark to this PR? Since it relies on the latest Docker build of master, and master is broken, the docker image won't get updated.

Not quite, this PR has modified the .repo file, and has thus broken the "Upstream Cache" wrt that first created by the CI docker image. CircleCI will then attempt to build your upstream workspace using the .repo file in the PR. At present, https://circleci.com/gh/ros-planning/navigation2/46 , it looks like this upstream build is failing:

--- stderr: image_transport
/opt/ros_ws/src/image_common/image_transport/test/test_remapping.cpp: In member function ‘virtual void TestPublisher::SetUp()’:
/opt/ros_ws/src/image_common/image_transport/test/test_remapping.cpp:22:13: error: ‘NodeOptions’ is not a member of ‘rclcpp’
     rclcpp::NodeOptions node_options;
             ^~~~~~~~~~~
/opt/ros_ws/src/image_common/image_transport/test/test_remapping.cpp:22:13: note: suggested alternative: ‘InitOptions’
     rclcpp::NodeOptions node_options;
             ^~~~~~~~~~~
             InitOptions
/opt/ros_ws/src/image_common/image_transport/test/test_remapping.cpp:23:5: error: ‘node_options’ was not declared in this scope
     node_options.arguments(arguments);
     ^~~~~~~~~~~~
/opt/ros_ws/src/image_common/image_transport/test/test_remapping.cpp:23:5: note: suggested alternative: ‘rcl_node_options_t’
     node_options.arguments(arguments);
     ^~~~~~~~~~~~
     rcl_node_options_t
make[2]: *** [CMakeFiles/image_transport-remapping.dir/test/test_remapping.cpp.o] Error 1
make[1]: *** [CMakeFiles/image_transport-remapping.dir/all] Error 2
make: *** [all] Error 2
---
Failed   <<< image_transport	[ Exited with code 2 ]

As a result Circle CI is always building against an old version of the ROS 2 codebase that doesn't have the new interface?

The docker hub repo should auto rebuild whenever a dependent base image repo is rebuilt upstream, that being osrf/ros2:nightly, so should never be more than about 24hrs old.

---

Oh, I see what you mean now. We have the docker image attempting to build the upstream package, but those themselves were broken. Hmm, ideally it shouldn't come to this; i.e. having our upstream .repo file point to broken branches, but its all is a moving target given out nightly image is shifting underneath the .repos.

I suppose we could short circuit the CI dockerfile build to only rosdep install the upstream/navigation2 dependencies, and skip the colcon builds; thus so rely on the circleci to once a day rebuild the upstream cache for each new CI image. This would just push the upstream failure from dockerhub to circleci, but at least rosplanning/navigation2:master would always keep fresh as long as osrf/ros2:nightly stays fresh as well.

[nuclearsandwich]

The build farm from OSRF via Jinkens? I'm not sure which commit it builds against, but I would have thought it'd be master. @nuclearsandwich ?

the Cpr_* jobs test packages against the Crystal release using debs. We don't currently have a publicly available buildfarm for Dashing.

As changes to ROS 2's core are made which are incompatible with Crystal, we've been creating crystal branches and updating the Crystal release track to point to those branches instead of the development head. This NodeOptions API change might be the first Crystal-incompatible change that has affected navigation2, in which case it might make sense to branch for Crystal here as well. Once that's done we would update the rosdistro source and doc entry for navigation2 in Crystal to reference the crystal branch rather than master. Which would limit pull requests which have the Cpr jobs triggered to PRs actually targeting the crystal branch.

[crdelsey]

I suppose we could short circuit the CI dockerfile build to only rosdep install the upstream/navigation2 dependencies, and skip the colcon builds; thus so rely on the circleci to once a day rebuild the upstream cache for each new CI image. This would just push the upstream failure from dockerhub to circleci, but at least rosplanning/navigation2:master would always keep fresh as long as osrf/ros2:nightly stays fresh as well.

I'm OK with keeping it as it is. The master branch failures should be fairly rare. So long as we are aware that the CircleCI build can fail in these circumstances, we can just ignore the build failure and merge anyway.