Add drake_ros_bazel_installed Bazel workspace.

[hidmic]

This PR introduces a drake_bazel_installed equivalent incorporating ROS 2. It includes:

• Build infrastructure to pull a ROS 2 Rolling binary installation into the WORKSPACE as a local repository.
• Build rules to generate and build rosidl messages, services, and actions in C++ and Python.
• Demo packages that build and use their own rosidl interfaces, both in C++ and Python.

---

This change is 

[hidmic]

I have a few CMakeLists.txt files to fix upstream before I can setup that ROS 2 local_repository.

Edit: if I'm not mistaken, it was a total of four upstream packages that needed their exported dependencies updated:

• ros2/urdf#25 (released)
• ros2/message_filters#54 (released)
• ros2/rmw_fastrtps#530 (released)
• ros-perception/laser_geometry#77 (released)

[hidmic]

It took a few more fixes to get it to build, upstream in:

• ros2/rviz#687
• ros2/rviz#688

and to the build infrastructure itself (a few more corner cases were introduced going from Eloquent to Rolling).

[hidmic]

Alright, we're getting closer. The core rosidl pipeline re-implementation lives in the rosidl.bzl.tpl file. Had to make some changes upstream (ros2/rosidl_python#131, ros2/rosidl#597) and to the original build infrastructure to help the process too.

But of course, the basic build infrastructure wasn't enough and it still isn't. Some dependencies within Rolling's binary underlay are not ROS packages but plain CMake packages (e.g. FastCDR) that the current tools miss. So I have to add support to scrap those as well.

[hidmic]

Getting closer after a few more upstream fixes and major surgery to the build infrastructure. Interfaces now build in Bazel, composing interfaces in the overlay and in another package within the Bazel workspace. Need to write some examples next and make sure runfiles and environment are OK.

[hidmic]

