[doc] Fix model_version_control instructions

doc/_pages/model_version_control.md

@@ -12,9 +12,7 @@ If your model's files are small enough (<100KB in a given directory), then you
 may add them directly to Git in a PR to master.
 
 Otherwise, you should add the large files to
-[RobotLocomotion/models](https://github.com/RobotLocomotion/models). Please do
-not commit files that are generally small, like ``*.sdf`` or ``*.urdf`` files,
-in ``RobotLocomotion/models``; instead, please commit those directly.
+[RobotLocomotion/models](https://github.com/RobotLocomotion/models).
 
 Before you decide to submit models, please ensure that you have tests that
 will need them. Do not submit a PR that adds models but has zero intent to use
@@ -24,17 +22,21 @@ See below for the suggested workflow.
 
 ## Develop Changes Locally
 
-1. Clone ``RobotLocomotion/models`` locally
+1. Clone ``RobotLocomotion/models`` locally.
 2. Create a Git branch in your local checkouts of *both* ``models`` and
    ``drake``.
 3. Update ``drake/tools/workspace/drake_models/repository.bzl`` to point to your
    ``models`` checkout using
    ``github_archive(..., local_repository_override = <path>)``.
-4. Update ``drake/tools/workspace/drake_models/files.bzl`` to incorporate the models
-   you want.
-5. Update ``drake/tools/workspace/drake_models/package.BUILD.bazel`` to export the
-   models.
-6. Ensure your tests pass under ``bazel test``.
+4. Incorporate the new ``models`` files into a Drake ``BUILD.bazel`` file where
+   they are needed, typically via ``filegroup(name = "models", ...)``.
+   1. You can grep for ``@drake_models`` to find examples of using existing
+      model files.
+   2. Do not ask the build system to "install" any files from ``drake_models``;
+      they are already available via remote fetching. In particular, don't add
+      any ``install()`` rule for them, nor add them to any ``filegroup()`` that
+      is already being installed.
+5. Ensure your tests pass under ``bazel test``.
 
 ## Submit Changes in a Pull Request
 

multibody/parsing/package_map.cc

@@ -278,7 +278,8 @@ bool PackageData::FindInCache() {
   if (!path_.needs_fetch()) {
     return true;
   }
-  internal::PathOrError try_cache = internal::FindOrCreateCache("package_map");
+  drake::internal::PathOrError try_cache =
+      drake::internal::FindOrCreateCache("package_map");
   if (!try_cache.error.empty()) {
     return false;
   }
@@ -312,7 +313,8 @@ const std::string& PackageData::GetPathWithAutomaticFetching(
   }
 
   // Find and/or create the cache_dir.
-  internal::PathOrError try_cache = internal::FindOrCreateCache("package_map");
+  drake::internal::PathOrError try_cache =
+      drake::internal::FindOrCreateCache("package_map");
   if (!try_cache.error.empty()) {
     throw std::runtime_error(fmt::format(
         "PackageMap: when downloading '{}', could not create temporary cache "
@@ -500,8 +502,10 @@ struct RepositoryMetadataSchema {
   std::string strip_prefix;
 };
 
-/* Loads and parses the metadata from tools/workspace/models_internal into the
-RemoteParams structure needed by PackageMap. */
+}  // namespace
+
+namespace internal {
+
 PackageMap::RemoteParams GetDrakeModelsRemoteParams() {
   const std::string json_filename =
       FindResourceOrThrow("drake/multibody/parsing/drake_models.json");
@@ -520,7 +524,7 @@ PackageMap::RemoteParams GetDrakeModelsRemoteParams() {
   return result;
 }
 
-}  // namespace
+}  // namespace internal
 
 PackageMap::PackageMap() : PackageMap{std::nullopt} {
   // FindResource is the source of truth for where Drake's first-party files
@@ -533,7 +537,7 @@ PackageMap::PackageMap() : PackageMap{std::nullopt} {
   // the if-else to ensure it receives test coverage under bazel (i.e., even if
   // we're never going to download anything).
   static const never_destroyed<RemoteParams> memoized_params(
-      GetDrakeModelsRemoteParams());
+      internal::GetDrakeModelsRemoteParams());
 
   // For drake_models (i.e., https://github.com/RobotLocomotion/models), the
   // location where we find the data will vary. If we have Bazel runfiles with

multibody/parsing/package_map.h

@@ -217,6 +217,13 @@ class PackageMap final {
   std::unique_ptr<Impl> impl_;
 };
 
+namespace internal {
+
+/* (Internal use only) Parses the metadata from `tools/workspace/drake_models`
+into the RemoteParams structure needed by PackageMap. */
+PackageMap::RemoteParams GetDrakeModelsRemoteParams();
+
+}  // namespace internal
 }  // namespace multibody
 }  // namespace drake
 

multibody/parsing/test/package_map_remote_test.cc

@@ -295,6 +295,24 @@ TEST_F(PackageMapRemoteTest, AddMapMismatch) {
   DRAKE_EXPECT_THROWS_MESSAGE(dut.AddMap(other), ".*parameters differ.*");
 }
 
+// Sanity check the `package://drake_models/...` defaults.
+TEST_F(PackageMapRemoteTest, DrakeModelsDefaults) {
+  // Check that the package exists by default.
+  std::string drake_models_path;
+  EXPECT_NO_THROW(drake_models_path = PackageMap().GetPath("drake_models"));
+  EXPECT_TRUE(fs::is_directory(drake_models_path)) << drake_models_path;
+
+  // Check its remote fetching configuration (using an internal API, because we
+  // don't actually want to fetch it from the network during a unit test).
+  const RemoteParams params = internal::GetDrakeModelsRemoteParams();
+  EXPECT_GE(params.urls.size(), 1);
+  EXPECT_THAT(params.urls, testing::Each(testing::StartsWith("https://")));
+  EXPECT_EQ(params.sha256.size(), 64);
+  EXPECT_EQ(params.archive_type, std::nullopt);
+  EXPECT_THAT(params.strip_prefix,
+              testing::Optional(testing::StartsWith("models-")));
+}
+
 }  // namespace
 }  // namespace multibody
 }  // namespace drake

tools/workspace/drake_models/package.BUILD.bazel

@@ -10,3 +10,24 @@ filegroup(
     name = "drake_models",
     srcs = _SRCS,
 )
+
+# Nominally, the `@drake_models` external is fetched via `github_archive()`,
+# which creates a json metadata files to explain what it downloaded. Drake's
+# install rules use that file to pin the lazy-download version of the models.
+#
+# However, in case a developer is using a local checkout of `@drake_models`,
+# the json file will not exist. In that case, we need to generate a stub file
+# to take its place, so that our Bazel install rules can still find it. We'll
+# fill it with dummy data. To guard against shipping a Drake release with the
+# dummy data, the package_map_remote_test checks the content of the json file.
+glob(["drake_repository_metadata.json"], allow_empty = True) or genrule(
+    name = "_gen_dummy_metadata",
+    outs = ["drake_repository_metadata.json"],
+    cmd = "echo '{}' > $@".format(
+        json.encode(dict(
+            urls = [],
+            sha256 = "",
+            strip_prefix = "",
+        )),
+    ),
+)