Skip to content

Commit

Permalink
Auto merge of #13071 - LuuuXXX:issue-11010, r=epage
Browse files Browse the repository at this point in the history
Have cargo add --optional <dep> create a <dep> = "dep:<dep> feature

### What does this PR try to resolve?

`cargo add --optional <dep>` would create a `<dep> = "dep:<dep>` feature iff

- `rust-version` is unset or is new enough for the syntax
- `dep:<dep>` doesn't already exist

Fixes #11010

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

As the `dep:` syntax is only available starting with Rust 1.60. this pr maintains the previous usage convention in the earlier version.

run

```shell
cargo add --optional <dep>
```

with different rust-version to verify.
  • Loading branch information
bors committed Dec 1, 2023
2 parents 60e65ee + 036a037 commit 78109c0
Show file tree
Hide file tree
Showing 21 changed files with 139 additions and 1 deletion.
35 changes: 35 additions & 0 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -196,6 +197,20 @@ 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 dep.optional == Some(true) {
let is_namespaced_features_supported =
check_rust_version_for_optional_dependency(options.spec.rust_version())?;
if is_namespaced_features_supported {
let dep_key = dep.toml_key();
if !manifest.is_explicit_dep_activation(dep_key) {
let table = manifest.get_table_mut(&[String::from("features")])?;
let dep_name = dep.rename.as_deref().unwrap_or(&dep.name);
let new_feature: toml_edit::Value =
[format!("dep:{dep_name}")].iter().collect();
table[dep_key] = toml_edit::value(new_feature);
}
}
}
manifest.gc_dep(dep.toml_key());
}

Expand Down Expand Up @@ -472,6 +487,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<bool> {
match rust_version {
Some(version) => {
let syntax_support_version = RustVersion::from_str("1.60.0")?;
Ok(&syntax_support_version <= version)
}
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
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/toml_mut/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ impl LocalManifest {
}
}

fn is_explicit_dep_activation(&self, dep_key: &str) -> bool {
pub fn is_explicit_dep_activation(&self, dep_key: &str) -> bool {
if let Some(toml_edit::Item::Table(feature_table)) = self.data.as_table().get("features") {
for values in feature_table
.iter()
Expand Down
3 changes: 3 additions & 0 deletions tests/testsuite/cargo_add/change_rename_target/out/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@ version = "0.0.0"

[dependencies]
foo = { workspace = true, optional = true }

[features]
foo = ["dep:foo"]
4 changes: 4 additions & 0 deletions tests/testsuite/cargo_add/optional/out/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@ version = "0.0.0"

[dependencies]
foo = { workspace = true, optional = true }

[features]
foo = ["dep:foo"]
3 changes: 3 additions & 0 deletions tests/testsuite/cargo_add/overwrite_name_noop/out/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
4 changes: 4 additions & 0 deletions tests/testsuite/cargo_add/overwrite_optional/out/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[workspace]

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

[dependencies]
my-package1 = { version = "99999.0.0", optional = true }

[features]
default = ["dep:my-package1"]
Empty file.
36 changes: 36 additions & 0 deletions tests/testsuite/cargo_add/overwrite_optional_with_optional/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
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 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("my-package1", 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 --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);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[workspace]

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

[dependencies]
my-package1 = { version = "99999.0.0", optional = true }

[features]
default = ["dep:my-package1"]
Original file line number Diff line number Diff line change
@@ -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.
Empty file.
3 changes: 3 additions & 0 deletions tests/testsuite/cargo_add/overwrite_path_noop/out/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ version = "0.0.0"

[dependencies]
a1 = { package = "versioned-package", version = "0.1.1", optional = true }

[features]
a1 = ["dep:a1"]
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

0 comments on commit 78109c0

Please sign in to comment.