From 5229a09d411b323c163916b417a196a55a520971 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Fri, 23 Aug 2024 15:51:24 +0800 Subject: [PATCH] force edition 2024 lint into action on crater --- src/bin/cargo/commands/check.rs | 58 ++++++- src/cargo/core/compiler/fingerprint/mod.rs | 183 +++++---------------- src/cargo/ops/fix.rs | 1 + src/cargo/util/toml/mod.rs | 18 +- 4 files changed, 107 insertions(+), 153 deletions(-) diff --git a/src/bin/cargo/commands/check.rs b/src/bin/cargo/commands/check.rs index 66f378c3e902..698f9961a138 100644 --- a/src/bin/cargo/commands/check.rs +++ b/src/bin/cargo/commands/check.rs @@ -1,7 +1,7 @@ -use crate::command_prelude::*; - use cargo::ops; +use crate::command_prelude::*; + pub fn cli() -> Command { subcommand("check") // subcommand aliases are handled in aliased_command() @@ -44,12 +44,13 @@ pub fn cli() -> Command { } pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { + if std::env::var("CARGO_REAL_CHECK").is_err() { + fixit()?; + return Ok(()); + } let ws = args.workspace(gctx)?; // This is a legacy behavior that causes `cargo check` to pass `--test`. - let test = matches!( - args.get_one::("profile").map(String::as_str), - Some("test") - ); + let test = matches!(args.get_one::("profile").map(String::as_str), Some("test")); let mode = CompileMode::Check { test }; let compile_opts = args.compile_options(gctx, mode, Some(&ws), ProfileChecking::LegacyTestOnly)?; @@ -57,3 +58,48 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { ops::compile(&ws, &compile_opts)?; Ok(()) } + +fn fixit() -> CliResult { + use std::path::Path; + + use anyhow::Context; + use cargo_util::{paths, ProcessBuilder}; + + eprintln!("Copying to /tmp/fixit"); + ProcessBuilder::new("cp").args(&["-a", ".", "/tmp/fixit"]).exec()?; + std::env::set_current_dir("/tmp/fixit").map_err(|e| anyhow::format_err!("cd failed {}", e))?; + + let ed_re = regex::Regex::new(r#"(?m)^ *edition *= *['"]([^'"]+)['"]"#).unwrap(); + let manifest = paths::read(Path::new("Cargo.toml"))?; + let ed_cap = match ed_re.captures(&manifest) { + None => { + eprintln!("no edition found in manifest, probably 2015, skipping"); + return Ok(()); + } + Some(caps) => caps.get(1).unwrap(), + }; + if ed_cap.as_str() != "2021" { + eprintln!("skipping non-2021 edition `{}`", ed_cap.as_str()); + return Ok(()); + } + eprintln!("Running `cargo fix --edition`"); + // Skip "cargo check" + let args: Vec<_> = std::env::args().skip(2).collect(); + ProcessBuilder::new("cargo") + .args(&["fix", "--edition", "--allow-no-vcs", "--allow-dirty"]) + .args(&args) + .exec() + .with_context(|| "failed to migrate to next edition")?; + let mut manifest = paths::read(Path::new("Cargo.toml"))?; + let ed_cap = ed_re.captures(&manifest).unwrap().get(1).unwrap(); + manifest.replace_range(ed_cap.range(), "2024"); + paths::write("Cargo.toml", manifest)?; + eprintln!("Running `cargo check` to verify 2024"); + ProcessBuilder::new("cargo") + .args(&["check"]) + .args(&args) + .env("CARGO_REAL_CHECK", "1") + .exec() + .with_context(|| "failed to check after updating to 2024")?; + Ok(()) +} diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index 22ac87ba85e5..92b0e8461d1e 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -356,36 +356,28 @@ mod dirty_reason; use std::collections::hash_map::{Entry, HashMap}; - -use std::env; use std::hash::{self, Hash, Hasher}; -use std::io; use std::path::{Path, PathBuf}; -use std::str; use std::sync::{Arc, Mutex}; use std::time::SystemTime; +use std::{env, io, str}; use anyhow::{bail, format_err, Context as _}; use cargo_util::{paths, ProcessBuilder}; +pub use dirty_reason::DirtyReason; use filetime::FileTime; -use serde::de; -use serde::ser; -use serde::{Deserialize, Serialize}; +use serde::{de, ser, Deserialize, Serialize}; use tracing::{debug, info}; +use super::custom_build::BuildDeps; +use super::{BuildContext, BuildRunner, FileFlavor, Job, Unit, Work}; use crate::core::compiler::unit_graph::UnitDep; use crate::core::Package; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; -use crate::util::{self, try_canonicalize}; -use crate::util::{internal, path_args, StableHasher}; +use crate::util::{self, internal, path_args, try_canonicalize, StableHasher}; use crate::{GlobalContext, CARGO_ENV}; -use super::custom_build::BuildDeps; -use super::{BuildContext, BuildRunner, FileFlavor, Job, Unit, Work}; - -pub use dirty_reason::DirtyReason; - /// Determines if a [`Unit`] is up-to-date, and if not prepares necessary work to /// update the persisted fingerprint. /// @@ -436,9 +428,7 @@ pub fn prepare_target( // changed then an error is issued. let source_id = unit.pkg.package_id().source_id(); let sources = bcx.packages.sources(); - let source = sources - .get(source_id) - .ok_or_else(|| internal("missing package source"))?; + let source = sources.get(source_id).ok_or_else(|| internal("missing package source"))?; source.verify(unit.pkg.package_id())?; // Clear out the old fingerprint file if it exists. This protects when @@ -488,14 +478,10 @@ pub fn prepare_target( let build_script_outputs = Arc::clone(&build_runner.build_script_outputs); let metadata = build_runner.get_run_build_script_metadata(unit); let (gen_local, _overridden) = build_script_local_fingerprints(build_runner, unit); - let output_path = build_runner.build_explicit_deps[unit] - .build_script_output - .clone(); + let output_path = build_runner.build_explicit_deps[unit].build_script_output.clone(); Work::new(move |_| { let outputs = build_script_outputs.lock().unwrap(); - let output = outputs - .get(metadata) - .expect("output must exist after running"); + let output = outputs.get(metadata).expect("output must exist after running"); let deps = BuildDeps::new(&output_path, Some(output)); // FIXME: it's basically buggy that we pass `None` to `call_box` @@ -626,11 +612,7 @@ pub enum FsStatus { StaleItem(StaleItem), /// A dependency was stale. - StaleDependency { - name: InternedString, - dep_mtime: FileTime, - max_mtime: FileTime, - }, + StaleDependency { name: InternedString, dep_mtime: FileTime, max_mtime: FileTime }, /// A dependency was stale. StaleDepFingerprint { name: InternedString }, @@ -657,13 +639,7 @@ impl Serialize for DepFingerprint { where S: ser::Serializer, { - ( - &self.pkg_id, - &self.name, - &self.public, - &self.fingerprint.hash_u64(), - ) - .serialize(ser) + (&self.pkg_id, &self.name, &self.public, &self.fingerprint.hash_u64()).serialize(ser) } } @@ -736,10 +712,7 @@ enum LocalFingerprint { /// /// This is considered up-to-date if all of the `paths` are older than /// `output`, otherwise we need to recompile. - RerunIfChanged { - output: PathBuf, - paths: Vec, - }, + RerunIfChanged { output: PathBuf, paths: Vec }, /// This represents a single `rerun-if-env-changed` annotation printed by a /// build script. The exact env var and value are hashed here. There's no @@ -976,14 +949,8 @@ impl Fingerprint { } } ( - LocalFingerprint::RerunIfChanged { - output: aout, - paths: apaths, - }, - LocalFingerprint::RerunIfChanged { - output: bout, - paths: bpaths, - }, + LocalFingerprint::RerunIfChanged { output: aout, paths: apaths }, + LocalFingerprint::RerunIfChanged { output: bout, paths: bpaths }, ) => { if aout != bout { return DirtyReason::RerunIfChangedOutputFileChanged { @@ -999,14 +966,8 @@ impl Fingerprint { } } ( - LocalFingerprint::RerunIfEnvChanged { - var: akey, - val: avalue, - }, - LocalFingerprint::RerunIfEnvChanged { - var: bkey, - val: bvalue, - }, + LocalFingerprint::RerunIfEnvChanged { var: akey, val: avalue }, + LocalFingerprint::RerunIfEnvChanged { var: bkey, val: bvalue }, ) => { if *akey != *bkey { return DirtyReason::EnvVarsChanged { @@ -1026,7 +987,7 @@ impl Fingerprint { return DirtyReason::LocalFingerprintTypeChanged { old: b.kind(), new: a.kind(), - } + }; } } } @@ -1039,10 +1000,7 @@ impl Fingerprint { } for (a, b) in self.deps.iter().zip(old.deps.iter()) { if a.name != b.name { - return DirtyReason::UnitDependencyNameChanged { - old: b.name, - new: a.name, - }; + return DirtyReason::UnitDependencyNameChanged { old: b.name, new: a.name }; } if a.fingerprint.hash_u64() != b.fingerprint.hash_u64() { @@ -1112,10 +1070,7 @@ impl Fingerprint { self.fs_status = FsStatus::UpToDate { mtimes }; return Ok(()); }; - debug!( - "max output mtime for {:?} is {:?} {}", - pkg_root, max_path, max_mtime - ); + debug!("max output mtime for {:?} is {:?} {}", pkg_root, max_path, max_mtime); for dep in self.deps.iter() { let dep_mtimes = match &dep.fingerprint.fs_status { @@ -1148,10 +1103,7 @@ impl Fingerprint { None => continue, } }; - debug!( - "max dep mtime for {:?} is {:?} {}", - pkg_root, dep_path, dep_mtime - ); + debug!("max dep mtime for {:?} is {:?} {}", pkg_root, dep_path, dep_mtime); // If the dependency is newer than our own output then it was // recompiled previously. We transitively become stale ourselves in @@ -1293,21 +1245,12 @@ impl StaleItem { StaleItem::MissingFile(path) => { info!("stale: missing {:?}", path); } - StaleItem::ChangedFile { - reference, - reference_mtime, - stale, - stale_mtime, - } => { + StaleItem::ChangedFile { reference, reference_mtime, stale, stale_mtime } => { info!("stale: changed {:?}", stale); info!(" (vs) {:?}", reference); info!(" {:?} < {:?}", reference_mtime, stale_mtime); } - StaleItem::ChangedEnv { - var, - previous, - current, - } => { + StaleItem::ChangedEnv { var, previous, current } => { info!("stale: changed env {:?}", var); info!(" {:?} != {:?}", previous, current); } @@ -1354,9 +1297,7 @@ fn calculate(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult )?; let fingerprint = Arc::new(fingerprint); - build_runner - .fingerprints - .insert(unit.clone(), Arc::clone(&fingerprint)); + build_runner.fingerprints.insert(unit.clone(), Arc::clone(&fingerprint)); Ok(fingerprint) } @@ -1390,10 +1331,7 @@ fn calculate_normal( let local = if unit.mode.is_doc() || unit.mode.is_doc_scrape() { // rustdoc does not have dep-info files. let fingerprint = pkg_fingerprint(build_runner.bcx, &unit.pkg).with_context(|| { - format!( - "failed to determine package fingerprint for documenting {}", - unit.pkg - ) + format!("failed to determine package fingerprint for documenting {}", unit.pkg) })?; vec![LocalFingerprint::Precalculated(fingerprint)] } else { @@ -1468,7 +1406,7 @@ fn calculate_normal( let compile_kind = unit.kind.fingerprint_hash(); let mut declared_features = unit.pkg.summary().features().keys().collect::>(); declared_features.sort(); // to avoid useless rebuild if the user orders it's features - // differently + // differently Ok(Fingerprint { rustc: util::hash_u64(&build_runner.bcx.rustc().verbose_version), target: util::hash_u64(&unit.target), @@ -1613,11 +1551,9 @@ fn build_script_local_fingerprints( if let Some(fingerprint) = build_script_override_fingerprint(build_runner, unit) { debug!("override local fingerprints deps {}", unit.pkg); return ( - Box::new( - move |_: &BuildDeps, _: Option<&dyn Fn() -> CargoResult>| { - Ok(Some(vec![fingerprint])) - }, - ), + Box::new(move |_: &BuildDeps, _: Option<&dyn Fn() -> CargoResult>| { + Ok(Some(vec![fingerprint])) + }), true, // this is an overridden build script ); } @@ -1647,10 +1583,7 @@ fn build_script_local_fingerprints( // be stored here rather than the mtime of them. Some(f) => { let s = f()?; - debug!( - "old local fingerprints deps {:?} precalculated={:?}", - pkg_root, s - ); + debug!("old local fingerprints deps {:?} precalculated={:?}", pkg_root, s); return Ok(Some(vec![LocalFingerprint::Precalculated(s)])); } None => return Ok(None), @@ -1679,10 +1612,7 @@ fn build_script_override_fingerprint( let metadata = build_runner.get_run_build_script_metadata(unit); // Returns None if it is not overridden. let output = build_script_outputs.get(metadata)?; - let s = format!( - "overridden build state with hash: {}", - util::hash_u64(output) - ); + let s = format!("overridden build state with hash: {}", util::hash_u64(output)); Some(LocalFingerprint::Precalculated(s)) } @@ -1703,11 +1633,7 @@ fn local_fingerprints_deps( // Note that like the module comment above says we are careful to never // store an absolute path in `LocalFingerprint`, so ensure that we strip // absolute prefixes from them. - let output = deps - .build_script_output - .strip_prefix(target_root) - .unwrap() - .to_path_buf(); + let output = deps.build_script_output.strip_prefix(target_root).unwrap().to_path_buf(); let paths = deps .rerun_if_changed .iter() @@ -1716,11 +1642,7 @@ fn local_fingerprints_deps( local.push(LocalFingerprint::RerunIfChanged { output, paths }); } - local.extend( - deps.rerun_if_env_changed - .iter() - .map(LocalFingerprint::from_env), - ); + local.extend(deps.rerun_if_env_changed.iter().map(LocalFingerprint::from_env)); local } @@ -1792,17 +1714,11 @@ fn compare_old_fingerprint( match compare.as_ref() { Ok(None) => {} Ok(Some(reason)) => { - info!( - "fingerprint dirty for {}/{:?}/{:?}", - unit.pkg, unit.mode, unit.target, - ); + info!("fingerprint dirty for {}/{:?}/{:?}", unit.pkg, unit.mode, unit.target,); info!(" dirty: {reason:?}"); } Err(e) => { - info!( - "fingerprint error for {}/{:?}/{:?}", - unit.pkg, unit.mode, unit.target, - ); + info!("fingerprint error for {}/{:?}/{:?}", unit.pkg, unit.mode, unit.target,); info!(" err: {e:?}"); } } @@ -1830,12 +1746,12 @@ fn _compare_old_fingerprint( let old_fingerprint: Fingerprint = serde_json::from_str(&old_fingerprint_json) .with_context(|| internal("failed to deserialize json"))?; // Fingerprint can be empty after a failed rebuild (see comment in prepare_target). - if !old_fingerprint_short.is_empty() { - debug_assert_eq!( - util::to_hex(old_fingerprint.hash_u64()), - old_fingerprint_short - ); - } + // if !old_fingerprint_short.is_empty() { + // debug_assert_eq!( + // util::to_hex(old_fingerprint.hash_u64()), + // old_fingerprint_short + // ); + // } Ok(Some(new_fingerprint.compare(&old_fingerprint))) } @@ -1880,9 +1796,7 @@ fn pkg_fingerprint(bcx: &BuildContext<'_, '_>, pkg: &Package) -> CargoResult = ["git", "registry"] - .into_iter() - .map(|subfolder| cargo_home.join(subfolder)) - .collect(); + let skipable_dirs: Vec<_> = + ["git", "registry"].into_iter().map(|subfolder| cargo_home.join(subfolder)).collect(); Some(skipable_dirs) } else { None @@ -1962,10 +1874,7 @@ where }); } - debug!( - "all paths up-to-date relative to {:?} mtime={}", - reference, reference_mtime - ); + debug!("all paths up-to-date relative to {:?} mtime={}", reference, reference_mtime); None } @@ -2046,9 +1955,7 @@ pub fn translate_dep_info( // This also includes `CARGO` since if the code is explicitly wanting to // know that path, it should be rebuilt if it changes. The CARGO path is // not tracked elsewhere in the fingerprint. - on_disk_info - .env - .retain(|(key, _)| !rustc_cmd.get_envs().contains_key(key) || key == CARGO_ENV); + on_disk_info.env.retain(|(key, _)| !rustc_cmd.get_envs().contains_key(key) || key == CARGO_ENV); for file in depinfo.files { // The path may be absolute or relative, canonical or not. Make sure diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index de17527745e6..82ba2891f022 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -1251,6 +1251,7 @@ impl FixArgs { // the entire migration to fail. But those lints aren't needed to // migrate. cmd.arg("--cap-lints=allow"); + cmd.arg("-Wif-let-rescope"); } else { // This allows `cargo fix` to work even if the crate has #[deny(warnings)]. cmd.arg("--cap-lints=warn"); diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 44b3b6812c80..d6cb8d07b942 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1261,15 +1261,15 @@ pub fn to_real_manifest( // features.require(Feature::edition20xx())?; // } // ``` - if edition == Edition::Edition2024 { - features.require(Feature::edition2024())?; - } else if !edition.is_stable() { - // Guard in case someone forgets to add .require() - return Err(util::errors::internal(format!( - "edition {} should be gated", - edition - ))); - } + // if edition == Edition::Edition2024 { + // features.require(Feature::edition2024())?; + // } else if !edition.is_stable() { + // // Guard in case someone forgets to add .require() + // return Err(util::errors::internal(format!( + // "edition {} should be gated", + // edition + // ))); + // } if original_toml.project.is_some() { if Edition::Edition2024 <= edition {