Support `extra_deps` arg for `rosidl` both `cc` and `py`
This stems from a problem I ran into yesterday, where python_rules numpy wasn't providing the cpp headers and I would end up in a following scenario -
ali@bdai_docker:/workspaces/bdai/projects/dexterity$ bazel build //...
INFO: Options provided by the client:
Inherited 'common' options: --isatty=1 --terminal_columns=103
INFO: Reading rc options for 'build' from /workspaces/bdai/projects/dexterity/.bazelrc:
'build' options: --announce_rc --cxxopt=-std=c++20 --cxxopt=-Werror --cxxopt=-Wall --cxxopt=-Wextra --cxxopt=-Wdouble-promotion --cxxopt=-Wformat --cxxopt=-Wunused --cxxopt=-Wfloat-equal --cxxopt=-Wshadow --cxxopt=-Wconversion --cxxopt=-Winline --cxxopt=-Wno-variadic-macros --cxxopt=-Wnon-virtual-dtor --cxxopt=-Wpedantic --incompatible_strict_action_env --@aspect_rules_py//py:interpreter_version=3.10.12
INFO: Analyzed 59 targets (5 packages loaded, 10431 targets configured).
INFO: Found 59 targets...
ERROR: /workspaces/bdai/projects/dexterity/ws/src/mocap_print/BUILD:5:24: Compiling ws/src/mocap_print/mocap_generated_interfaces/py/mocap_generated_interfaces/msg/_marker_s.c failed: (Exit 1): gcc failed: error executing command (from target //ws/src/mocap_print:_mocap_generated_interfaces_py_c) /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -MD -MF ... (remaining 110 arguments skipped)
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
bazel-out/k8-fastbuild/bin/ws/src/mocap_print/mocap_generated_interfaces/py/mocap_generated_interfaces/msg/_marker_s.c:11:10: fatal error: numpy/ndarrayobject.h: No such file or directory
11 | #include "numpy/ndarrayobject.h"
| ^~~~~~~~~~~~~~~~~~~~~~~
And unsurprisingly -
ali@bdai_docker:/workspaces/bdai/projects/dexterity$ bazelisk query "@pypi_numpy//:*" | grep ndarrayobject
Loading: 0 packages loaded
@pypi_numpy//:site-packages/numpy/core/include/numpy/ndarrayobject.h
Loading: 0 packages loaded
Via this patch users would be able to provide the cc dependency as an extra. I don't think it's a perfect solution, but it is a solution that doesn't involve touching python_rules themselves. An example to use this would be -
rosidl_interfaces_group(
name = "mocap_generated_interfaces",
interfaces = [
"msg/RigidBody.msg",
"msg/Marker.msg",
"msg/RigidBodies.msg",
],
visibility = ["//:__pkg__"],
deps = [
"@ros2//:builtin_interfaces",
"@ros2//:geometry_msgs",
],
extra_deps = [
"@numpy_repo//:numpy_cc",
],
)
cc: thanks @dta-bdai for this patch :)
#include "numpy/ndarrayobject.h"
Hm... I'm a bit suspicious that either
(a) we have a bug in drake-ros that we're not able to observe (i.e., we have system-installed numpy, and you do not), or
(b) there is something misconfigured on your side that requires numpy when really it shouldn't be required.
Do you know if you have system-wide installed numpy, or is it purely local to Bazel?
Also, can you reproduce this error in drake-ros itself, perhaps by adjusting the installed dependencies and running within a container?
#include "numpy/ndarrayobject.h"Hm... I'm a bit suspicious that either (a) we have a bug in
drake-rosthat we're not able to observe (i.e., we have system-installednumpy, and you do not), or (b) there is something misconfigured on your side that requiresnumpywhen really it shouldn't be required.Do you know if you have system-wide installed
numpy, or is it purely local to Bazel?
- We do have a broken version of systemwide numpy but we're going for a more aggressive hermetic environment where we don't depend on system deps where we don't absolutely have to. This seemed to have solved the problem in the docker container we're running into. On our end, it's much better to modify our bazel approach than to change dockerfiles as of now.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @ali-bdai)
a discussion (no related file): nit The usage of
extra_depsmay be ambiguous w.r.t. what language this is intended for; you may get mixed providers (e.g.ccgoing wherepyshould be or vice-versa).If this approach is indeed needed, then the language type should be qualified.
Absolutely, that's why we wanted to push stuff upstream as well. Currently this works for us "as it is" as a temporary solution.
If this approach is indeed needed, ..
As primary the maintainer of TRI's starlark rule set, I'll advocate that drake-ros should accept & merge something like this, even if it's not "needed" for numpy. Macros that wrap things like creating a library should always do their best to pass through options from higher layer callers above them down into the lower layer primitives like cc_library and py_library. It's not really feasible to argue that pass-throughs are never necessary, because we can't foresee every circumstance.
Certainly root causing and fixing the numpy thing the "right" way is ideal, but having an escape hatch is important for improving iteration speed and allowing for hotfixes during emergencies.
... the language type should be qualified.
I agree with this part. It should be named cc_extra_deps and py_extra_deps, or cc_deps_extra and py_deps_extra.