From 5030f231ba3a2508b4a2c51e9ecbec3d1b08b3e8 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 2 Mar 2024 18:24:52 -0800 Subject: [PATCH] Clarify warning for using `features` or `default-features` in `patch` When attempting to substitute a local version of a dependency, since the `patch` section uses syntax similar to a dependency (and the documentation even says "The syntax is similar to the `[dependencies]` section", it's natural to assume that other things from `[dependencies]` also work, such as `features` or `default-features`. Attempting to use them produces this warning: > patch for `crate-name-here` uses the features mechanism. > default-features and features will not take effect because the patch dependency does not support this mechanism The phrasing "the patch dependency does not support this mechanism" makes it seem at first glance like `patch` just doesn't support this at all. But the actual problem is that the user needs to move the `features`/`default-features` keys to an entry in `dependencies` instead. Modify the message, to make this more obvious. Modify the corresponding warning for `replace` as well. Update tests accordingly. --- src/cargo/core/registry.rs | 22 +++++++++++++++++----- src/cargo/ops/resolve.rs | 26 ++++++++++++++++++-------- tests/testsuite/patch.rs | 36 ++++++++++++++++++++++++------------ tests/testsuite/replace.rs | 18 ++++++++++++------ 4 files changed, 71 insertions(+), 31 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index ac91c0948c5..d116c1e91fe 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -379,11 +379,23 @@ impl<'gctx> PackageRegistry<'gctx> { dep.package_name() ); - if dep.features().len() != 0 || !dep.uses_default_features() { - self.source_config.gctx().shell().warn(format!( - "patch for `{}` uses the features mechanism. \ - default-features and features will not take effect because the patch dependency does not support this mechanism", - dep.package_name() + let mut unused_fields = Vec::new(); + if dep.features().len() != 0 { + unused_fields.push("`features`"); + } + if !dep.uses_default_features() { + unused_fields.push("`default-features`") + } + if !unused_fields.is_empty() { + let mut shell = self.source_config.gctx().shell(); + shell.warn(format!( + "unused field in patch for `{}`: {}", + dep.package_name(), + unused_fields.join(", ") + ))?; + shell.note(format!( + "configure {} in the `dependencies` entry", + unused_fields.join(", ") ))?; } diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index d9d10e73550..c741ee4be37 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -209,14 +209,24 @@ pub fn resolve_ws_with_opts<'gctx>( .warn(format!("package replacement is not used: {}", replace_spec))? } - if dep.features().len() != 0 || !dep.uses_default_features() { - ws.gctx() - .shell() - .warn(format!( - "replacement for `{}` uses the features mechanism. \ - default-features and features will not take effect because the replacement dependency does not support this mechanism", - dep.package_name() - ))? + let mut unused_fields = Vec::new(); + if dep.features().len() != 0 { + unused_fields.push("`features`"); + } + if !dep.uses_default_features() { + unused_fields.push("`default-features`") + } + if !unused_fields.is_empty() { + let mut shell = ws.gctx().shell(); + shell.warn(format!( + "unused field in replacement for `{}`: {}", + dep.package_name(), + unused_fields.join(", ") + ))?; + shell.note(format!( + "configure {} in the `dependencies` entry", + unused_fields.join(", ") + ))?; } } diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index b5918e3aecf..d65235db4f3 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -928,20 +928,26 @@ fn add_patch_with_features() { .file("bar/src/lib.rs", r#""#) .build(); - p.cargo("check").with_stderr_data(str![[r#" -[WARNING] patch for `bar` uses the features mechanism. default-features and features will not take effect because the patch dependency does not support this mechanism + p.cargo("check") + .with_stderr_data(str![[r#" +[WARNING] unused field in patch for `bar`: `features` +[NOTE] configure `features` in the `dependencies` entry [UPDATING] `dummy-registry` index [LOCKING] 1 package to latest compatible version [CHECKING] bar v0.1.0 ([ROOT]/foo/bar) [CHECKING] foo v0.0.1 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s -"#]]).run(); - p.cargo("check").with_stderr_data(str![[r#" -[WARNING] patch for `bar` uses the features mechanism. default-features and features will not take effect because the patch dependency does not support this mechanism +"#]]) + .run(); + p.cargo("check") + .with_stderr_data(str![[r#" +[WARNING] unused field in patch for `bar`: `features` +[NOTE] configure `features` in the `dependencies` entry [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s -"#]]).run(); +"#]]) + .run(); } #[cargo_test] @@ -970,20 +976,26 @@ fn add_patch_with_setting_default_features() { .file("bar/src/lib.rs", r#""#) .build(); - p.cargo("check").with_stderr_data(str![[r#" -[WARNING] patch for `bar` uses the features mechanism. default-features and features will not take effect because the patch dependency does not support this mechanism + p.cargo("check") + .with_stderr_data(str![[r#" +[WARNING] unused field in patch for `bar`: `features`, `default-features` +[NOTE] configure `features`, `default-features` in the `dependencies` entry [UPDATING] `dummy-registry` index [LOCKING] 1 package to latest compatible version [CHECKING] bar v0.1.0 ([ROOT]/foo/bar) [CHECKING] foo v0.0.1 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s -"#]]).run(); - p.cargo("check").with_stderr_data(str![[r#" -[WARNING] patch for `bar` uses the features mechanism. default-features and features will not take effect because the patch dependency does not support this mechanism +"#]]) + .run(); + p.cargo("check") + .with_stderr_data(str![[r#" +[WARNING] unused field in patch for `bar`: `features`, `default-features` +[NOTE] configure `features`, `default-features` in the `dependencies` entry [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s -"#]]).run(); +"#]]) + .run(); } #[cargo_test] diff --git a/tests/testsuite/replace.rs b/tests/testsuite/replace.rs index 8f9b3eb0fed..1191c1303eb 100644 --- a/tests/testsuite/replace.rs +++ b/tests/testsuite/replace.rs @@ -89,16 +89,19 @@ fn override_with_features() { ) .build(); - p.cargo("check").with_stderr_data(str![[r#" + p.cargo("check") + .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index [UPDATING] git repository `[ROOTURL]/override` [LOCKING] 2 packages to latest compatible versions -[WARNING] replacement for `bar` uses the features mechanism. default-features and features will not take effect because the replacement dependency does not support this mechanism +[WARNING] unused field in replacement for `bar`: `features` +[NOTE] configure `features` in the `dependencies` entry [CHECKING] bar v0.1.0 ([ROOTURL]/override#[..]) [CHECKING] foo v0.0.1 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s -"#]]).run(); +"#]]) + .run(); } #[cargo_test] @@ -136,16 +139,19 @@ fn override_with_setting_default_features() { ) .build(); - p.cargo("check").with_stderr_data(str![[r#" + p.cargo("check") + .with_stderr_data(str![[r#" [UPDATING] `dummy-registry` index [UPDATING] git repository `[ROOTURL]/override` [LOCKING] 2 packages to latest compatible versions -[WARNING] replacement for `bar` uses the features mechanism. default-features and features will not take effect because the replacement dependency does not support this mechanism +[WARNING] unused field in replacement for `bar`: `features`, `default-features` +[NOTE] configure `features`, `default-features` in the `dependencies` entry [CHECKING] bar v0.1.0 ([ROOTURL]/override#[..]) [CHECKING] foo v0.0.1 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s -"#]]).run(); +"#]]) + .run(); } #[cargo_test]