Skip to content

Commit

Permalink
Auto merge of #13433 - arlosi:builtin-replacement-messages, r=epage
Browse files Browse the repository at this point in the history
Fix confusing error messages for sparse index replaced source

The built-in sparse crates.io index `index.crates.io` is implemented in Cargo using source replacement.

When an error occurs downloading from the sparse index, the message includes text that looks like the user configured source replacement, even when they did not.

This change extends the special case for the built-in replacement of crates.io to include all the error messages for source replacement in addition the the description special case.
  • Loading branch information
bors committed Feb 12, 2024
2 parents 06a19e6 + dbd7c98 commit e4d6786
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 32 deletions.
67 changes: 47 additions & 20 deletions src/cargo/sources/replaced.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ use crate::sources::IndexSummary;
use crate::util::errors::CargoResult;
use std::task::Poll;

use anyhow::Context as _;

/// A source that replaces one source with the other. This manages the [source
/// replacement] feature.
///
Expand Down Expand Up @@ -37,6 +35,14 @@ impl<'cfg> ReplacedSource<'cfg> {
inner: src,
}
}

/// Is this source a built-in replacement of crates.io?
///
/// Built-in source replacement of crates.io for sparse registry or tests
/// should not show messages indicating source replacement.
fn is_builtin_replacement(&self) -> bool {
self.replace_with.is_crates_io() && self.to_replace.is_crates_io()
}
}

impl<'cfg> Source for ReplacedSource<'cfg> {
Expand Down Expand Up @@ -70,10 +76,14 @@ impl<'cfg> Source for ReplacedSource<'cfg> {
f(summary.map_summary(|s| s.map_source(replace_with, to_replace)))
})
.map_err(|e| {
e.context(format!(
"failed to query replaced source {}",
self.to_replace
))
if self.is_builtin_replacement() {
e
} else {
e.context(format!(
"failed to query replaced source {}",
self.to_replace
))
}
})
}

Expand All @@ -87,10 +97,16 @@ impl<'cfg> Source for ReplacedSource<'cfg> {

fn download(&mut self, id: PackageId) -> CargoResult<MaybePackage> {
let id = id.with_source_id(self.replace_with);
let pkg = self
.inner
.download(id)
.with_context(|| format!("failed to download replaced source {}", self.to_replace))?;
let pkg = self.inner.download(id).map_err(|e| {
if self.is_builtin_replacement() {
e
} else {
e.context(format!(
"failed to download replaced source {}",
self.to_replace
))
}
})?;
Ok(match pkg {
MaybePackage::Ready(pkg) => {
MaybePackage::Ready(pkg.map_source(self.replace_with, self.to_replace))
Expand All @@ -101,10 +117,16 @@ impl<'cfg> Source for ReplacedSource<'cfg> {

fn finish_download(&mut self, id: PackageId, data: Vec<u8>) -> CargoResult<Package> {
let id = id.with_source_id(self.replace_with);
let pkg = self
.inner
.finish_download(id, data)
.with_context(|| format!("failed to download replaced source {}", self.to_replace))?;
let pkg = self.inner.finish_download(id, data).map_err(|e| {
if self.is_builtin_replacement() {
e
} else {
e.context(format!(
"failed to download replaced source {}",
self.to_replace
))
}
})?;
Ok(pkg.map_source(self.replace_with, self.to_replace))
}

Expand All @@ -118,9 +140,7 @@ impl<'cfg> Source for ReplacedSource<'cfg> {
}

fn describe(&self) -> String {
if self.replace_with.is_crates_io() && self.to_replace.is_crates_io() {
// Built-in source replacement of crates.io for sparse registry or tests
// doesn't need duplicate description (crates.io replacing crates.io).
if self.is_builtin_replacement() {
self.inner.describe()
} else {
format!(
Expand Down Expand Up @@ -148,8 +168,15 @@ impl<'cfg> Source for ReplacedSource<'cfg> {
}

fn block_until_ready(&mut self) -> CargoResult<()> {
self.inner
.block_until_ready()
.with_context(|| format!("failed to update replaced source {}", self.to_replace))
self.inner.block_until_ready().map_err(|e| {
if self.is_builtin_replacement() {
e
} else {
e.context(format!(
"failed to update replaced source {}",
self.to_replace
))
}
})
}
}
15 changes: 3 additions & 12 deletions tests/testsuite/credential_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,7 @@ fn not_found() {
r#"[UPDATING] [..]
[CREDENTIAL] [..]not_found[..] get crates-io
{"v":1[..]
[ERROR] failed to query replaced source registry `crates-io`
Caused by:
no token found, please run `cargo login`
[ERROR] no token found, please run `cargo login`
"#,
)
.run();
Expand Down Expand Up @@ -314,10 +311,7 @@ fn all_not_found() {
r#"[UPDATING] [..]
[CREDENTIAL] [..]not_found[..] get crates-io
{"v":1,"registry":{"index-url":"[..]","name":"crates-io","headers":[[..]"WWW-Authenticate: Cargo login_url=\"https://test-registry-login/me\""[..]]},"kind":"get","operation":"read"}
[ERROR] failed to query replaced source registry `crates-io`
Caused by:
no token found, please run `cargo login`
[ERROR] no token found, please run `cargo login`
"#,
)
.run();
Expand Down Expand Up @@ -353,10 +347,7 @@ fn all_not_supported() {
r#"[UPDATING] [..]
[CREDENTIAL] [..]not_supported[..] get crates-io
{"v":1,"registry":{"index-url":"[..]","name":"crates-io","headers":[[..]"WWW-Authenticate: Cargo login_url=\"https://test-registry-login/me\""[..]]},"kind":"get","operation":"read"}
[ERROR] failed to query replaced source registry `crates-io`
Caused by:
no credential providers could handle the request
[ERROR] no credential providers could handle the request
"#,
)
.run();
Expand Down
39 changes: 39 additions & 0 deletions tests/testsuite/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3664,3 +3664,42 @@ fn differ_only_by_metadata_with_lockfile() {
)
.run();
}

#[cargo_test]
fn builtin_source_replacement() {
// errors for builtin source replacement of crates.io
// should not include mention of source replacement in the error message.
let server = RegistryBuilder::new().build();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
[dependencies]
bad-cksum = ">= 0.0.0"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

let pkg = Package::new("bad-cksum", "0.0.1");
pkg.publish();
t!(File::create(&pkg.archive_dst()));

p.cargo("check -v")
.replace_crates_io(&server.index_url())
.with_status(101)
.with_stderr(
"\
[UPDATING] [..] index
[DOWNLOADING] crates ...
[DOWNLOADED] bad-cksum [..]
[ERROR] failed to verify the checksum of `bad-cksum v0.0.1`
",
)
.run();
}

0 comments on commit e4d6786

Please sign in to comment.