Skip to content

Commit

Permalink
refactor: merge name and alt_registry_key into one enum
Browse files Browse the repository at this point in the history
Both `name` and `alt_registry_key` are mainly used for displaying.
We can safely merge them into one enum.
However, `alt_registry_key` is also used for telling if a SourceId is
from `[registries]` so we need to provide functions to distinguish that.
  • Loading branch information
weihanglo committed Sep 21, 2023
1 parent a30d9fd commit d1ffef6
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 40 deletions.
90 changes: 57 additions & 33 deletions src/cargo/core/source_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,11 @@ struct SourceIdInner {
kind: SourceKind,
/// For example, the exact Git revision of the specified branch for a Git Source.
precise: Option<String>,
/// Name of the registry source for alternative registries
/// WARNING: this is not always set for alt-registries when the name is
/// not known.
name: Option<String>,
/// Name of the alt registry in the `[registries]` table.
/// WARNING: this is not always set for alt-registries when the name is
/// not known.
alt_registry_key: Option<String>,
/// Name of the remote registry.
///
/// WARNING: this is not always set when the name is not known,
/// e.g. registry coming from `--index` or Cargo.lock
registry_key: Option<KeyOf>,
}

/// The possible kinds of code source.
Expand Down Expand Up @@ -93,11 +90,23 @@ pub enum GitReference {
DefaultBranch,
}

