Skip to content

Commit

Permalink
Fix confusing error messages for sparse index replaced source
Browse files Browse the repository at this point in the history
  • Loading branch information
arlosi committed Feb 12, 2024
1 parent 06a19e6 commit dbd7c98
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 dbd7c98

Please sign in to comment.