From b7fc417132b7195275b16d59228f478d2360f984 Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Wed, 8 Mar 2023 11:50:26 -0800 Subject: [PATCH] [parsing] Add support for remote packages TODO needs mutex for const function --- BUILD.bazel | 1 + bindings/pydrake/multibody/parsing_py.cc | 15 +- .../pydrake/multibody/test/parsing_test.py | 10 + multibody/parsing/BUILD.bazel | 31 ++- multibody/parsing/package_map.cc | 188 ++++++++++++++- multibody/parsing/package_map.h | 45 +++- .../parsing/test/package_map_remote_test.cc | 218 ++++++++++++++++++ multibody/parsing/test/package_map_test.cc | 21 +- .../package_map_test_packages/compressed.zip | Bin 0 -> 569 bytes tools/install/bazel/BUILD.bazel | 1 + tools/install/bazel/drake.BUILD.bazel | 1 + .../bazel/drake__multibody.BUILD.bazel | 21 ++ 12 files changed, 538 insertions(+), 14 deletions(-) create mode 100644 multibody/parsing/test/package_map_remote_test.cc create mode 100644 multibody/parsing/test/package_map_test_packages/compressed.zip create mode 100644 tools/install/bazel/drake__multibody.BUILD.bazel diff --git a/BUILD.bazel b/BUILD.bazel index 7476880d1f2c..3e081d5c85ab 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -89,6 +89,7 @@ install( "//geometry:install", "//lcmtypes:install", "//manipulation/models:install_data", + "//multibody/parsing:install", "//setup:install", "//tools/install/libdrake:install", "//tools/workspace:install_external_packages", diff --git a/bindings/pydrake/multibody/parsing_py.cc b/bindings/pydrake/multibody/parsing_py.cc index 4b5f0ece42e1..bfaf11781c17 100644 --- a/bindings/pydrake/multibody/parsing_py.cc +++ b/bindings/pydrake/multibody/parsing_py.cc @@ -32,12 +32,25 @@ PYBIND11_MODULE(parsing, m) { using Class = PackageMap; constexpr auto& cls_doc = doc.PackageMap; py::class_ cls(m, "PackageMap", cls_doc.doc); + { + using Nested = PackageMap::RemotePackageParams; + constexpr auto& nested_doc = cls_doc.RemotePackageParams; + py::class_ nested(cls, "RemotePackageParams", nested_doc.doc); + nested.def(ParamInit()); + DefAttributesUsingSerialize(&nested, nested_doc); + DefReprUsingSerialize(&nested); + DefCopyAndDeepCopy(&nested); + } cls // BR .def(py::init<>(), cls_doc.ctor.doc) .def(py::init(), py::arg("other"), "Copy constructor") .def("Add", &Class::Add, py::arg("package_name"), py::arg("package_path"), cls_doc.Add.doc) .def("AddMap", &Class::AddMap, py::arg("other_map"), cls_doc.AddMap.doc) + .def("AddPackageXml", &Class::AddPackageXml, py::arg("filename"), + cls_doc.AddPackageXml.doc) + .def("AddRemotePackage", &Class::AddRemotePackage, + py::arg("package_name"), py::arg("params")) .def("Contains", &Class::Contains, py::arg("package_name"), cls_doc.Contains.doc) .def("Remove", &Class::Remove, py::arg("package_name"), @@ -53,8 +66,6 @@ PYBIND11_MODULE(parsing, m) { return self.GetPath(package_name); }, py::arg("package_name"), cls_doc.GetPath.doc) - .def("AddPackageXml", &Class::AddPackageXml, py::arg("filename"), - cls_doc.AddPackageXml.doc) .def("PopulateFromFolder", &Class::PopulateFromFolder, py::arg("path"), cls_doc.PopulateFromFolder.doc) .def("PopulateFromEnvironment", &Class::PopulateFromEnvironment, diff --git a/bindings/pydrake/multibody/test/parsing_test.py b/bindings/pydrake/multibody/test/parsing_test.py index ad42044fe146..cf4e529fc21b 100644 --- a/bindings/pydrake/multibody/test/parsing_test.py +++ b/bindings/pydrake/multibody/test/parsing_test.py @@ -68,6 +68,16 @@ def test_package_map(self): dut.PopulateFromEnvironment(environment_variable='TEST_TMPDIR') dut.PopulateFromFolder(path=tmpdir) + # Simple coverage for remote packages (and their Params). + params = PackageMap.RemotePackageParams() + params.urls = ["file:///tmp/does-not-exist.zip"] + params.sha256 = "0" * 64 + params.archive_type = "zip" + params.strip_prefix = "prefix" + dut.AddRemotePackage(package_name="remote", params=params) + with self.assertRaisesRegex(RuntimeError, "downloader.*error"): + dut.GetPath("remote") + def test_parser_file(self): """Calls every combination of arguments for the Parser methods which use a file_name (not contents) and inspects their return type. diff --git a/multibody/parsing/BUILD.bazel b/multibody/parsing/BUILD.bazel index 97463ed28a12..febfa37ebc0b 100644 --- a/multibody/parsing/BUILD.bazel +++ b/multibody/parsing/BUILD.bazel @@ -12,6 +12,7 @@ load( "drake_py_library", "drake_py_unittest", ) +load("@drake//tools/install:install.bzl", "install_files") load("//tools/lint:lint.bzl", "add_lint_tests") load("@drake//tools/workspace:forward_files.bzl", "forward_files") load( @@ -60,15 +61,22 @@ drake_cc_library( srcs = ["package_map.cc"], hdrs = ["package_map.h"], data = [ + ":package_downloader.py", "//:package.xml", "@models_internal//:package.xml", ], visibility = ["//visibility:public"], interface_deps = [ "//common:essential", + "//common:name_value", ], deps = [ - "//common", + "//common:find_cache", + "//common:find_resource", + "//common:find_runfiles", + "//common:scope_exit", + "//common:unused", + "//common/yaml", "@tinyxml2_internal//:tinyxml2", ], ) @@ -615,6 +623,20 @@ drake_cc_googletest( ], ) +drake_cc_googletest( + name = "package_map_remote_test", + data = [ + "test/package_map_test_packages/compressed.zip", + ], + deps = [ + ":package_map", + "//common:find_cache", + "//common:find_resource", + "//common:scope_exit", + "//common/test_utilities:expect_throws_message", + ], +) + drake_cc_googletest( name = "drake_manifest_resolution_test", data = [ @@ -663,4 +685,11 @@ drake_cc_googletest( ], ) +install_files( + name = "install", + dest = "share/drake/multibody/parsing", + files = ["package_downloader.py"], + visibility = ["//visibility:public"], +) + add_lint_tests() diff --git a/multibody/parsing/package_map.cc b/multibody/parsing/package_map.cc index 209493a5dc68..5861865e416e 100644 --- a/multibody/parsing/package_map.cc +++ b/multibody/parsing/package_map.cc @@ -1,6 +1,9 @@ #include "drake/multibody/parsing/package_map.h" +#include + #include +#include #include #include #include @@ -14,13 +17,15 @@ #include #include "drake/common/drake_assert.h" -#include "drake/common/drake_path.h" #include "drake/common/drake_throw.h" +#include "drake/common/find_cache.h" #include "drake/common/find_resource.h" #include "drake/common/find_runfiles.h" #include "drake/common/never_destroyed.h" +#include "drake/common/scope_exit.h" #include "drake/common/text_logging.h" #include "drake/common/unused.h" +#include "drake/common/yaml/yaml_io.h" namespace drake { namespace multibody { @@ -31,13 +36,108 @@ using tinyxml2::XMLDocument; using tinyxml2::XMLElement; namespace { + struct PackageData { - /* Directory in which the manifest resides. */ + // XXX FetchStatus is_fetched; + + /* Directory in which the manifest resides. If this is an undownloaded + remote package, this path will be empty until it is downloaded. */ std::string path; /* Optional message declaring deprecation of the package. */ std::optional deprecated_message; + + /* Iff this is a remote package, this will contain the details. */ + std::optional remote; + + // Below, we have a few tiny "summary" getters for the struct fields, + // to make the call sites a bit more readable. + + /* Returns true iff this package exists on disk (i.e., iff the `path` + has been set). */ + bool is_fetched() const { return path.size() > 0; } + + /* Returns `path` if it is known, or else (for unfetched packages) the + first of the `remote->urls` . */ + std::string printable_path() const { + return !path.empty() ? path : remote.value().urls.front(); + } +}; + +struct DownloaderArgs { + template + void Serialize(Archive* a) { + a->Visit(DRAKE_NVP(package_name)); + remote.Serialize(a); + a->Visit(DRAKE_NVP(output_dir)); + } + std::string package_name; + PackageMap::RemotePackageParams remote; + std::string output_dir; }; + +void FetchContent(std::string_view package_name, PackageData* data) { + DRAKE_DEMAND(!package_name.empty()); + DRAKE_DEMAND(data != nullptr); + DRAKE_DEMAND(!data->is_fetched()); + DRAKE_DEMAND(data->remote.has_value()); + DRAKE_DEMAND(data->path.empty()); + + // Find and/or create the cache_dir. + auto try_cache = internal::FindOrCreateCache("package_map"); + if (!try_cache.error.empty()) { + throw std::runtime_error(fmt::format( + "PackageMap: when downloading '{}', could not create temporary cache " + "directory: {}", + package_name, try_cache.error)); + } + const fs::path cache_dir = std::move(try_cache.abspath); + + // See if the package has already been fetched. + const fs::path package_dir = cache_dir / data->remote->sha256; + std::error_code ec; + if (fs::is_directory(package_dir, ec)) { + data->path = package_dir.string(); + return; + } + + // Write the downloader arguments to a JSON file. + DownloaderArgs args{.package_name = std::string(package_name), + .remote = *data->remote, + .output_dir = package_dir.string()}; + std::string json_filename = fs::path(cache_dir / ".fetch_XXXXXX").string(); + const int temp_fd = ::mkstemp(json_filename.data()); + ::close(temp_fd); + ScopeExit remove_yaml([&json_filename]() { + fs::remove(json_filename); + }); + yaml::SaveJsonFile(json_filename, args); + + // Shell out to the downloader to fetch the package. + const std::string downloader = + FindResourceOrThrow("drake/multibody/parsing/package_downloader.py"); + const std::string command = + fmt::format("/usr/bin/python3 {} {}", downloader, json_filename); + const int returncode = std::system(command.c_str()); + if (returncode != 0) { + throw std::runtime_error(fmt::format( + "PackageMap: when downloading '{}', the downloader experienced an " + "error", + package_name)); + } + + // Confirm that it actually fetched. + if (!fs::is_directory(package_dir, ec)) { + throw std::runtime_error(fmt::format( + "PackageMap: when downloading '{}', the downloader claimed success but " + "somehow did not actually download anything?!", + package_name)); + } + + // Success + data->path = package_dir.string(); +} + } // namespace struct PackageMap::Impl { @@ -111,9 +211,75 @@ void PackageMap::Add(const std::string& package_name, void PackageMap::AddMap(const PackageMap& other_map) { for (const auto& [package_name, data] : other_map.impl_->map) { - Add(package_name, data.path); - SetDeprecated(package_name, data.deprecated_message); + if (data.is_fetched()) { + Add(package_name, data.path); + SetDeprecated(package_name, data.deprecated_message); + } else { + DRAKE_DEMAND(data.remote.has_value()); + AddRemotePackage(package_name, *data.remote); + } + } +} + +void PackageMap::AddRemotePackage(std::string package_name, + RemotePackageParams params) { + // Validate our arguments. + auto iter = impl_->map.find(package_name); + if (iter != impl_->map.end()) { + // Adding a 100% identical package is supported (and a no-op). + // Otherwise, it's an error. + if (iter->second.remote.has_value()) { + const RemotePackageParams& old_params = iter->second.remote.value(); + const std::string old_json = yaml::SaveJsonString(old_params); + const std::string new_json = yaml::SaveJsonString(params); + if (new_json == old_json) { + drake::log()->trace("AddRemotePackage skipping duplicate '{}'", + package_name); + return; + } + } + throw std::logic_error(fmt::format( + "PackageMap::AddRemotePackage cannot add '{}' because a package of " + "that name has already been registered", + package_name)); + } + if (params.urls.empty()) { + throw std::logic_error(fmt::format( + "PackageMap::AddRemotePackage on '{}' requires at least one URL", + package_name)); + } + for (const std::string_view url : params.urls) { + if (!((url.substr(0, 8) == "https://") || (url.substr(0, 7) == "http://") || + (url.substr(0, 7) == "file://"))) { + throw std::logic_error(fmt::format( + "PackageMap::AddRemotePackage on '{}' used an unsupported URL '{}'", + package_name, url)); + } } + if (!((params.sha256.size() == 64) && + (std::all_of(params.sha256.begin(), params.sha256.end(), [](char ch) { + return std::isxdigit(ch); + })))) { + throw std::logic_error(fmt::format( + "PackageMap::AddRemotePackage on '{}' with invalid sha256 '{}'", + package_name, params.sha256)); + } + if (params.archive_type.has_value()) { + const std::initializer_list known_types = { + "zip", "tar", "gztar", "bztar", "xztar"}; + if (std::count(known_types.begin(), known_types.end(), + *params.archive_type) == 0) { + throw std::logic_error(fmt::format( + "PackageMap::AddRemotePackage on '{}' has unsupported archive " + "type '{}'", + package_name, *params.archive_type)); + } + } + + // Everything checks out, so we can add it now. + PackageData data; + data.remote = std::move(params); + impl_->map.emplace_hint(iter, std::move(package_name), std::move(data)); } bool PackageMap::Contains(const std::string& package_name) const { @@ -179,6 +345,12 @@ const std::string& PackageMap::GetPath( drake::log()->warn("PackageMap: {}", *warning); } + // If this is a remote package and we haven't fetched it yet, do that now. + if (!package_data.is_fetched()) { + FetchContent(package_name, const_cast(&package_data)); + } + + DRAKE_DEMAND(!package_data.path.empty()); return package_data.path; } @@ -313,7 +485,9 @@ bool PackageMap::AddPackageIfNew(const std::string& package_name, "does not exist", package_name, path)); } - impl_->map.insert(make_pair(package_name, PackageData{path})); + PackageData data; + data.path = path; + impl_->map.emplace(package_name, data); } else { // Don't warn if we've found the same path with a different spelling. const PackageData existing_data = impl_->map.at(package_name); @@ -321,7 +495,7 @@ bool PackageMap::AddPackageIfNew(const std::string& package_name, drake::log()->warn( "PackageMap is ignoring newly-found path \"{}\" for package \"{}\"" " and will continue using the previously-known path at \"{}\".", - path, package_name, existing_data.path); + path, package_name, existing_data.printable_path()); return false; } } @@ -383,7 +557,7 @@ std::ostream& operator<<(std::ostream& out, const PackageMap& package_map) { out << " [EMPTY!]\n"; } for (const auto& [package_name, data] : package_map.impl_->map) { - out << " - " << package_name << ": " << data.path << "\n"; + out << " - " << package_name << ": " << data.printable_path() << "\n"; } return out; } diff --git a/multibody/parsing/package_map.h b/multibody/parsing/package_map.h index 3360a85f1ba3..f76f6902e124 100644 --- a/multibody/parsing/package_map.h +++ b/multibody/parsing/package_map.h @@ -7,13 +7,15 @@ #include "drake/common/drake_copyable.h" #include "drake/common/fmt_ostream.h" +#include "drake/common/name_value.h" namespace drake { namespace multibody { /** Maps ROS package names to their full path on the local file system. It is used by the SDF and URDF parsers when parsing files that reference ROS packages -for resources like mesh files. */ +for resources like mesh files. This class can also to download remote packages +from the internet on an as-needed basis via AddRemotePackage(). */ class PackageMap final { public: /** A constructor that initializes a default map containing only the top-level @@ -69,7 +71,8 @@ class PackageMap final { void Add(const std::string& package_name, const std::string& package_path); /** Adds package->path mappings from another PackageMap `other_map`. Throws - if the other PackageMap contains the same package with a different path. */ + if the other PackageMap contains the same package with a different path, + or if unable to merge overlapping remote packages. */ void AddMap(const PackageMap& other_map); /** Adds an entry into this PackageMap for the given `package.xml` filename. @@ -77,6 +80,44 @@ class PackageMap final { this map. */ void AddPackageXml(const std::string& filename); + /** Parameters used for AddRemotePackage(). */ + struct RemotePackageParams { + template + void Serialize(Archive* a) { + a->Visit(DRAKE_NVP(urls)); + a->Visit(DRAKE_NVP(sha256)); + a->Visit(DRAKE_NVP(archive_type)); + a->Visit(DRAKE_NVP(strip_prefix)); + } + + /** The list of remote URLs for this resource. The urls are used in the + other they appear here, so preferred mirror(s) should come first. Valid + methods are "http://" or "https://" or "file://". */ + std::vector urls; + + /** The cryptographic checksum of the file to be downloaded, as a + 64-character hexadecimal string. */ + std::string sha256; + + /** (Optional) The archive type of the downloaded file. By default, the + archive type is determined from the file extension of the URL. If the file + has no extension, you can explicitly specify one of the following: "zip", + "tar", "gztar", "bztar", or "xztar" */ + std::optional archive_type; + + /** (Optional) A directory prefix to strip from the extracted files. If + there are files outside of this directory, they will be discarded and + inaccessible. */ + std::optional strip_prefix; + }; + + /** Adds an entry into this PackageMap for the given `package_name`, which + will be downloaded from the internet (with local caching). The data will not + be downloaded until necessary, i.e., when GetPath() is first called for the + `package_name`. Downloading requires a valid `/usr/bin/python3` interpreter, + which will be invoked as a subprocess. */ + void AddRemotePackage(std::string package_name, RemotePackageParams params); + /** Crawls down the directory tree starting at `path` searching for directories containing the file `package.xml`. For each of these directories, this method adds a new entry into this PackageMap where the key is the package diff --git a/multibody/parsing/test/package_map_remote_test.cc b/multibody/parsing/test/package_map_remote_test.cc new file mode 100644 index 000000000000..c7a135d1889f --- /dev/null +++ b/multibody/parsing/test/package_map_remote_test.cc @@ -0,0 +1,218 @@ +#include + +#include +#include + +#include "drake/common/drake_assert.h" +#include "drake/common/find_cache.h" +#include "drake/common/find_resource.h" +#include "drake/common/scope_exit.h" +#include "drake/common/test_utilities/expect_throws_message.h" +#include "drake/multibody/parsing/package_map.h" + +namespace drake { +namespace multibody { +namespace { + +namespace fs = std::filesystem; +using RemotePackageParams = PackageMap::RemotePackageParams; + +class PackageMapRemoteTest : public ::testing::Test { + protected: + /* Creates a mock-up of the ~/.cache/drake/package_map/{sha256} directory + containing just the package.xml file. Returns the package directory. */ + fs::path PrepopulateCache(std::string_view package_name, + std::string_view sha256) { + const auto try_cache = drake::internal::FindOrCreateCache("package_map"); + DRAKE_DEMAND(try_cache.error.empty()); + const fs::path package_dir = try_cache.abspath / sha256; + const bool exists = fs::create_directory(package_dir); + DRAKE_DEMAND(exists); + std::ofstream xml(package_dir / "package.xml"); + static constexpr char kPattern[] = R"""( + + + {} + +)"""; + xml << fmt::format(kPattern, package_name); + return package_dir; + } +}; + +// When the cache directory already containts the content-addressable {sha256} +// directory, the map returns that directory without downloading anything. +TEST_F(PackageMapRemoteTest, GetPathPrepopulated) { + const std::string package_name("some_remote_name"); + const std::string sha256(size_t{64}, '0'); + + // Adding the remote package doesn't download anything. + PackageMap dut = PackageMap::MakeEmpty(); + RemotePackageParams params; + params.urls.push_back("http://127.0.0.1/missing.zip"); + params.sha256 = sha256; + dut.AddRemotePackage(package_name, params); + + // Only a call to GetPath kicks off the resolution. + const fs::path package_dir = PrepopulateCache(package_name, sha256); + const std::string resolved = dut.GetPath(package_name); + EXPECT_EQ(resolved, package_dir.string()); +} + +// Returns a valid remote params object, i.e., one that can actually fetch. +RemotePackageParams MakeGoodParams() { + RemotePackageParams params; + params.urls.push_back(fmt::format( + "file://{}", + FindResourceOrThrow("drake/multibody/parsing/test/" + "package_map_test_packages/compressed.zip"))); + params.sha256 = + "b4bdbad313293ca61fe8f4ed1b5579dadadb3a5c08f0a6d06a8e39e5f97f1bd1"; + params.strip_prefix = "compressed_prefix"; + return params; +} + +// Cover the sunny-day control flow for fetching a remote zip file. +TEST_F(PackageMapRemoteTest, ActuallyFetch) { + // Declare a remote package (using a file:// URI). + PackageMap dut = PackageMap::MakeEmpty(); + const std::string package_name("compressed"); + dut.AddRemotePackage(package_name, MakeGoodParams()); + + // Resolve the package, to force a "download" and extract. + const std::string resolved = dut.GetPath(package_name); + + // Check that the extracted data lives where it should. + const fs::path readme_path = fs::path(resolved) / "README"; + EXPECT_TRUE(fs::is_regular_file(readme_path)) << readme_path; + + // Double-check the file content. + std::ifstream readme(readme_path); + std::stringstream buffer; + buffer << readme.rdbuf(); + EXPECT_EQ(buffer.str(), "This package is empty.\n"); +} + +// Cannot duplicate another package, even if its local-only. +TEST_F(PackageMapRemoteTest, RejectDuplicates) { + PackageMap dut = PackageMap::MakeEmpty(); + fs::create_directory("local_dir"); + dut.Add("foo", "local_dir"); + + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemotePackage("foo", RemotePackageParams{}), + ".*foo.*already.*registered.*"); +} + +// Invalid params produce exception messages. +TEST_F(PackageMapRemoteTest, InputSanitization) { + PackageMap dut = PackageMap::MakeEmpty(); + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemotePackage("foo", RemotePackageParams{}), + ".*at least one URL.*"); + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemotePackage("foo", + RemotePackageParams{ + .urls = {"mailfrom:rico@drake"}, + }), + ".*unsupported URL.*"); + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemotePackage("foo", + RemotePackageParams{ + .urls = {"http://127.0.0.1/missing.zip"}, + .sha256 = "256", + }), + ".*invalid sha256.*"); + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemotePackage("foo", + RemotePackageParams{ + .urls = {"http://127.0.0.1/missing.zip"}, + .sha256 = std::string(64u, 'X'), + }), + ".*invalid sha256.*"); + + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemotePackage("foo", + RemotePackageParams{ + .urls = {"http://127.0.0.1/missing.zip"}, + .sha256 = std::string(64u, '0'), + .archive_type = "arj", + }), + ".*unsupported.*type.*arj.*"); +} + +TEST_F(PackageMapRemoteTest, CouldNotCreateCache) { + // Create the cache directory (it probably exists already, but *shrug*). + const char* const test_tmpdir = std::getenv("TEST_TMPDIR"); + DRAKE_DEMAND(test_tmpdir != nullptr); + const fs::path cache = fs::path(test_tmpdir) / ".cache"; + fs::create_directory(cache); + + // Remove all permissions. + const fs::perms old_perms = fs::status(cache).permissions(); + fs::permissions(cache, fs::perms{}); + ScopeExit guard([&cache, &old_perms]() { + fs::permissions(cache, old_perms); + }); + + // Add and fetch a remote package. It will fail due to lack of cache. + PackageMap dut = PackageMap::MakeEmpty(); + dut.AddRemotePackage("foo", MakeGoodParams()); + DRAKE_EXPECT_THROWS_MESSAGE(dut.GetPath("foo"), ".*create.*cache.*"); +} + +// Merge an unfetched remote package into the the current map. +TEST_F(PackageMapRemoteTest, AddMapMergeUnfetched) { + PackageMap other = PackageMap::MakeEmpty(); + other.AddRemotePackage("compressed", MakeGoodParams()); + + // Merge a remote package into the dut. + PackageMap dut = PackageMap::MakeEmpty(); + dut.AddMap(other); + EXPECT_THAT(dut.GetPackageNames(), testing::ElementsAre("compressed")); + + // Fetch it and make sure it came through. + const fs::path readme = fs::path(dut.GetPath("compressed")) / "README"; + EXPECT_TRUE(fs::is_regular_file(readme)) << readme; +} + +// Merge a fetched remote package into the the current map. +TEST_F(PackageMapRemoteTest, AddMapMergeFetched) { + PackageMap other = PackageMap::MakeEmpty(); + other.AddRemotePackage("compressed", MakeGoodParams()); + other.GetPath("compressed"); + + // Merge the fetched package into the dut. + PackageMap dut = PackageMap::MakeEmpty(); + dut.AddMap(other); + EXPECT_THAT(dut.GetPackageNames(), testing::ElementsAre("compressed")); + const fs::path readme = fs::path(dut.GetPath("compressed")) / "README"; + EXPECT_TRUE(fs::is_regular_file(readme)) << readme; +} + +// Merging identical remote packages is a no-op. +TEST_F(PackageMapRemoteTest, AddMapDeduplicate) { + PackageMap dut = PackageMap::MakeEmpty(); + dut.AddRemotePackage("compressed", MakeGoodParams()); + PackageMap other(dut); + dut.AddMap(other); + EXPECT_THAT(dut.GetPackageNames(), testing::ElementsAre("compressed")); +} + +// Merging differing remote packages is a failure. +TEST_F(PackageMapRemoteTest, AddMapMismatch) { + RemotePackageParams params1 = MakeGoodParams(); + RemotePackageParams params2 = params1; + params2.urls.push_back(params1.urls.back()); + + PackageMap dut = PackageMap::MakeEmpty(); + dut.AddRemotePackage("compressed", params1); + PackageMap other = PackageMap::MakeEmpty(); + other.AddRemotePackage("compressed", params2); + + DRAKE_EXPECT_THROWS_MESSAGE(dut.AddMap(other), ".*already.*registered.*"); +} + +} // namespace +} // namespace multibody +} // namespace drake diff --git a/multibody/parsing/test/package_map_test.cc b/multibody/parsing/test/package_map_test.cc index eb60f97b2cd9..e67e4c2bbbdb 100644 --- a/multibody/parsing/test/package_map_test.cc +++ b/multibody/parsing/test/package_map_test.cc @@ -19,6 +19,8 @@ namespace { namespace fs = std::filesystem; +// N.B. See also package_map_remote_test.cc for additional test cases. + string GetTestDataRoot() { const string desired_dir = "drake/multibody/parsing/test/package_map_test_packages/"; @@ -165,6 +167,16 @@ GTEST_TEST(PackageMapTest, TestManualPopulation) { EXPECT_THROW(package_map.Remove("package_baz"), std::runtime_error); } +// Default-constructed maps must always be merge-able. +GTEST_TEST(PackageMapTest, AddDefauledMaps) { + const PackageMap foo; + const PackageMap bar; + PackageMap dut; + dut.AddMap(foo); + dut.AddMap(bar); + EXPECT_EQ(dut.size(), foo.size()); +} + // Tests that PackageMaps can be combined via AddMap. GTEST_TEST(PackageMapTest, TestAddMap) { fs::create_directory("package_foo"); @@ -348,6 +360,10 @@ GTEST_TEST(PackageMapTest, TestStreamingToString) { for (const auto& it : expected_packages) { package_map.Add(it.first, it.second); } + const std::string url = "http://127.0.0.1/missing.zip"; + package_map.AddRemotePackage("remote", { + .urls = {url}, + .sha256 = std::string(64u, '0')}); std::stringstream string_buffer; string_buffer << package_map; @@ -360,10 +376,11 @@ GTEST_TEST(PackageMapTest, TestStreamingToString) { EXPECT_NE(resulting_string.find(it.first), std::string::npos); EXPECT_NE(resulting_string.find(it.second), std::string::npos); } + EXPECT_NE(resulting_string.find(url), std::string::npos); - // Verifies that there are three lines in the resulting string. + // Verifies the number of lines in the resulting string. EXPECT_EQ(std::count(resulting_string.begin(), resulting_string.end(), '\n'), - 3); + 4); } // Tests that PackageMap is parsing deprecation messages diff --git a/multibody/parsing/test/package_map_test_packages/compressed.zip b/multibody/parsing/test/package_map_test_packages/compressed.zip new file mode 100644 index 0000000000000000000000000000000000000000..f91c9fdac6c2107fa94624914d9fdb38784aabb8 GIT binary patch literal 569 zcmWIWW@h1H00I86w6Fz1F9*0WO>QVzyKyfGBS%53KEmE6Vp=_fXvj~f|5!-t^lA#B0ys~82*N) zg;`u*V6h0uV`XGuki}{d)C9eX+#IlNaX_20Fm3boKg)MWfoI=mkrsA|%f`u4wr?L; zo2;=?s%K35_T$iP@;#o_*Ea@V-V`S!=&<);>z9{{TkmW%p4<}j z@W7&wdu<(>4>f*xg{0nAxV>!31M4-HIy_xvi;ix0{&pgKKI_|}%pQ@`IaX%sJ>oG3 z*qVJUPd|B}?*3km_5bec24^Zi1@b-kusq-8YERe;p;PSpx%M$D{rI*-B|gBLkx7mj zR~SnG1A&177zzwq8bK`FA