Skip to content

Commit 720c2fa

Browse files
arlosiarlosi
committed
Implement RFC 3289: source replacement ambiguity
1 parent 646e9a0 commit 720c2fa

30 files changed

+552
-325
lines changed

‎crates/cargo-test-support/src/lib.rs

+11
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,17 @@ impl Execs {
818818
self
819819
}
820820

821+
/// Overrides the crates.io URL for testing.
822+
///
823+
/// Can be used for testing crates-io functionality where alt registries
824+
/// cannot be used.
825+
pub fn replace_crates_io(&mut self, url: &Url) -> &mut Self {
826+
if let Some(ref mut p) = self.process_builder {
827+
p.env("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS", url.as_str());
828+
}
829+
self
830+
}
831+
821832
pub fn enable_mac_dsym(&mut self) -> &mut Self {
822833
if cfg!(target_os = "macos") {
823834
self.env("CARGO_PROFILE_DEV_SPLIT_DEBUGINFO", "packed")

‎crates/cargo-test-support/src/registry.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ impl RegistryBuilder {
103103
pub fn new() -> RegistryBuilder {
104104
RegistryBuilder {
105105
alternative: None,
106-
token: Some("api-token".to_string()),
106+
token: None,
107107
http_api: false,
108108
http_index: false,
109109
api: true,
@@ -195,6 +195,7 @@ impl RegistryBuilder {
195195
let dl_url = generate_url(&format!("{prefix}dl"));
196196
let dl_path = generate_path(&format!("{prefix}dl"));
197197
let api_path = generate_path(&format!("{prefix}api"));
198+
let token = Some(self.token.unwrap_or_else(|| format!("{prefix}sekrit")));
198199

199200
let (server, index_url, api_url, dl_url) = if !self.http_index && !self.http_api {
200201
// No need to start the HTTP server.
@@ -203,7 +204,7 @@ impl RegistryBuilder {
203204
let server = HttpServer::new(
204205
registry_path.clone(),
205206
dl_path,
206-
self.token.clone(),
207+
token.clone(),
207208
self.custom_responders,
208209
);
209210
let index_url = if self.http_index {
@@ -226,7 +227,7 @@ impl RegistryBuilder {
226227
_server: server,
227228
dl_url,
228229
path: registry_path,
229-
token: self.token,
230+
token,
230231
};
231232

232233
if self.configure_registry {
@@ -250,8 +251,8 @@ impl RegistryBuilder {
250251
[source.crates-io]
251252
replace-with = 'dummy-registry'
252253
253-
[source.dummy-registry]
254-
registry = '{}'",
254+
[registries.dummy-registry]
255+
index = '{}'",
255256
registry.index_url
256257
)
257258
.as_bytes(),

‎src/cargo/core/compiler/rustdoc.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ pub fn add_root_urls(
128128
if !sid.is_registry() {
129129
return false;
130130
}
131-
if sid.is_default_registry() {
131+
if sid.is_crates_io() {
132132
return registry == CRATES_IO_REGISTRY;
133133
}
134134
if let Some(index_url) = name2url.get(registry) {

‎src/cargo/core/package_id.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ impl fmt::Display for PackageId {
211211
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
212212
write!(f, "{} v{}", self.inner.name, self.inner.version)?;
213213

214-
if !self.inner.source_id.is_default_registry() {
214+
if !self.inner.source_id.is_crates_io() {
215215
write!(f, " ({})", self.inner.source_id)?;
216216
}
217217

‎src/cargo/core/source/source_id.rs

+19-4
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ struct SourceIdInner {
3939
/// WARNING: this is not always set for alt-registries when the name is
4040
/// not known.
4141
name: Option<String>,
42+
/// Name of the alt registry in the [registries] table.
43+
/// WARNING: this is not always set for alt-registries when the name is
44+
/// not known.
45+
alt_registry_key: Option<String>,
4246
}
4347

4448
/// The possible kinds of code source. Along with `SourceIdInner`, this fully defines the
@@ -81,6 +85,7 @@ impl SourceId {
8185
url,
8286
precise: None,
8387
name: name.map(|n| n.into()),
88+
alt_registry_key: None,
8489
});
8590
Ok(source_id)
8691
}
@@ -228,6 +233,7 @@ impl SourceId {
228233
url,
229234
precise: None,
230235
name: Some(key.to_string()),
236+
alt_registry_key: Some(key.to_string()),
231237
}))
232238
}
233239

@@ -243,15 +249,15 @@ impl SourceId {
243249
}
244250

245251
pub fn display_index(self) -> String {
246-
if self.is_default_registry() {
252+
if self.is_crates_io() {
247253
format!("{} index", CRATES_IO_DOMAIN)
248254
} else {
249255
format!("`{}` index", self.display_registry_name())
250256
}
251257
}
252258

253259
pub fn display_registry_name(self) -> String {
254-
if self.is_default_registry() {
260+
if self.is_crates_io() {
255261
CRATES_IO_REGISTRY.to_string()
256262
} else if let Some(name) = &self.inner.name {
257263
name.clone()
@@ -264,6 +270,13 @@ impl SourceId {
264270
}
265271
}
266272

273+
/// Gets the name of the remote registry as defined in the [registries] table.
274+
/// WARNING: alt registries that come from Cargo.lock, or --index will
275+
/// not have a name.
276+
pub fn alt_registry_key(&self) -> Option<&str> {
277+
self.inner.alt_registry_key.as_deref()
278+
}
279+
267280
/// Returns `true` if this source is from a filesystem path.
268281
pub fn is_path(self) -> bool {
269282
self.inner.kind == SourceKind::Path
@@ -364,13 +377,15 @@ impl SourceId {
364377
}
365378

366379
/// Returns `true` if the remote registry is the standard <https://crates.io>.
367-
pub fn is_default_registry(self) -> bool {
380+
pub fn is_crates_io(self) -> bool {
368381
match self.inner.kind {
369382
SourceKind::Registry => {}
370383
_ => return false,
371384
}
372385
let url = self.inner.url.as_str();
373-
url == CRATES_IO_INDEX || url == CRATES_IO_HTTP_INDEX
386+
url == CRATES_IO_INDEX
387+
|| url == CRATES_IO_HTTP_INDEX
388+
|| std::env::var("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS").as_deref() == Ok(url)
374389
}
375390

376391
/// Hashes `self`.

‎src/cargo/ops/registry.rs

+66-79
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ fn verify_dependencies(
209209
// This extra hostname check is mostly to assist with testing,
210210
// but also prevents someone using `--index` to specify
211211
// something that points to crates.io.
212-
if registry_src.is_default_registry() || registry.host_is_crates_io() {
212+
if registry_src.is_crates_io() || registry.host_is_crates_io() {
213213
bail!("crates cannot be published to crates.io with dependencies sourced from other\n\
214214
registries. `{}` needs to be published to crates.io before publishing this crate.\n\
215215
(crate `{}` is pulled from {})",
@@ -391,6 +391,22 @@ pub fn registry_configuration(
391391
};
392392
// `registry.default` is handled in command-line parsing.
393393
let (token, process) = match registry {
394+
Some("crates-io") | None => {
395+
// Use crates.io default.
396+
config.check_registry_index_not_set()?;
397+
let token = config.get_string("registry.token")?.map(|p| p.val);
398+
let process = if config.cli_unstable().credential_process {
399+
let process =
400+
config.get::<Option<config::PathAndArgs>>("registry.credential-process")?;
401+
if token.is_some() && process.is_some() {
402+
return err_both("registry.token", "registry.credential-process");
403+
}
404+
process
405+
} else {
406+
None
407+
};
408+
(token, process)
409+
}
394410
Some(registry) => {
395411
let token_key = format!("registries.{registry}.token");
396412
let token = config.get_string(&token_key)?.map(|p| p.val);
@@ -411,22 +427,6 @@ pub fn registry_configuration(
411427
};
412428
(token, process)
413429
}
414-
None => {
415-
// Use crates.io default.
416-
config.check_registry_index_not_set()?;
417-
let token = config.get_string("registry.token")?.map(|p| p.val);
418-
let process = if config.cli_unstable().credential_process {
419-
let process =
420-
config.get::<Option<config::PathAndArgs>>("registry.credential-process")?;
421-
if token.is_some() && process.is_some() {
422-
return err_both("registry.token", "registry.credential-process");
423-
}
424-
process
425-
} else {
426-
None
427-
};
428-
(token, process)
429-
}
430430
};
431431

432432
let credential_process =
@@ -444,11 +444,9 @@ pub fn registry_configuration(
444444
///
445445
/// * `token`: The token from the command-line. If not set, uses the token
446446
/// from the config.
447-
/// * `index`: The index URL from the command-line. This is ignored if
448-
/// `registry` is set.
447+
/// * `index`: The index URL from the command-line.
449448
/// * `registry`: The registry name from the command-line. If neither
450-
/// `registry`, or `index` are set, then uses `crates-io`, honoring
451-
/// `[source]` replacement if defined.
449+
/// `registry`, or `index` are set, then uses `crates-io`.
452450
/// * `force_update`: If `true`, forces the index to be updated.
453451
/// * `validate_token`: If `true`, the token must be set.
454452
fn registry(
@@ -459,24 +457,8 @@ fn registry(
459457
force_update: bool,
460458
validate_token: bool,
461459
) -> CargoResult<(Registry, RegistryConfig, SourceId)> {
462-
if index.is_some() && registry.is_some() {
463-
// Otherwise we would silently ignore one or the other.
464-
bail!("both `--index` and `--registry` should not be set at the same time");
465-
}
466-
// Parse all configuration options
460+
let (sid, sid_no_replacement) = get_source_id(config, index, registry)?;
467461
let reg_cfg = registry_configuration(config, registry)?;
468-
let opt_index = registry
469-
.map(|r| config.get_registry_index(r))
470-
.transpose()?
471-
.map(|u| u.to_string());
472-
let sid = get_source_id(config, opt_index.as_deref().or(index), registry)?;
473-
if !sid.is_remote_registry() {
474-
bail!(
475-
"{} does not support API commands.\n\
476-
Check for a source-replacement in .cargo/config.",
477-
sid
478-
);
479-
}
480462
let api_host = {
481463
let _lock = config.acquire_package_cache_lock()?;
482464
let mut src = RegistrySource::remote(sid, &HashSet::new(), config)?;
@@ -503,42 +485,18 @@ fn registry(
503485
}
504486
token
505487
} else {
506-
// Check `is_default_registry` so that the crates.io index can
507-
// change config.json's "api" value, and this won't affect most
508-
// people. It will affect those using source replacement, but
509-
// hopefully that's a relatively small set of users.
510-
if token.is_none()
511-
&& reg_cfg.is_token()
512-
&& registry.is_none()
513-
&& !sid.is_default_registry()
514-
&& !crates_io::is_url_crates_io(&api_host)
515-
{
516-
config.shell().warn(
517-
"using `registry.token` config value with source \
518-
replacement is deprecated\n\
519-
This may become a hard error in the future; \
520-
see <https://github.com/rust-lang/cargo/issues/xxx>.\n\
521-
Use the --token command-line flag to remove this warning.",
522-
)?;
523-
reg_cfg.as_token().map(|t| t.to_owned())
524-
} else {
525-
let token =
526-
auth::auth_token(config, token.as_deref(), &reg_cfg, registry, &api_host)?;
527-
Some(token)
528-
}
488+
let token = auth::auth_token(config, token.as_deref(), &reg_cfg, registry, &api_host)?;
489+
Some(token)
529490
}
530491
} else {
531492
None
532493
};
533494
let handle = http_handle(config)?;
534-
// Workaround for the sparse+https://index.crates.io replacement index. Use the non-replaced
535-
// source_id so that the original (github) url is used when publishing a crate.
536-
let sid = if sid.is_default_registry() {
537-
SourceId::crates_io(config)?
538-
} else {
539-
sid
540-
};
541-
Ok((Registry::new_handle(api_host, token, handle), reg_cfg, sid))
495+
Ok((
496+
Registry::new_handle(api_host, token, handle),
497+
reg_cfg,
498+
sid_no_replacement,
499+
))
542500
}
543501

544502
/// Creates a new HTTP handle with appropriate global configuration for cargo.
@@ -943,16 +901,45 @@ pub fn yank(
943901
/// Gets the SourceId for an index or registry setting.
944902
///
945903
/// The `index` and `reg` values are from the command-line or config settings.
946-
/// If both are None, returns the source for crates.io.
947-
fn get_source_id(config: &Config, index: Option<&str>, reg: Option<&str>) -> CargoResult<SourceId> {
948-
match (reg, index) {
949-
(Some(r), _) => SourceId::alt_registry(config, r),
950-
(_, Some(i)) => SourceId::for_registry(&i.into_url()?),
951-
_ => {
952-
let map = SourceConfigMap::new(config)?;
953-
let src = map.load(SourceId::crates_io(config)?, &HashSet::new())?;
954-
Ok(src.replaced_source_id())
904+
/// If both are None, and no source-replacement is configured, returns the source for crates.io.
905+
/// If both are None, and source replacement is configured, returns an error.
906+
///
907+
/// The source for crates.io may be GitHub, index.crates.io, or a test-only registry depending
908+
/// on configuration.
909+
///
910+
/// If `reg` is set, source replacement is not followed.
911+
///
912+
/// The return value is a pair of `SourceId`s: The first may be a built-in replacement of
913+
/// crates.io (such as index.crates.io), while the second is always the original source.
914+
fn get_source_id(
915+
config: &Config,
916+
index: Option<&str>,
917+
reg: Option<&str>,
918+
) -> CargoResult<(SourceId, SourceId)> {
919+
let sid = match (reg, index) {
920+
(None, None) | (Some("crates-io"), None) => SourceId::crates_io(config)?,
921+
(Some(r), None) => SourceId::alt_registry(config, r)?,
922+
(None, Some(i)) => SourceId::for_registry(&i.into_url()?)?,
923+
(Some(_), Some(_)) => {
924+
bail!("both `--index` and `--registry` should not be set at the same time")
925+
}
926+
};
927+
// Load source replacements that are built-in to Cargo.
928+
let builtin_replacement_sid = SourceConfigMap::empty(config)?
929+
.load(sid, &HashSet::new())?
930+
.replaced_source_id();
931+
let replacement_sid = SourceConfigMap::new(config)?
932+
.load(sid, &HashSet::new())?
933+
.replaced_source_id();
934+
if reg.is_none() && index.is_none() && replacement_sid != builtin_replacement_sid {
935+
// Neither --registry nor --index was passed and the user has configured source-replacement.
936+
if let Some(replacement_name) = replacement_sid.alt_registry_key() {
937+
bail!("crates-io is replaced with remote registry {replacement_name};\ninclude `--registry {replacement_name}` or `--registry crates-io`");
938+
} else {
939+
bail!("crates-io is replaced with non-remote-registry source {replacement_sid};\ninclude `--registry crates-io` to publish to crates.io");
955940
}
941+
} else {
942+
Ok((builtin_replacement_sid, sid))
956943
}
957944
}
958945

@@ -1032,7 +1019,7 @@ pub fn search(
10321019
&ColorSpec::new(),
10331020
);
10341021
} else if total_crates > limit && limit >= search_max_limit {
1035-
let extra = if source_id.is_default_registry() {
1022+
let extra = if source_id.is_crates_io() {
10361023
format!(
10371024
" (go to https://crates.io/search?q={} to see more)",
10381025
percent_encode(query.as_bytes(), NON_ALPHANUMERIC)

‎src/cargo/ops/tree/format/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ impl<'a> fmt::Display for Display<'a> {
8484
)?;
8585

8686
let source_id = package.package_id().source_id();
87-
if !source_id.is_default_registry() {
87+
if !source_id.is_crates_io() {
8888
write!(fmt, " ({})", source_id)?;
8989
}
9090
}

‎src/cargo/ops/vendor.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -252,13 +252,13 @@ fn sync(
252252

253253
// replace original sources with vendor
254254
for source_id in sources {
255-
let name = if source_id.is_default_registry() {
255+
let name = if source_id.is_crates_io() {
256256
CRATES_IO_REGISTRY.to_string()
257257
} else {
258258
source_id.url().to_string()
259259
};
260260

261-
let source = if source_id.is_default_registry() {
261+
let source = if source_id.is_crates_io() {
262262
VendorSource::Registry {
263263
registry: None,
264264
replace_with: merged_source_name.to_string(),

0 commit comments

Comments
 (0)