From 4c38aeed80d2f32b2c1860768da611e6e367ae73 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 19 Sep 2024 15:52:49 +0200 Subject: [PATCH 1/2] Cleanup duplicated check-cfg lints logic --- src/cargo/core/compiler/fingerprint/mod.rs | 22 --------------- src/cargo/core/compiler/mod.rs | 33 ++-------------------- src/cargo/util/toml/mod.rs | 29 +++++++++++++++++-- tests/testsuite/check_cfg.rs | 14 ++++++--- 4 files changed, 39 insertions(+), 59 deletions(-) diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index 22ac87ba85e..e2f61bdfb1f 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -1421,34 +1421,12 @@ fn calculate_normal( } .to_vec(); - // Include all the args from `[lints.rust.unexpected_cfgs.check-cfg]` - // - // HACK(#13975): duplicating the lookup logic here until `--check-cfg` is supported - // on Cargo's MSRV and we can centralize the logic in `lints_to_rustflags` - let mut lint_check_cfg = Vec::new(); - if let Ok(Some(lints)) = unit.pkg.manifest().normalized_toml().normalized_lints() { - if let Some(rust_lints) = lints.get("rust") { - if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") { - if let Some(config) = unexpected_cfgs.config() { - if let Some(check_cfg) = config.get("check-cfg") { - if let Ok(check_cfgs) = - toml::Value::try_into::>(check_cfg.clone()) - { - lint_check_cfg = check_cfgs; - } - } - } - } - } - } - let profile_hash = util::hash_u64(( &unit.profile, unit.mode, build_runner.bcx.extra_args_for(unit), build_runner.lto[unit], unit.pkg.manifest().lint_rustflags(), - lint_check_cfg, )); // Include metadata since it is exposed as environment variables. let m = unit.pkg.manifest().metadata(); diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 310e4f2a84b..f09b7e349ef 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -63,7 +63,7 @@ use std::io::{BufRead, Write}; use std::path::{Path, PathBuf}; use std::sync::Arc; -use anyhow::{bail, Context as _, Error}; +use anyhow::{Context as _, Error}; use lazycell::LazyCell; use tracing::{debug, trace}; @@ -1391,39 +1391,12 @@ fn check_cfg_args(unit: &Unit) -> CargoResult> { // Cargo and docs.rs than rustc and docs.rs. In particular, all users of docs.rs use // Cargo, but not all users of rustc (like Rust-for-Linux) use docs.rs. - let mut args = vec![ + Ok(vec![ OsString::from("--check-cfg"), OsString::from("cfg(docsrs)"), OsString::from("--check-cfg"), arg_feature, - ]; - - // Also include the custom arguments specified in `[lints.rust.unexpected_cfgs.check_cfg]` - if let Ok(Some(lints)) = unit.pkg.manifest().normalized_toml().normalized_lints() { - if let Some(rust_lints) = lints.get("rust") { - if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") { - if let Some(config) = unexpected_cfgs.config() { - if let Some(check_cfg) = config.get("check-cfg") { - if let Ok(check_cfgs) = - toml::Value::try_into::>(check_cfg.clone()) - { - for check_cfg in check_cfgs { - args.push(OsString::from("--check-cfg")); - args.push(OsString::from(check_cfg)); - } - // error about `check-cfg` not being a list-of-string - } else { - bail!( - "`lints.rust.unexpected_cfgs.check-cfg` must be a list of string" - ); - } - } - } - } - } - } - - Ok(args) + ]) } /// Adds LTO related codegen flags. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 6f74ff6e8b2..85dcc1a0540 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1446,7 +1446,7 @@ pub fn to_real_manifest( .normalized_lints() .expect("previously normalized") .unwrap_or(&default), - ); + )?; let metadata = ManifestMetadata { description: normalized_package @@ -2545,7 +2545,7 @@ switch to nightly channel you can pass warnings.push(message); } -fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec { +fn lints_to_rustflags(lints: &manifest::TomlLints) -> CargoResult> { let mut rustflags = lints .iter() // We don't want to pass any of the `cargo` lints to `rustc` @@ -2575,7 +2575,30 @@ fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec { }) .collect::>(); rustflags.sort(); - rustflags.into_iter().map(|(_, _, option)| option).collect() + + let mut rustflags: Vec<_> = rustflags.into_iter().map(|(_, _, option)| option).collect(); + + // Also include the custom arguments specified in `[lints.rust.unexpected_cfgs.check_cfg]` + if let Some(rust_lints) = lints.get("rust") { + if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") { + if let Some(config) = unexpected_cfgs.config() { + if let Some(check_cfg) = config.get("check-cfg") { + if let Ok(check_cfgs) = toml::Value::try_into::>(check_cfg.clone()) + { + for check_cfg in check_cfgs { + rustflags.push("--check-cfg".to_string()); + rustflags.push(check_cfg); + } + // error about `check-cfg` not being a list-of-string + } else { + bail!("`lints.rust.unexpected_cfgs.check-cfg` must be a list of string"); + } + } + } + } + } + + Ok(rustflags) } fn emit_diagnostic( diff --git a/tests/testsuite/check_cfg.rs b/tests/testsuite/check_cfg.rs index 83f208c96c4..6b856007123 100644 --- a/tests/testsuite/check_cfg.rs +++ b/tests/testsuite/check_cfg.rs @@ -707,8 +707,11 @@ fn config_invalid_not_list() { p.cargo("check") .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `lints.rust.unexpected_cfgs.check-cfg` must be a list of string -... +[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` + +Caused by: + `lints.rust.unexpected_cfgs.check-cfg` must be a list of string + "#]]) .run(); } @@ -734,8 +737,11 @@ fn config_invalid_not_list_string() { p.cargo("check") .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `lints.rust.unexpected_cfgs.check-cfg` must be a list of string -... +[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` + +Caused by: + `lints.rust.unexpected_cfgs.check-cfg` must be a list of string + "#]]) .run(); } From 3058f50f7128d2a6514fd3aae86a58839d21ff47 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 19 Sep 2024 15:55:22 +0200 Subject: [PATCH 2/2] Make check_cfg_args fn not-fallible anymore --- src/cargo/core/compiler/build_runner/mod.rs | 2 +- src/cargo/core/compiler/mod.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/compiler/build_runner/mod.rs b/src/cargo/core/compiler/build_runner/mod.rs index 43d00e1ec85..2dd6a90e4f4 100644 --- a/src/cargo/core/compiler/build_runner/mod.rs +++ b/src/cargo/core/compiler/build_runner/mod.rs @@ -246,7 +246,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> { let mut args = compiler::extern_args(&self, unit, &mut unstable_opts)?; args.extend(compiler::lto_args(&self, unit)); args.extend(compiler::features_args(unit)); - args.extend(compiler::check_cfg_args(unit)?); + args.extend(compiler::check_cfg_args(unit)); let script_meta = self.find_build_script_metadata(unit); if let Some(meta) = script_meta { diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index f09b7e349ef..8e9e2ac214d 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -743,7 +743,7 @@ fn prepare_rustdoc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResu let doc_dir = build_runner.files().out_dir(unit); rustdoc.arg("-o").arg(&doc_dir); rustdoc.args(&features_args(unit)); - rustdoc.args(&check_cfg_args(unit)?); + rustdoc.args(&check_cfg_args(unit)); add_error_format_and_color(build_runner, &mut rustdoc); add_allow_features(build_runner, &mut rustdoc); @@ -1140,7 +1140,7 @@ fn build_base_args( } cmd.args(&features_args(unit)); - cmd.args(&check_cfg_args(unit)?); + cmd.args(&check_cfg_args(unit)); let meta = build_runner.files().metadata(unit); cmd.arg("-C").arg(&format!("metadata={}", meta)); @@ -1354,7 +1354,7 @@ fn package_remap(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> OsString { } /// Generates the `--check-cfg` arguments for the `unit`. -fn check_cfg_args(unit: &Unit) -> CargoResult> { +fn check_cfg_args(unit: &Unit) -> Vec { // The routine below generates the --check-cfg arguments. Our goals here are to // enable the checking of conditionals and pass the list of declared features. // @@ -1391,12 +1391,12 @@ fn check_cfg_args(unit: &Unit) -> CargoResult> { // Cargo and docs.rs than rustc and docs.rs. In particular, all users of docs.rs use // Cargo, but not all users of rustc (like Rust-for-Linux) use docs.rs. - Ok(vec![ + vec![ OsString::from("--check-cfg"), OsString::from("cfg(docsrs)"), OsString::from("--check-cfg"), arg_feature, - ]) + ] } /// Adds LTO related codegen flags.