/// The name of the registry to display.
///
/// The purpose of this is to distinguish between different sources of registry
/// keys to provide better diagnostics.
#[derive(Debug, Clone, PartialEq, Eq)]
enum KeyOf {
/// Registry defined in the `[registries]` stable or the built-in `crates-io` key.
AlternativeRegistry(String),
/// Registry defined in the `[source]` replacement table.
SourceReplacement(String),
}

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, name: Option<&str>) -> CargoResult<SourceId> {
fn new(kind: SourceKind, url: Url, key: Option<KeyOf>) -> CargoResult<SourceId> {
if kind == SourceKind::SparseRegistry {
// Sparse URLs are different because they store the kind prefix (sparse+)
// in the URL. This is because the prefix is necessary to differentiate
Expand All @@ -111,8 +120,7 @@ impl SourceId {
canonical_url: CanonicalUrl::new(&url)?,
url,
precise: None,
name: name.map(|n| n.into()),
alt_registry_key: None,
registry_key: key,
});
Ok(source_id)
}
Expand Down Expand Up @@ -230,10 +238,18 @@ impl SourceId {
SourceId::new(kind, url.to_owned(), None)
}

/// Creates a `SourceId` from a remote registry URL with given name.
pub fn for_alt_registry(url: &Url, name: &str) -> CargoResult<SourceId> {
/// Creates a `SourceId` for a remote registry from the `[registries]` table or crates.io.
pub fn for_alt_registry(url: &Url, key: &str) -> CargoResult<SourceId> {
let kind = Self::remote_source_kind(url);
SourceId::new(kind, url.to_owned(), Some(name))
let key = KeyOf::AlternativeRegistry(key.into());
SourceId::new(kind, url.to_owned(), Some(key))
}

/// Creates a `SourceId` for a remote registry from the `[source]` replacement table.
pub fn for_source_replacement_registry(url: &Url, key: &str) -> CargoResult<SourceId> {
let kind = Self::remote_source_kind(url);
let key = KeyOf::SourceReplacement(key.into());
SourceId::new(kind, url.to_owned(), Some(key))
}

/// Creates a `SourceId` from a local registry path.
Expand Down Expand Up @@ -262,7 +278,8 @@ impl SourceId {
if Self::crates_io_is_sparse(config)? {
config.check_registry_index_not_set()?;
let url = CRATES_IO_HTTP_INDEX.into_url().unwrap();
SourceId::new(SourceKind::SparseRegistry, url, Some(CRATES_IO_REGISTRY))
let key = KeyOf::AlternativeRegistry(CRATES_IO_REGISTRY.into());
SourceId::new(SourceKind::SparseRegistry, url, Some(key))
} else {
Self::crates_io(config)
}
Expand All @@ -289,15 +306,7 @@ impl SourceId {
return Self::crates_io(config);
}
let url = config.get_registry_index(key)?;
let kind = Self::remote_source_kind(&url);
Ok(SourceId::wrap(SourceIdInner {
kind,
canonical_url: CanonicalUrl::new(&url)?,
url,
precise: None,
name: Some(key.to_string()),
alt_registry_key: Some(key.to_string()),
}))
Self::for_alt_registry(&url, key)
}

/// Gets this source URL.
Expand All @@ -324,8 +333,8 @@ impl SourceId {
pub fn display_registry_name(self) -> String {
if self.is_crates_io() {
CRATES_IO_REGISTRY.to_string()
} else if let Some(name) = &self.inner.name {
name.clone()
} else if let Some(key) = self.inner.registry_key.as_ref().map(|k| k.key()) {
key.into()
} else if self.precise().is_some() {
// We remove `precise` here to retrieve an permissive version of
// `SourceIdInner`, which may contain the registry name.
Expand All @@ -335,11 +344,10 @@ impl SourceId {
}
}

/// Gets the name of the remote registry as defined in the `[registries]` table.
/// WARNING: alt registries that come from Cargo.lock, or --index will
/// not have a name.
/// Gets the name of the remote registry as defined in the `[registries]` table,
/// or the built-in `crates-io` key.
pub fn alt_registry_key(&self) -> Option<&str> {
self.inner.alt_registry_key.as_deref()
self.inner.registry_key.as_ref()?.alternative_registry()
}

/// Returns `true` if this source is from a filesystem path.
Expand Down Expand Up @@ -652,10 +660,9 @@ impl Hash for SourceId {
/// The hash of `SourceIdInner` is used to retrieve its interned value from
/// `SOURCE_ID_CACHE`. We only care about fields that make `SourceIdInner`
/// unique. Optional fields not affecting the uniqueness must be excluded,
/// such as [`name`] and [`alt_registry_key`]. That's why this is not derived.
/// such as [`registry_key`]. That's why this is not derived.
///
/// [`name`]: SourceIdInner::name
/// [`alt_registry_key`]: SourceIdInner::alt_registry_key
/// [`registry_key`]: SourceIdInner::registry_key
impl Hash for SourceIdInner {
fn hash<S: hash::Hasher>(&self, into: &mut S) {
self.kind.hash(into);
Expand Down Expand Up @@ -868,6 +875,23 @@ impl<'a> fmt::Display for PrettyRef<'a> {
}
}

impl KeyOf {
/// Gets the underlying key.
fn key(&self) -> &str {
match self {
KeyOf::AlternativeRegistry(k) | KeyOf::SourceReplacement(k) => k,
}
}

/// Gets the key if it's from an alternative registry.
fn alternative_registry(&self) -> Option<&str> {
match self {
KeyOf::AlternativeRegistry(k) => Some(k),
_ => None,
}
}
}

#[cfg(test)]
mod tests {
use super::{GitReference, SourceId, SourceKind};
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,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_alt_registry(&url, &name)?);
srcs.push(SourceId::for_source_replacement_registry(&url, &name)?);
}
if let Some(local_registry) = def.local_registry {
let path = local_registry.resolve_path(self.config);
Expand Down
7 changes: 1 addition & 6 deletions src/cargo/util/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
use crate::{
core::features::cargo_docs_link,
sources::CRATES_IO_REGISTRY,
util::{config::ConfigKey, CanonicalUrl, CargoResult, Config, IntoUrl},
};
use anyhow::{bail, Context as _};
Expand Down Expand Up @@ -506,11 +505,7 @@ fn credential_action(
args: &[&str],
require_cred_provider_config: bool,
) -> CargoResult<CredentialResponse> {
let name = if sid.is_crates_io() {
Some(CRATES_IO_REGISTRY)
} else {
sid.alt_registry_key()
};
let name = sid.alt_registry_key();
let registry = RegistryInfo {
index_url: sid.url().as_str(),
name,
Expand Down

0 comments on commit d1ffef6

Please sign in to comment.