Skip to content

Commit

Permalink
Auto merge of #10907 - arlosi:sr2, r=ehuss
Browse files Browse the repository at this point in the history
Implement RFC 3289: source replacement ambiguity

### Implements [RFC 3289](rust-lang/rfcs#3289)
* When the crates-io source is replaced, the user needs to specify `--registry <NAME>` when running an API operation to disambiguate which registry to use. Otherwise, cargo will issue a new error.
* In source replacement, the `replace-with` key can reference the name of an alt registry in the `[registries]` table.
* Publishing to source-replaced crates.io is no longer permitted using the crates.io token (`registry.token`). We have had a deprecation warning in place since #7973 (1.45.0).

### Testing
* Tests that interacting with crates.io use the new `replace_crates_io` function, which internally sets an environment variable to change the URL of crates.io.

Changes are insta-stable.

cc #10894
r? `@Eh2406`
  • Loading branch information
bors committed Oct 8, 2022
2 parents 8743325 + dd5134c commit d9b05a1
Show file tree
Hide file tree
Showing 32 changed files with 661 additions and 330 deletions.
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\
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

0 comments on commit d9b05a1

Please sign in to comment.