Skip to content

Commit

Permalink
Fix issue-11010
Browse files Browse the repository at this point in the history
  • Loading branch information
LuuuXXX committed Nov 29, 2023
1 parent 35ea623 commit 0822415
Show file tree
Hide file tree
Showing 16 changed files with 171 additions and 0 deletions.
28 changes: 28 additions & 0 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,14 @@ 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(workspace.rust_version())?;
if option && rust_version_check {
let features_table: Vec<String> = vec![String::from("features")];
manifest.insert_into_feature_table(&features_table, &dep)?;
}
}
manifest.gc_dep(dep.toml_key());
}

Expand Down Expand Up @@ -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<bool> {
match rust_version {
Some(version) => {
let syntax_support_version = String::from("1.60.0");
Ok(version.to_string().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
Expand Down
77 changes: 77 additions & 0 deletions src/cargo/util/toml_mut/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,13 @@ impl Dependency {
unreachable!("Invalid dependency type: {}", item.type_name());
}
}

/// Convert dependency to Feature.
pub fn to_feature(&self) -> Feature {
let feature = Feature::new(self.name.to_string(), self.rename.to_owned());
let feature_value = feature.as_feature();
feature.add_feature(feature_value)
}
}

fn overwrite_value(
Expand Down Expand Up @@ -921,6 +928,76 @@ impl Display for WorkspaceSource {
}
}

pub struct Feature {
/// The name of the feature (as it is set in its `Cargo.toml`).
pub name: String,
/// List of features to add.
pub features: IndexSet<String>,
/// this is the new name for the dependency
/// as a string. None if it is not renamed.
pub rename: Option<String>,
}

impl Feature {
pub fn new(name: String, rename: Option<String>) -> Self {
Self {
name: name,
features: IndexSet::new(),
rename: rename,
}
}

pub fn add_feature(mut self, value: String) -> Self {
self.features.insert(value);
self
}

pub fn as_feature(&self) -> String {
if let Some(rename) = &self.rename {
format!("dep:{}", rename)
} else {
format!("dep:{}", self.name)
}
}

/// Convert feature to TOML.
/// Panics if the path is relative
pub fn to_toml(&self, crate_root: &Path) -> toml_edit::Item {
assert!(
crate_root.is_absolute(),
"Absolute path needed, got: {}",
crate_root.display()
);
let features: toml_edit::Value = self.features.iter().cloned().collect();
toml_edit::value(features)
}

/// Check whether `dep:<dep>` is defined in the features section.
///
/// `true` if there is a define `dep:<dep>`
/// `false` if `dep:<dep>` is not used ever.
///
/// Make sure that `dep:<dep>` is included in one of features in the features table.
pub fn check_duplicate_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
}
})
}
}

impl std::fmt::Display for Feature {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
self.name.fmt(f)
}
}

#[cfg(test)]
mod tests {
use std::path::Path;
Expand Down
24 changes: 24 additions & 0 deletions src/cargo/util/toml_mut/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,30 @@ impl LocalManifest {
Ok(())
}

/// Add feature entry to a Cargo.toml.
pub fn insert_into_feature_table(
&mut self,
table_path: &[String],
dep: &Dependency,
) -> CargoResult<()> {
let crate_root = self
.path
.parent()
.expect("manifest path is absolute")
.to_owned();
let dep_key = dep.toml_key();
let feature = dep.to_feature();
let table = self.get_table_mut(table_path)?;

// Check whether `dep:<dep>` is defined in the [features] section.
if !feature.check_duplicate_feature(table) {
let new_feature = feature.to_toml(&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)?;
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"]
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 0822415

Please sign in to comment.