RobotLocomotion · rpoyner-tri · Jan 26, 2022 · Jan 26, 2022
diff --git a/bindings/pydrake/multibody/BUILD.bazel b/bindings/pydrake/multibody/BUILD.bazel
@@ -125,7 +125,10 @@ drake_pybind_library(
 
 drake_pybind_library(
     name = "parsing_py",
-    cc_deps = ["//bindings/pydrake:documentation_pybind"],
+    cc_deps = [
+        "//bindings/pydrake:documentation_pybind",
+        "//bindings/pydrake/common:deprecation_pybind",
+    ],
     cc_srcs = ["parsing_py.cc"],
     package_info = PACKAGE_INFO,
     py_deps = [

diff --git a/bindings/pydrake/multibody/parsing_py.cc b/bindings/pydrake/multibody/parsing_py.cc
@@ -1,6 +1,7 @@
 #include "pybind11/pybind11.h"
 #include "pybind11/stl.h"
 
+#include "drake/bindings/pydrake/common/deprecation_pybind.h"
 #include "drake/bindings/pydrake/documentation_pybind.h"
 #include "drake/bindings/pydrake/pydrake_pybind.h"
 #include "drake/multibody/parsing/package_map.h"
@@ -27,7 +28,8 @@ PYBIND11_MODULE(parsing, m) {
   {
     using Class = PackageMap;
     constexpr auto& cls_doc = doc.PackageMap;
-    py::class_<Class>(m, "PackageMap", cls_doc.doc)
+    py::class_<Class> cls(m, "PackageMap", cls_doc.doc);
+    cls  // BR
         .def(py::init<>(), cls_doc.ctor.doc)
         .def("Add", &Class::Add, py::arg("package_name"),
             py::arg("package_path"), cls_doc.Add.doc)
@@ -48,9 +50,14 @@ PYBIND11_MODULE(parsing, m) {
         .def("PopulateFromEnvironment", &Class::PopulateFromEnvironment,
             py::arg("environment_variable"),
             cls_doc.PopulateFromEnvironment.doc)
-        .def("PopulateUpstreamToDrake", &Class::PopulateUpstreamToDrake,
-            py::arg("model_file"), cls_doc.PopulateUpstreamToDrake.doc)
         .def_static("MakeEmpty", &Class::MakeEmpty, cls_doc.MakeEmpty.doc);
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+    cls.def("PopulateUpstreamToDrake",
+        WrapDeprecated(cls_doc.PopulateUpstreamToDrake.doc_deprecated,
+            &Class::PopulateUpstreamToDrake),
+        py::arg("model_file"), cls_doc.PopulateUpstreamToDrake.doc_deprecated);
+#pragma GCC diagnostic pop
   }
 
   // Parser

diff --git a/bindings/pydrake/multibody/test/parsing_test.py b/bindings/pydrake/multibody/test/parsing_test.py
@@ -15,6 +15,7 @@
 import unittest
 
 from pydrake.common import FindResourceOrThrow
+from pydrake.common.test_utilities.deprecation import catch_drake_warnings
 from pydrake.multibody.tree import (
     ModelInstanceIndex,
 )
@@ -51,7 +52,8 @@ def test_package_map(self):
         self.assertEqual(dut2.size(), 0)
 
         # Simple coverage test for Drake paths.
-        dut.PopulateUpstreamToDrake(model_file=model)
+        with catch_drake_warnings(expected_count=1):
+            dut.PopulateUpstreamToDrake(model_file=model)
         self.assertGreater(dut.size(), 1)
 
         # Simple coverage test for folder and environment.

diff --git a/examples/pr2/test/load_pr2_simplified_test.cc b/examples/pr2/test/load_pr2_simplified_test.cc
@@ -17,7 +17,6 @@ GTEST_TEST(LoadPr2SimplifiedTest, TestIfPr2SimplifiedLoads) {
   const std::string& pathname = FindResourceOrThrow(
       "drake/examples/pr2/models/pr2_description/urdf/pr2_simplified.urdf");
   multibody::Parser parser(&plant);
-  parser.package_map().PopulateUpstreamToDrake(pathname);
   parser.AddModelFromFile(pathname);
   plant.Finalize();
 

diff --git a/multibody/parsing/package_map.cc b/multibody/parsing/package_map.cc
@@ -255,6 +255,8 @@ void PackageMap::PopulateUpstreamToDrakeHelper(
       GetParentDirectory(directory), stop_at_directory);
 }
 
+// N.B. When removing this deprecated function, also be sure to remove
+// the PopulateUpstreamToDrakeHelper, immediately above.
 void PackageMap::PopulateUpstreamToDrake(const string& model_file) {
   DRAKE_DEMAND(!model_file.empty());
   drake::log()->trace("PopulateUpstreamToDrake: {}", model_file);

diff --git a/multibody/parsing/package_map.h b/multibody/parsing/package_map.h
@@ -7,6 +7,7 @@
 #include <vector>
 
 #include "drake/common/drake_copyable.h"
+#include "drake/common/drake_deprecated.h"
 
 namespace drake {
 namespace multibody {
@@ -100,6 +101,11 @@ class PackageMap final {
   ///
   /// @param[in] model_file The model file whose directory is the start of the
   /// search for `package.xml` files. This file must be an SDF or URDF file.
+  DRAKE_DEPRECATED("2022-05-01",
+      "You should remove all calls to this function. There is no replacement,"
+      " because the feature should be irrelevant. This function is a no-op"
+      " unless you are also using one of the deprecated package.xml files"
+      " internal to Drake without using Drake's multibody::Parser class.")
   void PopulateUpstreamToDrake(const std::string& model_file);
 
   friend std::ostream& operator<<(std::ostream& out,

diff --git a/multibody/parsing/parser.cc b/multibody/parsing/parser.cc
@@ -40,9 +40,12 @@ std::vector<ModelInstanceIndex> Parser::AddAllModelsFromFile(
     const std::string& file_name) {
   DataSource data_source;
   data_source.file_name = &file_name;
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
   // Always search for a package.xml file, starting the crawl upward from
   // the file's path.
   package_map_.PopulateUpstreamToDrake(file_name);
+#pragma GCC diagnostic pop
   const FileType type = DetermineFileType(file_name);
   if (type == FileType::kSdf) {
     return AddModelsFromSdf(
@@ -58,9 +61,12 @@ ModelInstanceIndex Parser::AddModelFromFile(
     const std::string& model_name) {
   DataSource data_source;
   data_source.file_name = &file_name;
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
   // Always search for a package.xml file, starting the crawl upward from
   // the file's path.
   package_map_.PopulateUpstreamToDrake(file_name);
+#pragma GCC diagnostic pop
   const FileType type = DetermineFileType(file_name);
   if (type == FileType::kSdf) {
     return AddModelFromSdf(

diff --git a/multibody/parsing/test/detail_path_utils_test.cc b/multibody/parsing/test/detail_path_utils_test.cc
@@ -169,7 +169,10 @@ GTEST_TEST(ResolveUriTest, TestModel) {
 
   // Create the package map.
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(sdf_file_name);
+  package_map.AddPackageXml(FindResourceOrThrow(
+      "drake/multibody/parsing/test/"
+          "package_map_test_packages/package_map_test_package_a/"
+          "package.xml"));
 
   // Set the root directory - it will not end up being used in ResolveUri().
   const std::string root_dir = "/no/such/root";

diff --git a/multibody/parsing/test/detail_sdf_parser_test.cc b/multibody/parsing/test/detail_sdf_parser_test.cc
@@ -144,7 +144,6 @@ GTEST_TEST(MultibodyPlantSdfParserTest, ModelInstanceTest) {
       "drake/multibody/parsing/test/"
       "links_with_visuals_and_collisions.sdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
 
   ModelInstanceIndex instance1 =
       AddModelFromSdfFile(full_name, "instance1", package_map, &plant);
@@ -652,7 +651,6 @@ GTEST_TEST(SdfParserThrowsWhen, JointDampingIsNegative) {
       "drake/multibody/parsing/test/sdf_parser_test/"
       "negative_damping_joint.sdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(sdf_file_path);
   MultibodyPlant<double> plant(0.0);
   DRAKE_EXPECT_THROWS_MESSAGE(
       AddModelFromSdfFile(sdf_file_path, "", package_map, &plant),
@@ -738,7 +736,6 @@ GTEST_TEST(SdfParser, TestOptionalSceneGraph) {
       "drake/multibody/parsing/test/"
       "links_with_visuals_and_collisions.sdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
 
   int num_visuals_explicit{};
   {
@@ -770,7 +767,6 @@ GTEST_TEST(MultibodyPlantSdfParserTest, JointParsingTest) {
       "drake/multibody/parsing/test/sdf_parser_test/"
       "joint_parsing_test.sdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
 
   // Read in the SDF file.
   const std::vector<ModelInstanceIndex> instances =
@@ -927,7 +923,6 @@ GTEST_TEST(MultibodyPlantSdfParserTest, JointActuatorParsingTest) {
       "drake/multibody/parsing/test/sdf_parser_test/"
       "joint_actuator_parsing_test.sdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
 
   // Read in the SDF file.
   AddModelFromSdfFile(full_name, "", package_map, &plant, nullptr);
@@ -962,7 +957,6 @@ GTEST_TEST(MultibodyPlantSdfParserTest, RevoluteSpringParsingTest) {
       "drake/multibody/parsing/test/sdf_parser_test/"
       "revolute_spring_parsing_test.sdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
 
   // Reads in the SDF file.
   AddModelFromSdfFile(full_name, "", package_map, &plant, nullptr);
@@ -1225,7 +1219,6 @@ ::testing::AssertionResult FrameHasShape(geometry::FrameId frame_id,
 void TestForParsedGeometry(const char* sdf_name, geometry::Role role) {
   const std::string full_name = FindResourceOrThrow(sdf_name);
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
   MultibodyPlant<double> plant(0.0);
   SceneGraph<double> scene_graph;
   plant.RegisterAsSourceForSceneGraph(&scene_graph);
@@ -1474,7 +1467,6 @@ GTEST_TEST(SdfParser, LoadDirectlyNestedModelsInWorld) {
   ASSERT_EQ(plant.num_joints(), 0);
 
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
   AddModelsFromSdfFile(full_name, package_map, &plant);
   plant.Finalize();
 
@@ -1533,7 +1525,6 @@ GTEST_TEST(SdfParser, LoadDirectlyNestedModelsInModel) {
   ASSERT_EQ(plant.num_joints(), 0);
 
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
   AddModelsFromSdfFile(full_name, package_map, &plant);
   plant.Finalize();
 
@@ -1961,7 +1952,6 @@ GTEST_TEST(SdfParser, FramesAsJointParentOrChild) {
   MultibodyPlant<double> plant(0.0);
 
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
   AddModelsFromSdfFile(full_name, package_map, &plant);
   ASSERT_TRUE(plant.HasModelInstanceNamed("parent_model"));
 
@@ -2016,7 +2006,9 @@ GTEST_TEST(SdfParser, InterfaceAPI) {
       "drake/multibody/parsing/test/sdf_parser_test/interface_api_test/"
       "top.sdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(sdf_file_path);
+  package_map.AddPackageXml(FindResourceOrThrow(
+      "drake/multibody/parsing/test/sdf_parser_test/interface_api_test/"
+      "package.xml"));
   MultibodyPlant<double> plant(0.0);
 
   DRAKE_ASSERT_NO_THROW(AddModelFromSdfFile(sdf_file_path, "", package_map,
@@ -2123,7 +2115,6 @@ GTEST_TEST(SdfParser, CollisionFilterGroupParsingTest) {
   MultibodyPlant<double> plant(0.0);
   SceneGraph<double> scene_graph;
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_sdf_filename);
 
   // Read in the SDF file.
   AddModelFromSdfFile(full_sdf_filename, "", package_map, &plant, &scene_graph);

diff --git a/multibody/parsing/test/detail_urdf_geometry_test.cc b/multibody/parsing/test/detail_urdf_geometry_test.cc
@@ -234,9 +234,6 @@ class UrdfGeometryTests : public testing::Test {
       root_dir_ = full_path.substr(0, found);
     }
 
-    // TODO(sam.creasey) Add support for using an existing package map.
-    package_map_.PopulateUpstreamToDrake(full_path);
-
     const XMLElement* node = xml_doc_.FirstChildElement("robot");
     ASSERT_TRUE(node);
 

diff --git a/multibody/parsing/test/detail_urdf_parser_test.cc b/multibody/parsing/test/detail_urdf_parser_test.cc
@@ -80,7 +80,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, DoublePendulum) {
   std::string full_name = FindResourceOrThrow(
       "drake/multibody/benchmarks/acrobot/double_pendulum.urdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
   AddModelFromUrdfFile(full_name, "", package_map, &plant, &scene_graph);
   plant.Finalize();
 
@@ -119,7 +118,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, TestAtlasMinimalContact) {
   std::string full_name = FindResourceOrThrow(
       "drake/examples/atlas/urdf/atlas_minimal_contact.urdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
 
   AddModelFromUrdfFile(full_name, "", package_map, &plant, &scene_graph);
   plant.Finalize();
@@ -140,7 +138,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, TestAddWithQuaternionFloatingDof) {
   const std::string model_file =
       FindResourceOrThrow(resource_dir + "zero_dof_robot.urdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(model_file);
 
   MultibodyPlant<double> plant(0.0);
   SceneGraph<double> scene_graph;
@@ -155,7 +152,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, TestOptionalSceneGraph) {
   const std::string full_name = FindResourceOrThrow(
       "drake/examples/atlas/urdf/atlas_minimal_contact.urdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
   int num_visuals_explicit{};
   {
     // Test explicitly specifying `scene_graph`.
@@ -182,7 +178,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, JointParsingTest) {
       "drake/multibody/parsing/test/urdf_parser_test/"
       "joint_parsing_test.urdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
 
   MultibodyPlant<double> plant(0.0);
   SceneGraph<double> scene_graph;
@@ -440,7 +435,6 @@ ::testing::AssertionResult FrameHasShape(geometry::FrameId frame_id,
 void TestForParsedGeometry(const char* sdf_name, geometry::Role role) {
   const std::string full_name = FindResourceOrThrow(sdf_name);
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
   MultibodyPlant<double> plant(0.0);
   SceneGraph<double> scene_graph;
   plant.RegisterAsSourceForSceneGraph(&scene_graph);
@@ -831,7 +825,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, CollisionFilterGroupParsingTest) {
       "drake/multibody/parsing/test/urdf_parser_test/"
       "collision_filter_group_parsing_test.urdf");
   PackageMap package_map;
-  package_map.PopulateUpstreamToDrake(full_name);
 
   MultibodyPlant<double> plant(0.0);
   SceneGraph<double> scene_graph;

diff --git a/multibody/parsing/test/package_map_test.cc b/multibody/parsing/test/package_map_test.cc
@@ -215,8 +215,10 @@ GTEST_TEST(PackageMapTest, TestPopulateMapFromFolderExtraTrailingSlashes) {
   VerifyMatchWithTestDataRoot(package_map);
 }
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 // Tests that PackageMap can be populated by crawling up a directory tree.
-GTEST_TEST(PackageMapTest, TestPopulateUpstreamToDrake) {
+GTEST_TEST(PackageMapTest, DeprecatedTestPopulateUpstreamToDrake) {
   const string root_path = GetTestDataRoot();
   const string sdf_file_name = FindResourceOrThrow(
       "drake/multibody/parsing/test/"
@@ -236,6 +238,7 @@ GTEST_TEST(PackageMapTest, TestPopulateUpstreamToDrake) {
   package_map.PopulateUpstreamToDrake(sdf_file_name);
   VerifyMatch(package_map, expected_packages);
 }
+#pragma GCC diagnostic pop
 
 // Tests that PackageMap can be populated from an env var.
 GTEST_TEST(PackageMapTest, TestPopulateFromEnvironment) {