Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in 1.61 error message for crate not found in local registry #10682

Closed
jonhoo opened this issue May 20, 2022 · 5 comments · Fixed by #10683
Closed

Regression in 1.61 error message for crate not found in local registry #10682

jonhoo opened this issue May 20, 2022 · 5 comments · Fixed by #10683
Labels
C-bug Category: bug

Comments

@jonhoo
Copy link
Contributor

jonhoo commented May 20, 2022

Problem

The error message that's presented when a crate doesn't exist in a local registry has regressed from 1.60 to 1.61.

Steps

cargo new local-reg-regression
cd local-reg-regression
echo 'itoa = "1"' >> Cargo.toml
cargo check
mkdir .cargo
cargo local-registry -s Cargo.lock local-registry > .cargo/config.toml
$EDITOR .cargo/config.toml # make it valid TOML
sed -i 's/itoa/gxxy/' Cargo.toml
cargo check

I expected to see (and did in 1.60):

error: no matching package named `gxxy` found
location searched: registry `crates-io`
required by package `local-reg-regression v0.1.0 (/local/home/jongje/dev/tmp/local-reg-regression)`

Instead, with 1.61, I see:

error: failed to get `gxxy` as a dependency of package `local-reg-regression v0.1.0 (/local/home/jongje/dev/tmp/local-reg-regression)`

Caused by:
  failed to query replaced source registry `crates-io`

Caused by:
  failed to read `/local/home/jongje/dev/tmp/local-reg-regression/local-registry/index/gx/xy/gxxy`

Caused by:
  No such file or directory (os error 2)

Possible Solution(s)

I'm not entirely sure how this change in behavior arose. I think it may be related to #10482 and how we don't translate a ErrorKind::NotFound from read_bytes into LoadResponse::NotFound here:

Poll::Ready(Ok(LoadResponse::Data {
raw_data: paths::read_bytes(&root.join(path))?,
index_version: None,
}))

cc @arlosi

Notes

No response

Version

cargo 1.61.0 (a028ae4 2022-04-29)
release: 1.61.0
commit-hash: a028ae42fc1376571de836be702e840ca8e060c2
commit-date: 2022-04-29
host: aarch64-unknown-linux-gnu
libgit2: 1.4.2 (sys:0.14.2 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:OpenSSL/1.1.1m)
os: Amazon Linux AMI 2.0.0 [64-bit]
@jonhoo jonhoo added the C-bug Category: bug label May 20, 2022
@jonhoo
Copy link
Contributor Author

jonhoo commented May 20, 2022

Bisection suggests regression happened in the 2022-03-20 nightly, and specifically with rust-lang/rust#95103.

@jonhoo
Copy link
Contributor Author

jonhoo commented May 20, 2022

Cargo bisection of the relevant commit range points at f12f025 (part of #10064) being the first bad commit

@jonhoo
Copy link
Contributor Author

jonhoo commented May 20, 2022

I think I found it. That commit changed (in part) this:

});
if matches!(err, Poll::Pending) {
assert!(!hit_closure);
return Poll::Pending;
}
// We ignore lookup failures as those are just crates which don't exist
// or we haven't updated the registry yet. If we actually ran the
// closure though then we care about those errors.
if !hit_closure {
debug_assert!(cache_contents.is_none());
return Poll::Ready(Ok(None));
}
let _ = err?;

to this

})?;
if matches!(err, Poll::Pending) {
assert!(!hit_closure);
return Poll::Pending;
}
// We ignore lookup failures as those are just crates which don't exist
// or we haven't updated the registry yet. If we actually ran the
// closure though then we care about those errors.
if !hit_closure {
debug_assert!(cache_contents.is_none());
return Poll::Ready(Ok(None));
}

Note how we now error immediately if .load() fails (the added ?), whereas previously we would ignore the error if the closure was never hit.

@jonhoo
Copy link
Contributor Author

jonhoo commented May 20, 2022

And that change carried over through #10482, which doesn't map a local registry NotFound error into LoadResponse::NotFound, and thus continues to take the early-exit path.

@jonhoo
Copy link
Contributor Author

jonhoo commented May 20, 2022

Fix in #10683

bors added a commit that referenced this issue May 23, 2022
Restore proper error for crate not in local reg

Fixes #10682.
@bors bors closed this as completed in c262f10 May 23, 2022
Hezuikn pushed a commit to Hezuikn/cargo that referenced this issue Sep 22, 2022
Hezuikn pushed a commit to Hezuikn/cargo that referenced this issue Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant