Do not rely on external symlink for RMW_LIBRARY_PATH

[ahans]

We use a different relative path to librmw_cyclonedds.so to be compatible with Bazel's --nolegacy_external_runfiles option. Since we depend on the respective target, it can still be found in the runfiles tree, only in a different place.

Fixes #64

[mvukov]

This is a good solution for the moment, thanks. I hoped we could use https://github.com/bazelbuild/bazel/blob/master/tools/cpp/runfiles/runfiles_src.h, but looks like it's difficult to be used. Plus BAZEL_CURRENT_REPOSITORY is empty on my system -- strange; dunno if that would help at all. We can revisit this later. FWIW, it's very easy to use runfiles library from rules_python...

[ahans]

Using the runfiles library would make us more robust against changes to the layout of the runfiles tree. It's possible it would also enable some support for Linux Windows and macOS. But it would not fix the issue of #57 when you want to use a binary directly and not as part of bazel run.

I wonder how you would want to integrate the runfiles library here. Change the patch that currently injects RMW_LIBRARY_PATH to use the runfiles library? Or have a wrapper that uses the runfiles library to set an environment variable that you then use?

We would probably need a combination of $(rlocationpath @ros2_rmw_cyclonedds//:rmw_cyclonedds) in a BUILD.bazel file (probably the one I changed in this PR) and then call runfiles->Rlocation(RMW_LIBRARY_PATH); from C++ to fully resolve the path. But it would still be a path pointing to some runfiles tree, so we would still rely on that. Maybe it would create an absolute path, so that we could at least call a node binary from anywhere directly.

Btw, BAZEL_CURRENT_REPOSITORY is not an environment variable. It's injected as a pre-processor define via -DBAZEL_CURRENT_REPOSITORY=.... For me it is set, so things should work as expected.

[ahans]

I quickly tried it out.

This goes into a BUILD.bazel:

cc_binary(
    name = "hello",
    srcs = ["hello.cc"],
    data = [
        "@ros2_rmw_cyclonedds//:rmw_cyclonedds",
    ],
    local_defines = [
        "RMW_LIBRARY_PATH=\\\"$(rlocationpath @ros2_rmw_cyclonedds//:rmw_cyclonedds)\\\"",
    ],
    deps = [
        "@bazel_tools//tools/cpp/runfiles",
    ],
)

And here's hello.cc:

#include "tools/cpp/runfiles/runfiles.h"

#include <iostream>

int main(int argc, char** argv) {
  auto runfiles = bazel::tools ::cpp::runfiles::Runfiles::Create(
      argv[0], BAZEL_CURRENT_REPOSITORY);
  std::cout << "RMW_LIBRARY_PATH=" << RMW_LIBRARY_PATH << std::endl;
  std::cout << runfiles->Rlocation(RMW_LIBRARY_PATH) << std::endl;
}

Output is this:

❯ bazel run ros2:hello
INFO: Analyzed target //ros2:hello (1 packages loaded, 2 targets configured).
INFO: Found 1 target...
Target //ros2:hello up-to-date:
  bazel-bin/ros2/hello
INFO: Elapsed time: 0.249s, Critical Path: 0.17s
INFO: 3 processes: 1 internal, 2 linux-sandbox.
INFO: Build completed successfully, 3 total actions
INFO: Running command line: bazel-bin/ros2/hello
RMW_LIBRARY_PATH=ros2_rmw_cyclonedds/librmw_cyclonedds.so
/home/ahans/.cache/bazel/_bazel_ahans/c02197da98e070ec8a47cb18b5c98285/execroot/com_github_mvukov_rules_ros2/bazel-out/k8-fastbuild/bin/external/ros2_rmw_cyclonedds/librmw_cyclonedds.so

So it gives us an absolute path. Not too bad. Not sure it would solve @matzipan's use-case from #57.

[ahans]

Btw, in real code I would wrap the raw pointer in a std::unique_ptr, so we don't accidentally leak the runfiles object. I wonder why they didn't do that in the first place and return a raw pointer instead. Or, even simpler, just return the Runfiles instance by value?!

[ahans]

And another btw, I found this talk useful: https://www.youtube.com/watch?v=5NbgUMH1OGo

[mvukov]

Thanks for the link! :) I fixed the issue in a more general way in #66 . bazel::tools ::cpp::runfiles::Runfiles::Create didn't work out for me without argv[0] set to {} -- which is the case I'd need it in a rclcpp patch. Luckily, the patch doesn't need to be modified.