RobotLocomotion / drake-ros

Experimental prototyping (for now)
Apache License 2.0
83 stars 32 forks source link

[bazel] Should ensure we share version info among separate workspaces / CMake projects #201

Closed EricCousineau-TRI closed 1 year ago

EricCousineau-TRI commented 1 year ago

We should do this either via symlinks or shared tooling, or simple linting.

This should do things like ensure we are:

Alternatively, we may want to reconsider the multi-Bazel-workspace approach we're using here.

sloretz commented 1 year ago

Alternatively, we may want to reconsider the multi-Bazel-workspace approach we're using here.

It does feel like we're fighting bazel by having so many WORKSPACEs. What if we combined drake_ros_core, drake_ros_tf2, and drake_ros_viz into a single package drake_ros? That removes some of the duplication.

Having them in separate packages as they are now allows someone to depend on drake_ros_core without getting the extra dependencies of drake_ros_tf2 or drake_ros_viz, but those extra dependencies are pretty small: geometry_msgs, tf2_ros, tf2_ros_py, visualization_msgs. It might be worth removing that ability to make maintenance of this repo easier.

EricCousineau-TRI commented 1 year ago

Yeah, I'd be fine if Bazel's consumption is monorepo-esque, as Bazel itself has great mechanisms for granularity.

My only concern is how that might affect ergonomics of colcon/ament workflows. Is it possible for us to have nested stack (in ye olde ROS 1 parlance) for CMake, and then merged for Bazel?

Example:

drake_ros_core/
├──WORKSPACE
└──CMakeLists.txt
drake_ros_tf2/
├──WORKSPACE
└──CMakeLists.txt
drake_ros_viz/
├──WORKSPACE
└──CMakeLists.txt

to

drake_ros/
├──WORKSPACE
├──drake_ros_core/
│  └──CMakeLists.txt
├──drake_ros_tf2/
│  └──CMakeLists.txt
└──drake_ros_viz/
   └──CMakeLists.txt
sloretz commented 1 year ago

My only concern is how that might affect ergonomics of colcon/ament workflows. Is it possible for us to have nested stack (in ye olde ROS 1 parlance) for CMake, and then merged for Bazel?

It would be possible; though, in this case I think combining them might improve the ergonomics. It would make the CMake required to depend on them a bit shorter.

# ---- Before ----
find_package(drake_ros_core REQUIRED)
find_package(drake_ros_tf2 REQUIRED)
find_package(drake_ros_viz REQUIRED)

# Depending on one library
target_link_libraries(mylib PUBLIC drake_ros_core::drake_ros_core)
# Depending on all libraries
target_link_libraries(mylib PUBLIC drake_ros_core::drake_ros_core drake_ros_tf2::drake_ros_tf2 drake_ros_viz::drake_ros_viz)

# ---- After---
find_package(drake_ros REQUIRED)

#...

# Depending on one library
target_link_libraries(mylib PUBLIC drake_ros::core)
# Depending on all libraries
target_link_libraries(mylib PUBLIC drake_ros::core drake_ros::viz drake_ros::tf2)
# Now could have a shortcut to depend on all libraries?
target_link_libraries(mylib PUBLIC drake_ros::drake_ros)

Having separate packages also allows users to install only the dependencies they need, but In this case the difference between drake_ros_core and drake_ros_viz is pretty small. I made an Ubuntu 22.04 container with just the dependencies for drake_ros_core excluding Drake. Then I checked how much more disk space was needed for the dependencies of drake_ros_viz and drake_ros_tf2. It's only 13 MB. I also checked how much Drake's dependencies are, and found that to be 1527 MB more. Since we're already depending on Drake I don't think it's worth trying to give users the opportunity to save another 13 MB.

