diff --git a/bindings/pydrake/multibody/parsing_py.cc b/bindings/pydrake/multibody/parsing_py.cc index 4b5f0ece42e1..2e4d382fce10 100644 --- a/bindings/pydrake/multibody/parsing_py.cc +++ b/bindings/pydrake/multibody/parsing_py.cc @@ -32,12 +32,26 @@ PYBIND11_MODULE(parsing, m) { using Class = PackageMap; constexpr auto& cls_doc = doc.PackageMap; py::class_ cls(m, "PackageMap", cls_doc.doc); + { + using Nested = PackageMap::RemoteParams; + constexpr auto& nested_doc = cls_doc.RemoteParams; + py::class_ nested(cls, "RemoteParams", nested_doc.doc); + nested.def(ParamInit()); + nested.def("ToJson", &Nested::ToJson, nested_doc.ToJson.doc); + 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("AddRemote", &Class::AddRemote, 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 +67,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..71ab7dec1066 100644 --- a/bindings/pydrake/multibody/test/parsing_test.py +++ b/bindings/pydrake/multibody/test/parsing_test.py @@ -68,6 +68,34 @@ def test_package_map(self): dut.PopulateFromEnvironment(environment_variable='TEST_TMPDIR') dut.PopulateFromFolder(path=tmpdir) + def test_package_map_remote_params(self): + dut = PackageMap.RemoteParams( + urls=["file:///tmp/missing.zip"], + sha256="0" * 64, + archive_type="zip", + strip_prefix="prefix",) + self.assertIn("missing.zip", dut.ToJson()) + copy.copy(dut) + copy.deepcopy(dut) + + def test_package_map_add_remote(self): + """Runs a full lifecycle of AddRemote + GetPath to check that Python + bindings calling C++ code that shells out to Python all plays nice. + """ + dut = PackageMap.MakeEmpty() + zipfile = FindResourceOrThrow( + "drake/multibody/parsing/test/package_map_test_packages/" + "compressed.zip") + dut.AddRemote(package_name="compressed", + params=PackageMap.RemoteParams( + urls=[f"file://{zipfile}"], + sha256=("b4bdbad313293ca61fe8f4ed1b5579da" + "dadb3a5c08f0a6d06a8e39e5f97f1bd1"), + strip_prefix="compressed_prefix")) + path = dut.GetPath("compressed") + with open(f"{path}/README", encoding="utf-8") as f: + self.assertEqual(f.read(), "This package is empty.\n") + 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 dc6ba06426d9..651c60acfec2 100644 --- a/multibody/parsing/BUILD.bazel +++ b/multibody/parsing/BUILD.bazel @@ -23,12 +23,13 @@ filegroup( testonly = 1, srcs = glob([ "test/**/*.config", + "test/**/*.dmd.yaml", "test/**/*.obj", + "test/**/*.png", "test/**/*.sdf", "test/**/*.urdf", "test/**/*.xml", - "test/**/*.dmd.yaml", - "test/**/*.png", + "test/**/*.zip", "test/**/COLCON_IGNORE", ]), visibility = ["//visibility:public"], @@ -66,10 +67,14 @@ drake_cc_library( visibility = ["//visibility:public"], interface_deps = [ "//common:essential", + "//common:name_value", ], deps = [ + "//common:find_cache", "//common:find_resource", "//common:find_runfiles", + "//common:scope_exit", + "//common/yaml", "@tinyxml2_internal//:tinyxml2", ], ) @@ -622,6 +627,20 @@ drake_cc_googletest( ], ) +drake_cc_googletest( + name = "package_map_remote_test", + data = [ + ":test_models", + ], + 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 = [ diff --git a/multibody/parsing/package_downloader.py b/multibody/parsing/package_downloader.py index 2f04578b7dbf..425fe5c215e9 100644 --- a/multibody/parsing/package_downloader.py +++ b/multibody/parsing/package_downloader.py @@ -147,8 +147,13 @@ def _run(*, temp_dir: Path, package_name: str, urls: List[str], sha256: str, def _main(argv): + # We expect exactly two command-line arguments to this program: + # - The filename containing JSON data with our *actual* arguments. + # - A dummy argument that we'll ignore. (It's sometimes used by our caller + # to suppress valgrind when we're called from C++ code). + config_json, _ = argv + # Read our config file. - config_json, = argv with open(config_json, "r") as f: kwargs = json.load(f) diff --git a/multibody/parsing/package_map.cc b/multibody/parsing/package_map.cc index 67dbb0a45600..d0f2442e36de 100644 --- a/multibody/parsing/package_map.cc +++ b/multibody/parsing/package_map.cc @@ -1,6 +1,10 @@ #include "drake/multibody/parsing/package_map.h" +#include + #include +#include +#include #include #include #include @@ -15,10 +19,13 @@ #include "drake/common/drake_assert.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/yaml/yaml_io.h" namespace drake { namespace multibody { @@ -28,11 +35,52 @@ namespace fs = std::filesystem; using tinyxml2::XMLDocument; using tinyxml2::XMLElement; +std::string PackageMap::RemoteParams::ToJson() const { + return yaml::SaveJsonString(*this); +} + +bool operator==(const PackageMap::RemoteParams& left, + const PackageMap::RemoteParams& right) { + return left.ToJson() == right.ToJson(); +} + namespace { +/* Provides a little encapsulation for whether a package is both remote and +unfetched, especially in terms of enabling PackageData to use the default +implementations of the special member functions (i.e., copy & move). */ +class NeedsFetch { + public: + /* Default constructs to `false`. */ + NeedsFetch() : needs_fetch_{false} {} + + /* Copying preserves the value, but produces a distinct atomic variable. */ + NeedsFetch(const NeedsFetch& other) + : needs_fetch_(other.needs_fetch_.load()) {} + + // Does not support copy-assignment nor move-assignment. + NeedsFetch& operator=(const NeedsFetch&) = delete; + + /* Returns true iff the package is both remote and unfetched. */ + operator bool() const { return needs_fetch_.load(); } + + /* Sets this to a new value. */ + void store(bool value) { needs_fetch_ = value; } + + /* Returns a lock_guard for our encapsulated mutex. */ + [[nodiscard]] auto lock_guard() const { return std::lock_guard(mutex_); } + + private: + std::atomic needs_fetch_; + mutable std::mutex mutex_; +}; + /* PackageData encapsulates everything we know about an added package: - The settings that were given to us when it was added. - - A deprecation status that can be added after construction. */ + - A deprecation status that can be added after construction. + - For remote packages, whether it's been fetched locally yet. + - Atomic access and a mutex to guard the above. +*/ class PackageData { public: /* Declares a local package (with a local filesystem path). */ @@ -42,6 +90,15 @@ class PackageData { return result; } + /* Declares a remote package (with URLs, etc). Does not fetch anything. + @pre `params` has already been validated to be well-formed. */ + static PackageData MakeRemote(PackageMap::RemoteParams params) { + PackageData result; + result.remote_params_ = std::move(params); + result.needs_fetch_.store(true); + return result; + } + // Supports copy and move. PackageData(const PackageData&) = default; PackageData(PackageData&&) = default; @@ -49,6 +106,18 @@ class PackageData { // Does not support copy-assignment nor move-assignment. void operator=(const PackageData&) = delete; + /* Returns true iff this package was added using a local path, e.g., + Add() or AddPackageXml() or PopulateFrom... etc. */ + bool is_local() const { return !remote_params_.has_value(); } + + /* Returns true iff this package was added using AddRemote(). */ + bool is_remote() const { return remote_params_.has_value(); } + + /* Returns the remote params; throws when is_remote() is false. */ + const PackageMap::RemoteParams& remote_params() const { + return remote_params_.value(); + } + /* Returns true iff this package is deprecated. */ bool is_deprecated() const { return deprecation_.has_value(); } @@ -69,34 +138,151 @@ class PackageData { } } - /* Returns the path suitable for use in error messages. */ - const std::string& display_path() const { return path_; } + /* Returns the path suitable for use in error messages. For local packages, + returns the local path; for remote packages, returns the first URL. */ + const std::string& display_path() const { + return remote_params_.has_value() ? remote_params_.value().urls.front() + : path_; + } /* Returns true iff `other` specifies a suitably identical package to `this` - such that we can fold the two together: the path specification must match, but - the deprecation status can differ. In case they do not match, the error (if - provided) will be reset to describe the incompatibility. */ + such that we can fold the two together: the path specification (whether local + or remote) must match, but the deprecation or fetch status can differ. In case + they do not match, the error (if provided) will be reset to describe the + incompatibility. */ bool CanMerge(const PackageData& other, std::string* error = nullptr) const; /* Merges `other` into `this`. Throws an exception if CanMerge() is false. The `package_name` is non-functional (only used when reporting errors). */ void Merge(std::string_view package_name, const PackageData& other); - /* Returns the local filesystem path for this package. - TODO(jwnimmer-tri) We should fetch remote packages here. */ - const std::string& GetPath() const { return path_; } + /* Returns the local filesystem path for this package. In case this package + is remote and not already fetched, this function will fetch before returning. + The `package_name` is non-functional (only used when reporting errors). */ + const std::string& GetPathWithAutomaticFetching( + std::string_view package_name) const; private: PackageData() = default; - /* Directory in which the manifest resides. */ + /* When a package is added using AddRemote(), this starts out 'true'. + After the downloader finishes successfully, it changes to 'false'. + For local packages, this is always 'false'. */ + NeedsFetch needs_fetch_; + + /* Directory in which the manifest resides. When needs_fetch() is true, it is + NOT ALLOWED to access this member field (to avoid race conditions). */ std::string path_; + /* If this was added using AddRemote(), this will contain the details, even + after the package has been fetched and is_remote has changed to 'false'. In + other words, this is a record of what was declared, even if `path_` has since + been changed to point to the downloaded copy.) */ + std::optional remote_params_; + /* Optional message declaring deprecation of the package. If it is non-nullopt then it is also guaranteed to be non-empty. */ std::optional deprecation_; }; +/* A little helper struct that gathers the args for package_downloader into a +single place to make it easy to convert to JSON. */ +struct DownloaderArgs { + template + void Serialize(Archive* a) { + a->Visit(DRAKE_NVP(package_name)); + remote_params.Serialize(a); + a->Visit(DRAKE_NVP(output_dir)); + } + std::string package_name; + PackageMap::RemoteParams remote_params; + std::string output_dir; +}; + +const std::string& PackageData::GetPathWithAutomaticFetching( + std::string_view package_name) const { + DRAKE_DEMAND(!package_name.empty()); + + if (!needs_fetch_) { + DRAKE_DEMAND(!path_.empty()); + return path_; + } + DRAKE_DEMAND(is_remote()); + + // Note that we are a *const* member function, but we're about to start + // modifying member data as we fetch the package. Everything from here on + // needs to have exclusive access to this package's PackageData object. + auto guard = needs_fetch_.lock_guard(); + auto* mutable_this = const_cast(this); + + // We need to do the "double-checked locking" pattern; the bool might have + // changed value while we were waiting for the lock. + if (!needs_fetch_) { + DRAKE_DEMAND(!path_.empty()); + return path_; + } + + // Find and/or create the cache_dir. + internal::PathOrError 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 / remote_params().sha256; + std::error_code ec; + if (fs::is_directory(package_dir, ec)) { + mutable_this->path_ = package_dir.string(); + mutable_this->needs_fetch_.store(false); + return path_; + } + + drake::log()->info("PackageMap: Downloading {}", display_path()); + + // Write the downloader arguments to a JSON file. + DownloaderArgs args{.package_name = std::string(package_name), + .remote_params = remote_params(), + .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, + "--disable-drake-valgrind-tracing"); + 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! + mutable_this->path_ = package_dir.string(); + mutable_this->needs_fetch_.store(false); + return path_; +} + } // namespace class PackageMap::Impl { @@ -156,8 +342,9 @@ class PackageMap::Impl { if (iter != map_.end()) { std::string error; if (!iter->second.CanMerge(data, &error)) { - throw std::runtime_error(fmt::format( - "PackageMap::Add() cannot add '{}' {}", package_name, error)); + throw std::runtime_error( + fmt::format("PackageMap::Add{}() cannot add '{}' {}", + data.is_remote() ? "Remote" : "", package_name, error)); } return; } @@ -241,16 +428,41 @@ void PackageMap::Add(const std::string& package_name, } bool PackageData::CanMerge(const PackageData& other, std::string* error) const { - // The paths must resolve to the same dir. - if (!fs::equivalent(this->path_, other.path_)) { - if (error != nullptr) { - *error = fmt::format( - "because the local paths are not equivalent ('{}' vs '{}')", - this->display_path(), other.display_path()); + // When both packages are local, the paths must resolve to the same dir. + if (this->is_local() && other.is_local()) { + if (!fs::equivalent(this->path_, other.path_)) { + if (error != nullptr) { + *error = fmt::format( + "because the local paths are not equivalent ('{}' vs '{}')", + this->display_path(), other.display_path()); + } + return false; + } + return true; + } + + // When both packages are remote, the specification must match exactly. + if (this->is_remote() && other.is_remote()) { + if (!(this->remote_params() == other.remote_params())) { + if (error != nullptr) { + *error = fmt::format( + "because the remote package parameters differ ({} vs {})", + this->remote_params().ToJson(), other.remote_params().ToJson()); + } + return false; } - return false; + return true; } - return true; + + // Cannot merge local paths with remote params. + if (error != nullptr) { + *error = fmt::format( + "because the existing path is {} ('{}') " + "but the the new path is {} ('{}')", + this->is_local() ? "local" : "remote", this->display_path(), + other.is_local() ? "local" : "remote", other.display_path()); + } + return false; } void PackageData::Merge(std::string_view package_name, @@ -275,6 +487,47 @@ void PackageMap::AddMap(const PackageMap& other_map) { impl_->AddMap(*other_map.impl_); } +void PackageMap::AddRemote(std::string package_name, RemoteParams params) { + drake::log()->trace("PackageMap.AddRemote('{}', '{}', ...)", package_name, + params.urls.empty() ? "" : params.urls.front()); + + // Validate our arguments. + if (params.urls.empty()) { + throw std::logic_error( + fmt::format("PackageMap::AddRemote 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::AddRemote on '{}' has 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::AddRemote on '{}' has 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::AddRemote on '{}' has unsupported archive type '{}'", + package_name, *params.archive_type)); + } + } + + // Add it now. Emplace will handle rejection of duplicates. + impl_->Emplace(package_name, PackageData::MakeRemote(std::move(params))); +} + bool PackageMap::Contains(const std::string& package_name) const { return impl_->map().find(package_name) != impl_->map().end(); } @@ -328,7 +581,8 @@ const std::string& PackageMap::GetPath( drake::log()->warn("PackageMap: {}", *warning); } - return data.GetPath(); + // If this is a remote package and we haven't fetched it yet, do that now. + return data.GetPathWithAutomaticFetching(package_name); } void PackageMap::PopulateFromFolder(const std::string& path) { diff --git a/multibody/parsing/package_map.h b/multibody/parsing/package_map.h index ad82a1a2489b..4b886e2d699a 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 download remote packages from +the internet on an as-needed basis via AddRemote(). */ class PackageMap final { public: /** A constructor that initializes a default map containing only the top-level @@ -68,8 +70,9 @@ class PackageMap final { if `package_path` does not exist. */ 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. */ + /** Adds all packages from `other_map` into `this`. Throws if `other` contains + a package with the same `package_name` as one already in this map but with + incompatible details (e.g., a different local path). */ void AddMap(const PackageMap& other_map); /** Adds an entry into this PackageMap for the given `package.xml` filename. @@ -77,6 +80,54 @@ class PackageMap final { this map. */ void AddPackageXml(const std::string& filename); + /** Parameters used for AddRemote(). */ + struct RemoteParams { + 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)); + } + + /** Returns the JSON serialization of these params. */ + std::string ToJson() const; + + /** Equality operator. */ + friend bool operator==(const RemoteParams&, const RemoteParams&); + + /** 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. Valid options are + "zip", "tar", "gztar", "bztar", or "xztar". By default, the archive type is + determined from the file extension of the URL; in case the URL has no + filename extension, you should explicitly specify one here. */ + std::optional archive_type; + + /** (Optional) A directory prefix to remove from the extracted files. In + many cases, an archive will prefix all filenames with something like + "package-v1.2.3/" so that it extracts into a convenient directory. This + option will discard that common prefix when extracting the archive for the + PackageMap. It is an error if the archive does not contain any diectory with + this prefix, but if there are files outside of this directory they will be + silently discarded. */ + 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 AddRemote(std::string package_name, RemoteParams 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_downloader_stress_test.py b/multibody/parsing/test/package_downloader_stress_test.py index 7d7b3166d934..55169624a7f7 100644 --- a/multibody/parsing/test/package_downloader_stress_test.py +++ b/multibody/parsing/test/package_downloader_stress_test.py @@ -54,7 +54,7 @@ def test_many_threads(self): while (len(children) + 1 < cpus) and (launch_count < N): children.append(subprocess.Popen([ sys.executable, "multibody/parsing/package_downloader.py", - kwargs])) + kwargs, ""])) launch_count += 1 # Reap completed processes. for i in reversed(range(len(children))): diff --git a/multibody/parsing/test/package_downloader_test.py b/multibody/parsing/test/package_downloader_test.py index 885ce8472d54..6cfb80b462f6 100644 --- a/multibody/parsing/test/package_downloader_test.py +++ b/multibody/parsing/test/package_downloader_test.py @@ -70,7 +70,7 @@ def _call_main(self, expect_success=True, **kwargs): # Wrap a call to main. try: - mut._main([filename]) + mut._main([filename, ""]) returncode = 0 except SystemExit as e: returncode = e.code 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..4bc2018b1c6d --- /dev/null +++ b/multibody/parsing/test/package_map_remote_test.cc @@ -0,0 +1,248 @@ +#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 RemoteParams = PackageMap::RemoteParams; + +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 contains 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(64u, '0'); + + // Adding the remote package doesn't download anything. + PackageMap dut = PackageMap::MakeEmpty(); + RemoteParams params; + params.urls.push_back("http://127.0.0.1/missing.zip"); + params.sha256 = sha256; + dut.AddRemote(package_name, params); + + // Only a call to GetPath kicks off the resolution, and even then nothing is + // downloaded (the missing URL would have been a 404 error). + 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. +RemoteParams MakeGoodParams() { + RemoteParams 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.AddRemote(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. (Note that here we're + // checking the README file within the downloaded package, not the metadata + // {sha256}.README file from the downloader program in the parent directory.) + 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(); + dut.Add("foo", "."); + + DRAKE_EXPECT_THROWS_MESSAGE(dut.AddRemote("foo", MakeGoodParams()), + ".*existing path is local.*"); +} + +// Invalid params produce exception messages. +TEST_F(PackageMapRemoteTest, InputSanitization) { + PackageMap dut = PackageMap::MakeEmpty(); + + DRAKE_EXPECT_THROWS_MESSAGE(dut.AddRemote("foo", RemoteParams{}), + ".*at least one URL.*"); + + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemote("foo", + RemoteParams{ + .urls = {"mailfrom:rico@drake.mit.edu"}, + }), + ".*unsupported URL.*"); + + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemote("foo", + RemoteParams{ + .urls = {"http://127.0.0.1/missing.zip"}, + .sha256 = "256", + }), + ".*invalid sha256.*"); + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemote("foo", + RemoteParams{ + .urls = {"http://127.0.0.1/missing.zip"}, + .sha256 = std::string(64u, 'X'), + }), + ".*invalid sha256.*"); + + DRAKE_EXPECT_THROWS_MESSAGE( + dut.AddRemote("foo", + RemoteParams{ + .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 (most notably, write permission). + 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.AddRemote("foo", MakeGoodParams()); + DRAKE_EXPECT_THROWS_MESSAGE(dut.GetPath("foo"), ".*create.*cache.*"); +} + +TEST_F(PackageMapRemoteTest, CouldNotFetch) { + // Declare a remote package with a well-formed but incorrect checksum. + auto params = MakeGoodParams(); + params.sha256.front() = 'f'; + PackageMap dut = PackageMap::MakeEmpty(); + const std::string package_name("compressed"); + dut.AddRemote(package_name, params); + + // Fetching the package throws. + DRAKE_EXPECT_THROWS_MESSAGE(dut.GetPath(package_name), + ".*downloader.*error.*"); + + // An incomplete download should not corrupt the cache dir. Trying again + // should still fail. + DRAKE_EXPECT_THROWS_MESSAGE(dut.GetPath(package_name), + ".*downloader.*error.*"); +} + +// Merge an unfetched remote package into the the current map. Our purpose here +// is a sanity check that nothing crashes when copying an unfetched PackageData. +TEST_F(PackageMapRemoteTest, AddMapMergeUnfetched) { + PackageMap compressed = PackageMap::MakeEmpty(); + compressed.AddRemote("compressed", MakeGoodParams()); + + // Merge a remote package into the dut. + PackageMap dut = PackageMap::MakeEmpty(); + dut.AddMap(compressed); + 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. Our purpose here is +// a sanity check that nothing crashes when copying a fetched PackageData, but +// also to confirm that the presence of the known local path does not prevent us +// from preserving the remote params for later use. +TEST_F(PackageMapRemoteTest, AddMapMergeFetched) { + PackageMap compressed = PackageMap::MakeEmpty(); + compressed.AddRemote("compressed", MakeGoodParams()); + compressed.GetPath("compressed"); + + // Merge the fetched package into the dut. + PackageMap dut = PackageMap::MakeEmpty(); + dut.AddMap(compressed); + 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; + + // Confirm the remote data (URLs) was still preserved, even though we already + // had a fetched path in available during the AddMap. + EXPECT_THAT(fmt::to_string(fmt_streamed(dut)), + testing::HasSubstr("package_map_test_packages/compressed.zip")); +} + +// Merging identical remote packages is a no-op. +TEST_F(PackageMapRemoteTest, AddMapDeduplicate) { + PackageMap dut = PackageMap::MakeEmpty(); + dut.AddRemote("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) { + RemoteParams params1 = MakeGoodParams(); + RemoteParams params2 = params1; + params2.urls.push_back(params1.urls.back()); + + PackageMap dut = PackageMap::MakeEmpty(); + dut.AddRemote("compressed", params1); + PackageMap other = PackageMap::MakeEmpty(); + other.AddRemote("compressed", params2); + + DRAKE_EXPECT_THROWS_MESSAGE(dut.AddMap(other), ".*parameters differ.*"); +} + +} // namespace +} // namespace multibody +} // namespace drake diff --git a/multibody/parsing/test/package_map_test.cc b/multibody/parsing/test/package_map_test.cc index 7a101a33662c..3fb9f732c08d 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/"; @@ -393,6 +395,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.AddRemote("remote", { + .urls = {url}, + .sha256 = std::string(64u, '0')}); std::stringstream string_buffer; string_buffer << package_map; @@ -405,10 +411,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 000000000000..f91c9fdac6c2 Binary files /dev/null and b/multibody/parsing/test/package_map_test_packages/compressed.zip differ diff --git a/tools/wheel/image/build-wheel.sh b/tools/wheel/image/build-wheel.sh index 362729da7ad9..c467253c8d23 100755 --- a/tools/wheel/image/build-wheel.sh +++ b/tools/wheel/image/build-wheel.sh @@ -88,6 +88,7 @@ cp -r -t ${WHEEL_SHARE_DIR}/drake \ /opt/drake/share/drake/examples \ /opt/drake/share/drake/geometry \ /opt/drake/share/drake/manipulation \ + /opt/drake/share/drake/multibody \ /opt/drake/share/drake/tutorials # TODO(#15774) Eventually we will download these at runtime, instead of shipping # them in the wheel.