Alright, after some more upstream changes (see ros2/rosidl#601 & co) we now have a pair of C++ publisher and subscription working 🎉 Service and action come next. Then Python.

[hidmic]

Alright, topics, services, and action interfaces built within Bazel are fully functional in C++.

[EricCousineau-TRI]

Reviewed 1 of 34 files at r17, 29 of 29 files at r23, all commit messages.
Reviewable status: 100 of 101 files reviewed, 3 unresolved discussions (waiting on @hidmic and @IanTheEngineer)

a discussion (no related file):

Previously, hidmic (Michel Hidalgo) wrote…

Done in 24c5c7b, but only to the extent that it is possible. Several binaries, namely those that are either CPython extensions or rosidl typesupport shared libraries, must have a specific name for dependent code to be able to dlopen them later. I have yet to find a linker-agnostic way to force a cc_binary output name to be anything but the target name.

OK Thanks!

---

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/package.BUILD.bazel, line 33 at r23 (raw file):

    data = [":generate_isolated_rmw_env"],
    local_defines = [
        "RMW_ISOLATION_ROOTPATH={}/rmw_isolation".format(

TIL local_defines - nice!

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/package.BUILD.bazel, line 76 at r23 (raw file):

    data = [":isolated_talker_py", ":isolated_listener_py"],
    size = "small",
    flaky = True,

nit (Here and below) Can you mention why this is marked as flaky?

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/test/isolated_talker.py, line 31 at r23 (raw file):

    uuid = os.environ.get('TEST_TMPDIR', 'none')
    try:

BTW Why not try ... finally: rclpy.shutdown()?

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/test/isolated_talker.py, line 41 at r23 (raw file):

        raise

nit Extra lines?

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/ament_cmake.py, line 62 at r23 (raw file):

            ])
        # Fail on any /usr/local libraries
        local_link_libraries = [

BTW We may want to allow people to opt into this in the future, but I do like it being off by default - thanks!

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scrapping/system.py, line 6 at r22 (raw file):

Previously, hidmic (Michel Hidalgo) wrote…

We can't change an already linked binary nor the fact that compiler and linker will look under these directories after exhausting all user provided ones.

What we can do is to fail when we detect it. See 93fa595.

Gotcha; can you add a brief comment to that effect for DEFAULT_*?

[cottsay]

Reviewed 1 of 34 files at r17, all commit messages.
Reviewable status: all files reviewed, 18 unresolved discussions (waiting on @hidmic and @IanTheEngineer)

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 67 at r23 (raw file):

- A `<package_name>_c` C/C++ library for C libraries. Typically an alias of the
  `<package>_cc` target if C and C++ libraries cannot be told apart.
- A `<package_name>_transitive_py` Python library if the package has does not

Nix the 'has'.

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 74 at r23 (raw file):

  installed at the package-level (i.e. under `lib/<package_name>`, where
  `ros2 run` can find them).
- A `<executable_name>` Python binary for each executable installed under the

What about non-Python executables installed to the global bin directory? I'm not sure there are any in ros2.repos, but it's certainly possible to do.

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 83 at r23 (raw file):

`ros_cc_test` rules are available in the `ros_cc.bzl` file. These rules,
equivalent to the native `cc_binary` and `cc_test` rules, ensure these binaries
run in a environment that is tightly coupled with the underlying ROS 2 workspace

...run in an environment...
Same in the next paragraph.

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 99 at r23 (raw file):

following strict naming conventions (e.g. C++ and Python interface libraries
carry `_cc` and `_py` suffixes, respectively), though finer-grained control over
what is generated and built can be achieved through via other rules available in

Nix either through or via

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 111 at r23 (raw file):

**DISCLAIMER**
: Isolation relies on `rmw`-specific configuration. Support is available for 

nitpick: trailing whitespace

---

drake_ros_bazel_installed/tools/skylark/ros2/cmake_tools/init.py, line 6 at r23 (raw file):

from .server_mode import server_mode
from .packages import get_packages_with_prefixes

nitpick: alphabetize these imports

There are a lot of these later in the review. I'm going to stop calling them out.

---

drake_ros_bazel_installed/drake_ros_apps/inquirer.cc, line 10 at r23 (raw file):

#include "drake_ros_apps_msgs/msg/status.hpp"
#include "drake_ros_common_msgs/srv/query.hpp"
#include "drake_ros_common_msgs/action/do.hpp"

nitpick: Alphabetize these two includes

---

drake_ros_bazel_installed/drake_ros_apps/inquirer.cc, line 34 at r23 (raw file):

private:
  using QueryClient = rclcpp::Client<drake_ros_common_msgs::srv::Query>;

nitpick: this one has a blank line between, the next doesn't. I don't care which, but it would be nice to be consistent (same in oracle.cc)

---

drake_ros_bazel_installed/drake_ros_apps/inquirer.py, line 80 at r23 (raw file):

        else:
            self.get_logger().warning('oracle not available for actions')

nitpick: I believe there should be a blank line here.

---

drake_ros_bazel_installed/drake_ros_apps/oracle.cc, line 41 at r23 (raw file):

private:

nitpick: drop the extra blank line for consistency

---

drake_ros_bazel_installed/drake_ros_apps/README.md, line 10 at r23 (raw file):

- Equivalent `inquirer_(cc|py)` applications that subscribe to `drake_ros_apps_msgs/msg/Status` messages, make `drake_ros_common_msgs/srv/Query` service requests, and invoke the `drake_ros_common_msgs/action/Do` action.

You may run a C++ oracle against a Python inquirer, and viceversa:

sp: "vice versa"

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/init.py, line 39 at r23 (raw file):

SKIPPED_GROUP_DEPENDENCIES = ('rmw_implementation_packages',)

What makes rmw_implementation_packages special? Did I miss a note somewhere?

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/init.py, line 55 at r23 (raw file):

        for group_name in metadata['groups']:
            if group_name not in groups:
                groups[group_name] = []

Simplify these two lines:

Code snippet:

groups.setdefault(group_name, [])

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/init.py, line 62 at r23 (raw file):

        name = package_set.pop()
        metadata = packages[name]
        dependencies = set(metadata.get('build_dependencies', []))

Why do we care about the build dependencies of an already built package? Do you mean to get the build export dependencies?

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/metadata.py, line 81 at r23 (raw file):

def collect_ros_package_metadata(name, prefix):
    share_directory = os.path.join(prefix, 'share', name)

Does this hold for isolated workspaces?

[hidmic]

Reviewable status: 79 of 101 files reviewed, 17 unresolved discussions (waiting on @cottsay, @EricCousineau-TRI, and @IanTheEngineer)

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 67 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

Nix the 'has'.

Fixed in d6dceee

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 74 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

What about non-Python executables installed to the global bin directory? I'm not sure there are any in ros2.repos, but it's certainly possible to do.

Those are picked up as well. That's why I didn't qualify the noun. See d6dceee.

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 83 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

...run in an environment...
Same in the next paragraph.

Fixed in d6dceee.

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 99 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

Nix either through or via

I was indecisive it seems :) Fixed in 674f879

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 111 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

nitpick: trailing whitespace

See 674f879

---

drake_ros_bazel_installed/tools/skylark/ros2/cmake_tools/init.py, line 6 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

nitpick: alphabetize these imports

There are a lot of these later in the review. I'm going to stop calling them out.

Done in d6dceee.

---

drake_ros_bazel_installed/drake_ros_apps/inquirer.cc, line 10 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

nitpick: Alphabetize these two includes

Done in d6dceee

---

drake_ros_bazel_installed/drake_ros_apps/inquirer.cc, line 34 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

nitpick: this one has a blank line between, the next doesn't. I don't care which, but it would be nice to be consistent (same in oracle.cc)

Fixed in d6dceee.

---

drake_ros_bazel_installed/drake_ros_apps/inquirer.py, line 80 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

nitpick: I believe there should be a blank line here.

Fixed in d6dceee

---

drake_ros_bazel_installed/drake_ros_apps/oracle.cc, line 41 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

nitpick: drop the extra blank line for consistency

Fixed in d6dceee

---

drake_ros_bazel_installed/drake_ros_apps/README.md, line 10 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

sp: "vice versa"

Done in 674f879

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/package.BUILD.bazel, line 76 at r23 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

nit (Here and below) Can you mention why this is marked as flaky?

Done in b159cbe

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/test/isolated_talker.py, line 31 at r23 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

BTW Why not try ... finally: rclpy.shutdown()?

In this case, if rclpy.executors.ExternalShutdownException is raised that means rclpy.shutdown has been already invoked. Attempting to shut down twice raises.

FWIW there's rclpy.try_shutdown() to circumvent that, but last time I checked it wasn't working properly. ros2/rclpy#868 may have fixed it (got in yesterday).

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/test/isolated_talker.py, line 41 at r23 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

nit Extra lines?

One extra line indeed. Fixed in d6dceee.

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/init.py, line 39 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

What makes rmw_implementation_packages special? Did I miss a note somewhere?

This is actually a workaround for an issue that doesn't exist anymore (at some point in time, RMW_IMPLEMENTATION envvar values would be propagated down the build graph and this group dependency would cause collisions when multiple rmw implementation packages were brought in as dependencies through the rmw_implementationpackage).

Great catch. Removed in 700752a.

Edit: this uncovered a bug :) I was explicitly linking targets to their dependencies through deps, whether they were direct or group dependencies. Should've been only direct ones. Commit amended

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/init.py, line 55 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

