RobotLocomotion / drake-ros

Experimental prototyping (for now)
Apache License 2.0
91 stars 38 forks source link

Unclear when failure occurs due to lack of `dload` shim? #113

Open EricCousineau-TRI opened 2 years ago

EricCousineau-TRI commented 2 years ago

In Anzu, we had an issue where (somehow?) a node could run without a dload shim, but it would not communicate with other processes (e.g. because ROS_LOCALHOST_ONLY was a non-default value). See Anzu 9280.

There should be some way to warn about this, ideally? (not sure if via Starlark via deps analysis, runtime check, etc.)

\cc @IanTheEngineer

EricCousineau-TRI commented 2 years ago

Per DM w/ Shane, simple analysis can check with "@ros2//:" (in whatever project). However, this won't catch transitive deps.

sloretz commented 2 years ago

Reproduction in https://github.com/RobotLocomotion/drake-ros/pull/114

EricCousineau-TRI commented 2 years ago

My suggestion, if we were to check transitive dependencies via a rule() method, we should (ideally?) be able to do something like:

Alternatively, we have some crazy non-Bazel friendly thing that uses bazel query. I don't like this option, though.

I would recommend reviewing Bazel docs on available mechanisms for querying within the analysis phase...

sloretz commented 1 year ago

I think this was resolved in Anzu using a Bazel aspect that looked for ROS 2 dependencies. At analysis phase it errors if ROS 2 is depended upon without using one of the ros_*() bazel rules (determined by checking if the rule target has a specific "ros" tag).

There are Anzu specific carve outs in the aspect's implementation there that make me think it wouldn't be able to use an upstreamed version of that check. Do you still want it upstreamed here?

EricCousineau-TRI commented 1 year ago

Hm... what aspects seem like it'd be hard to modularize? And yup, I'd like at least an example of it carved out and used in drake_ros_examples. (We find it to be a good convention, have used it for a few months, would be nice to share.)