Skip to content

Commit

Permalink
Implement RFC 3289: source replacement ambiguity
Browse files Browse the repository at this point in the history
  • Loading branch information
arlosi committed Jul 27, 2022
1 parent 85b500c commit 028aa47
Show file tree
Hide file tree
Showing 20 changed files with 213 additions and 167 deletions.
15 changes: 15 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,20 @@ impl Execs {
self
}

/// Allows replacement of crates-io source without specifying --registry <NAME>
///
/// Can be used for testing crates-io specific items where alt registries
/// cannot be used.
pub fn allow_silent_crates_io_replacement(&mut self) -> &mut Self {
if let Some(ref mut p) = self.process_builder {
p.env(
"__CARGO_TEST_ALLOW_CRATES_IO_REPLACEMENT_DO_NOT_USE",
"true",
);
}
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 Expand Up @@ -1214,6 +1228,7 @@ pub trait TestEnv: Sized {
// stable channel yet. Once incremental support hits the stable compiler we
// can switch this to one and then fix the tests.
.env("CARGO_INCREMENTAL", "0")
.env("__CARGO_TEST_CRATESIO_REPLACEMENT", "true")
.env_remove("__CARGO_DEFAULT_LIB_METADATA")
.env_remove("RUSTC")
.env_remove("RUSTDOC")
Expand Down
7 changes: 5 additions & 2 deletions crates/cargo-test-support/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,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 Expand Up @@ -282,6 +282,9 @@ impl RegistryBuilder {
r#"
[registry]
token = "{token}"
[registries.dummy-registry]
token = "{token}"
"#
)
.as_bytes(),
Expand Down
13 changes: 13 additions & 0 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 @@ -228,6 +233,7 @@ impl SourceId {
url,
precise: None,
name: Some(key.to_string()),
alt_registry_key: Some(key.to_string()),
}))
}

Expand Down Expand Up @@ -264,6 +270,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
105 changes: 44 additions & 61 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 @@ -943,16 +901,41 @@ 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 (except when testing Cargo).
///
/// If `reg` is set, source replacement is not followed.
fn get_source_id(
config: &Config,
index: Option<&str>,
reg: Option<&str>,
) -> CargoResult<(SourceId, SourceId)> {
let map = SourceConfigMap::new(config)?;
let sid = match (reg, index) {
(None, None) | (Some("crates-io"), 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")
}
};
let src = map.load(sid, &HashSet::new())?;
if src.is_replaced() && reg.is_none() && index.is_none() {
// Neither --registry nor --index was passed and source replacement is configured.
let replacement_sid = src.replaced_source_id();
if replacement_sid.is_default_registry()
|| std::env::var("__CARGO_TEST_ALLOW_CRATES_IO_REPLACEMENT_DO_NOT_USE").is_ok()
{
// Allow replacement of crates.io for the sparse crates.io endpoint or for Cargo's
// testing framework.
Ok((replacement_sid, sid))
} else 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 publish to crates.io");
}
} else {
Ok((sid.clone(), sid))
}
}

Expand Down
27 changes: 16 additions & 11 deletions src/cargo/sources/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,18 +121,24 @@ impl<'cfg> SourceConfigMap<'cfg> {
};
let mut cfg_loc = "";
let orig_name = name;
let new_id;
loop {
let new_id = loop {
let cfg = match self.cfgs.get(name) {
Some(cfg) => cfg,
None => bail!(
"could not find a configured source with the \
None => {
// Attempt to interpret the source name as an alt registry name
if let Ok(alt_id) = SourceId::alt_registry(self.config, name) {
debug!("following pointer to registry {}", name);
break alt_id.with_precise(id.precise().map(str::to_string));
}
bail!(
"could not find a configured source with the \
name `{}` when attempting to lookup `{}` \
(configuration in `{}`)",
name,
orig_name,
cfg_loc
),
name,
orig_name,
cfg_loc
);
}
};
match &cfg.replace_with {
Some((s, c)) => {
Expand All @@ -141,8 +147,7 @@ impl<'cfg> SourceConfigMap<'cfg> {
}
None if id == cfg.id => return id.load(self.config, yanked_whitelist),
None => {
new_id = cfg.id.with_precise(id.precise().map(|s| s.to_string()));
break;
break cfg.id.with_precise(id.precise().map(|s| s.to_string()));
}
}
debug!("following pointer to {}", name);
Expand All @@ -155,7 +160,7 @@ impl<'cfg> SourceConfigMap<'cfg> {
cfg_loc
)
}
}
};

let new_src = new_id.load(
self.config,
Expand Down
3 changes: 3 additions & 0 deletions src/doc/src/reference/source-replacement.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ directory = "vendor"
# The crates.io default source for crates is available under the name
# "crates-io", and here we use the `replace-with` key to indicate that it's
# replaced with our source above.
#
# The `replace-with` key can also reference an alternative registry name
# defined in the `[registries]` table.
[source.crates-io]
replace-with = "my-vendor-source"

Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1900,7 +1900,7 @@ fn publish_artifact_dep() {
.file("src/lib.rs", "")
.build();

p.cargo("publish -Z bindeps --no-verify --token sekrit")
p.cargo("publish --registry dummy-registry -Z bindeps --no-verify --token sekrit")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr(
"\
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fn bad3() {
.build();
Package::new("foo", "1.0.0").publish();

p.cargo("publish -v")
p.cargo("publish --registry dummy-registry -v")
.with_status(101)
.with_stderr(
"\
Expand Down Expand Up @@ -125,7 +125,7 @@ fn bad6() {
.build();
Package::new("foo", "1.0.0").publish();

p.cargo("publish -v")
p.cargo("publish --registry dummy-registry -v")
.with_status(101)
.with_stderr(
"\
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ fn publish_allowed() {
)
.file("src/lib.rs", "")
.build();
p.cargo("publish --token sekrit")
p.cargo("publish --registry dummy-registry --token sekrit")
.masquerade_as_nightly_cargo(&["test-dummy-unstable"])
.run();
}
Expand Down
Loading

0 comments on commit 028aa47

Please sign in to comment.