Fix issue with custom messages (#317)

* Fix issue with custom messages Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com> --------- Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com> Co-authored-by: Eric Cousineau <eric.cousineau@tri.global>
RobotLocomotion · Nov 28, 2023 · 9063021 · 9063021
1 parent d7aa28f
commit 9063021
Show file tree

Hide file tree

Showing 10 changed files with 331 additions and 17 deletions.
diff --git a/bazel_ros2_rules/ros2/rosidl.bzl b/bazel_ros2_rules/ros2/rosidl.bzl
@@ -1083,6 +1083,56 @@ def rosidl_typesupport_cc_library(
         **kwargs
     )
 
+def _symlink_typesupport_workaround_issue311_impl(ctx):
+    filename = paths.join(
+        ctx.attr.prefix,
+        "lib" + ctx.attr.pkgname + ".so",
+    )
+
+    library = ctx.actions.declare_file(filename)
+    ctx.actions.symlink(
+        output = library,
+        target_file = ctx.executable.executable,
+        is_executable = True,
+    )
+
+    runfiles_symlinks = {
+        filename: library,
+    }
+
+    return [
+        AmentIndex(prefix = ctx.attr.prefix),
+        DefaultInfo(
+            executable = library,
+            files = depset(direct = [library]),
+            runfiles = ctx.runfiles(root_symlinks = runfiles_symlinks),
+        ),
+    ]
+
+"""
+Symlinks a shared library.
+
+This is done as a workaround to drake-ros#311, where applications like
+`ros2 bag record` may look for typesupport via ${AMENT_PREFIX_PATH}/lib instead
+of using ${LD_LIBRARY_PATH}.
+"""
+
+_symlink_typesupport_workaround_issue311 = rule(
+    _symlink_typesupport_workaround_issue311_impl,
+    attrs = {
+        "executable": attr.label(
+            executable = True,
+            cfg = "target",
+        ),
+        "prefix": attr.string(
+            default = "rosidl_generate_ament_index_entry/lib",
+        ),
+        "pkgname": attr.string(mandatory = True),
+    },
+    executable = True,
+    doc = "Creates a new target for the given executable.",
+)
+
 def rosidl_cc_support(
         name,
         interfaces,
@@ -1120,6 +1170,7 @@ def rosidl_cc_support(
         **kwargs
     )
 
+    data = list(data)
     typesupports = {}
 
     # NOTE: typesupport binary files must not have any leading
@@ -1145,6 +1196,20 @@ def rosidl_cc_support(
         typesupports["rosidl_typesupport_introspection_cpp"] = \
             _make_public_label(name, "__rosidl_typesupport_introspection_cpp")
 
+        _symlink_typesupport_workaround_issue311(
+            name = name + "_symlink_introspection_cpp",
+            executable = ":" + _make_public_name(
+                name,
+                "__rosidl_typesupport_introspection_cpp",
+            ),
+            pkgname = _make_public_name(
+                name,
+                "__rosidl_typesupport_introspection_cpp",
+            ),
+            **kwargs
+        )
+        data += [name + "_symlink_introspection_cpp"]
+
     if "rosidl_typesupport_fastrtps_cpp" in AVAILABLE_TYPESUPPORT_LIST:
         rosidl_typesupport_fastrtps_cc_library(
             name = _make_public_name(
@@ -1165,6 +1230,20 @@ def rosidl_cc_support(
         typesupports["rosidl_typesupport_fastrtps_cpp"] = \
             _make_public_label(name, "__rosidl_typesupport_fastrtps_cpp")
 
+        _symlink_typesupport_workaround_issue311(
+            name = name + "_symlink_fastrtps_cpp",
+            executable = ":" + _make_public_name(
+                name,
+                "__rosidl_typesupport_fastrtps_cpp",
+            ),
+            pkgname = _make_public_name(
+                name,
+                "__rosidl_typesupport_fastrtps_cpp",
+            ),
+            **kwargs
+        )
+        data += [name + "_symlink_fastrtps_cpp"]
+
     rosidl_typesupport_cc_library(
         name = _make_public_name(name, "__rosidl_typesupport_cpp"),
         typesupports = typesupports,
@@ -1179,6 +1258,20 @@ def rosidl_cc_support(
         **kwargs
     )
 
+    _symlink_typesupport_workaround_issue311(
+        name = name + "_symlink_typesupport_cpp",
+        executable = ":" + _make_public_name(
+            name,
+            "__rosidl_typesupport_cpp",
+        ),
+        pkgname = _make_public_name(
+            name,
+            "__rosidl_typesupport_cpp",
+        ),
+        **kwargs
+    )
+    data += [name + "_symlink_typesupport_cpp"]
+
     cc_library_rule(
         name = _make_public_name(name, "_cc"),
         srcs = [
@@ -1229,6 +1322,7 @@ def rosidl_py_support(
         **kwargs
     )
 
+    data = list(data)
     typesupports = {}
 
     # NOTE: typesupport binary files must not have any leading
@@ -1254,6 +1348,20 @@ def rosidl_py_support(
         typesupports["rosidl_typesupport_introspection_c"] = \
             _make_public_label(name, "__rosidl_typesupport_introspection_c")
 
+        _symlink_typesupport_workaround_issue311(
+            name = name + "_symlink_introspection_c",
+            executable = ":" + _make_public_name(
+                name,
+                "__rosidl_typesupport_introspection_c",
+            ),
+            pkgname = _make_public_name(
+                name,
+                "__rosidl_typesupport_introspection_c",
+            ),
+            **kwargs
+        )
+        data += [name + "_symlink_introspection_c"]
+
     if "rosidl_typesupport_fastrtps_c" in AVAILABLE_TYPESUPPORT_LIST:
         rosidl_typesupport_fastrtps_c_library(
             name = _make_public_name(
@@ -1274,6 +1382,20 @@ def rosidl_py_support(
         typesupports["rosidl_typesupport_fastrtps_c"] = \
             _make_public_label(name, "__rosidl_typesupport_fastrtps_c")
 
+        _symlink_typesupport_workaround_issue311(
+            name = name + "_symlink_fastrtps_c",
+            executable = ":" + _make_public_name(
+                name,
+                "__rosidl_typesupport_fastrtps_c",
+            ),
+            pkgname = _make_public_name(
+                name,
+                "__rosidl_typesupport_fastrtps_c",
+            ),
+            **kwargs
+        )
+        data += [name + "_symlink_fastrtps_c"]
+
     rosidl_typesupport_c_library(
         name = _make_public_name(name, "__rosidl_typesupport_c"),
         typesupports = typesupports,
@@ -1290,6 +1412,20 @@ def rosidl_py_support(
     typesupports["rosidl_typesupport_c"] = \
         _make_public_label(name, "__rosidl_typesupport_c")
 
+    _symlink_typesupport_workaround_issue311(
+        name = name + "_symlink_typesupport_c",
+        executable = ":" + _make_public_name(
+            name,
+            "__rosidl_typesupport_c",
+        ),
+        pkgname = _make_public_name(
+            name,
+            "__rosidl_typesupport_c",
+        ),
+        **kwargs
+    )
+    data += [name + "_symlink_typesupport_c"]
+
     cc_library_rule(
         name = _make_public_name(name, "_c"),
         srcs = typesupports.values(),
@@ -1331,8 +1467,11 @@ def rosidl_interfaces_group(
     """
     Generates and builds C++ and Python ROS 2 interfaces.
 
+    To depend on IDL definitions, use the `<name>_defs` target.
     To depend on C++ interfaces, use the `<name>_cc` target.
-    To depend on Python interfaces, use the `<name>_py` target.
+    To depend on Python interfaces, use the `<name>_py` target. You should
+    depend on this target for tools like `ros2 bag record` and
+    `ros2 topic echo` to work.
 
     Args:
         name: interface group name, used as prefix for target names
@@ -1381,11 +1520,13 @@ def rosidl_interfaces_group(
         cc_library_rule = cc_library_rule,
         **kwargs
     )
+    cc_name = _make_public_name(name, "_cc")
 
     rosidl_py_support(
         name,
         interfaces = interfaces,
-        data = data,
+        # Add the C++ target so that we can support `ros2 bag record`.
+        data = data + [cc_name],
         deps = deps,
         group = group,
         cc_binary_rule = cc_binary_rule,

diff --git a/bazel_ros2_rules/ros2/tools/dload.bzl b/bazel_ros2_rules/ros2/tools/dload.bzl
@@ -61,6 +61,19 @@ def get_dload_shim_attributes():
         "env_changes": attr.string_list_dict(),
     }
 
+def _workaround_issue311(ament_prefixes, env_changes):
+    # Work around drake-ros#311 and ensure that we have our
+    # ${AMEND_PREFIX_PATH}/lib libraries also exposed on LD_LIBRARY_PATH.
+    lib_folders = []
+    for prefix in ament_prefixes:
+        if prefix.endswith("/lib"):
+            lib_folders.append(prefix)
+    if len(lib_folders) > 0:
+        if "LD_LIBRARY_PATH" not in env_changes:
+            env_changes["LD_LIBRARY_PATH"] = ["path-prepend"]
+    lib_folders = depset(lib_folders).to_list()
+    env_changes["LD_LIBRARY_PATH"].extend(lib_folders)
+
 def do_dload_shim(ctx, template, to_list):
     """
     Implements common dload_shim rule functionality.
@@ -108,6 +121,8 @@ def do_dload_shim(ctx, template, to_list):
         ament_prefixes = depset(ament_prefixes).to_list()
         env_changes["AMENT_PREFIX_PATH"].extend(ament_prefixes)
 
+        _workaround_issue311(ament_prefixes, env_changes)
+
     envvars = env_changes.keys()
 
     # TODO(eric.cousineau): Should deduplicate entries for path-prepend and

diff --git a/ros2_example_bazel_installed/BUILD.bazel b/ros2_example_bazel_installed/BUILD.bazel
@@ -85,3 +85,13 @@ ros_cc_test(
     srcs = ["test/use_console_bridge.cc"],
     deps = ["@ros2//:console_bridge_vendor_cc"],
 )
+
+# Provide a roll-up of all generated IDL types for `//tools:ros2`.
+py_library(
+    name = "ros_msgs_all_py",
+    visibility = ["__subpackages__"],
+    deps = [
+        "//ros2_example_apps:ros2_example_apps_msgs_py",
+        "//ros2_example_common:ros2_example_common_msgs_py",
+    ],
+)
diff --git a/ros2_example_bazel_installed/WORKSPACE b/ros2_example_bazel_installed/WORKSPACE
@@ -72,6 +72,7 @@ ROS2_PACKAGES = [
     "rclpy",
     "ros2cli",
     "ros2cli_common_extensions",
+    "rosbag2",
     "rosidl_default_generators",
     "tf2_py",
 ] + [

diff --git a/ros2_example_bazel_installed/ros2_example_apps/BUILD.bazel b/ros2_example_bazel_installed/ros2_example_apps/BUILD.bazel
@@ -12,6 +12,7 @@ rosidl_interfaces_group(
     interfaces = [
         "msg/Status.msg",
     ],
+    visibility = ["//:__pkg__"],
     deps = [
         "//ros2_example_common:ros2_example_common_msgs",
         "@ros2//:builtin_interfaces",
@@ -201,21 +202,31 @@ ros_py_binary(
     ],
 )
 
+ros_py_test(
+    name = "custom_message_rosbag_test",
+    srcs = ["test/custom_message_rosbag_test.py"],
+    data = [
+        ":simple_talker",
+        "//tools:ros2",
+    ],
+    main = "test/custom_message_rosbag_test.py",
+    deps = [
+        "@bazel_tools//tools/python/runfiles",
+        "@ros2//resources/rmw_isolation:rmw_isolation_py",
+    ],
+)
+
 # This shows how to ensure a Bazel-provided `ros2` CLI can print out custom
 # messages (#118). See note below.
 ros_py_test(
     name = "custom_message_echo_test",
     srcs = ["test/custom_message_echo_test.py"],
     data = [
         ":simple_talker",
-        "@ros2",
+        "//tools:ros2",
     ],
     main = "test/custom_message_echo_test.py",
     deps = [
-        # This is the main key - you must provide the generated messages as a
-        # Python dependency. Otherwise, attempting to use `ros2 topic ...` will
-        # indicate the message type is invalid.
-        ":ros2_example_apps_msgs_py",
         "@bazel_tools//tools/python/runfiles",
         "@ros2//resources/rmw_isolation:rmw_isolation_py",
     ],
@@ -227,14 +238,7 @@ ros_py_test(
     name = "custom_message_list_test",
     srcs = ["test/custom_message_list_test.py"],
     data = [
-        # This is the main key. You must provide the generated definitions as
-        # a dependency. It can come through data such as the C++ (*_cc) or
-        # Python (*_py) targets as is done here, or directly via the
-        # definitions targets (*_defs).
-        # Otherwise, `ros2 interface ...` will indicate the message type is
-        # invalid.
-        ":ros2_example_apps_msgs_py",
-        "@ros2",
+        "//tools:ros2",
     ],
     main = "test/custom_message_list_test.py",
     deps = [

diff --git a/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_echo_test.py b/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_echo_test.py
@@ -11,7 +11,7 @@ def main():
         os.environ["ROS_HOME"] = os.path.join(os.environ["TEST_TMPDIR"])
 
     manifest = runfiles.Create()
-    ros2_bin = manifest.Rlocation("ros2/ros2")
+    ros2_bin = manifest.Rlocation("ros2_example_bazel_installed/tools/ros2")
     talker_bin = manifest.Rlocation(
         "ros2_example_bazel_installed/ros2_example_apps/simple_talker")
 

diff --git a/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_list_test.py b/ros2_example_bazel_installed/ros2_example_apps/test/custom_message_list_test.py
@@ -11,7 +11,7 @@ def main():
         os.environ["ROS_HOME"] = os.path.join(os.environ["TEST_TMPDIR"])
 
     manifest = runfiles.Create()
-    ros2_bin = manifest.Rlocation("ros2/ros2")
+    ros2_bin = manifest.Rlocation("ros2_example_bazel_installed/tools/ros2")
 
     interfaces = subprocess.run(
         [ros2_bin, "interface", "list"],