Merge b5fd82686f into eee4ea2f5a

fix(cargo-lints): Don't always inherit workspace lints

When working on changes for #13805, I noticed that we always passed the contents of `[workspace.lints.cargo]` into the currently implemented lints,  even if `[lints]` was not specified or did not contain `workspace = true`. This PR makes it so we only pass in the workspace cargo lints if `[lints]` contains `workspace = true`.

You can verify this change by looking at the first commit, where I added a test showing the current behavior, and looking at the second commit and seeing the test output no longer shows a warning about specifying `im-a-teapot`.

src/cargo/core/features.rs

@@ -761,7 +761,9 @@ unstable_cli_options!(
     dual_proc_macros: bool = ("Build proc-macros for both the host and the target"),
     features: Option<Vec<String>>,
     gc: bool = ("Track cache usage and \"garbage collect\" unused files"),
+    #[serde(deserialize_with = "deserialize_git_features")]
     git: Option<GitFeatures> = ("Enable support for shallow git fetch operations"),
+    #[serde(deserialize_with = "deserialize_gitoxide_features")]
     gitoxide: Option<GitoxideFeatures> = ("Use gitoxide for the given git interactions, or all of them if no argument is given"),
     host_config: bool = ("Enable the `[host]` section in the .cargo/config.toml file"),
     minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum"),
@@ -868,7 +870,7 @@ where
     ))
 }
 
