Updated PackageMap mechanism for finding Drake

BUILD.bazel

@@ -16,6 +16,7 @@ exports_files([
     ".bazelproject",
     ".clang-format",
     ".drake-find_resource-sentinel",
+    "package.xml",
 ])
 
 # Drake's top-level module; all drake_py_stuff rules add this to deps.

multibody/parsing/BUILD.bazel

@@ -50,6 +50,9 @@ drake_cc_library(
     hdrs = [
         "package_map.h",
     ],
+    data = [
+        "//:package.xml",
+    ],
     visibility = [
         "//visibility:public",
     ],

multibody/parsing/package_map.cc

@@ -12,6 +12,7 @@
 #include "drake/common/drake_path.h"
 #include "drake/common/drake_throw.h"
 #include "drake/common/filesystem.h"
+#include "drake/common/find_resource.h"
 #include "drake/common/text_logging.h"
 
 namespace drake {
@@ -23,13 +24,8 @@ using tinyxml2::XMLDocument;
 using tinyxml2::XMLElement;
 
 PackageMap::PackageMap() {
-  const std::optional<string> maybe_drake_path = MaybeGetDrakePath();
-  if (!maybe_drake_path) {
-    drake::log()->warn("Could not determine Drake's path.");
-  } else {
-    Add("drake", *maybe_drake_path);
-    // drake::log()->warn("Found Drake's path: {}", *maybe_drake_path);
-  }
+  std::string drake_pkg = FindResourceOrThrow("drake/package.xml");
+  AddPackageXml(drake_pkg);
 }
 
 void PackageMap::Add(const string& package_name, const string& package_path) {

multibody/parsing/test/package_map_test.cc

@@ -10,9 +10,6 @@
 using std::map;
 using std::string;
 
-// FIXME(imcmahon): add constant for expected minumum package path
-// and then use it to check for the correct PackgeMap size
-
 namespace drake {
 namespace multibody {
 namespace {
@@ -30,7 +27,7 @@ string GetTestDataRoot() {
 
 void VerifyMatch(const PackageMap& package_map,
     const map<string, string>& expected_packages) {
-  EXPECT_EQ(package_map.size()-1, static_cast<int>(expected_packages.size()));
+  EXPECT_EQ(package_map.size(), static_cast<int>(expected_packages.size()));
   for (const auto& path_entry : expected_packages) {
     const std::string& package_name = path_entry.first;
     const std::string& package_path = path_entry.second;
@@ -72,6 +69,8 @@ GTEST_TEST(PackageMapTest, TestManualPopulation) {
   };
 
   PackageMap package_map;
+  package_map.Remove("drake");
+
   for (const auto& it : expected_packages) {
     package_map.Add(it.first, it.second);
   }
@@ -98,6 +97,7 @@ GTEST_TEST(PackageMapTest, TestPopulateFromXml) {
   const string xml_dirname =
       filesystem::path(xml_filename).parent_path().string();
   PackageMap package_map;
+  package_map.Remove("drake");
   package_map.AddPackageXml(xml_filename);
 
   map<string, string> expected_packages = {
@@ -110,6 +110,7 @@ GTEST_TEST(PackageMapTest, TestPopulateFromXml) {
 GTEST_TEST(PackageMapTest, TestPopulateMapFromFolder) {
   const string root_path = GetTestDataRoot();
   PackageMap package_map;
+  package_map.Remove("drake");
   package_map.PopulateFromFolder(root_path);
   VerifyMatchWithTestDataRoot(package_map);
 }
@@ -119,6 +120,7 @@ GTEST_TEST(PackageMapTest, TestPopulateMapFromFolder) {
 GTEST_TEST(PackageMapTest, TestPopulateMapFromFolderExtraTrailingSlashes) {
   const string root_path = GetTestDataRoot();
   PackageMap package_map;
+  package_map.Remove("drake");
   package_map.PopulateFromFolder(root_path + "///////");
   VerifyMatchWithTestDataRoot(package_map);
 }
@@ -132,6 +134,7 @@ GTEST_TEST(PackageMapTest, TestPopulateUpstreamToDrake) {
       "sdf/test_model.sdf");
 
   PackageMap package_map;
+  package_map.Remove("drake");
   package_map.PopulateUpstreamToDrake(sdf_file_name);
 
   map<string, string> expected_packages = {
@@ -149,15 +152,16 @@ GTEST_TEST(PackageMapTest, TestPopulateUpstreamToDrake) {
 // Tests that PackageMap can be populated from an env var.
 GTEST_TEST(PackageMapTest, TestPopulateFromEnvironment) {
   PackageMap package_map;
+  package_map.Remove("drake");
 
-  // Test a null environment with only the Drake package.
+  // Test a null environment.
   package_map.PopulateFromEnvironment("FOOBAR");
-  EXPECT_EQ(package_map.size(), 1);
+  EXPECT_EQ(package_map.size(), 0);
 
-  // Test an empty environment with only the Drake package.
+  // Test an empty environment.
   ::setenv("FOOBAR", "", 1);
   package_map.PopulateFromEnvironment("FOOBAR");
-  EXPECT_EQ(package_map.size(), 1);
+  EXPECT_EQ(package_map.size(), 0);
 
   // Test three environment entries, concatenated:
   // - one bad path
@@ -179,6 +183,7 @@ GTEST_TEST(PackageMapTest, TestStreamingToString) {
   };
 
   PackageMap package_map;
+  package_map.Remove("drake");
   for (const auto& it : expected_packages) {
     package_map.Add(it.first, it.second);
   }
@@ -197,7 +202,7 @@ GTEST_TEST(PackageMapTest, TestStreamingToString) {
 
   // Verifies that there are three lines in the resulting string.
   EXPECT_EQ(std::count(resulting_string.begin(), resulting_string.end(), '\n'),
-            4);
+            3);
 }
 
 }  // namespace

multibody/parsing/test/parser_test.cc

@@ -186,14 +186,14 @@ GTEST_TEST(FileParserTest, FindDrakePackageWhenAdding) {
       MultibodyPlant<double> plant(0.0);
       geometry::SceneGraph<double> scene_graph;
       Parser parser(&plant, &scene_graph);
-      EXPECT_EQ(parser.package_map().size(), 1);
+      const int orig_package_size = parser.package_map().size();
 
       // Because the box.sdf references an obj via a package: URI, this would
       // throw if the package were not found.
       EXPECT_NO_THROW(add_func(FindResourceOrThrow(file_name), &parser));
 
       // Now we explicitly confirm the package map has been modified.
-      EXPECT_EQ(parser.package_map().size(), 2);
+      EXPECT_EQ(parser.package_map().size(), orig_package_size+1);
       EXPECT_TRUE(parser.package_map().Contains("box_model"));
     }
   }

package.xml

@@ -1,6 +1,5 @@
 <package format="2">
   <name>drake</name>
-  <version>0.24.0</version>
   <description>
   Model-Based Design and Verification for Robotics.
   </description>