Simplify these two lines:

Sounds good. Done in 700752a

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/init.py, line 62 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

Why do we care about the build dependencies of an already built package? Do you mean to get the build export dependencies?

metadata['build_dependencies'] is indeed populated with build_export_depends, depends, and buildtool_depends. I'll the key to 'build_export_dependencies' to avoid confusion. Also, buildtool_depends shouldn't be there. See 700752a

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/metadata.py, line 81 at r23 (raw file):

Previously, cottsay (Scott K Logan) wrote…

Does this hold for isolated workspaces?

It holds for symlink- and merge-installed workspaces.

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scrapping/system.py, line 6 at r22 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Gotcha; can you add a brief comment to that effect for DEFAULT_*?

Done in efc8a6c (I think)

[hidmic]

Had to manage another issue in 7e44de5 that 700752a uncovered. @cottsay PTAL!

Reviewable status: 78 of 101 files reviewed, 17 unresolved discussions (waiting on @cottsay, @EricCousineau-TRI, and @IanTheEngineer)

[EricCousineau-TRI]

Reviewed 5 of 23 files at r24.
Reviewable status: 83 of 101 files reviewed, 17 unresolved discussions (waiting on @cottsay, @EricCousineau-TRI, @hidmic, and @IanTheEngineer)

---

drake_ros_bazel_installed/tools/skylark/ros2/cmake_tools/server_mode.py, line 22 at r24 (raw file):

        if not address:
            address = tempfile.mktemp()
        self.__address = address

nit Use of double-under score not clearly merited (no inheritance here, so no need for mangling?).

Consider demoting / simplifying to single underscore.

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/package.BUILD.bazel, line 76 at r24 (raw file):

    data = [":isolated_talker_py", ":isolated_listener_py"],
    size = "small",
    flaky = True,  # because collision rates are non-zero

nit For formatting / clearer context, consider hoisting comments to line above code, e.g.

# Set flaky because collision rates are non-zero. See [...]
flaky = True,

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/test/isolated_talker.py, line 31 at r23 (raw file):

