From 7e3d375ec4978d1cf4a11e80558856b3e380c845 Mon Sep 17 00:00:00 2001 From: LuuuX Date: Wed, 29 Nov 2023 11:42:50 +0800 Subject: [PATCH] Fix issue-11010 --- src/cargo/ops/cargo_add/mod.rs | 28 +++++++++++++ src/cargo/util/toml_mut/dependency.rs | 41 +++++++++++++++++++ src/cargo/util/toml_mut/manifest.rs | 19 +++++++++ .../change_rename_target/out/Cargo.toml | 3 ++ .../out/primary/Cargo.toml | 3 ++ .../cargo_add/optional/out/Cargo.toml | 4 ++ .../out/primary/Cargo.toml | 3 ++ .../out/primary/Cargo.toml | 3 ++ .../overwrite_name_noop/out/Cargo.toml | 3 ++ .../out/Cargo.toml | 4 ++ .../overwrite_optional/out/Cargo.toml | 4 ++ .../in/Cargo.toml | 12 ++++++ .../in/src/lib.rs | 0 .../overwrite_optional_with_optional/mod.rs | 38 +++++++++++++++++ .../out/Cargo.toml | 12 ++++++ .../stderr.log | 3 ++ .../stdout.log | 0 .../overwrite_path_noop/out/Cargo.toml | 3 ++ .../out/primary/Cargo.toml | 3 ++ .../out/Cargo.toml | 3 ++ .../overwrite_version_with_git/out/Cargo.toml | 3 ++ .../out/primary/Cargo.toml | 3 ++ 22 files changed, 195 insertions(+) create mode 100644 tests/testsuite/cargo_add/overwrite_optional_with_optional/in/Cargo.toml create mode 100644 tests/testsuite/cargo_add/overwrite_optional_with_optional/in/src/lib.rs create mode 100644 tests/testsuite/cargo_add/overwrite_optional_with_optional/mod.rs create mode 100644 tests/testsuite/cargo_add/overwrite_optional_with_optional/out/Cargo.toml create mode 100644 tests/testsuite/cargo_add/overwrite_optional_with_optional/stderr.log create mode 100644 tests/testsuite/cargo_add/overwrite_optional_with_optional/stdout.log diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index 5afc8d0c9d81..dd5572c173a6 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -7,6 +7,7 @@ use std::collections::BTreeSet; use std::collections::VecDeque; use std::fmt::Write; use std::path::Path; +use std::str::FromStr; use anyhow::Context as _; use cargo_util::paths; @@ -196,6 +197,13 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<( print_dep_table_msg(&mut options.config.shell(), &dep)?; manifest.insert_into_table(&dep_table, &dep)?; + if let Some(option) = dep.optional { + let rust_version_check = + check_rust_version_for_optional_dependency(options.spec.rust_version())?; + if option && rust_version_check { + manifest.insert_into_feature_table(&dep)?; + } + } manifest.gc_dep(dep.toml_key()); } @@ -469,6 +477,26 @@ fn check_invalid_ws_keys(toml_key: &str, arg: &DepOp) -> CargoResult<()> { Ok(()) } +/// When the `--optional` option is added using `cargo add`, we need to +/// check the current rust-version. As the `dep:` syntax is only avaliable +/// starting with Rust 1.60.0 +/// +/// `true` means that the rust-version is None or the rust-version is higher +/// than the version needed. +/// +/// Note: Previous versions can only use the implicit feature name. +fn check_rust_version_for_optional_dependency( + rust_version: Option<&RustVersion>, +) -> CargoResult { + match rust_version { + Some(version) => { + let syntax_support_version = RustVersion::from_str("1.60.0")?; + Ok(version.cmp(&syntax_support_version).is_ge()) + } + None => Ok(true), + } +} + /// Provide the existing dependency for the target table /// /// If it doesn't exist but exists in another table, let's use that as most likely users diff --git a/src/cargo/util/toml_mut/dependency.rs b/src/cargo/util/toml_mut/dependency.rs index 88298fa8d548..5386aec5188e 100644 --- a/src/cargo/util/toml_mut/dependency.rs +++ b/src/cargo/util/toml_mut/dependency.rs @@ -921,6 +921,47 @@ impl Display for WorkspaceSource { } } +impl Dependency { + /// Convert denpency as feature to TOML. + /// Panics if the path is relative + pub fn to_toml_as_feature(&self, crate_root: &Path) -> toml_edit::Item { + assert!( + crate_root.is_absolute(), + "Absolute path needed, got: {}", + crate_root.display() + ); + let features: toml_edit::Value = vec![self.as_feature()].iter().collect(); + toml_edit::value(features) + } + + pub fn as_feature(&self) -> String { + if let Some(rename) = &self.rename { + format!("dep:{}", rename) + } else { + format!("dep:{}", self.name) + } + } + + /// Check whether `dep:` is defined in the features section. + /// + /// `true` if there is a define `dep:` + /// `false` if `dep:` is not used ever. + /// + /// Make sure that `dep:` is included in one of features in the features table. + pub fn is_active_as_feature(&self, item: &mut toml_edit::Item) -> bool { + item.as_table_like_mut().unwrap().iter().any(|(_, values)| { + if let Some(values) = &values.as_array() { + values.iter().any(|value| match value.as_str() { + Some(val) => val.to_string().eq(&self.as_feature()), + None => false, + }) + } else { + false + } + }) + } +} + #[cfg(test)] mod tests { use std::path::Path; diff --git a/src/cargo/util/toml_mut/manifest.rs b/src/cargo/util/toml_mut/manifest.rs index e859af2153a3..458d6cd349d6 100644 --- a/src/cargo/util/toml_mut/manifest.rs +++ b/src/cargo/util/toml_mut/manifest.rs @@ -363,6 +363,25 @@ impl LocalManifest { Ok(()) } + /// Add feature entry to a Cargo.toml. + pub fn insert_into_feature_table(&mut self, dep: &Dependency) -> CargoResult<()> { + let crate_root = self + .path + .parent() + .expect("manifest path is absolute") + .to_owned(); + let dep_key = dep.toml_key(); + let table = self.get_table_mut(&vec![String::from("features")])?; + + // Check whether `dep:` is defined in the [features] section. + if !dep.is_active_as_feature(table) { + let new_feature = dep.to_toml_as_feature(&crate_root); + table[dep_key] = new_feature; + } + + Ok(()) + } + /// Remove entry from a Cargo.toml. pub fn remove_from_table(&mut self, table_path: &[String], name: &str) -> CargoResult<()> { let parent_table = self.get_table_mut(table_path)?; diff --git a/tests/testsuite/cargo_add/change_rename_target/out/Cargo.toml b/tests/testsuite/cargo_add/change_rename_target/out/Cargo.toml index 70cd318268cf..bc29fac8ebdd 100644 --- a/tests/testsuite/cargo_add/change_rename_target/out/Cargo.toml +++ b/tests/testsuite/cargo_add/change_rename_target/out/Cargo.toml @@ -6,3 +6,6 @@ version = "0.0.0" [dependencies] some-package = { package = "my-package2", version = "99999.0.0", optional = true } + +[features] +some-package = ["dep:some-package"] diff --git a/tests/testsuite/cargo_add/detect_workspace_inherit_optional/out/primary/Cargo.toml b/tests/testsuite/cargo_add/detect_workspace_inherit_optional/out/primary/Cargo.toml index 6dd7fb6d65d0..38ff36eb38b4 100644 --- a/tests/testsuite/cargo_add/detect_workspace_inherit_optional/out/primary/Cargo.toml +++ b/tests/testsuite/cargo_add/detect_workspace_inherit_optional/out/primary/Cargo.toml @@ -4,3 +4,6 @@ version = "0.0.0" [dependencies] foo = { workspace = true, optional = true } + +[features] +foo = ["dep:foo"] diff --git a/tests/testsuite/cargo_add/optional/out/Cargo.toml b/tests/testsuite/cargo_add/optional/out/Cargo.toml index eda5445c50f3..5ed3b820ef45 100644 --- a/tests/testsuite/cargo_add/optional/out/Cargo.toml +++ b/tests/testsuite/cargo_add/optional/out/Cargo.toml @@ -7,3 +7,7 @@ version = "0.0.0" [dependencies] my-package1 = { version = "99999.0.0", optional = true } my-package2 = { version = "0.4.1", optional = true } + +[features] +my-package1 = ["dep:my-package1"] +my-package2 = ["dep:my-package2"] diff --git a/tests/testsuite/cargo_add/overwrite_git_with_path/out/primary/Cargo.toml b/tests/testsuite/cargo_add/overwrite_git_with_path/out/primary/Cargo.toml index ad1205481235..27e6e175d6f6 100644 --- a/tests/testsuite/cargo_add/overwrite_git_with_path/out/primary/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_git_with_path/out/primary/Cargo.toml @@ -6,3 +6,6 @@ version = "0.0.0" [dependencies] cargo-list-test-fixture-dependency = { optional = true, path = "../dependency", version = "0.0.0" } + +[features] +cargo-list-test-fixture-dependency = ["dep:cargo-list-test-fixture-dependency"] diff --git a/tests/testsuite/cargo_add/overwrite_inherit_optional_noop/out/primary/Cargo.toml b/tests/testsuite/cargo_add/overwrite_inherit_optional_noop/out/primary/Cargo.toml index 6dd7fb6d65d0..38ff36eb38b4 100644 --- a/tests/testsuite/cargo_add/overwrite_inherit_optional_noop/out/primary/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_inherit_optional_noop/out/primary/Cargo.toml @@ -4,3 +4,6 @@ version = "0.0.0" [dependencies] foo = { workspace = true, optional = true } + +[features] +foo = ["dep:foo"] diff --git a/tests/testsuite/cargo_add/overwrite_name_noop/out/Cargo.toml b/tests/testsuite/cargo_add/overwrite_name_noop/out/Cargo.toml index bbaf4f5521c2..7172521917c9 100644 --- a/tests/testsuite/cargo_add/overwrite_name_noop/out/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_name_noop/out/Cargo.toml @@ -7,3 +7,6 @@ version = "0.0.0" [dependencies] your-face = { version = "0.0.0", path = "dependency", optional = true, default-features = false, features = ["nose", "mouth"], registry = "alternative" } + +[features] +your-face = ["dep:your-face"] diff --git a/tests/testsuite/cargo_add/overwrite_no_optional_with_optional/out/Cargo.toml b/tests/testsuite/cargo_add/overwrite_no_optional_with_optional/out/Cargo.toml index eda5445c50f3..5ed3b820ef45 100644 --- a/tests/testsuite/cargo_add/overwrite_no_optional_with_optional/out/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_no_optional_with_optional/out/Cargo.toml @@ -7,3 +7,7 @@ version = "0.0.0" [dependencies] my-package1 = { version = "99999.0.0", optional = true } my-package2 = { version = "0.4.1", optional = true } + +[features] +my-package1 = ["dep:my-package1"] +my-package2 = ["dep:my-package2"] diff --git a/tests/testsuite/cargo_add/overwrite_optional/out/Cargo.toml b/tests/testsuite/cargo_add/overwrite_optional/out/Cargo.toml index eda5445c50f3..5ed3b820ef45 100644 --- a/tests/testsuite/cargo_add/overwrite_optional/out/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_optional/out/Cargo.toml @@ -7,3 +7,7 @@ version = "0.0.0" [dependencies] my-package1 = { version = "99999.0.0", optional = true } my-package2 = { version = "0.4.1", optional = true } + +[features] +my-package1 = ["dep:my-package1"] +my-package2 = ["dep:my-package2"] diff --git a/tests/testsuite/cargo_add/overwrite_optional_with_optional/in/Cargo.toml b/tests/testsuite/cargo_add/overwrite_optional_with_optional/in/Cargo.toml new file mode 100644 index 000000000000..d1fe51025994 --- /dev/null +++ b/tests/testsuite/cargo_add/overwrite_optional_with_optional/in/Cargo.toml @@ -0,0 +1,12 @@ +[workspace] + +[package] +name = "cargo-list-test-fixture" +version = "0.0.0" + +[dependencies] +my-package1 = "99999.0.0" +my-package2 = "0.4.1" + +[features] +default = ["dep:my-package1", "dep:my-package2"] \ No newline at end of file diff --git a/tests/testsuite/cargo_add/overwrite_optional_with_optional/in/src/lib.rs b/tests/testsuite/cargo_add/overwrite_optional_with_optional/in/src/lib.rs new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/testsuite/cargo_add/overwrite_optional_with_optional/mod.rs b/tests/testsuite/cargo_add/overwrite_optional_with_optional/mod.rs new file mode 100644 index 000000000000..408a46ed3b3f --- /dev/null +++ b/tests/testsuite/cargo_add/overwrite_optional_with_optional/mod.rs @@ -0,0 +1,38 @@ +use cargo_test_support::compare::assert_ui; +use cargo_test_support::prelude::*; +use cargo_test_support::Project; + +use cargo_test_support::curr_dir; + +#[cargo_test] +fn case() { + cargo_test_support::registry::init(); + for name in ["my-package1", "my-package2"] { + for ver in [ + "0.1.1+my-package", + "0.2.0+my-package", + "0.2.3+my-package", + "0.4.1+my-package", + "20.0.0+my-package", + "99999.0.0+my-package", + "99999.0.0-alpha.1+my-package", + ] { + cargo_test_support::registry::Package::new(name, ver).publish(); + } + } + + let project = Project::from_template(curr_dir!().join("in")); + let project_root = project.root(); + let cwd = &project_root; + + snapbox::cmd::Command::cargo_ui() + .arg("add") + .arg_line("my-package1 my-package2@0.4.1 --optional") + .current_dir(cwd) + .assert() + .success() + .stdout_matches_path(curr_dir!().join("stdout.log")) + .stderr_matches_path(curr_dir!().join("stderr.log")); + + assert_ui().subset_matches(curr_dir!().join("out"), &project_root); +} diff --git a/tests/testsuite/cargo_add/overwrite_optional_with_optional/out/Cargo.toml b/tests/testsuite/cargo_add/overwrite_optional_with_optional/out/Cargo.toml new file mode 100644 index 000000000000..6075e6f5e7f7 --- /dev/null +++ b/tests/testsuite/cargo_add/overwrite_optional_with_optional/out/Cargo.toml @@ -0,0 +1,12 @@ +[workspace] + +[package] +name = "cargo-list-test-fixture" +version = "0.0.0" + +[dependencies] +my-package1 = { version = "99999.0.0", optional = true } +my-package2 = { version = "0.4.1", optional = true } + +[features] +default = ["dep:my-package1", "dep:my-package2"] diff --git a/tests/testsuite/cargo_add/overwrite_optional_with_optional/stderr.log b/tests/testsuite/cargo_add/overwrite_optional_with_optional/stderr.log new file mode 100644 index 000000000000..8cf4812cf172 --- /dev/null +++ b/tests/testsuite/cargo_add/overwrite_optional_with_optional/stderr.log @@ -0,0 +1,3 @@ + Updating `dummy-registry` index + Adding my-package1 v99999.0.0 to optional dependencies. + Adding my-package2 v0.4.1 to optional dependencies. diff --git a/tests/testsuite/cargo_add/overwrite_optional_with_optional/stdout.log b/tests/testsuite/cargo_add/overwrite_optional_with_optional/stdout.log new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/testsuite/cargo_add/overwrite_path_noop/out/Cargo.toml b/tests/testsuite/cargo_add/overwrite_path_noop/out/Cargo.toml index bbaf4f5521c2..7172521917c9 100644 --- a/tests/testsuite/cargo_add/overwrite_path_noop/out/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_path_noop/out/Cargo.toml @@ -7,3 +7,6 @@ version = "0.0.0" [dependencies] your-face = { version = "0.0.0", path = "dependency", optional = true, default-features = false, features = ["nose", "mouth"], registry = "alternative" } + +[features] +your-face = ["dep:your-face"] diff --git a/tests/testsuite/cargo_add/overwrite_path_with_version/out/primary/Cargo.toml b/tests/testsuite/cargo_add/overwrite_path_with_version/out/primary/Cargo.toml index a20f2095db95..c45f794912eb 100644 --- a/tests/testsuite/cargo_add/overwrite_path_with_version/out/primary/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_path_with_version/out/primary/Cargo.toml @@ -6,3 +6,6 @@ version = "0.0.0" [dependencies] cargo-list-test-fixture-dependency = { optional = true, version = "20.0" } + +[features] +cargo-list-test-fixture-dependency = ["dep:cargo-list-test-fixture-dependency"] diff --git a/tests/testsuite/cargo_add/overwrite_rename_with_rename_noop/out/Cargo.toml b/tests/testsuite/cargo_add/overwrite_rename_with_rename_noop/out/Cargo.toml index 4502292454a9..0217d4176021 100644 --- a/tests/testsuite/cargo_add/overwrite_rename_with_rename_noop/out/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_rename_with_rename_noop/out/Cargo.toml @@ -6,3 +6,6 @@ version = "0.0.0" [dependencies] a1 = { package = "versioned-package", version = "0.1.1", optional = true } + +[features] +a1 = ["dep:a1"] diff --git a/tests/testsuite/cargo_add/overwrite_version_with_git/out/Cargo.toml b/tests/testsuite/cargo_add/overwrite_version_with_git/out/Cargo.toml index 260014024143..a3eabd06559b 100644 --- a/tests/testsuite/cargo_add/overwrite_version_with_git/out/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_version_with_git/out/Cargo.toml @@ -6,3 +6,6 @@ version = "0.0.0" [dependencies] versioned-package = { version = "0.3.0", optional = true, git = "[ROOTURL]/versioned-package" } + +[features] +versioned-package = ["dep:versioned-package"] diff --git a/tests/testsuite/cargo_add/overwrite_version_with_path/out/primary/Cargo.toml b/tests/testsuite/cargo_add/overwrite_version_with_path/out/primary/Cargo.toml index 07253670afe4..bd460a11b212 100644 --- a/tests/testsuite/cargo_add/overwrite_version_with_path/out/primary/Cargo.toml +++ b/tests/testsuite/cargo_add/overwrite_version_with_path/out/primary/Cargo.toml @@ -6,3 +6,6 @@ version = "0.0.0" [dependencies] cargo-list-test-fixture-dependency = { version = "0.0.0", optional = true, path = "../dependency" } + +[features] +cargo-list-test-fixture-dependency = ["dep:cargo-list-test-fixture-dependency"]