Per this comment in drake/common/BUILD, it looks like GetDrakePath() may soon meet its demise.
Additionally, given how bazel / package_drake.sh function, this method effectively has no meaning outside of a bazel sandbox build environment since it is no longer an absolute path, and that GetDrakePath() will break in downstream projects when they switch to bazel.
Per RobotLocomotion/spartan#30, GetDrakePath() does provide good and useful convenience for external consumers, and it would be nice to either (a) keep it and think of a workaround, or (b) provide a robust and succinct mechanism to easily replace being able to locate development resources for drake, such as example plants, URDFs, etc.
(For option (a), I can only think of doing some dirty-nasty environment variables, such as if_in_bazel ? "drake" : getenv("DRAKE_SOURCE_DIR")... Can't say that I am personally a fan of that.)
@jwnimmer-tri Can I ask what your thoughts are on this, and if you have suggestions for workarounds?
Additionally, as of present, package_drake.sh does not incorporate resource files. Do we have a plan for that as well?
\cc @mwoehlke-kitware @sammy-tri @patmarion @manuelli
Previous discussion on the topic:
https://github.com/RobotLocomotion/drake/issues/2174
https://github.com/RobotLocomotion/drake/issues/1295#issuecomment-136156776
@EricCousineau-TRI Please give one or two very specific examples of what you would like to accomplish. I assume its something like obtaining the complete path to some URDF(s) and related resource(s) that are checked into Drake, after having run the Drake superbuild? And you're worried that this will stop working after the superbuild switches to using Bazel?
@jwnimmer-tri Yes, the worry is that either access to existing resources would stop working, or that there may be just a few more hoops to jump through.
A concrete example:
apps/pr2/bhpnTranslator.py:103: filename = os.path.join(pydrake.getDrakePath(), 'examples/PR2/pr2.urdf')
However:
spartan/apps, three in director python code, and two in MATLAB code - which I assume may be deprecated in the near future)pydrake.getDrakePath() uses a relative path and won't be sensitive to C++'s GetDrakePath().With things as they are, I am guessing that this and other downstream projects could just map their own paths to drake source-level resources.
@patmarion Do you think things would be best if (a) those resources are incorporated in the superbuild, or (b) if it is expected that they be used only for development / a separate repository?
(b) may be most ideal for Drake, and pave way to deprecating GetDrakePath() and place the burden of locating resources on some other system, such as catkin or some environment variable such as $DRAKE_PACKAGE_PATH that will be the responsibility of downstream projects, not Drake, to provide.
EDIT: It looks like this has been discussed a bit in #3257, but was closed due to non-urgency. I think (b) would be the most compatible with whatever solution may come up from that issue if it is re-opened.
To forestall too much of a sidetrack: whatever other resource-hosting solutions may arise, if (1) there are resource files committed to Drake's source tree, and (2) we want to make them part of our supported contract (e.g., stuff from drake/examples yes, but drake/multibody/geometry/test/my_unit_test_cube.urdf no), then Drake should provide a way to directly access those resources after a superbuild.
I will write up a longer reply a bit later on, with more information.
My rough proposal goes like:
(1) Add a C++ API something like drake::FindResource(const std::string& relpath) documented as MagicHappensHere™. Given a string like drake/examples/Acrobot/Acrobot.urdf it would return the fully-qualified filesystem path to that file as provided by Drake.
If the resource is not found, it would provide a useful fail-fast diagnostic (probably should not be an exception; but should be difficult to brush past without noticing; std::expected or std::optional could be a nice fit).
(If we wanted to allow relpath to be a Bazel label for convenience (start with // which is ignored, and allow : to mean /), we could document that and then the same string would be used in data= in BUILD files all well as the C++ code that refers to the resource. Probably not worth it, though.)
For the MagicHappensHere implementation logic, we have:
(1a) When running from Bazel within a Bazel sandbox, the implementation is simple return relpath.
(1b) When running by hand somewhere within a Bazel development workspace, or with the sandbox disabled, the implementation prepends bazel info workspace to the relpath (probably just reimplementing it's logic by hand).
(1c) When running from a CMake-wrapped superbuild, probably do (1d) below -- but we could consider (1b) instead if we're doing some kind of incremental CMake rebuilding where we want to avoid the tarball.
(1d) When running from a binary release, we package the resources into the binary release tarball and the implementation prepends the installed location of our release. (We could encode that location into either a tiny shim library, a magic .drake_install_root dotfile, an environment variables, etc.) If we are cute, we might be able to make option (a) fail-fast if the artifact is not included in the binary release, to push accessibility-labeling failures earlier into the development process.
(1e) We could document a DRAKE_RESOURCE_ROOT env var override that could trump any or all of the above, for random user purposes and not to be used directly within Drake without an especially good reason.
Once that is all working, we:
(2a) Port all of Drake's tests an examples to use (1), and deprecate C++ GetDrakePath.
(2b) Remove C++ GetDrakePath.
I don't _think_ the ROS package map needs to interact with this at all, but if it does I suspect the FindResource mnenomic will suit better that GetDrakePath could anyway.
Sorry for the late reply, and sorry for the naive question(s), but what would the advantage be for supplying FindResource(resource) vs. GetDrakePath() (or something like GetResourceRoot()) to merit the downstream changes?
Same question, different forms:
What use cases would FindResource(resource) (which right now, seems like GetDrakePath() + "/" + resource) cover thatGetDrakePath()` could not?
What does knowing the relative path ahead of time have to offer, if we are not using something like a list of search paths / ROS package map?
Using a method means that (1) we can return an error when the resource does not exist and (2) we can change the resource-finding implementation while keeping the API intact, such as having more than one way to locate the resource.
Makes sense - thanks!
WIP will be at https://github.com/jwnimmer-tri/drake/tree/build-drakepath. ~(There's not very much there yet; just an API sketch.)~
Update: WIP branch has (1a) and (1b) done.
It also has a new innovation -- changing the C++ GetDrakePath in Bazel to be correct from anywhere within the workspace, not just from drake-distro, by relying on FindResource to find a well-known Drake resource and then backing out the root path from that. That means that once (1d) is done under the hood, then C++ GetDrakePath will work in both from-source and installed releases! We should probably still deprecate it, but it also won't be as much of a problem anymore.
GetDrakePath is now supported in all build systems, but is marked deprecated.
Most helpful comment
To forestall too much of a sidetrack: whatever other resource-hosting solutions may arise, if (1) there are resource files committed to Drake's source tree, and (2) we want to make them part of our supported contract (e.g., stuff from
drake/examplesyes, butdrake/multibody/geometry/test/my_unit_test_cube.urdfno), then Drake should provide a way to directly access those resources after a superbuild.I will write up a longer reply a bit later on, with more information.