Skip to content

Commit 17fdd58

Browse files
committed
refactor: merge name and alt_registry_key into one enum
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.
1 parent a30d9fd commit 17fdd58

File tree

3 files changed

+58
-40
lines changed

3 files changed

+58
-40
lines changed

src/cargo/core/source_id.rs

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,11 @@ struct SourceIdInner {
5151
kind: SourceKind,
5252
/// For example, the exact Git revision of the specified branch for a Git Source.
5353
precise: Option<String>,
54-
/// Name of the registry source for alternative registries
55-
/// WARNING: this is not always set for alt-registries when the name is
56-
/// not known.
57-
name: Option<String>,
58-
/// Name of the alt registry in the `[registries]` table.
59-
/// WARNING: this is not always set for alt-registries when the name is
60-
/// not known.
61-
alt_registry_key: Option<String>,
54+
/// Name of the remote registry.
55+
///
56+
/// WARNING: this is not always set when the name is not known,
57+
/// e.g. registry coming from `--index` or Cargo.lock
58+
registry_key: Option<KeyOf>,
6259
}
6360

6461
/// The possible kinds of code source.
@@ -93,11 +90,22 @@ pub enum GitReference {
9390
DefaultBranch,
9491
}
9592

93+
/// Where the remote source key is defined.
94+
///
95+
/// The purpose of this is to provide better diagnostics for different sources of keys.
96+
#[derive(Debug, Clone, PartialEq, Eq)]
97+
enum KeyOf {
98+
/// Defined in the `[registries]` table or the built-in `crates-io` key.
99+
Registry(String),
100+
/// Defined in the `[source]` replacement table.
101+
Source(String),
102+
}
103+
96104
impl SourceId {
97105
/// Creates a `SourceId` object from the kind and URL.
98106
///
99107
/// The canonical url will be calculated, but the precise field will not
100-
fn new(kind: SourceKind, url: Url, name: Option<&str>) -> CargoResult<SourceId> {
108+
fn new(kind: SourceKind, url: Url, key: Option<KeyOf>) -> CargoResult<SourceId> {
101109
if kind == SourceKind::SparseRegistry {
102110
// Sparse URLs are different because they store the kind prefix (sparse+)
103111
// in the URL. This is because the prefix is necessary to differentiate
@@ -111,8 +119,7 @@ impl SourceId {
111119
canonical_url: CanonicalUrl::new(&url)?,
112120
url,
113121
precise: None,
114-
name: name.map(|n| n.into()),
115-
alt_registry_key: None,
122+
registry_key: key,
116123
});
117124
Ok(source_id)
118125
}
@@ -230,10 +237,18 @@ impl SourceId {
230237
SourceId::new(kind, url.to_owned(), None)
231238
}
232239

233-
/// Creates a `SourceId` from a remote registry URL with given name.
234-
pub fn for_alt_registry(url: &Url, name: &str) -> CargoResult<SourceId> {
240+
/// Creates a `SourceId` for a remote registry from the `[registries]` table or crates.io.
241+
pub fn for_alt_registry(url: &Url, key: &str) -> CargoResult<SourceId> {
235242
let kind = Self::remote_source_kind(url);
236-
SourceId::new(kind, url.to_owned(), Some(name))
243+
let key = KeyOf::Registry(key.into());
244+
SourceId::new(kind, url.to_owned(), Some(key))
245+
}
246+
247+
/// Creates a `SourceId` for a remote registry from the `[source]` replacement table.
248+
pub fn for_source_replacement_registry(url: &Url, key: &str) -> CargoResult<SourceId> {
249+
let kind = Self::remote_source_kind(url);
250+
let key = KeyOf::Source(key.into());
251+
SourceId::new(kind, url.to_owned(), Some(key))
237252
}
238253

239254
/// Creates a `SourceId` from a local registry path.
@@ -262,7 +277,8 @@ impl SourceId {
262277
if Self::crates_io_is_sparse(config)? {
263278
config.check_registry_index_not_set()?;
264279
let url = CRATES_IO_HTTP_INDEX.into_url().unwrap();
265-
SourceId::new(SourceKind::SparseRegistry, url, Some(CRATES_IO_REGISTRY))
280+
let key = KeyOf::Registry(CRATES_IO_REGISTRY.into());
281+
SourceId::new(SourceKind::SparseRegistry, url, Some(key))
266282
} else {
267283
Self::crates_io(config)
268284
}
@@ -289,15 +305,7 @@ impl SourceId {
289305
return Self::crates_io(config);
290306
}
291307
let url = config.get_registry_index(key)?;
292-
let kind = Self::remote_source_kind(&url);
293-
Ok(SourceId::wrap(SourceIdInner {
294-
kind,
295-
canonical_url: CanonicalUrl::new(&url)?,
296-
url,
297-
precise: None,
298-
name: Some(key.to_string()),
299-
alt_registry_key: Some(key.to_string()),
300-
}))
308+
Self::for_alt_registry(&url, key)
301309
}
302310

303311
/// Gets this source URL.
@@ -324,8 +332,8 @@ impl SourceId {
324332
pub fn display_registry_name(self) -> String {
325333
if self.is_crates_io() {
326334
CRATES_IO_REGISTRY.to_string()
327-
} else if let Some(name) = &self.inner.name {
328-
name.clone()
335+
} else if let Some(key) = self.inner.registry_key.as_ref().map(|k| k.key()) {
336+
key.into()
329337
} else if self.precise().is_some() {
330338
// We remove `precise` here to retrieve an permissive version of
331339
// `SourceIdInner`, which may contain the registry name.
@@ -335,11 +343,10 @@ impl SourceId {
335343
}
336344
}
337345

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

345352
/// Returns `true` if this source is from a filesystem path.
@@ -652,10 +659,9 @@ impl Hash for SourceId {
652659
/// The hash of `SourceIdInner` is used to retrieve its interned value from
653660
/// `SOURCE_ID_CACHE`. We only care about fields that make `SourceIdInner`
654661
/// unique. Optional fields not affecting the uniqueness must be excluded,
655-
/// such as [`name`] and [`alt_registry_key`]. That's why this is not derived.
662+
/// such as [`registry_key`]. That's why this is not derived.
656663
///
657-
/// [`name`]: SourceIdInner::name
658-
/// [`alt_registry_key`]: SourceIdInner::alt_registry_key
664+
/// [`registry_key`]: SourceIdInner::registry_key
659665
impl Hash for SourceIdInner {
660666
fn hash<S: hash::Hasher>(&self, into: &mut S) {
661667
self.kind.hash(into);
@@ -868,6 +874,23 @@ impl<'a> fmt::Display for PrettyRef<'a> {
868874
}
869875
}
870876

877+
impl KeyOf {
878+
/// Gets the underlying key.
879+
fn key(&self) -> &str {
880+
match self {
881+
KeyOf::Registry(k) | KeyOf::Source(k) => k,
882+
}
883+
}
884+
885+
/// Gets the key if it's from an alternative registry.
886+
fn alternative_registry(&self) -> Option<&str> {
887+
match self {
888+
KeyOf::Registry(k) => Some(k),
889+
_ => None,
890+
}
891+
}
892+
}
893+
871894
#[cfg(test)]
872895
mod tests {
873896
use super::{GitReference, SourceId, SourceKind};

src/cargo/sources/config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ restore the source replacement configuration to continue the build
239239
let mut srcs = Vec::new();
240240
if let Some(registry) = def.registry {
241241
let url = url(&registry, &format!("source.{}.registry", name))?;
242-
srcs.push(SourceId::for_alt_registry(&url, &name)?);
242+
srcs.push(SourceId::for_source_replacement_registry(&url, &name)?);
243243
}
244244
if let Some(local_registry) = def.local_registry {
245245
let path = local_registry.resolve_path(self.config);

src/cargo/util/auth/mod.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
33
use crate::{
44
core::features::cargo_docs_link,
5-
sources::CRATES_IO_REGISTRY,
65
util::{config::ConfigKey, CanonicalUrl, CargoResult, Config, IntoUrl},
76
};
87
use anyhow::{bail, Context as _};
@@ -506,11 +505,7 @@ fn credential_action(
506505
args: &[&str],
507506
require_cred_provider_config: bool,
508507
) -> CargoResult<CredentialResponse> {
509-
let name = if sid.is_crates_io() {
510-
Some(CRATES_IO_REGISTRY)
511-
} else {
512-
sid.alt_registry_key()
513-
};
508+
let name = sid.alt_registry_key();
514509
let registry = RegistryInfo {
515510
index_url: sid.url().as_str(),
516511
name,

0 commit comments

Comments
 (0)