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

Implement RFC 3289: source replacement ambiguity #10907

Merged
merged 1 commit into from
Oct 8, 2022
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
11 changes: 11 additions & 0 deletions crates/cargo-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,17 @@ impl Execs {
self
}

/// Overrides the crates.io URL for testing.
///
/// Can be used for testing crates-io functionality where alt registries
/// cannot be used.
pub fn replace_crates_io(&mut self, url: &Url) -> &mut Self {
if let Some(ref mut p) = self.process_builder {
p.env("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS", url.as_str());
}
self
}

pub fn enable_mac_dsym(&mut self) -> &mut Self {
if cfg!(target_os = "macos") {
self.env("CARGO_PROFILE_DEV_SPLIT_DEBUGINFO", "packed")
Expand Down
11 changes: 6 additions & 5 deletions crates/cargo-test-support/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl RegistryBuilder {
pub fn new() -> RegistryBuilder {
RegistryBuilder {
alternative: None,
token: Some("api-token".to_string()),
token: None,
http_api: false,
http_index: false,
api: true,
Expand Down Expand Up @@ -197,6 +197,7 @@ impl RegistryBuilder {
let dl_url = generate_url(&format!("{prefix}dl"));
let dl_path = generate_path(&format!("{prefix}dl"));
let api_path = generate_path(&format!("{prefix}api"));
let token = Some(self.token.unwrap_or_else(|| format!("{prefix}sekrit")));

let (server, index_url, api_url, dl_url) = if !self.http_index && !self.http_api {
// No need to start the HTTP server.
Expand All @@ -205,7 +206,7 @@ impl RegistryBuilder {
let server = HttpServer::new(
registry_path.clone(),
dl_path,
self.token.clone(),
token.clone(),
self.custom_responders,
);
let index_url = if self.http_index {
Expand All @@ -228,7 +229,7 @@ impl RegistryBuilder {
_server: server,
dl_url,
path: registry_path,
token: self.token,
token,
};

if self.configure_registry {
Expand All @@ -252,8 +253,8 @@ impl RegistryBuilder {
[source.crates-io]
replace-with = 'dummy-registry'

[source.dummy-registry]
registry = '{}'",
[registries.dummy-registry]
index = '{}'",
registry.index_url
)
.as_bytes(),
Expand Down
6 changes: 5 additions & 1 deletion src/bin/cargo/commands/add.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use cargo::sources::CRATES_IO_REGISTRY;
use indexmap::IndexMap;
use indexmap::IndexSet;

Expand Down Expand Up @@ -207,7 +208,10 @@ fn parse_dependencies(config: &Config, matches: &ArgMatches) -> CargoResult<Vec<
let rev = matches.get_one::<String>("rev");
let tag = matches.get_one::<String>("tag");
let rename = matches.get_one::<String>("rename");
let registry = matches.registry(config)?;
let registry = match matches.registry(config)? {
Some(reg) if reg == CRATES_IO_REGISTRY => None,
reg => reg,
};
let default_features = default_features(matches);
let optional = optional(matches);

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub fn add_root_urls(
if !sid.is_registry() {
return false;
}
if sid.is_default_registry() {
if sid.is_crates_io() {
return registry == CRATES_IO_REGISTRY;
}
if let Some(index_url) = name2url.get(registry) {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/package_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl fmt::Display for PackageId {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
write!(f, "{} v{}", self.inner.name, self.inner.version)?;

if !self.inner.source_id.is_default_registry() {
if !self.inner.source_id.is_crates_io() {
write!(f, " ({})", self.inner.source_id)?;
}

Expand Down
26 changes: 22 additions & 4 deletions src/cargo/core/source/source_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ struct SourceIdInner {
/// 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>,
}

/// The possible kinds of code source. Along with `SourceIdInner`, this fully defines the
Expand Down Expand Up @@ -81,6 +85,7 @@ impl SourceId {
url,
precise: None,
name: name.map(|n| n.into()),
alt_registry_key: None,
});
Ok(source_id)
}
Expand Down Expand Up @@ -221,13 +226,17 @@ impl SourceId {

/// Gets the `SourceId` associated with given name of the remote registry.
pub fn alt_registry(config: &Config, key: &str) -> CargoResult<SourceId> {
if key == CRATES_IO_REGISTRY {
return Self::crates_io(config);
}
let url = config.get_registry_index(key)?;
Ok(SourceId::wrap(SourceIdInner {
kind: SourceKind::Registry,
canonical_url: CanonicalUrl::new(&url)?,
url,
precise: None,
name: Some(key.to_string()),
alt_registry_key: Some(key.to_string()),
}))
}

Expand All @@ -243,15 +252,15 @@ impl SourceId {
}

pub fn display_index(self) -> String {
if self.is_default_registry() {
if self.is_crates_io() {
format!("{} index", CRATES_IO_DOMAIN)
} else {
format!("`{}` index", self.display_registry_name())
}
}

pub fn display_registry_name(self) -> String {
if self.is_default_registry() {
if self.is_crates_io() {
CRATES_IO_REGISTRY.to_string()
} else if let Some(name) = &self.inner.name {
name.clone()
Expand All @@ -264,6 +273,13 @@ 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.
pub fn alt_registry_key(&self) -> Option<&str> {
self.inner.alt_registry_key.as_deref()
}

/// Returns `true` if this source is from a filesystem path.
pub fn is_path(self) -> bool {
self.inner.kind == SourceKind::Path
Expand Down Expand Up @@ -364,13 +380,15 @@ impl SourceId {
}

/// Returns `true` if the remote registry is the standard <https://crates.io>.
pub fn is_default_registry(self) -> bool {
pub fn is_crates_io(self) -> bool {
match self.inner.kind {
SourceKind::Registry => {}
_ => return false,
}
let url = self.inner.url.as_str();
url == CRATES_IO_INDEX || url == CRATES_IO_HTTP_INDEX
url == CRATES_IO_INDEX
|| url == CRATES_IO_HTTP_INDEX
|| std::env::var("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS").as_deref() == Ok(url)
}

/// Hashes `self`.
Expand Down
145 changes: 66 additions & 79 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ fn verify_dependencies(
// This extra hostname check is mostly to assist with testing,
// but also prevents someone using `--index` to specify
// something that points to crates.io.
if registry_src.is_default_registry() || registry.host_is_crates_io() {
if registry_src.is_crates_io() || registry.host_is_crates_io() {
bail!("crates cannot be published to crates.io with dependencies sourced from other\n\
registries. `{}` needs to be published to crates.io before publishing this crate.\n\
(crate `{}` is pulled from {})",
Expand Down Expand Up @@ -391,6 +391,22 @@ pub fn registry_configuration(
};
// `registry.default` is handled in command-line parsing.
let (token, process) = match registry {
Some("crates-io") | None => {
// Use crates.io default.
config.check_registry_index_not_set()?;
let token = config.get_string("registry.token")?.map(|p| p.val);
let process = if config.cli_unstable().credential_process {
let process =
config.get::<Option<config::PathAndArgs>>("registry.credential-process")?;
if token.is_some() && process.is_some() {
return err_both("registry.token", "registry.credential-process");
}
process
} else {
None
};
(token, process)
}
Some(registry) => {
let token_key = format!("registries.{registry}.token");
let token = config.get_string(&token_key)?.map(|p| p.val);
Expand All @@ -411,22 +427,6 @@ pub fn registry_configuration(
};
(token, process)
}
None => {
// Use crates.io default.
config.check_registry_index_not_set()?;
let token = config.get_string("registry.token")?.map(|p| p.val);
let process = if config.cli_unstable().credential_process {
let process =
config.get::<Option<config::PathAndArgs>>("registry.credential-process")?;
if token.is_some() && process.is_some() {
return err_both("registry.token", "registry.credential-process");
}
process
} else {
None
};
(token, process)
}
};

let credential_process =
Expand All @@ -444,11 +444,9 @@ pub fn registry_configuration(
///
/// * `token`: The token from the command-line. If not set, uses the token
/// from the config.
/// * `index`: The index URL from the command-line. This is ignored if
/// `registry` is set.
/// * `index`: The index URL from the command-line.
/// * `registry`: The registry name from the command-line. If neither
/// `registry`, or `index` are set, then uses `crates-io`, honoring
/// `[source]` replacement if defined.
/// `registry`, or `index` are set, then uses `crates-io`.
/// * `force_update`: If `true`, forces the index to be updated.
/// * `validate_token`: If `true`, the token must be set.
fn registry(
Expand All @@ -459,24 +457,8 @@ fn registry(
force_update: bool,
validate_token: bool,
) -> CargoResult<(Registry, RegistryConfig, SourceId)> {
if index.is_some() && registry.is_some() {
// Otherwise we would silently ignore one or the other.
bail!("both `--index` and `--registry` should not be set at the same time");
}
// Parse all configuration options
let (sid, sid_no_replacement) = get_source_id(config, index, registry)?;
let reg_cfg = registry_configuration(config, registry)?;
let opt_index = registry
.map(|r| config.get_registry_index(r))
.transpose()?
.map(|u| u.to_string());
let sid = get_source_id(config, opt_index.as_deref().or(index), registry)?;
if !sid.is_remote_registry() {
bail!(
"{} does not support API commands.\n\
Eh2406 marked this conversation as resolved.
Show resolved Hide resolved
Check for a source-replacement in .cargo/config.",
sid
);
}
let api_host = {
let _lock = config.acquire_package_cache_lock()?;
let mut src = RegistrySource::remote(sid, &HashSet::new(), config)?;
Expand All @@ -503,42 +485,18 @@ fn registry(
}
token
} else {
// Check `is_default_registry` so that the crates.io index can
// change config.json's "api" value, and this won't affect most
// people. It will affect those using source replacement, but
// hopefully that's a relatively small set of users.
if token.is_none()
&& reg_cfg.is_token()
&& registry.is_none()
&& !sid.is_default_registry()
&& !crates_io::is_url_crates_io(&api_host)
{
config.shell().warn(
"using `registry.token` config value with source \
replacement is deprecated\n\
This may become a hard error in the future; \
see <https://github.com/rust-lang/cargo/issues/xxx>.\n\
Use the --token command-line flag to remove this warning.",
)?;
reg_cfg.as_token().map(|t| t.to_owned())
} else {
let token =
auth::auth_token(config, token.as_deref(), &reg_cfg, registry, &api_host)?;
Some(token)
}
let token = auth::auth_token(config, token.as_deref(), &reg_cfg, registry, &api_host)?;
Some(token)
}
} else {
None
};
let handle = http_handle(config)?;
// Workaround for the sparse+https://index.crates.io replacement index. Use the non-replaced
// source_id so that the original (github) url is used when publishing a crate.
let sid = if sid.is_default_registry() {
SourceId::crates_io(config)?
} else {
sid
};
Ok((Registry::new_handle(api_host, token, handle), reg_cfg, sid))
Ok((
Registry::new_handle(api_host, token, handle),
reg_cfg,
sid_no_replacement,
))
}

/// Creates a new HTTP handle with appropriate global configuration for cargo.
Expand Down Expand Up @@ -947,16 +905,45 @@ pub fn yank(
/// Gets the SourceId for an index or registry setting.
///
/// The `index` and `reg` values are from the command-line or config settings.
/// If both are None, returns the source for crates.io.
fn get_source_id(config: &Config, index: Option<&str>, reg: Option<&str>) -> CargoResult<SourceId> {
match (reg, index) {
(Some(r), _) => SourceId::alt_registry(config, r),
(_, Some(i)) => SourceId::for_registry(&i.into_url()?),
_ => {
let map = SourceConfigMap::new(config)?;
let src = map.load(SourceId::crates_io(config)?, &HashSet::new())?;
Ok(src.replaced_source_id())
/// If both are None, and no source-replacement is configured, returns the source for crates.io.
/// If both are None, and source replacement is configured, returns an error.
///
/// The source for crates.io may be GitHub, index.crates.io, or a test-only registry depending
/// on configuration.
///
/// If `reg` is set, source replacement is not followed.
///
/// The return value is a pair of `SourceId`s: The first may be a built-in replacement of
/// crates.io (such as index.crates.io), while the second is always the original source.
fn get_source_id(
config: &Config,
index: Option<&str>,
reg: Option<&str>,
) -> CargoResult<(SourceId, SourceId)> {
let sid = match (reg, index) {
(None, None) => SourceId::crates_io(config)?,
(Some(r), None) => SourceId::alt_registry(config, r)?,
(None, Some(i)) => SourceId::for_registry(&i.into_url()?)?,
(Some(_), Some(_)) => {
bail!("both `--index` and `--registry` should not be set at the same time")
}
};
// Load source replacements that are built-in to Cargo.
let builtin_replacement_sid = SourceConfigMap::empty(config)?
.load(sid, &HashSet::new())?
.replaced_source_id();
let replacement_sid = SourceConfigMap::new(config)?
.load(sid, &HashSet::new())?
.replaced_source_id();
if reg.is_none() && index.is_none() && replacement_sid != builtin_replacement_sid {
// Neither --registry nor --index was passed and the user has configured source-replacement.
if let Some(replacement_name) = replacement_sid.alt_registry_key() {
bail!("crates-io is replaced with remote registry {replacement_name};\ninclude `--registry {replacement_name}` or `--registry crates-io`");
} else {
bail!("crates-io is replaced with non-remote-registry source {replacement_sid};\ninclude `--registry crates-io` to use crates.io");
}
} else {
Ok((builtin_replacement_sid, sid))
}
}

Expand Down Expand Up @@ -1024,7 +1011,7 @@ pub fn search(
&ColorSpec::new(),
);
} else if total_crates > limit && limit >= search_max_limit {
let extra = if source_id.is_default_registry() {
let extra = if source_id.is_crates_io() {
format!(
" (go to https://crates.io/search?q={} to see more)",
percent_encode(query.as_bytes(), NON_ALPHANUMERIC)
Expand Down
Loading