drake_ros_tf2 and drake_ros_viz extra deps ``` The following NEW packages will be installed: libconsole-bridge-dev libconsole-bridge1.0 ros-humble-class-loader ros-humble-composition-interfaces ros-humble-console-bridge-vendor ros-humble-message-filters ros-humble-rclcpp-action ros-humble-rclcpp-components ros-humble-sensor-msgs ros-humble-tf2 ros-humble-tf2-msgs ros-humble-tf2-py ros-humble-tf2-ros ros-humble-tf2-ros-py ros-humble-visualization-msgs 0 upgraded, 15 newly installed, 0 to remove and 0 not upgraded. Need to get 1408 kB of archives. After this operation, 12.7 MB of additional disk space will be used. ```
Drake extra deps ``` The following NEW packages will be installed: adwaita-icon-theme alsa-topology-conf alsa-ucm-conf at-spi2-core blt ca-certificates-java coinor-libclp1 coinor-libcoinutils3v5 coinor-libipopt1v5 coinor-libosi1v5 dbus dbus-user-session dconf-gsettings-backend dconf-service default-jre default-jre-headless dmsetup fontconfig fonts-dejavu-extra fonts-font-awesome fonts-freefont-ttf fonts-glyphicons-halflings fonts-liberation fonts-lyx fonts-mathjax g++ g++-11 gir1.2-glib-2.0 graphviz gsettings-desktop-schemas gtk-update-icon-cache hicolor-icon-theme humanity-icon-theme ibverbs-providers java-common jupyter-core jupyter-nbextension-jupyter-js-widgets jupyter-notebook libaec0 libamd2 libann0 libapache-pom-java libapparmor1 libargon2-1 libasound2 libasound2-data libasyncns0 libatk-bridge2.0-0 libatk-wrapper-java libatk-wrapper-java-jni libatk1.0-0 libatk1.0-data libatspi2.0-0 libavahi-client3 libavahi-common-data libavahi-common3 libboost-dev libboost1.74-dev libcairo-gobject2 libcairo2 libcdt5 libcgraph6 libcmark-gfm-extensions0.29.0.gfm.3 libcmark-gfm0.29.0.gfm.3 libcolord2 libcommons-io-java libcommons-logging-java libcommons-parent-java libcryptsetup12 libcups2 libcurl3-gnutls libdatrie1 libdbus-1-3 libdconf1 libdecor-0-0 libdecor-0-plugin-1-cairo libdevmapper1.02.1 libdouble-conversion3 libdpkg-perl libdrm-amdgpu1 libdrm-common libdrm-intel1 libdrm-nouveau2 libdrm-radeon1 libdrm2 libedit2 libegl-mesa0 libegl1 libeigen3-dev libelf1 libepoxy0 libevdev2 libevent-core-2.1-7 libevent-pthreads-2.1-7 libfabric1 libfile-fcntllock-perl libflac8 libfluidsynth3 libfontenc1 libgbm1 libgdk-pixbuf-2.0-0 libgdk-pixbuf2.0-bin libgdk-pixbuf2.0-common libgflags2.2 libgif7 libgirepository-1.0-1 libgl1 libgl1-amber-dri libgl1-mesa-dri libgl2ps1.4 libglapi-mesa libglew2.2 libglu1-mesa libglvnd0 libglx-mesa0 libglx0 libgtk-3-0 libgtk-3-bin libgtk-3-common libgts-0.7-5 libgts-bin libgudev-1.0-0 libgvc6 libgvpr2 libhdf5-103 libhdf5-103-1 libhdf5-fortran-102 libhdf5-hl-100 libhdf5-hl-fortran-100 libhwloc-plugins libhwloc15 libibverbs1 libice6 libinput-bin libinput10 libinstpatch-1.0-2 libip4tc2 libjack-jackd2-0 libjchart2d-java libjide-oss-java libjs-backbone libjs-bootstrap libjs-bootstrap-tour libjs-codemirror libjs-es6-promise libjs-jed libjs-jquery-typeahead libjs-jquery-ui libjs-marked libjs-mathjax libjs-moment libjs-requirejs libjs-requirejs-text libjs-text-encoding libjs-xterm libjson-c5 libkmod2 liblab-gamut1 liblbfgsb0 libldl2 libllvm13 liblocale-gettext-perl libltdl7 libmd4c0 libmodplug1 libmpg123-0 libmsgpackc2 libmtdev1 libmumps-seq-5.4 libnetcdf19 libnl-3-200 libnl-route-3-200 libnlopt-cxx0 libnorm1 libnspr4 libnss-systemd libnss3 libnuma1 libogg0 libopenblas-dev libopenblas-pthread-dev libopenblas0 libopenblas0-pthread libopengl0 libopenmpi3 libopus0 libopusfile0 libpam-systemd libpango-1.0-0 libpangocairo-1.0-0 libpangoft2-1.0-0 libpathplan4 libpciaccess0 libpcre2-16-0 libpcsclite1 libpgm-5.3-0 libpixman-1-0 libpmix2 libportmidi0 libproj22 libpsm-infinipath1 libpsm2-2 libpulse0 libqhull-r8.0 libqt5core5a libqt5dbus5 libqt5gui5 libqt5network5 libqt5printsupport5 libqt5svg5 libqt5widgets5 librdmacm1 librsvg2-2 librsvg2-common libsamplerate0 libscotch-6.1 libsdl2-2.0-0 libsdl2-image-2.0-0 libsdl2-mixer-2.0-0 libsdl2-ttf-2.0-0 libsensors-config libsensors5 libsm6 libsndfile1 libsodium23 libstdc++-11-dev libsuitesparseconfig5 libsz2 libtbb12 libtbbmalloc2 libtcl8.6 libthai-data libthai0 libtheora0 libtinyxml2.6.2v5 libtk8.6 libucx0 libvorbis0a libvorbisenc2 libvorbisfile3 libvtk9.1 libvulkan1 libwacom-bin libwacom-common libwacom9 libwayland-client0 libwayland-cursor0 libwayland-egl1 libwayland-server0 libx11-xcb1 libxaw7 libxcb-dri2-0 libxcb-dri3-0 libxcb-glx0 libxcb-icccm4 libxcb-image0 libxcb-keysyms1 libxcb-present0 libxcb-randr0 libxcb-render-util0 libxcb-render0 libxcb-shape0 libxcb-shm0 libxcb-sync1 libxcb-util1 libxcb-xfixes0 libxcb-xinerama0 libxcb-xinput0 libxcb-xkb1 libxcomposite1 libxcursor1 libxdamage1 libxext6 libxfixes3 libxft2 libxi6 libxinerama1 libxkbcommon-x11-0 libxkbcommon0 libxkbfile1 libxmlgraphics-commons-java libxmu6 libxmuu1 libxnvctrl0 libxrandr2 libxrender1 libxshmfence1 libxsimd-dev libxslt1.1 libxss1 libxt6 libxtst6 libxv1 libxxf86dga1 libxxf86vm1 libyaml-cpp-dev libyaml-cpp0.7 libzmq5 mesa-vulkan-drivers networkd-dispatcher node-jed ocl-icd-libopencl1 openjdk-11-jre openjdk-11-jre-headless pandoc pandoc-data pkg-config proj-data python-babel-localedata python-matplotlib-data python3-appdirs python3-argon2 python3-babel python3-backcall python3-beniget python3-bleach python3-brotli python3-bs4 python3-cffi-backend python3-chardet python3-cycler python3-dbus python3-decorator python3-defusedxml python3-entrypoints python3-fonttools python3-fs python3-gast python3-gi python3-html5lib python3-ipykernel python3-ipython python3-ipython-genutils python3-ipywidgets python3-jedi python3-jinja2 python3-jsonschema python3-jupyter-client python3-jupyter-core python3-jupyterlab-pygments python3-kiwisolver python3-lxml python3-lz4 python3-markupsafe python3-matplotlib python3-matplotlib-inline python3-mpmath python3-nbclient python3-nbconvert python3-nbformat python3-nest-asyncio python3-notebook python3-pandocfilters python3-parso python3-pexpect python3-pickleshare python3-pil.imagetk python3-ply python3-prometheus-client python3-prompt-toolkit python3-ptyprocess python3-pydot python3-pygame python3-pyrsistent python3-pythran python3-scipy python3-send2trash python3-soupsieve python3-sympy python3-terminado python3-testpath python3-tk python3-tornado python3-traitlets python3-tz python3-u-msgpack python3-ufolib2 python3-unicodedata2 python3-wcwidth python3-webencodings python3-widgetsnbextension python3-yaml python3-zmq qt5-gtk-platformtheme qttranslations5-l10n session-migration systemd systemd-sysv systemd-timesyncd timgm6mb-soundfont tk8.6-blt2.5 ubuntu-mono unicode-data x11-common x11-utils xkb-data 0 upgraded, 396 newly installed, 0 to remove and 0 not upgraded. Need to get 309 MB of archives. After this operation, 1527 MB of additional disk space will be used. ```

How about

bazel_ros2_rules/ ...
drake_model_interop/...
drake_ros/
├──BUILD.bazel
├──CMakeLists.txt
├──package.xml
├──WORKSPACE
├──core/
├──tf2/
└──viz/
drake_ros_examples/
├──BUILD.bazel
├──CMakeLists.txt
├──package.xml
└──WORKSPACE
ros2_example_bazel_installed/...
EricCousineau-TRI commented 1 year ago

Sweet, that all sounds like a great and convincing analysis to me!

EricCousineau-TRI commented 1 year ago

Resolved by #201, thanks!