Set docs.rs as the default extern-map for crates.io

[jyn514]

Helps address #8296, specifically the bit needed for rust-lang/docs.rs#1177.
r? @ehuss

[alexcrichton]

Thanks and seems reasonable to me! Instead of manually adding this after deserializing, though, could this update the Deserialize integration to add this in if it wasn't found previously? I think that would be a bit more natural to integrate with what we already have.

[jyn514]

Instead of manually adding this after deserializing, though, could this update the Deserialize integration to add this in if it wasn't found previously?

I'm not sure what you mean - Deserialize is derived using serde and I don't see anything relevant in the deserialize attributes: https://serde.rs/field-attrs.html. default is probably the closest, but won't add the field if there's an entry that's not for crates-io.

[alexcrichton]

I believe deserialize_with would work for this purpose.

[jyn514]

So, this is what I have so far for deserialize_with:

diff --git a/src/cargo/core/compiler/rustdoc.rs b/src/cargo/core/compiler/rustdoc.rs
index 18c6898ee..1008652c2 100644
--- a/src/cargo/core/compiler/rustdoc.rs
+++ b/src/cargo/core/compiler/rustdoc.rs
@@ -55,10 +55,32 @@ impl<'de> serde::de::Deserialize<'de> for RustdocExternMode {
 #[derive(serde::Deserialize, Debug, Default)]
 #[serde(default)]
 pub struct RustdocExternMap {
+    #[serde(deserialize_with = "default_crates_io_to_docs_rs")]
     pub(crate) registries: HashMap<String, String>,
     std: Option<RustdocExternMode>,
 }
 
+fn default_crates_io_to_docs_rs<'de, D: serde::Deserializer<'de>>(de: D) -> Result<HashMap<String, String>, D::Error> {
+    struct DocsrsDeserializer;
+    impl<'de> serde::de::Visitor<'de> for DocsrsDeserializer {
+        type Value = HashMap<String, String>;
+
+        fn expecting(&self, _f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            Ok(())
+        }
+
+        fn visit_map<A: serde::de::MapAccess<'de>>(self, mut a: A) -> Result<Self::Value, A::Error> {
+            let mut map = HashMap::new();
+            while let Some((k, v)) = a.next_entry()? {
+                map.insert(k, v);
+            }
+            map.entry("crates-io".into()).or_insert("https://docs.rs/".into());
+            Ok(map)
+        }
+    }
+    de.deserialize_map(DocsrsDeserializer)
+}
+
 impl hash::Hash for RustdocExternMap {
     fn hash<H: hash::Hasher>(&self, into: &mut H) {
         self.std.hash(into);
diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs
index 24ac86e83..24a4e41c7 100644
--- a/src/cargo/util/config/mod.rs
+++ b/src/cargo/util/config/mod.rs
@@ -1218,13 +1218,7 @@ impl Config {
         // fundamentally does not have access to the registry name, so there is
         // nothing to query. Plumbing the name into SourceId is quite challenging.
         self.doc_extern_map.try_borrow_with(|| {
-            let mut extern_map = self.get::<RustdocExternMap>("doc.extern-map");
-            if let Ok(map) = &mut extern_map {
-                map.registries
-                    .entry("crates-io".into())
-                    .or_insert("https://docs.rs/".into());
-            }
-            extern_map
+            self.get::<RustdocExternMap>("doc.extern-map")
         })
     }
 

which fails with

---- rustdoc_extern_html::lib_name stdout ----
running `/home/joshua/.local/lib/cargo/target/debug/cargo doc -v --no-deps -Zrustdoc-map`
thread 'rustdoc_extern_html::lib_name' panicked at '
Expected: execs
    but: expected to find:
[RUNNING] `rustdoc [..]--crate-name foo [..]rumpelstiltskin=https://docs.rs/bar/1.0.0/[..]

did not find in output:
    Updating `/home/joshua/.local/lib/cargo/target/cit/t0/registry` index
 Downloading crates ...
  Downloaded bar v1.0.0 (registry `/home/joshua/.local/lib/cargo/target/cit/t0/registry`)
    Checking bar v1.0.0
     Running `rustc --crate-name rumpelstiltskin /home/joshua/.local/lib/cargo/target/cit/t0/home/.cargo/registry/src/-4e778f3a05112758/bar-1.0.0/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 -C metadata=769adf23c2ce482d -C extra-filename=-769adf23c2ce482d --out-dir /home/joshua/.local/lib/cargo/target/cit/t0/foo/target/debug/deps -L dependency=/home/joshua/.local/lib/cargo/target/cit/t0/foo/target/debug/deps --cap-lints allow`
 Documenting foo v0.1.0 (/home/joshua/.local/lib/cargo/target/cit/t0/foo)
     Running `rustdoc --crate-type lib --crate-name foo src/lib.rs -o /home/joshua/.local/lib/cargo/target/cit/t0/foo/target/doc --error-format=json --json=diagnostic-rendered-ansi -L dependency=/home/joshua/.local/lib/cargo/target/cit/t0/foo/target/debug/deps --extern rumpelstiltskin=/home/joshua/.local/lib/cargo/target/cit/t0/foo/target/debug/deps/librumpelstiltskin-769adf23c2ce482d.rmeta --crate-version 0.1.0`
    Finished dev [unoptimized + debuginfo] target(s) in 0.78s
', crates/cargo-test-support/src/lib.rs:729:13

and I'm starting to wonder whether this is worth it :/ It seems a lot more complicated than before.

What's the reason for going through Deserialize directly? Is the idea that it will be consistent even if you use self.get::<RustdocExternMap>() instead of self.doc_extern_map()? That will already be - not exactly wrong, but not really correct either, since it bypasses the caching.

[alexcrichton]

I don't think you need to deal with serde::de::Visitor, you should be able to do something like:

let map = HashMap::deserialize(d)?;
if !map.contains_key(...) {
    map.insert(...);
}

The reason I'm asking for the change is that it feels more idiomatic to fix this at the source rather than have it fixed after-the-fact once we've deserialized. That should ideally make future refactorings easier.

[jyn514]

Ok, I finally got this working with serde - the issue was that deserialize_with was being skipped when doc.extern-map wasn't present in the config, it went straight to the Default impl instead. This now overrides the Default to include docs.rs and adds docs.rs using deserialize_with if doc.extern-map was present.

[alexcrichton]

@bors: r+

👍

[bors]

📌 Commit 8a48c87 has been approved by alexcrichton

[bors]

⌛ Testing commit 8a48c87 with merge 6de98d1...

[bors]

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing 6de98d1 to master...

[jyn514]

Thanks for all the help getting this to work! You could probably tell this was my first time working with serde 😅