Previously, hidmic (Michel Hidalgo) wrote…

In this case, if rclpy.executors.ExternalShutdownException is raised that means rclpy.shutdown has been already invoked. Attempting to shut down twice raises.

FWIW there's rclpy.try_shutdown() to circumvent that, but last time I checked it wasn't working properly. ros2/rclpy#868 may have fixed it (got in yesterday).

OK Ah, gotcha. Consider linking to upstream issue (but I won't block on that).

[hidmic]

Reviewable status: 83 of 101 files reviewed, 15 unresolved discussions (waiting on @cottsay, @EricCousineau-TRI, and @IanTheEngineer)

---

drake_ros_bazel_installed/tools/skylark/ros2/cmake_tools/server_mode.py, line 22 at r24 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

nit Use of double-under score not clearly merited (no inheritance here, so no need for mangling?).

Consider demoting / simplifying to single underscore.

Done in 8d5a489.

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/package.BUILD.bazel, line 76 at r24 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

nit For formatting / clearer context, consider hoisting comments to line above code, e.g.

# Set flaky because collision rates are non-zero. See [...]
flaky = True,

Done in efb4d54

---

drake_ros_bazel_installed/tools/skylark/ros2/resources/rmw_isolation/test/isolated_talker.py, line 31 at r23 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

OK Ah, gotcha. Consider linking to upstream issue (but I won't block on that).

Done in cae236a

[EricCousineau-TRI]

Reviewed 3 of 4 files at r25.
Reviewable status: 83 of 101 files reviewed, 15 unresolved discussions (waiting on @cottsay, @EricCousineau-TRI, and @IanTheEngineer)

[hidmic]

@EricCousineau-TRI @cottsay anything else?

[EricCousineau-TRI]

Nothing on my side!

Reviewable status: 83 of 101 files reviewed, 15 unresolved discussions (waiting on @cottsay, @EricCousineau-TRI, and @IanTheEngineer)

[EricCousineau-TRI]

Reviewed 17 of 23 files at r24, 1 of 4 files at r25, all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @cottsay and @IanTheEngineer)

[EricCousineau-TRI]

Per Slack DM w/ Miche, I'll provide additional approval for Scott's comments given that he's on vacation (hence dismissals / additional comments on discussion items).

For this, I'm fine w/ serving as sole LGTM'er

Dismissed @cottsay and @hidmic from 15 discussions.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @hidmic and @IanTheEngineer)

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 74 at r23 (raw file):

Previously, hidmic (Michel Hidalgo) wrote…

Those are picked up as well. That's why I didn't qualify the noun. See d6dceee.

nit Better, but I think "binary or not" can be confusing, esp. given that binary is a bit ambiguous - e.g. ASCII vs. binary (intended context?); vs. py_binary vs. py_library (nearby context?).

Consider different qualifier, e.g:

A `<exectuable_name>` for *any* kind of executable (Python, shell script, compiled binary, etc.). For simplicity, this is exposed as a Python binary in Bazel.

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/metadata.py, line 81 at r23 (raw file):

Previously, hidmic (Michel Hidalgo) wrote…

It holds for symlink- and merge-installed workspaces.

nit Consider providing this info as a comment for this function.

(As a non-ROS2 expert, I'm not sure if there are workspace types that aren't supported, and/if it'd be easy for me to determine how the workspace is or isn't compatible.)

[hidmic]

Thank you @EricCousineau-TRI !

Reviewable status: 99 of 101 files reviewed, all discussions resolved (waiting on @EricCousineau-TRI)

---

drake_ros_bazel_installed/tools/skylark/ros2/README.md, line 74 at r23 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

nit Better, but I think "binary or not" can be confusing, esp. given that binary is a bit ambiguous - e.g. ASCII vs. binary (intended context?); vs. py_binary vs. py_library (nearby context?).

Consider different qualifier, e.g:

A `<exectuable_name>` for *any* kind of executable (Python, shell script, compiled binary, etc.). For simplicity, this is exposed as a Python binary in Bazel.

Done in 74e2e58

---

drake_ros_bazel_installed/tools/skylark/ros2/ros2bzl/scraping/metadata.py, line 81 at r23 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

nit Consider providing this info as a comment for this function.

(As a non-ROS2 expert, I'm not sure if there are workspace types that aren't supported, and/if it'd be easy for me to determine how the workspace is or isn't compatible.)

Done in f068da4

[EricCousineau-TRI]

Welcome!

Reviewed 2 of 2 files at r26, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @hidmic)