Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(SourceId): merge name and alt_registry_key into one enum #12675

Merged
merged 3 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 0 additions & 37 deletions src/cargo/core/package_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,43 +251,6 @@ mod tests {
assert!(PackageId::new("foo", "", repo).is_err());
}

#[test]
arlosi marked this conversation as resolved.
Show resolved Hide resolved
fn debug() {
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 `crates-io`" }"#,
format!("{:?}", pkg_id)
);

let expected = r#"
PackageId {
name: "foo",
version: "1.0.0",
source: "registry `crates-io`",
}
"#
.trim();

// Can be removed once trailing commas in Debug have reached the stable
// channel.
let expected_without_trailing_comma = r#"
PackageId {
name: "foo",
version: "1.0.0",
source: "registry `crates-io`"
}
"#
.trim();

let actual = format!("{:#?}", pkg_id);
if actual.ends_with(",\n}") {
assert_eq!(actual, expected);
} else {
assert_eq!(actual, expected_without_trailing_comma);
}
}

#[test]
fn display() {
let loc = CRATES_IO_INDEX.into_url().unwrap();
Expand Down
91 changes: 56 additions & 35 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,22 @@ pub enum GitReference {
DefaultBranch,
}

/// Where the remote source key is defined.
///
/// The purpose of this is to provide better diagnostics for different sources of keys.
#[derive(Debug, Clone, PartialEq, Eq)]
enum KeyOf {
/// Defined in the `[registries]` table or the built-in `crates-io` key.
Registry(String),
/// Defined in the `[source]` replacement table.
Source(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 +119,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 +237,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::Registry(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::Source(key.into());
SourceId::new(kind, url.to_owned(), Some(key))
}

/// Creates a `SourceId` from a local registry path.
Expand Down Expand Up @@ -262,7 +277,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::Registry(CRATES_IO_REGISTRY.into());
SourceId::new(SourceKind::SparseRegistry, url, Some(key))
} else {
Self::crates_io(config)
}
Expand All @@ -289,15 +305,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 @@ -322,10 +330,8 @@ impl SourceId {

/// Displays the name of a registry if it has one. Otherwise just the URL.
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()
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 +341,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 +657,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 +872,23 @@ impl<'a> fmt::Display for PrettyRef<'a> {
}
}

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

/// Gets the key if it's from an alternative registry.
fn alternative_registry(&self) -> Option<&str> {
match self {
KeyOf::Registry(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
45 changes: 45 additions & 0 deletions tests/testsuite/source_replacement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,3 +248,48 @@ fn undefined_default() {
)
.run();
}

#[cargo_test]
fn source_replacement_with_registry_url() {
let alternative = RegistryBuilder::new().alternative().http_api().build();
Package::new("bar", "0.0.1").alternative(true).publish();

let crates_io = setup_replacement(&format!(
r#"
[source.crates-io]
replace-with = 'using-registry-url'

[source.using-registry-url]
registry = '{}'
"#,
alternative.index_url()
));

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
[dependencies.bar]
version = "0.0.1"
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check")
.replace_crates_io(crates_io.index_url())
.with_stderr(
"\
[UPDATING] `using-registry-url` index
[DOWNLOADING] crates ...
[DOWNLOADED] bar v0.0.1 (registry `using-registry-url`)
[CHECKING] bar v0.0.1
[CHECKING] foo v0.0.1 ([CWD])
[FINISHED] dev [..]
",
)
.run();
}