-#[derive(Debug, Copy, Clone, Default, Deserialize)]
+#[derive(Debug, Copy, Clone, Default, Deserialize, Ord, PartialOrd, Eq, PartialEq)]
 pub struct GitFeatures {
     /// When cloning the index, perform a shallow clone. Maintain shallowness upon subsequent fetches.
     pub shallow_index: bool,
@@ -877,12 +879,59 @@ pub struct GitFeatures {
 }
 
 impl GitFeatures {
-    fn all() -> Self {
+    pub fn all() -> Self {
         GitFeatures {
             shallow_index: true,
             shallow_deps: true,
         }
     }
+
+    fn expecting() -> String {
+        let fields = vec!["'all'", "'shallow-index'", "'shallow-deps'"];
+        format!(
+            "unstable 'git' only takes {} as valid inputs, your can use 'all' to turn out all git features",
+            fields.join(" and ")
+        )
+    }
+}
+
+fn deserialize_git_features<'de, D>(deserializer: D) -> Result<Option<GitFeatures>, D::Error>
+where
+    D: serde::de::Deserializer<'de>,
+{
+    struct GitFeaturesVisitor;
+
+    impl<'de> serde::de::Visitor<'de> for GitFeaturesVisitor {
+        type Value = Option<GitFeatures>;
+
+        fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+            formatter.write_str(&GitFeatures::expecting())
+        }
+
+        fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
+        where
+            E: serde::de::Error,
+        {
+            Ok(parse_git(s.split(",")).map_err(serde::de::Error::custom)?)
+        }
+
+        fn visit_none<E>(self) -> Result<Self::Value, E>
+        where
+            E: serde::de::Error,
+        {
+            Ok(None)
+        }
+
+        fn visit_map<V>(self, map: V) -> Result<Self::Value, V::Error>
+        where
+            V: serde::de::MapAccess<'de>,
+        {
+            let mvd = serde::de::value::MapAccessDeserializer::new(map);
+            Ok(Some(GitFeatures::deserialize(mvd)?))
+        }
+    }
+
+    deserializer.deserialize_any(GitFeaturesVisitor)
 }
 
 fn parse_git(it: impl Iterator<Item = impl AsRef<str>>) -> CargoResult<Option<GitFeatures>> {
@@ -894,19 +943,18 @@ fn parse_git(it: impl Iterator<Item = impl AsRef<str>>) -> CargoResult<Option<Gi
 
     for e in it {
         match e.as_ref() {
+            "all" => return Ok(Some(GitFeatures::all())),
             "shallow-index" => *shallow_index = true,
             "shallow-deps" => *shallow_deps = true,
             _ => {
-                bail!(
-                    "unstable 'git' only takes 'shallow-index' and 'shallow-deps' as valid inputs"
-                )
+                bail!(GitFeatures::expecting())
             }
         }
     }
     Ok(Some(out))
 }
 
-#[derive(Debug, Copy, Clone, Default, Deserialize)]
+#[derive(Debug, Copy, Clone, Default, Deserialize, Ord, PartialOrd, Eq, PartialEq)]
 pub struct GitoxideFeatures {
     /// All fetches are done with `gitoxide`, which includes git dependencies as well as the crates index.
     pub fetch: bool,
@@ -920,7 +968,7 @@ pub struct GitoxideFeatures {
 }
 
 impl GitoxideFeatures {
-    fn all() -> Self {
+    pub fn all() -> Self {
         GitoxideFeatures {
             fetch: true,
             checkout: true,
@@ -937,6 +985,55 @@ impl GitoxideFeatures {
             internal_use_git2: false,
         }
     }
+
+    fn expecting() -> String {
+        let fields = vec!["'all'", "'fetch'", "'checkout'", "'internal-use-git2'"];
+        format!(
+            "unstable 'gitoxide' only takes {} as valid inputs, your can use 'all' to turn out all gitoxide features, for shallow fetches see `shallow-index,shallow-deps`",
+            fields.join(" and ")
+        )
+    }
+}
+
+fn deserialize_gitoxide_features<'de, D>(
+    deserializer: D,
+) -> Result<Option<GitoxideFeatures>, D::Error>
+where
+    D: serde::de::Deserializer<'de>,
+{
+    struct GitoxideFeaturesVisitor;
+
+    impl<'de> serde::de::Visitor<'de> for GitoxideFeaturesVisitor {
+        type Value = Option<GitoxideFeatures>;
+
+        fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+            formatter.write_str(&GitoxideFeatures::expecting())
+        }
+
+        fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
+        where
+            E: serde::de::Error,
+        {
+            Ok(parse_gitoxide(s.split(",")).map_err(serde::de::Error::custom)?)
+        }
+
+        fn visit_none<E>(self) -> Result<Self::Value, E>
+        where
+            E: serde::de::Error,
+        {
+            Ok(None)
+        }
+
+        fn visit_map<V>(self, map: V) -> Result<Self::Value, V::Error>
+        where
+            V: serde::de::MapAccess<'de>,
+        {
+            let mvd = serde::de::value::MapAccessDeserializer::new(map);
+            Ok(Some(GitoxideFeatures::deserialize(mvd)?))
+        }
+    }
+
+    deserializer.deserialize_any(GitoxideFeaturesVisitor)
 }
 
 fn parse_gitoxide(
@@ -951,11 +1048,12 @@ fn parse_gitoxide(
 
     for e in it {
         match e.as_ref() {
+            "all" => return Ok(Some(GitoxideFeatures::all())),
             "fetch" => *fetch = true,
             "checkout" => *checkout = true,
             "internal-use-git2" => *internal_use_git2 = true,
             _ => {
-                bail!("unstable 'gitoxide' only takes `fetch` and 'checkout' as valid input, for shallow fetches see `-Zgit=shallow-index,shallow-deps`")
+                bail!(GitoxideFeatures::expecting())
             }
         }
     }

src/cargo/core/workspace.rs

@@ -1147,11 +1147,26 @@ impl<'gctx> Workspace<'gctx> {
     }
 
     pub fn emit_warnings(&self) -> CargoResult<()> {
+        let ws_lints = self
+            .root_maybe()
+            .workspace_config()
+            .inheritable()
+            .and_then(|i| i.lints().ok())
+            .unwrap_or_default();
+
+        let ws_cargo_lints = ws_lints
+            .get("cargo")
+            .cloned()
+            .unwrap_or_default()
+            .into_iter()
+            .map(|(k, v)| (k.replace('-', "_"), v))
+            .collect();
+
         for (path, maybe_pkg) in &self.packages.packages {
             let path = path.join("Cargo.toml");
             if let MaybePackage::Package(pkg) = maybe_pkg {
                 if self.gctx.cli_unstable().cargo_lints {
-                    self.emit_lints(pkg, &path)?
+                    self.emit_lints(pkg, &path, &ws_cargo_lints)?
                 }
             }
             let warnings = match maybe_pkg {
@@ -1179,22 +1194,12 @@ impl<'gctx> Workspace<'gctx> {
         Ok(())
     }
 
-    pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
-        let ws_lints = self
-            .root_maybe()
-            .workspace_config()
-            .inheritable()
-            .and_then(|i| i.lints().ok())
-            .unwrap_or_default();
-
-        let ws_cargo_lints = ws_lints
-            .get("cargo")
-            .cloned()
-            .unwrap_or_default()
-            .into_iter()
-            .map(|(k, v)| (k.replace('-', "_"), v))
-            .collect();
-
+    pub fn emit_lints(
+        &self,
+        pkg: &Package,
+        path: &Path,
+        ws_cargo_lints: &manifest::TomlToolLints,
+    ) -> CargoResult<()> {
         let mut error_count = 0;
         let toml_lints = pkg
             .manifest()
@@ -1212,11 +1217,21 @@ impl<'gctx> Workspace<'gctx> {
             .map(|(name, lint)| (name.replace('-', "_"), lint))
             .collect();
 
+        // We should only be using workspace lints if the `[lints]` table is
+        // present in the manifest, and `workspace` is set to `true`
+        let ws_cargo_lints = pkg
+            .manifest()
+            .resolved_toml()
+            .lints
+            .as_ref()
+            .is_some_and(|l| l.workspace)
+            .then(|| ws_cargo_lints);
+
         check_im_a_teapot(
             pkg,
             &path,
             &normalized_lints,
-            &ws_cargo_lints,
+            ws_cargo_lints,
             &mut error_count,
             self.gctx,
         )?;
@@ -1224,7 +1239,7 @@ impl<'gctx> Workspace<'gctx> {
             pkg,
             &path,
             &normalized_lints,
-            &ws_cargo_lints,
+            ws_cargo_lints,
             &mut error_count,
             self.gctx,
         )?;
@@ -1232,7 +1247,7 @@ impl<'gctx> Workspace<'gctx> {
             pkg,
             &path,
             &normalized_lints,
-            &ws_cargo_lints,
+            ws_cargo_lints,
             &mut error_count,
             self.gctx,
         )?;

src/cargo/util/context/de.rs

@@ -62,7 +62,12 @@ impl<'de, 'gctx> de::Deserializer<'de> for Deserializer<'gctx> {
             let (res, def) = res;
             return res.map_err(|e| e.with_key_context(&self.key, def));
         }
-        Err(ConfigError::missing(&self.key))
+
+        // Let's assume that `unstable.git` had defined its `deserialize_any`` method,
+        // then run `CARGO_UNSTABLE_GIT_SHALLOW_INDEX cargo fetch`, here will return `missing config key unstable.git`.
+        // It seems that anything that starts with CARGO_UNSTABLE_GIT, even like CARGO_UNSTABLE_GITOXIDE can trigger this.
+        // This is a workaround for now, but should be fixed in the future.
+        visitor.visit_none()
     }
 
     deserialize_method!(deserialize_bool, visit_bool, get_bool);

src/cargo/util/lints.rs

@@ -88,7 +88,7 @@ impl Lint {
     pub fn level(
         &self,
         pkg_lints: &TomlToolLints,
-        ws_lints: &TomlToolLints,
+        ws_lints: Option<&TomlToolLints>,
         edition: Edition,
     ) -> (LintLevel, LintLevelReason) {
         self.groups
@@ -188,7 +188,7 @@ fn level_priority(
     default_level: LintLevel,
     edition_lint_opts: Option<(Edition, LintLevel)>,
     pkg_lints: &TomlToolLints,
-    ws_lints: &TomlToolLints,
+    ws_lints: Option<&TomlToolLints>,
     edition: Edition,
 ) -> (LintLevel, LintLevelReason, i8) {
     let (unspecified_level, reason) = if let Some(level) = edition_lint_opts
@@ -211,7 +211,7 @@ fn level_priority(
             LintLevelReason::Package,
             defined_level.priority(),
         )
-    } else if let Some(defined_level) = ws_lints.get(name) {
+    } else if let Some(defined_level) = ws_lints.and_then(|l| l.get(name)) {
         (
             defined_level.level().into(),
             LintLevelReason::Workspace,
@@ -234,7 +234,7 @@ pub fn check_im_a_teapot(
     pkg: &Package,
     path: &Path,
     pkg_lints: &TomlToolLints,
-    ws_lints: &TomlToolLints,
+    ws_lints: Option<&TomlToolLints>,
     error_count: &mut usize,
     gctx: &GlobalContext,
 ) -> CargoResult<()> {
@@ -306,7 +306,7 @@ pub fn check_implicit_features(
     pkg: &Package,
     path: &Path,
     pkg_lints: &TomlToolLints,
-    ws_lints: &TomlToolLints,
+    ws_lints: Option<&TomlToolLints>,
     error_count: &mut usize,
     gctx: &GlobalContext,
 ) -> CargoResult<()> {
@@ -390,7 +390,7 @@ pub fn unused_dependencies(
     pkg: &Package,
     path: &Path,
     pkg_lints: &TomlToolLints,
-    ws_lints: &TomlToolLints,
+    ws_lints: Option<&TomlToolLints>,
     error_count: &mut usize,
     gctx: &GlobalContext,
 ) -> CargoResult<()> {

src/cargo/util/network/sleep.rs

@@ -90,13 +90,15 @@ impl<T> SleepTracker<T> {
 #[test]
 fn returns_in_order() {
     let mut s = SleepTracker::new();
-    s.push(3, 3);
+    s.push(30_000, 30_000);
     s.push(1, 1);
-    s.push(6, 6);
-    s.push(5, 5);
-    s.push(2, 2);
-    s.push(10000, 10000);
-    assert_eq!(s.len(), 6);
-    std::thread::sleep(Duration::from_millis(100));
-    assert_eq!(s.to_retry(), &[1, 2, 3, 5, 6]);
+    assert_eq!(s.len(), 2);
+    std::thread::sleep(Duration::from_millis(2));
+    assert_eq!(s.to_retry(), &[1]);
+    assert!(s.to_retry().is_empty());
+    let next = s.time_to_next().expect("should be next");
+    assert!(
+        next < Duration::from_millis(30_000),
+        "{next:?} should be less than 30s"
+    );
 }

tests/testsuite/config.rs

@@ -1,5 +1,6 @@
 //! Tests for config settings.
 
+use cargo::core::features::{GitFeatures, GitoxideFeatures};
 use cargo::core::{PackageIdSpec, Shell};
 use cargo::util::context::{
     self, Definition, GlobalContext, JobsConfig, SslVersionConfig, StringList,
@@ -1870,3 +1871,259 @@ fn trim_paths_parsing() {
     let trim_paths: TomlTrimPaths = gctx.get("profile.dev.trim-paths").unwrap();
     assert_eq!(trim_paths, expected, "failed to parse {val}");
 }
+
+#[cargo_test]
+fn git_features_as_str() {
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GIT", "shallow-index")
+        .build();
+    verify(
+        gctx,
+        Some(GitFeatures {
+            shallow_index: true,
+            ..GitFeatures::default()
+        }),
+    );
+
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GIT", "all,shallow-index")
+        .build();
+    verify(gctx, Some(GitFeatures::all()));
+
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GIT", "shallow-index,shallow-deps")
+        .build();
+    verify(gctx, Some(GitFeatures::all()));
+
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GIT", "shallow-index,abc")
+        .build();
+    assert_error(
+        gctx.get::<Option<cargo::core::CliUnstable>>("unstable")
+            .unwrap_err(),
+        "\
+error in environment variable `CARGO_UNSTABLE_GIT`: could not load config key `unstable.git`
+
+Caused by:
+  unstable 'git' only takes 'all' and 'shallow-index' and 'shallow-deps' as valid inputs, your can use 'all' to turn out all git features",
+    );
+
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GIT", "shallow-deps")
+        .build();
+    verify(
+        gctx,
+        Some(GitFeatures {
+            shallow_index: false,
+            shallow_deps: true,
+        }),
+    );
+
+    write_config_toml(
+        "\
+    [unstable]
+    git = 'all'
+    ",
+    );
+    let gctx = GlobalContextBuilder::new().build();
+    verify(gctx, Some(GitFeatures::all()));
+
+    write_config_toml(
+        "\
+    [unstable]
+    git = \"shallow-index\"
+    ",
+    );
+    let gctx = GlobalContextBuilder::new().build();
+    verify(
+        gctx,
+        Some(GitFeatures {
+            shallow_index: true,
+            shallow_deps: false,
+        }),
+    );
+
+    fn verify(gctx: GlobalContext, expect: Option<GitFeatures>) {
+        let unstable_flags = gctx
+            .get::<Option<cargo::core::CliUnstable>>("unstable")
+            .unwrap()
+            .unwrap();
+        assert_eq!(unstable_flags.git, expect);
+    }
+}
+
+#[cargo_test]
+fn git_features_as_table() {
+    // A single feature in ENV can't be specified
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GIT_SHALLOW_INDEX", "true")
+        .build();
+    let unstable_flags = gctx
+        .get::<Option<cargo::core::CliUnstable>>("unstable")
+        .unwrap();
+    assert!(unstable_flags.unwrap().git.is_none());
+
+    write_config_toml(
+        "\
+    [unstable.git]
+    shallow_deps = false
+    shallow_index = true
+    ",
+    );
+    let gctx = GlobalContextBuilder::new().build();
+    verify(
+        gctx,
+        Some(GitFeatures {
+            shallow_index: true,
+            shallow_deps: false,
+        }),
+    );
+    // Still warning missing a field.
+    write_config_toml(
+        "\
+    [unstable.git]
+    ",
+    );
+    let gctx = GlobalContextBuilder::new().build();
+    assert_error(
+        gctx.get::<Option<cargo::core::CliUnstable>>("unstable")
+            .unwrap_err(),
+        "\
+error in [..]/.cargo/config.toml: could not load config key `unstable.git`
+
+Caused by:
+  missing field `shallow_index`",
+    );
+
+    fn verify(gctx: GlobalContext, expect: Option<GitFeatures>) {
+        let unstable_flags = gctx
+            .get::<Option<cargo::core::CliUnstable>>("unstable")
+            .unwrap()
+            .unwrap();
+        assert_eq!(unstable_flags.git, expect);
+    }
+}
+
+#[cargo_test]
+fn gitoxide_features_as_str() {
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GITOXIDE", "fetch")
+        .build();
+    verify(
+        gctx,
+        Some(GitoxideFeatures {
+            fetch: true,
+            ..GitoxideFeatures::default()
+        }),
+    );
+
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GITOXIDE", "all,fetch")
+        .build();
+    verify(gctx, Some(GitoxideFeatures::all()));
+
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GITOXIDE", "fetch,checkout")
+        .build();
+    verify(gctx, Some(GitoxideFeatures::all()));
+
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GITOXIDE", "fetch,abc")
+        .build();
+
+    assert_error(
+        gctx.get::<Option<cargo::core::CliUnstable>>("unstable")
+            .unwrap_err(),
+        "\
+error in environment variable `CARGO_UNSTABLE_GITOXIDE`: could not load config key `unstable.gitoxide`
+
+Caused by:
+  unstable 'gitoxide' only takes 'all' and 'fetch' and 'checkout' and 'internal-use-git2' as valid inputs, your can use 'all' to turn out all gitoxide features, for shallow fetches see `shallow-index,shallow-deps`",
+    );
+
+    write_config_toml(
+        "\
+    [unstable]
+    gitoxide = 'all'
+    ",
+    );
+    let gctx = GlobalContextBuilder::new().build();
+    verify(gctx, Some(GitoxideFeatures::all()));
+
+    write_config_toml(
+        "\
+    [unstable]
+    gitoxide = \"fetch\"
+    ",
+    );
+    let gctx = GlobalContextBuilder::new().build();
+    verify(
+        gctx,
+        Some(GitoxideFeatures {
+            fetch: true,
+            ..GitoxideFeatures::default()
+        }),
+    );
+
+    fn verify(gctx: GlobalContext, expect: Option<GitoxideFeatures>) {
+        let unstable_flags = gctx
+            .get::<Option<cargo::core::CliUnstable>>("unstable")
+            .unwrap()
+            .unwrap();
+        assert_eq!(unstable_flags.gitoxide, expect);
+    }
+}
+
+#[cargo_test]
+fn gitoxide_features_as_table() {
+    // A single feature in ENV can't be specified
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_UNSTABLE_GITOXIDE_SHALLOW_INDEX", "true")
+        .build();
+    let unstable_flags = gctx
+        .get::<Option<cargo::core::CliUnstable>>("unstable")
+        .unwrap();
+    assert!(unstable_flags.unwrap().gitoxide.is_none());
+
+    write_config_toml(
+        "\
+    [unstable.gitoxide]
+    fetch = true
+    checkout = false
+    internal_use_git2 = false
+    ",
+    );
+    let gctx = GlobalContextBuilder::new().build();
+    verify(
+        gctx,
+        Some(GitoxideFeatures {
+            fetch: true,
+            checkout: false,
+            internal_use_git2: false,
+        }),
+    );
+    // Still warning missing a field.
+    write_config_toml(
+        "\
+    [unstable.gitoxide]
+    ",
+    );
+    let gctx = GlobalContextBuilder::new().build();
+    assert_error(
+        gctx.get::<Option<cargo::core::CliUnstable>>("unstable")
+            .unwrap_err(),
+        "\
+error in [..]/.cargo/config.toml: could not load config key `unstable.gitoxide`
+
+Caused by:
+  missing field `fetch`",
+    );
+
+    fn verify(gctx: GlobalContext, expect: Option<GitoxideFeatures>) {
+        let unstable_flags = gctx
+            .get::<Option<cargo::core::CliUnstable>>("unstable")
+            .unwrap()
+            .unwrap();
+        assert_eq!(unstable_flags.gitoxide, expect);
+    }
+}

tests/testsuite/lints_table.rs

@@ -982,3 +982,43 @@ error: `im_a_teapot` is specified
         )
         .run();
 }
+
+#[cargo_test]
+fn dont_always_inherit_workspace_lints() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+[workspace]
+members = ["foo"]
+
+[workspace.lints.cargo]
+im-a-teapot = "warn"
+"#,
+        )
+        .file(
+            "foo/Cargo.toml",
+            r#"
+cargo-features = ["test-dummy-unstable"]
+
+[package]
+name = "foo"
+version = "0.0.1"
+edition = "2015"
+authors = []
+im-a-teapot = true
+"#,
+        )
+        .file("foo/src/lib.rs", "")
+        .build();
+
+    p.cargo("check -Zcargo-lints")
+        .masquerade_as_nightly_cargo(&["cargo-lints"])
+        .with_stderr(
+            "\
+[CHECKING] foo v0.0.1 ([CWD]/foo)
+[FINISHED] [..]
+",
+        )
+        .run();
+}