Skip to content

Commit

Permalink
Auto merge of #13765 - dohse:fix-13702, r=epage
Browse files Browse the repository at this point in the history
Fix cargo add behaving different when translating package name

Fixes #13702
Fixes #10680

TODOs

- [x] ~Fuzzy match registry dependencies in `cargo remove` as well~
     `cargo remove` does not need fuzzy matching, because there is no unexpected behavior for the user
- [x] ~Don't duplicate name permutation generation~
     Unsure whether this is worth it
- [ ] Shall we reject cases where multiple different permutations match?
- [x] Add comments that explain the behavior
  • Loading branch information
bors committed Sep 4, 2024
2 parents 9627da3 + 9391bfb commit 40b6638
Show file tree
Hide file tree
Showing 26 changed files with 310 additions and 2 deletions.
43 changes: 41 additions & 2 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,9 @@ fn resolve_dependency(
};
selected_dep = populate_dependency(selected_dep, arg);

let old_dep = get_existing_dependency(manifest, selected_dep.toml_key(), section)?;
let lookup = |dep_key: &_| get_existing_dependency(manifest, dep_key, section);
let old_dep = fuzzy_lookup(&mut selected_dep, lookup, gctx)?;

let mut dependency = if let Some(mut old_dep) = old_dep.clone() {
if old_dep.name != selected_dep.name {
// Assuming most existing keys are not relevant when the package changes
Expand All @@ -383,7 +385,8 @@ fn resolve_dependency(
if dependency.source().is_none() {
// Checking for a workspace dependency happens first since a member could be specified
// in the workspace dependencies table as a dependency
if let Some(_dep) = find_workspace_dep(dependency.toml_key(), ws.root_manifest()).ok() {
let lookup = |toml_key: &_| Ok(find_workspace_dep(toml_key, ws.root_manifest()).ok());
if let Some(_dep) = fuzzy_lookup(&mut dependency, lookup, gctx)? {
dependency = dependency.set_source(WorkspaceSource::new());
} else if let Some(package) = ws.members().find(|p| p.name().as_str() == dependency.name) {
// Only special-case workspaces when the user doesn't provide any extra
Expand Down Expand Up @@ -449,6 +452,42 @@ fn resolve_dependency(
Ok(dependency)
}

fn fuzzy_lookup(
dependency: &mut Dependency,
lookup: impl Fn(&str) -> CargoResult<Option<Dependency>>,
gctx: &GlobalContext,
) -> CargoResult<Option<Dependency>> {
if let Some(rename) = dependency.rename() {
// Manually implement `toml_key` to restrict fuzzy lookups to only package names to mirror `PackageRegistry::query()`
return lookup(rename);
}

for name_permutation in [
dependency.name.clone(),
dependency.name.replace('-', "_"),
dependency.name.replace('_', "-"),
] {
let Some(dep) = lookup(&name_permutation)? else {
continue;
};

if dependency.name != name_permutation {
// Mirror the fuzzy matching policy of `PackageRegistry::query()`
if !matches!(dep.source, Some(Source::Registry(_))) {
continue;
}
gctx.shell().warn(format!(
"translating `{}` to `{}`",
dependency.name, &name_permutation,
))?;
dependency.name = name_permutation;
}
return Ok(Some(dep));
}

Ok(None)
}

/// When { workspace = true } you cannot define other keys that configure
/// the source of the dependency such as `version`, `registry`, `registry-index`,
/// `path`, `git`, `branch`, `tag`, `rev`, or `package`. You can also not define
Expand Down
11 changes: 11 additions & 0 deletions tests/testsuite/cargo_add/add_workspace_non_fuzzy/in/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[workspace]
members = ["primary", "fuzzy_name"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[package]
name = "fuzzy_name"
version = "0.0.0"
edition = "2015"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "bar"
version = "0.0.0"
edition = "2015"

[dependencies]
fuzzy_name = { path = "../fuzzy_name" }
Empty file.
24 changes: 24 additions & 0 deletions tests/testsuite/cargo_add/add_workspace_non_fuzzy/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
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::str;
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")
.args(["fuzzy-name", "-p", "bar"])
.current_dir(cwd)
.assert()
.code(101)
.stdout_eq(str![""])
.stderr_eq(file!["stderr.term.svg"]);

assert_ui().subset_matches(current_dir!().join("out"), &project_root);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[workspace]
members = ["primary", "fuzzy_name"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "bar"
version = "0.0.0"
edition = "2015"

[dependencies]
fuzzy_name = { path = "../fuzzy_name" }
Empty file.
30 changes: 30 additions & 0 deletions tests/testsuite/cargo_add/add_workspace_non_fuzzy/stderr.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary"]

[workspace.dependencies]
fuzzy_dependency = "1.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[package]
name = "bar"
version = "0.0.0"
edition = "2015"
Empty file.
29 changes: 29 additions & 0 deletions tests/testsuite/cargo_add/detect_workspace_inherit_fuzzy/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
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::registry::Package;
use cargo_test_support::str;
use cargo_test_support::Project;

#[cargo_test]
fn case() {
cargo_test_support::registry::init();

Package::new("fuzzy_dependency", "1.0.0").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")
.args(["fuzzy-dependency", "-p", "bar"])
.current_dir(cwd)
.assert()
.success()
.stdout_eq(str![""])
.stderr_eq(file!["stderr.term.svg"]);

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]
members = ["primary"]

[workspace.dependencies]
fuzzy_dependency = "1.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "bar"
version = "0.0.0"
edition = "2015"

[dependencies]
fuzzy_dependency.workspace = true
Empty file.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions tests/testsuite/cargo_add/features_fuzzy/in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[workspace]

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

[dependencies]
your_face = { version = "99999.0.0", features = ["eyes"] }
Empty file.
32 changes: 32 additions & 0 deletions tests/testsuite/cargo_add/features_fuzzy/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
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::str;
use cargo_test_support::Project;

#[cargo_test]
fn case() {
cargo_test_support::registry::init();
cargo_test_support::registry::Package::new("your_face", "99999.0.0+my-package")
.feature("nose", &[])
.feature("mouth", &[])
.feature("eyes", &[])
.feature("ears", &[])
.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("your-face")
.current_dir(cwd)
.assert()
.success()
.stdout_eq(str![""])
.stderr_eq(file!["stderr.term.svg"]);

assert_ui().subset_matches(current_dir!().join("out"), &project_root);
}
9 changes: 9 additions & 0 deletions tests/testsuite/cargo_add/features_fuzzy/out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[workspace]

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

[dependencies]
your_face = { version = "99999.0.0", features = ["eyes"] }
45 changes: 45 additions & 0 deletions tests/testsuite/cargo_add/features_fuzzy/stderr.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions tests/testsuite/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod add_no_vendored_package_with_alter_registry;
mod add_no_vendored_package_with_vendor;
mod add_normalized_name_external;
mod add_toolchain;
mod add_workspace_non_fuzzy;
mod build;
mod build_prefer_existing_version;
mod change_rename_target;
Expand All @@ -13,6 +14,7 @@ mod deprecated_default_features;
mod deprecated_section;
mod detect_workspace_inherit;
mod detect_workspace_inherit_features;
mod detect_workspace_inherit_fuzzy;
mod detect_workspace_inherit_optional;
mod detect_workspace_inherit_public;
mod dev;
Expand All @@ -24,6 +26,7 @@ mod features;
mod features_activated_over_limit;
mod features_deactivated_over_limit;
mod features_empty;
mod features_fuzzy;
mod features_multiple_occurrences;
mod features_preserve;
mod features_spaced_values;
Expand Down

0 comments on commit 40b6638

Please sign in to comment.