Auto merge of #9632 - weihanglo:issue-6691, r=ehuss

Display registry name instead of registry URL when possible

Fixes #6691

This PR can be divided into several parts:

- c5be3de: Try to display registry names instead of URLs for `impl Dipslay for SourceId`. This benefits almost all kinds of messages using`SourceId` directly or indrectly.
- 9394d48: This fixes `Updating <name> index` part of `[source]` replacement, which previously didn't preserve the registry name information.
- 4c2f9b5: This makes its best effort to show registry names for deps from `Cargo.lock`. Since current lockfile format does not serialize any registry name. We here try the best effort to restore registry name from either `[registries]` table or `[source]` replacement table. This is done by manually implementing `Hash` and `PartialEq` for `SourceIdInner`, of which two traits previously are simply derived.
    To make `SourceIdInner` generate the same hash no matter it contains `name` field or not, here we remove `name` field from hashing and only concern about `kind`, `precise` and `canonical_url`.

Feel free to ask me for adding more tests, though I am not sure what tests should be added 😅

Author: bors

src/cargo/core/compiler/rustdoc.rs

@@ -11,6 +11,8 @@ use std::fmt;
 use std::hash;
 use url::Url;
 
+const DOCS_RS_URL: &'static str = "https://docs.rs/";
+
 /// Mode used for `std`.
 #[derive(Debug, Hash)]
 pub enum RustdocExternMode {
@@ -63,7 +65,7 @@ pub struct RustdocExternMap {
 impl Default for RustdocExternMap {
     fn default() -> Self {
         let mut registries = HashMap::new();
-        registries.insert("crates-io".into(), "https://docs.rs/".into());
+        registries.insert(CRATES_IO_REGISTRY.into(), DOCS_RS_URL.into());
         Self {
             registries,
             std: None,
@@ -76,8 +78,8 @@ fn default_crates_io_to_docs_rs<'de, D: serde::Deserializer<'de>>(
 ) -> Result<HashMap<String, String>, D::Error> {
     use serde::Deserialize;
     let mut registries = HashMap::deserialize(de)?;
-    if !registries.contains_key("crates-io") {
-        registries.insert("crates-io".into(), "https://docs.rs/".into());
+    if !registries.contains_key(CRATES_IO_REGISTRY) {
+        registries.insert(CRATES_IO_REGISTRY.into(), DOCS_RS_URL.into());
     }
     Ok(registries)
 }

src/cargo/core/package_id.rs

@@ -252,15 +252,15 @@ mod tests {
         let loc = CRATES_IO_INDEX.into_url().unwrap();
         let pkg_id = PackageId::new("foo", "1.0.0", SourceId::for_registry(&loc).unwrap()).unwrap();
         assert_eq!(
-            r#"PackageId { name: "foo", version: "1.0.0", source: "registry `https://github.com/rust-lang/crates.io-index`" }"#,
+            r#"PackageId { name: "foo", version: "1.0.0", source: "registry `crates-io`" }"#,
             format!("{:?}", pkg_id)
         );
 
         let expected = r#"
 PackageId {
     name: "foo",
     version: "1.0.0",
-    source: "registry `https://github.com/rust-lang/crates.io-index`",
+    source: "registry `crates-io`",
 }
 "#
         .trim();
@@ -271,7 +271,7 @@ PackageId {
 PackageId {
     name: "foo",
     version: "1.0.0",
-    source: "registry `https://github.com/rust-lang/crates.io-index`"
+    source: "registry `crates-io`"
 }
 "#
         .trim();

src/cargo/core/source/source_id.rs

@@ -1,6 +1,6 @@
 use crate::core::PackageId;
-use crate::sources::DirectorySource;
-use crate::sources::{GitSource, PathSource, RegistrySource, CRATES_IO_INDEX};
+use crate::sources::{DirectorySource, CRATES_IO_DOMAIN, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
+use crate::sources::{GitSource, PathSource, RegistrySource};
 use crate::util::{CanonicalUrl, CargoResult, Config, IntoUrl};
 use log::trace;
 use serde::de;
@@ -24,7 +24,7 @@ pub struct SourceId {
     inner: &'static SourceIdInner,
 }
 
-#[derive(PartialEq, Eq, Clone, Debug, Hash)]
+#[derive(Eq, Clone, Debug)]
 struct SourceIdInner {
     /// The source URL.
     url: Url,
@@ -73,13 +73,13 @@ impl SourceId {
     /// Creates a `SourceId` object from the kind and URL.
     ///
     /// The canonical url will be calculated, but the precise field will not
-    fn new(kind: SourceKind, url: Url) -> CargoResult<SourceId> {
+    fn new(kind: SourceKind, url: Url, name: Option<&str>) -> CargoResult<SourceId> {
         let source_id = SourceId::wrap(SourceIdInner {
             kind,
             canonical_url: CanonicalUrl::new(&url)?,
             url,
             precise: None,
-            name: None,
+            name: name.map(|n| n.into()),
         });
         Ok(source_id)
     }
@@ -132,12 +132,12 @@ impl SourceId {
             }
             "registry" => {
                 let url = url.into_url()?;
-                Ok(SourceId::new(SourceKind::Registry, url)?
+                Ok(SourceId::new(SourceKind::Registry, url, None)?
                     .with_precise(Some("locked".to_string())))
             }
             "path" => {
                 let url = url.into_url()?;
-                SourceId::new(SourceKind::Path, url)
+                SourceId::new(SourceKind::Path, url, None)
             }
             kind => Err(anyhow::format_err!("unsupported source protocol: {}", kind)),
         }
@@ -155,43 +155,53 @@ impl SourceId {
     /// `path`: an absolute path.
     pub fn for_path(path: &Path) -> CargoResult<SourceId> {
         let url = path.into_url()?;
-        SourceId::new(SourceKind::Path, url)
+        SourceId::new(SourceKind::Path, url, None)
     }
 
     /// Creates a `SourceId` from a Git reference.
     pub fn for_git(url: &Url, reference: GitReference) -> CargoResult<SourceId> {
-        SourceId::new(SourceKind::Git(reference), url.clone())
+        SourceId::new(SourceKind::Git(reference), url.clone(), None)
     }
 
-    /// Creates a SourceId from a registry URL.
+    /// Creates a SourceId from a remote registry URL when the registry name
+    /// cannot be determined, e.g. an user passes `--index` directly from CLI.
+    ///
+    /// Use [`SourceId::for_alt_registry`] if a name can provided, which
+    /// generates better messages for cargo.
     pub fn for_registry(url: &Url) -> CargoResult<SourceId> {
-        SourceId::new(SourceKind::Registry, url.clone())
+        SourceId::new(SourceKind::Registry, url.clone(), None)
+    }
+
+    /// Creates a `SourceId` from a remote registry URL with given name.
+    pub fn for_alt_registry(url: &Url, name: &str) -> CargoResult<SourceId> {
+        SourceId::new(SourceKind::Registry, url.clone(), Some(name))
     }
 
     /// Creates a SourceId from a local registry path.
     pub fn for_local_registry(path: &Path) -> CargoResult<SourceId> {
         let url = path.into_url()?;
-        SourceId::new(SourceKind::LocalRegistry, url)
+        SourceId::new(SourceKind::LocalRegistry, url, None)
     }
 
     /// Creates a `SourceId` from a directory path.
     pub fn for_directory(path: &Path) -> CargoResult<SourceId> {
         let url = path.into_url()?;
-        SourceId::new(SourceKind::Directory, url)
+        SourceId::new(SourceKind::Directory, url, None)
     }
 
     /// Returns the `SourceId` corresponding to the main repository.
     ///
     /// This is the main cargo registry by default, but it can be overridden in
-    /// a `.cargo/config`.
+    /// a `.cargo/config.toml`.
     pub fn crates_io(config: &Config) -> CargoResult<SourceId> {
         config.crates_io_source_id(|| {
             config.check_registry_index_not_set()?;
             let url = CRATES_IO_INDEX.into_url().unwrap();
-            SourceId::for_registry(&url)
+            SourceId::new(SourceKind::Registry, url, Some(CRATES_IO_REGISTRY))
         })
     }
 
+    /// Gets the `SourceId` associated with given name of the remote regsitry.
     pub fn alt_registry(config: &Config, key: &str) -> CargoResult<SourceId> {
         let url = config.get_registry_index(key)?;
         Ok(SourceId::wrap(SourceIdInner {
@@ -216,17 +226,21 @@ impl SourceId {
 
     pub fn display_index(self) -> String {
         if self.is_default_registry() {
-            "crates.io index".to_string()
+            format!("{} index", CRATES_IO_DOMAIN)
         } else {
-            format!("`{}` index", url_display(self.url()))
+            format!("`{}` index", self.display_registry_name())
         }
     }
 
     pub fn display_registry_name(self) -> String {
         if self.is_default_registry() {
-            "crates.io".to_string()
+            CRATES_IO_REGISTRY.to_string()
         } else if let Some(name) = &self.inner.name {
             name.clone()
+        } else if self.precise().is_some() {
+            // We remove `precise` here to retrieve an permissive version of
+            // `SourceIdInner`, which may contain the registry name.
+            self.with_precise(None).display_registry_name()
         } else {
             url_display(self.url())
         }
@@ -463,7 +477,7 @@ impl fmt::Display for SourceId {
                 Ok(())
             }
             SourceKind::Path => write!(f, "{}", url_display(&self.inner.url)),
-            SourceKind::Registry => write!(f, "registry `{}`", url_display(&self.inner.url)),
+            SourceKind::Registry => write!(f, "registry `{}`", self.display_registry_name()),
             SourceKind::LocalRegistry => write!(f, "registry `{}`", url_display(&self.inner.url)),
             SourceKind::Directory => write!(f, "dir {}", url_display(&self.inner.url)),
         }
@@ -483,6 +497,29 @@ impl Hash for SourceId {
     }
 }
 
+impl Hash for SourceIdInner {
+    /// The hash of `SourceIdInner` is used to retrieve its interned value. We
+    /// only care about fields that make `SourceIdInner` unique, which are:
+    ///
+    /// - `kind`
+    /// - `precise`
+    /// - `canonical_url`
+    fn hash<S: hash::Hasher>(&self, into: &mut S) {
+        self.kind.hash(into);
+        self.precise.hash(into);
+        self.canonical_url.hash(into);
+    }
+}
+
+impl PartialEq for SourceIdInner {
+    /// This implementation must be synced with [`SourceIdInner::hash`].
+    fn eq(&self, other: &Self) -> bool {
+        self.kind == other.kind
+            && self.precise == other.precise
+            && self.canonical_url == other.canonical_url
+    }
+}
+
 // forward to `Ord`
 impl PartialOrd for SourceKind {
     fn partial_cmp(&self, other: &SourceKind) -> Option<Ordering> {
@@ -670,15 +707,15 @@ mod tests {
     fn github_sources_equal() {
         let loc = "https://github.com/foo/bar".into_url().unwrap();
         let default = SourceKind::Git(GitReference::DefaultBranch);
-        let s1 = SourceId::new(default.clone(), loc).unwrap();
+        let s1 = SourceId::new(default.clone(), loc, None).unwrap();
 
         let loc = "git://github.com/foo/bar".into_url().unwrap();
-        let s2 = SourceId::new(default, loc.clone()).unwrap();
+        let s2 = SourceId::new(default, loc.clone(), None).unwrap();
 
         assert_eq!(s1, s2);
 
         let foo = SourceKind::Git(GitReference::Branch("foo".to_string()));
-        let s3 = SourceId::new(foo, loc).unwrap();
+        let s3 = SourceId::new(foo, loc, None).unwrap();
         assert_ne!(s1, s3);
     }
 }

src/cargo/ops/mod.rs

@@ -1,3 +1,5 @@
+use crate::sources::CRATES_IO_DOMAIN;
+
 pub use self::cargo_clean::{clean, CleanOptions};
 pub use self::cargo_compile::{
     compile, compile_with_exec, compile_ws, create_bcx, print, resolve_all_features, CompileOptions,
@@ -66,7 +68,7 @@ fn check_dep_has_version(dep: &crate::core::Dependency, publish: bool) -> crate:
 
     if !dep.specified_req() && dep.is_transitive() {
         let dep_version_source = dep.registry_id().map_or_else(
-            || "crates.io".to_string(),
+            || CRATES_IO_DOMAIN.to_string(),
             |registry_id| registry_id.display_registry_name(),
         );
         anyhow::bail!(

src/cargo/ops/registry.rs

@@ -20,7 +20,7 @@ use crate::core::resolver::CliFeatures;
 use crate::core::source::Source;
 use crate::core::{Package, SourceId, Workspace};
 use crate::ops;
-use crate::sources::{RegistrySource, SourceConfigMap, CRATES_IO_REGISTRY};
+use crate::sources::{RegistrySource, SourceConfigMap, CRATES_IO_DOMAIN, CRATES_IO_REGISTRY};
 use crate::util::config::{self, Config, SslVersionConfig, SslVersionConfigRange};
 use crate::util::errors::CargoResult;
 use crate::util::important_paths::find_root_manifest_for_wd;
@@ -730,15 +730,15 @@ pub fn registry_login(
         "Login",
         format!(
             "token for `{}` saved",
-            reg.as_ref().map_or("crates.io", String::as_str)
+            reg.as_ref().map_or(CRATES_IO_DOMAIN, String::as_str)
         ),
     )?;
     Ok(())
 }
 
 pub fn registry_logout(config: &Config, reg: Option<String>) -> CargoResult<()> {
     let (registry, reg_cfg, _) = registry(config, None, None, reg.clone(), false, false)?;
-    let reg_name = reg.as_deref().unwrap_or("crates.io");
+    let reg_name = reg.as_deref().unwrap_or(CRATES_IO_DOMAIN);
     if reg_cfg.credential_process.is_none() && reg_cfg.token.is_none() {
         config.shell().status(
             "Logout",

src/cargo/ops/vendor.rs

@@ -2,6 +2,7 @@ use crate::core::shell::Verbosity;
 use crate::core::{GitReference, Workspace};
 use crate::ops;
 use crate::sources::path::PathSource;
+use crate::sources::CRATES_IO_REGISTRY;
 use crate::util::{CargoResult, Config};
 use anyhow::{bail, Context as _};
 use cargo_util::{paths, Sha256};
@@ -248,7 +249,7 @@ fn sync(
     // replace original sources with vendor
     for source_id in sources {
         let name = if source_id.is_default_registry() {
-            "crates-io".to_string()
+            CRATES_IO_REGISTRY.to_string()
         } else {
             source_id.url().to_string()
         };

src/cargo/sources/config.rs

@@ -207,7 +207,7 @@ restore the source replacement configuration to continue the build
         let mut srcs = Vec::new();
         if let Some(registry) = def.registry {
             let url = url(&registry, &format!("source.{}.registry", name))?;
-            srcs.push(SourceId::for_registry(&url)?);
+            srcs.push(SourceId::for_alt_registry(&url, &name)?);
         }
         if let Some(local_registry) = def.local_registry {
             let path = local_registry.resolve_path(self.config);
@@ -247,7 +247,7 @@ restore the source replacement configuration to continue the build
             check_not_set("tag", def.tag)?;
             check_not_set("rev", def.rev)?;
         }
-        if name == "crates-io" && srcs.is_empty() {
+        if name == CRATES_IO_REGISTRY && srcs.is_empty() {
             srcs.push(SourceId::crates_io(self.config)?);
         }
 

src/cargo/sources/mod.rs

@@ -2,7 +2,7 @@ pub use self::config::SourceConfigMap;
 pub use self::directory::DirectorySource;
 pub use self::git::GitSource;
 pub use self::path::PathSource;
-pub use self::registry::{RegistrySource, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
+pub use self::registry::{RegistrySource, CRATES_IO_DOMAIN, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
 pub use self::replaced::ReplacedSource;
 
 pub mod config;

src/cargo/sources/registry/mod.rs

@@ -184,6 +184,7 @@ use crate::util::{restricted_names, CargoResult, Config, Filesystem, OptVersionR
 const PACKAGE_SOURCE_LOCK: &str = ".cargo-ok";
 pub const CRATES_IO_INDEX: &str = "https://github.com/rust-lang/crates.io-index";
 pub const CRATES_IO_REGISTRY: &str = "crates-io";
+pub const CRATES_IO_DOMAIN: &str = "crates.io";
 const CRATE_TEMPLATE: &str = "{crate}";
 const VERSION_TEMPLATE: &str = "{version}";
 const PREFIX_TEMPLATE: &str = "{prefix}";