Skip to content

Commit

Permalink
Auto merge of #13281 - LuuuXXX:issue-10729, r=epage
Browse files Browse the repository at this point in the history
fix(add): Improve error when adding registry packages while vendored

### **What does this PR try to resolve?**

When a vendored directory is established, cargo add no longer adds new packages. Instead, it tries to translate a package name into a package that already exists in the vendored directory.
[More details](#10729 (comment))

Since `@epage` has done most of the work, here I do the rest of the finishing work.

Improves the error from #10729

### **How should we test and review this PR?**

The implementation procedure is as follows:
#10729 (comment)

Test steps:
1. Try to get an arbitrary crate and execute `cargo vendor` command.
2. Configure the vendor directory in .cargo/config.toml.
3. Add `alter-registry` to the config.toml file.
```
[registries]
alter-registry= { index = "XXX" }
```
4. run the same `cargo add` command.
```
cargo add another-crate --registry alter-registry
```
  • Loading branch information
bors committed Feb 22, 2024
2 parents 08c173a + 640c077 commit e08a813
Show file tree
Hide file tree
Showing 28 changed files with 134 additions and 10 deletions.
3 changes: 2 additions & 1 deletion crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ pub fn resolve_with_global_context_raw(
for summary in self.list.iter() {
let matched = match kind {
QueryKind::Exact => dep.matches(summary),
QueryKind::Fuzzy => true,
QueryKind::Alternatives => true,
QueryKind::Normalized => true,
};
if matched {
self.used.insert(summary.package_id());
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ pub(super) fn activation_error(
// Maybe the user mistyped the name? Like `dep-thing` when `Dep_Thing`
// was meant. So we try asking the registry for a `fuzzy` search for suggestions.
let candidates = loop {
match registry.query_vec(&new_dep, QueryKind::Fuzzy) {
match registry.query_vec(&new_dep, QueryKind::Alternatives) {
Poll::Ready(Ok(candidates)) => break candidates,
Poll::Ready(Err(e)) => return to_resolve_err(e),
Poll::Pending => match registry.block_until_ready() {
Expand Down
6 changes: 3 additions & 3 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ fn get_latest_dependency(
}
MaybeWorkspace::Other(query) => {
let possibilities = loop {
match registry.query_vec(&query, QueryKind::Fuzzy) {
match registry.query_vec(&query, QueryKind::Normalized) {
std::task::Poll::Ready(res) => {
break res?;
}
Expand Down Expand Up @@ -711,7 +711,7 @@ fn select_package(
MaybeWorkspace::Other(query) => {
let possibilities = loop {
// Exact to avoid returning all for path/git
match registry.query_vec(&query, QueryKind::Exact) {
match registry.query_vec(&query, QueryKind::Normalized) {
std::task::Poll::Ready(res) => {
break res?;
}
Expand Down Expand Up @@ -938,7 +938,7 @@ fn populate_available_features(
}

let possibilities = loop {
match registry.query_vec(&query, QueryKind::Exact) {
match registry.query_vec(&query, QueryKind::Normalized) {
std::task::Poll::Ready(res) => {
break res?;
}
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/sources/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ impl<'gctx> Source for DirectorySource<'gctx> {
let packages = self.packages.values().map(|p| &p.0);
let matches = packages.filter(|pkg| match kind {
QueryKind::Exact => dep.matches(pkg.summary()),
QueryKind::Fuzzy => true,
QueryKind::Alternatives => true,
QueryKind::Normalized => dep.matches(pkg.summary()),
});
for summary in matches.map(|pkg| pkg.summary().clone()) {
f(IndexSummary::Candidate(summary));
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/sources/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,8 @@ impl<'gctx> Source for PathSource<'gctx> {
for s in self.packages.iter().map(|p| p.summary()) {
let matched = match kind {
QueryKind::Exact => dep.matches(s),
QueryKind::Fuzzy => true,
QueryKind::Alternatives => true,
QueryKind::Normalized => dep.matches(s),
};
if matched {
f(IndexSummary::Candidate(s.clone()))
Expand Down
5 changes: 3 additions & 2 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,8 @@ impl<'gctx> Source for RegistrySource<'gctx> {
.query_inner(dep.package_name(), &req, &mut *self.ops, &mut |s| {
let matched = match kind {
QueryKind::Exact => dep.matches(s.as_summary()),
QueryKind::Fuzzy => true,
QueryKind::Alternatives => true,
QueryKind::Normalized => true,
};
if !matched {
return;
Expand Down Expand Up @@ -830,7 +831,7 @@ impl<'gctx> Source for RegistrySource<'gctx> {
return Poll::Ready(Ok(()));
}
let mut any_pending = false;
if kind == QueryKind::Fuzzy {
if kind == QueryKind::Alternatives || kind == QueryKind::Normalized {
// Attempt to handle misspellings by searching for a chain of related
// names to the original name. The resolver will later
// reject any candidates that have the wrong name, and with this it'll
Expand Down
5 changes: 4 additions & 1 deletion src/cargo/sources/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ pub enum QueryKind {
/// Path/Git sources may return all dependencies that are at that URI,
/// whereas an `Registry` source may return dependencies that have the same
/// canonicalization.
Fuzzy,
Alternatives,
/// Match a denpendency in all ways and will normalize the package name.
/// Each source defines what normalizing means.
Normalized,
}

/// A download status that represents if a [`Package`] has already been
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[source.crates-io]
replace-with = "vendored-sources"

[source.vendored-sources]
directory = "./vendor"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"files":{}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]

[package]
name = "aa"
version = "0.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::current_dir;
use cargo_test_support::file;
use cargo_test_support::prelude::*;
use cargo_test_support::Project;

#[cargo_test]
fn case() {
cargo_test_support::registry::alt_init();
cargo_test_support::registry::Package::new("linked-hash-map", "0.5.4")
.feature("clippy", &[])
.feature("heapsize", &[])
.feature("heapsize_impl", &[])
.feature("nightly", &[])
.feature("serde", &[])
.feature("serde_impl", &[])
.feature("serde_test", &[])
.alternative(true)
.publish();

let project = Project::from_template(current_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("add")
.arg_line("linked_hash_map --registry alternative")
.current_dir(cwd)
.assert()
.success()
.stdout_matches(file!["stdout.log"])
.stderr_matches(file!["stderr.log"]);

assert_ui().subset_matches(current_dir!().join("out"), &project_root);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"

[dependencies]
linked-hash-map = { version = "0.5.4", registry = "alternative" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Updating `alternative` index
warning: translating `linked_hash_map` to `linked-hash-map`
Adding linked-hash-map v0.5.4 to dependencies
Features:
- clippy
- heapsize
- heapsize_impl
- nightly
- serde
- serde_impl
- serde_test
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[source.crates-io]
replace-with = "vendored-sources"

[source.vendored-sources]
directory = "./vendor"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"files":{}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]

[package]
name = "aa"
version = "0.0.0"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::current_dir;
use cargo_test_support::file;
use cargo_test_support::prelude::*;
use cargo_test_support::Project;

#[cargo_test]
fn case() {
let project = Project::from_template(current_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("add")
.arg_line("cbindgen")
.current_dir(cwd)
.assert()
.failure()
.stdout_matches(file!["stdout.log"])
.stderr_matches(file!["stderr.log"]);

assert_ui().subset_matches(current_dir!().join("out"), &project_root);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]

[package]
name = "cargo-list-test-fixture"
version = "0.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error: the crate `cbindgen` could not be found in registry index.
Empty file.
2 changes: 2 additions & 0 deletions tests/testsuite/cargo_add/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
mod add_basic;
mod add_multiple;
mod add_no_vendored_package_with_alter_registry;
mod add_no_vendored_package_with_vendor;
mod add_normalized_name_external;
mod add_toolchain;
mod build;
Expand Down

0 comments on commit e08a813

Please sign in to comment.