Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show the merged set of activated features to the user when inheriting… #1

Merged
merged 1 commit into from
Apr 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/cargo/ops/cargo_add/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ pub struct Dependency {
pub features: Option<IndexSet<String>>,
/// Whether default features are enabled
pub default_features: Option<bool>,
/// List of features inherited from a workspace dependency
pub inherited_features: Option<IndexSet<String>>,

/// Where the dependency comes from
pub source: Option<Source>,
Expand All @@ -50,6 +52,7 @@ impl Dependency {
optional: None,
features: None,
default_features: None,
inherited_features: None,
source: None,
registry: None,
rename: None,
Expand Down Expand Up @@ -152,6 +155,12 @@ impl Dependency {
self
}

/// Set features as an array of string (does some basic parsing)
pub fn set_inherited_features(mut self, features: IndexSet<String>) -> Self {
self.inherited_features = Some(features);
self
}

/// Get the dependency source
pub fn source(&self) -> Option<&Source> {
self.source.as_ref()
Expand Down Expand Up @@ -350,6 +359,7 @@ impl Dependency {
features,
available_features,
optional,
inherited_features: None,
};
Ok(dep)
} else {
Expand Down
36 changes: 22 additions & 14 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,23 +106,27 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(
)
}
}

let available_features = dep
.available_features
.keys()
.map(|s| s.as_ref())
.collect::<BTreeSet<&str>>();
let mut unknown_features: Vec<&str> = Vec::new();
if let Some(req_feats) = dep.features.as_ref() {
let req_feats: BTreeSet<_> = req_feats.iter().map(|s| s.as_str()).collect();

let available_features = dep
.available_features
.keys()
.map(|s| s.as_ref())
.collect::<BTreeSet<&str>>();

let mut unknown_features: Vec<&&str> =
req_feats.difference(&available_features).collect();
unknown_features.sort();

if !unknown_features.is_empty() {
anyhow::bail!("unrecognized features: {unknown_features:?}");
}
unknown_features.extend(req_feats.difference(&available_features).copied());
}
if let Some(inherited_features) = dep.inherited_features.as_ref() {
let inherited_features: BTreeSet<_> =
inherited_features.iter().map(|s| s.as_str()).collect();
unknown_features.extend(inherited_features.difference(&available_features).copied());
}
unknown_features.sort();
if !unknown_features.is_empty() {
anyhow::bail!("unrecognized features: {unknown_features:?}");
}

manifest.insert_into_table(&dep_table, &dep)?;
manifest.gc_dep(dep.toml_key());
}
Expand Down Expand Up @@ -564,6 +568,9 @@ fn populate_available_features(
dependency.toml_key(),
dep_item,
)?;
if let Some(features) = dep.features.clone() {
dependency = dependency.set_inherited_features(features);
}
let query = dep.query(config)?;
match query {
MaybeWorkspace::Workspace(_) => {
Expand Down Expand Up @@ -645,6 +652,7 @@ fn print_msg(shell: &mut Shell, dep: &Dependency, section: &[String]) -> CargoRe
if dep.default_features().unwrap_or(true) {
activated.insert("default");
}
activated.extend(dep.inherited_features.iter().flatten().map(|s| s.as_str()));
let mut walk: VecDeque<_> = activated.iter().cloned().collect();
while let Some(next) = walk.pop_front() {
walk.extend(
Expand Down
5 changes: 5 additions & 0 deletions tests/snapshots/add/merge_activated_features.in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency", features = ["merge"] }
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "foo"
version = "0.0.0"

[features]
default-base = []
default-test-base = []
default-merge-base = []
default = ["default-base", "default-test-base", "default-merge-base"]
test-base = []
test = ["test-base", "default-test-base"]
merge-base = []
merge = ["merge-base", "default-merge-base"]
unrelated = []
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"

[dependencies]
foo = { workspace = true, features = ["test"] }
Empty file.
5 changes: 5 additions & 0 deletions tests/snapshots/add/merge_activated_features.out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency", features = ["merge"] }
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "foo"
version = "0.0.0"

[features]
default-base = []
default-test-base = []
default-merge-base = []
default = ["default-base", "default-test-base", "default-merge-base"]
test-base = []
test = ["test-base", "default-test-base"]
merge-base = []
merge = ["merge-base", "default-merge-base"]
unrelated = []
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"

[dependencies]
foo = { workspace = true, features = ["test"] }
10 changes: 10 additions & 0 deletions tests/snapshots/add/merge_activated_features.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Adding foo (workspace) to dependencies.
Features:
+ default-base
+ default-merge-base
+ default-test-base
+ merge
+ merge-base
+ test
+ test-base
- unrelated
Empty file.
5 changes: 5 additions & 0 deletions tests/snapshots/add/unknown_inherited_feature.in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency", features = ["not_recognized"] }
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "foo"
version = "0.0.0"

[features]
default-base = []
default-test-base = []
default-merge-base = []
default = ["default-base", "default-test-base", "default-merge-base"]
test-base = []
test = ["test-base", "default-test-base"]
merge-base = []
merge = ["merge-base", "default-merge-base"]
unrelated = []
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"

[dependencies]
foo = { workspace = true, features = ["test"] }
Empty file.
5 changes: 5 additions & 0 deletions tests/snapshots/add/unknown_inherited_feature.out/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
members = ["primary", "dependency"]

[workspace.dependencies]
foo = { version = "0.0.0", path = "./dependency", features = ["not_recognized"] }
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "foo"
version = "0.0.0"

[features]
default-base = []
default-test-base = []
default-merge-base = []
default = ["default-base", "default-test-base", "default-merge-base"]
test-base = []
test = ["test-base", "default-test-base"]
merge-base = []
merge = ["merge-base", "default-merge-base"]
unrelated = []
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
cargo-features = ["workspace-inheritance"]

[package]
name = "bar"
version = "0.0.0"

[dependencies]
foo = { workspace = true, features = ["test"] }
12 changes: 12 additions & 0 deletions tests/snapshots/add/unknown_inherited_feature.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Adding foo (workspace) to dependencies.
Features:
+ default-base
+ default-merge-base
+ default-test-base
+ not_recognized
+ test
+ test-base
- merge
- merge-base
- unrelated
error: unrecognized features: ["not_recognized"]
Empty file.
44 changes: 44 additions & 0 deletions tests/testsuite/cargo_add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1168,6 +1168,28 @@ fn manifest_path_package() {
);
}

#[cargo_test]
fn merge_activated_features() {
let project = Project::from_template("tests/snapshots/add/merge_activated_features.in");
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo()
.masquerade_as_nightly_cargo()
.arg("add")
.args(["foo", "-p", "bar"])
.current_dir(cwd)
.assert()
.success()
.stdout_matches_path("tests/snapshots/add/merge_activated_features.stdout")
.stderr_matches_path("tests/snapshots/add/merge_activated_features.stderr");

assert().subset_matches(
"tests/snapshots/add/merge_activated_features.out",
&project_root,
);
}

#[cargo_test]
fn multiple_conflicts_with_features() {
init_registry();
Expand Down Expand Up @@ -2019,6 +2041,28 @@ fn target_cfg() {
assert().subset_matches("tests/snapshots/add/target_cfg.out", &project_root);
}

#[cargo_test]
fn unknown_inherited_feature() {
let project = Project::from_template("tests/snapshots/add/unknown_inherited_feature.in");
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo()
.masquerade_as_nightly_cargo()
.arg("add")
.args(["foo", "-p", "bar"])
.current_dir(cwd)
.assert()
.failure()
.stdout_matches_path("tests/snapshots/add/unknown_inherited_feature.stdout")
.stderr_matches_path("tests/snapshots/add/unknown_inherited_feature.stderr");

assert().subset_matches(
"tests/snapshots/add/unknown_inherited_feature.out",
&project_root,
);
}

#[cargo_test]
fn vers() {
init_registry();
Expand Down