From aa5a2e06029ee2500f0c6c24d1e880309015b349 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 24 May 2025 15:08:03 -0700 Subject: [PATCH 1/2] Add a new "fix" mode This introduces a new "fix" mode to run `cargo fix`. This is primarily intended to support edition migration testing. This includes two related changes: - Adds the ability to change the CapLints behavior because setting CapLints interferes with how `cargo fix` works. - Sets the mount mode of the source directory to read-write because `cargo fix` inherently needs to be able to write to the source directory. It probably needs more work for general purpose `cargo fix` testing (non-edition) because otherwise it would need to rebuild the source directory between toolchains. I don't need that right now, so deferred that till later. --- Cargo.toml | 2 +- docs/bot-usage.md | 1 + src/experiments.rs | 1 + src/runner/tasks.rs | 3 ++ src/runner/test.rs | 64 +++++++++++++++++++++++++++-- src/runner/worker.rs | 7 +++- src/server/routes/ui/experiments.rs | 1 + 7 files changed, 73 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 66b233c8..e918f70f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,7 +52,7 @@ remove_dir_all = "0.7" reqwest = { version = "0.11", features = ["blocking", "json"] } rusqlite = { version = "0.32.1", features = ["chrono", "functions", "bundled"] } rust_team_data = { git = "https://github.com/rust-lang/team" } -rustwide = { version = "0.19.1", features = [ +rustwide = { version = "0.19.3", features = [ "unstable", "unstable-toolchain-ci", ] } diff --git a/docs/bot-usage.md b/docs/bot-usage.md index abff8895..a642c924 100644 --- a/docs/bot-usage.md +++ b/docs/bot-usage.md @@ -85,6 +85,7 @@ The following experiment modes are currently available: * `check-only`: run `cargo check` on every crate (faster) * `clippy`: run `cargo clippy` on every crate * `rustdoc`: run `cargo doc --no-deps` on every crate +* `fix`: run `cargo fix` on every crate (intended for edition migration testing) The mode you should use depends on what your experiment is testing: diff --git a/src/experiments.rs b/src/experiments.rs index f060db8a..1a25973f 100644 --- a/src/experiments.rs +++ b/src/experiments.rs @@ -30,6 +30,7 @@ string_enum!(pub enum Mode { Clippy => "clippy", Rustdoc => "rustdoc", UnstableFeatures => "unstable-features", + Fix => "fix", }); string_enum!(pub enum CapLints { diff --git a/src/runner/tasks.rs b/src/runner/tasks.rs index 6af70f40..8942b655 100644 --- a/src/runner/tasks.rs +++ b/src/runner/tasks.rs @@ -48,6 +48,7 @@ pub(super) enum TaskStep { Clippy { tc: Toolchain, quiet: bool }, Rustdoc { tc: Toolchain, quiet: bool }, UnstableFeatures { tc: Toolchain }, + Fix { tc: Toolchain, quiet: bool }, } impl fmt::Debug for TaskStep { @@ -59,6 +60,7 @@ impl fmt::Debug for TaskStep { TaskStep::Clippy { ref tc, quiet } => ("clippy", quiet, Some(tc)), TaskStep::Rustdoc { ref tc, quiet } => ("doc", quiet, Some(tc)), TaskStep::UnstableFeatures { ref tc } => ("find unstable features on", false, Some(tc)), + TaskStep::Fix { ref tc, quiet } => ("fix", quiet, Some(tc)), }; write!(f, "{name}")?; @@ -124,6 +126,7 @@ impl Task { tc, false, ), + TaskStep::Fix { ref tc, quiet } => (&build_dir[tc], "fixing", test::fix, tc, quiet), }; let ctx = TaskCtx::new(build_dir, config, ex, toolchain, &self.krate, quiet); diff --git a/src/runner/test.rs b/src/runner/test.rs index bd609a9a..d0a666b1 100644 --- a/src/runner/test.rs +++ b/src/runner/test.rs @@ -1,4 +1,5 @@ use crate::crates::Crate; +use crate::experiments::CapLints; use crate::prelude::*; use crate::results::DiagnosticCode; use crate::results::{BrokenReason, FailureReason, TestResult}; @@ -9,7 +10,7 @@ use cargo_metadata::diagnostic::DiagnosticLevel; use cargo_metadata::{Message, Metadata, Package, Target}; use docsrs_metadata::Metadata as DocsrsMetadata; use remove_dir_all::remove_dir_all; -use rustwide::cmd::{CommandError, ProcessLinesActions, SandboxBuilder}; +use rustwide::cmd::{CommandError, MountKind, ProcessLinesActions, SandboxBuilder}; use rustwide::logging::LogStorage; use rustwide::{Build, PrepareError}; use std::collections::{BTreeSet, HashMap, HashSet}; @@ -89,6 +90,8 @@ fn run_cargo( check_errors: bool, local_packages: &[Package], env: HashMap<&'static str, String>, + mount_kind: MountKind, + cap_lints: Option, ) -> Fallible<()> { let local_packages_id: HashSet<_> = local_packages.iter().map(|p| &p.id).collect(); @@ -100,13 +103,17 @@ fn run_cargo( args.extend(tc_cargoflags.split(' ')); } - let mut rustflags = format!("--cap-lints={}", ctx.experiment.cap_lints.to_str()); + let mut rustflags = cap_lints + .map(|cap| format!("--cap-lints={cap}")) + .unwrap_or_default(); if let Some(ref tc_rustflags) = ctx.toolchain.rustflags { rustflags.push(' '); rustflags.push_str(tc_rustflags); } - let mut rustdocflags = format!("--cap-lints={}", ctx.experiment.cap_lints.to_str()); + let mut rustdocflags = cap_lints + .map(|cap| format!("--cap-lints={cap}")) + .unwrap_or_default(); if let Some(ref tc_rustdocflags) = ctx.toolchain.rustdocflags { rustdocflags.push(' '); rustdocflags.push_str(tc_rustdocflags); @@ -187,6 +194,7 @@ fn run_cargo( let mut command = build_env .cargo() .args(&args) + .source_dir_mount_kind(mount_kind) .env("CARGO_INCREMENTAL", "0") .env("RUST_BACKTRACE", "full") .env("RUSTFLAGS", rustflags) @@ -263,6 +271,8 @@ fn build(ctx: &TaskCtx, build_env: &Build, local_packages: &[Package]) -> Fallib true, local_packages, HashMap::default(), + MountKind::ReadOnly, + Some(ctx.experiment.cap_lints), )?; run_cargo( ctx, @@ -271,6 +281,8 @@ fn build(ctx: &TaskCtx, build_env: &Build, local_packages: &[Package]) -> Fallib true, local_packages, HashMap::default(), + MountKind::ReadOnly, + Some(ctx.experiment.cap_lints), )?; Ok(()) } @@ -283,6 +295,8 @@ fn test(ctx: &TaskCtx, build_env: &Build) -> Fallible<()> { false, &[], HashMap::default(), + MountKind::ReadOnly, + Some(ctx.experiment.cap_lints), ) } @@ -336,6 +350,8 @@ pub(super) fn test_check_only( true, local_packages_id, HashMap::default(), + MountKind::ReadOnly, + Some(ctx.experiment.cap_lints), ) { Ok(TestResult::BuildFail(failure_reason(&err))) } else { @@ -361,6 +377,8 @@ pub(super) fn test_clippy_only( true, local_packages, HashMap::default(), + MountKind::ReadOnly, + Some(ctx.experiment.cap_lints), ) { Ok(TestResult::BuildFail(failure_reason(&err))) } else { @@ -374,7 +392,16 @@ pub(super) fn test_rustdoc( local_packages: &[Package], ) -> Fallible { let run = |cargo_args, env| { - let res = run_cargo(ctx, build_env, cargo_args, true, local_packages, env); + let res = run_cargo( + ctx, + build_env, + cargo_args, + true, + local_packages, + env, + MountKind::ReadOnly, + Some(ctx.experiment.cap_lints), + ); // Make sure to remove the built documentation // There is no point in storing it after the build is done @@ -433,6 +460,35 @@ fn is_library(target: &Target) -> bool { .all(|k| !["example", "test", "bench"].contains(&k.as_str())) } +pub(crate) fn fix( + ctx: &TaskCtx, + build_env: &Build, + local_packages_id: &[Package], +) -> Fallible { + if let Err(err) = run_cargo( + ctx, + build_env, + &[ + "fix", + "--allow-no-vcs", + "--allow-dirty", + "--frozen", + "--all", + "--all-targets", + "--message-format=json", + ], + true, + local_packages_id, + HashMap::default(), + MountKind::ReadWrite, + None, + ) { + Ok(TestResult::BuildFail(failure_reason(&err))) + } else { + Ok(TestResult::TestPass) + } +} + #[test] fn test_failure_reason() { let error: anyhow::Error = anyhow!(CommandError::IO(std::io::Error::other("Test"))); diff --git a/src/runner/worker.rs b/src/runner/worker.rs index 24c8b941..ce41e282 100644 --- a/src/runner/worker.rs +++ b/src/runner/worker.rs @@ -123,7 +123,8 @@ impl<'a> Worker<'a> { | TaskStep::CheckOnly { tc, .. } | TaskStep::Clippy { tc, .. } | TaskStep::Rustdoc { tc, .. } - | TaskStep::UnstableFeatures { tc } => Some(tc), + | TaskStep::UnstableFeatures { tc } + | TaskStep::Fix { tc, .. } => Some(tc), }; if let Some(toolchain) = toolchain { if toolchain == self.ex.toolchains.last().unwrap() { @@ -301,6 +302,10 @@ impl<'a> Worker<'a> { quiet, }, Mode::UnstableFeatures => TaskStep::UnstableFeatures { tc: tc.clone() }, + Mode::Fix => TaskStep::Fix { + tc: tc.clone(), + quiet, + }, }, }; diff --git a/src/server/routes/ui/experiments.rs b/src/server/routes/ui/experiments.rs index eeb3c1d0..bd62f6af 100644 --- a/src/server/routes/ui/experiments.rs +++ b/src/server/routes/ui/experiments.rs @@ -41,6 +41,7 @@ impl ExperimentData { Mode::Clippy => "cargo clippy", Mode::Rustdoc => "cargo doc", Mode::UnstableFeatures => "unstable features", + Mode::Fix => "cargo fix", }, assigned_to: experiment.assigned_to.as_ref().map(|a| a.to_string()), priority: experiment.priority, From 6a446f1dbf7cafc80af59726c0f824d7b7c39aa8 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 28 Jun 2025 14:36:48 -0700 Subject: [PATCH 2/2] Fix clippy manual_is_multiple_of This was recently added to nightly. --- src/report/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/report/mod.rs b/src/report/mod.rs index c9c003e1..f9091ce1 100644 --- a/src/report/mod.rs +++ b/src/report/mod.rs @@ -300,7 +300,7 @@ fn write_logs( } for (i, krate) in crates.iter().enumerate() { - if i % progress_every == 0 { + if i.is_multiple_of(progress_every) { info!("wrote logs for {i}/{num_crates} crates") }