From f91ed3ce50898bf503912d98d0679e60fc61caa0 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 18 Oct 2026 06:56:40 +0200 Subject: [PATCH] Refuse implicit compiler driver builds Add opt-in driver-build refusal to the library and CLI while retaining normal defaults and the independent metadata-library build control. Cover driver reuse, each refusal reason, build-error propagation and the public CLI's missing-driver path without provisioning a compiler. Keep fallible test setup and expected-first assertions, and repair a stale historical path comment required by the repository self-lint gate. --- cargo-dylint/src/main.rs | 13 +++ cargo-dylint/src/no_driver_build_tests.rs | 65 +++++++++++ cargo-dylint/tests/no_driver_build.rs | 58 ++++++++++ dylint/src/driver_builder.rs | 101 ++++++++++++------ .../driver_builder/no_driver_build_tests.rs | 99 +++++++++++++++++ dylint/src/opts.rs | 6 ++ utils/testing/src/lib.rs | 4 +- 7 files changed, 311 insertions(+), 35 deletions(-) create mode 100644 cargo-dylint/src/no_driver_build_tests.rs create mode 100644 cargo-dylint/tests/no_driver_build.rs create mode 100644 dylint/src/driver_builder/no_driver_build_tests.rs diff --git a/cargo-dylint/src/main.rs b/cargo-dylint/src/main.rs index f5f023d51..e2f159b27 100644 --- a/cargo-dylint/src/main.rs +++ b/cargo-dylint/src/main.rs @@ -50,6 +50,13 @@ struct Dylint { #[clap(long, help = "Do not check other packages within the workspace")] no_deps: bool, + #[clap( + global = true, + long, + help = "Do not build missing, outdated, or unverifiable compiler drivers" + )] + no_driver_build: bool, + #[clap( action = ArgAction::Append, number_of_values = 1, @@ -258,6 +265,7 @@ impl From for dylint::opts::Dylint { fix, keep_going, no_deps, + no_driver_build, packages, workspace, args, @@ -304,6 +312,7 @@ impl From for dylint::opts::Dylint { }), }; Self { + no_driver_build, pipe_stderr, pipe_stdout, quiet, @@ -425,3 +434,7 @@ mod tests { Opts::command().debug_assert(); } } + +#[cfg(test)] +#[path = "no_driver_build_tests.rs"] +mod no_driver_build_tests; diff --git a/cargo-dylint/src/no_driver_build_tests.rs b/cargo-dylint/src/no_driver_build_tests.rs new file mode 100644 index 000000000..9e4bd9ef3 --- /dev/null +++ b/cargo-dylint/src/no_driver_build_tests.rs @@ -0,0 +1,65 @@ +//! CLI parsing and library-option conversion for driver-build refusal. + +use super::*; + +#[test] +fn no_driver_build_is_explicit_and_independent_of_no_build() -> anyhow::Result<()> { + for (arguments, no_driver_build, no_build) in [ + (vec!["cargo", "dylint"], false, false), + (vec!["cargo", "dylint", "--no-driver-build"], true, false), + (vec!["cargo", "dylint", "--no-build"], false, true), + ( + vec!["cargo", "dylint", "--no-driver-build", "--no-build"], + true, + true, + ), + ] { + let parsed = Opts::try_parse_from(arguments)?; + let CargoSubcommand::Dylint(cli) = parsed.subcmd; + let opts = dylint::opts::Dylint::from(cli); + assert_eq!(opts.no_driver_build, no_driver_build); + let dylint::opts::Operation::Check(check) = opts.operation else { + anyhow::bail!("the default operation should remain checking"); + }; + assert_eq!(check.lib_sel.no_build, no_build); + } + Ok(()) +} + +#[test] +fn no_driver_build_reaches_the_list_operation() -> anyhow::Result<()> { + for arguments in [ + vec!["cargo", "dylint", "list", "--no-driver-build"], + vec!["cargo", "dylint", "list", "--no-driver-build", "--no-build"], + vec!["cargo", "dylint", "list", "--no-build", "--no-driver-build"], + ] { + let parsed = Opts::try_parse_from(arguments)?; + let CargoSubcommand::Dylint(cli) = parsed.subcmd; + let opts = dylint::opts::Dylint::from(cli); + assert!(opts.no_driver_build, "listing must retain driver refusal"); + assert!(matches!(opts.operation, dylint::opts::Operation::List(_))); + } + Ok(()) +} + +#[test] +fn no_driver_build_before_list_keeps_existing_argument_conflict() { + let error = Opts::try_parse_from(["cargo", "dylint", "--no-driver-build", "list"]) + .expect_err("Dylint options must follow the final subcommand"); + assert_eq!(clap::error::ErrorKind::UnknownArgument, error.kind()); +} + +#[test] +fn help_describes_driver_refusal_separately_from_library_builds() { + let error = Opts::try_parse_from(["cargo", "dylint", "--help"]) + .expect_err("help should exit before running Dylint"); + assert_eq!(clap::error::ErrorKind::DisplayHelp, error.kind()); + let help = error + .to_string() + .split_whitespace() + .collect::>() + .join(" "); + assert!(help.contains("--no-driver-build"), "{help}"); + assert!(help.contains("unverifiable compiler drivers"), "{help}"); + assert!(help.contains("Do not build metadata entries"), "{help}"); +} diff --git a/cargo-dylint/tests/no_driver_build.rs b/cargo-dylint/tests/no_driver_build.rs new file mode 100644 index 000000000..b588b6447 --- /dev/null +++ b/cargo-dylint/tests/no_driver_build.rs @@ -0,0 +1,58 @@ +//! Exercises missing-driver refusal through the real CLI without external tools. + +use assert_cmd::cargo::cargo_bin_cmd; +use dylint_internal::env; +use predicates::prelude::*; +use std::{env::consts, fs}; +use tempfile::{Builder, tempdir, tempdir_in}; + +#[test] +fn missing_driver_refusal_reaches_get_without_starting_a_build() -> std::io::Result<()> { + let tempdir = tempdir()?; + let library = Builder::new() + .prefix(&format!("{}refusal", consts::DLL_PREFIX)) + .suffix(&format!("@no-driver-build-test{}", consts::DLL_SUFFIX)) + .tempfile_in(&tempdir)?; + let empty_path = tempdir_in(&tempdir)?; + + // leynos: Listing an explicitly named library reaches get before loading the library + // or querying Cargo metadata. No executable is available if this regresses + // into the build path: CARGO and PATH are scoped only to this child process. + cargo_bin_cmd!("cargo-dylint") + .current_dir(&tempdir) + .env(env::DYLINT_DRIVER_PATH, tempdir.path()) + .env("CARGO", empty_path.path().join("cargo")) + .env("PATH", empty_path.path()) + .args([ + "dylint", + "list", + "--no-driver-build", + "--no-metadata", + "--lib-path", + ]) + .arg(library.path()) + .assert() + .failure() + .stderr( + predicate::str::contains("Compiler driver") + .and(predicate::str::contains("is missing")) + .and(predicate::str::contains( + "--no-driver-build requires a prebuilt compatible driver", + )), + ); + + assert!( + !tempdir + .path() + .join("no-driver-build-test/dylint-driver") + .exists(), + "refusal must not create a driver", + ); + let entries = fs::read_dir(&empty_path)?; + assert_eq!( + 0, + entries.count(), + "no tool should be provisioned in the isolated executable directory", + ); + Ok(()) +} diff --git a/dylint/src/driver_builder.rs b/dylint/src/driver_builder.rs index 94c2a6975..1757bb384 100644 --- a/dylint/src/driver_builder.rs +++ b/dylint/src/driver_builder.rs @@ -57,9 +57,20 @@ pub fn get(opts: &opts::Dylint, toolchain: &str) -> Result { } let driver = driver_dir.join("dylint-driver"); - if !driver.exists() || is_outdated(opts, toolchain, &driver)? { - build(opts, toolchain, &driver_dir)?; - } + let status = if driver.exists() { + is_outdated(toolchain, &driver).map(|outdated| { + if outdated { + DriverStatus::Outdated + } else { + DriverStatus::Current + } + }) + } else { + Ok(DriverStatus::Missing) + }; + ensure_driver(opts, &driver, status, || { + build(opts, toolchain, &driver_dir) + })?; Ok(driver) } @@ -88,28 +99,55 @@ fn dylint_drivers() -> Result { } } -fn is_outdated(opts: &opts::Dylint, toolchain: &str, driver: &Path) -> Result { - (|| -> Result { - let mut command = dylint_driver(toolchain, driver)?; - let output = command.args(["-V"]).logged_output(true)?; - let stdout = std::str::from_utf8(&output.stdout)?; - let theirs = stdout - .trim_end() - .rsplit_once(' ') - .map(|(_, s)| s) - .ok_or_else(|| anyhow!("Could not determine driver version"))?; - - let their_version = Version::parse(theirs) - .with_context(|| format!("Could not parse driver version `{theirs}`"))?; - - let our_version = Version::parse(env!("CARGO_PKG_VERSION"))?; - - Ok(their_version < our_version) - })() - .or_else(|error| { - warn(opts, &error.to_string()); - Ok(true) - }) +/// A cached driver's status under the existing version policy. +#[derive(Clone, Copy, Debug)] +enum DriverStatus { + Missing, + Current, + Outdated, +} + +fn is_outdated(toolchain: &str, driver: &Path) -> Result { + let mut command = dylint_driver(toolchain, driver)?; + let output = command.args(["-V"]).logged_output(true)?; + let stdout = std::str::from_utf8(&output.stdout)?; + let theirs = stdout + .trim_end() + .rsplit_once(' ') + .map(|(_, s)| s) + .ok_or_else(|| anyhow!("Could not determine driver version"))?; + + let their_version = Version::parse(theirs) + .with_context(|| format!("Could not parse driver version `{theirs}`"))?; + + let our_version = Version::parse(env!("CARGO_PKG_VERSION"))?; + + Ok(their_version < our_version) +} + +// leynos: Keep the refusal at the same decision that invokes the build operation. The +// injected operation lets tests exercise every outcome without invoking Cargo. +fn ensure_driver( + opts: &opts::Dylint, + driver: &Path, + status: Result, + build: impl FnOnce() -> Result<()>, +) -> Result<()> { + let status = match status { + Ok(DriverStatus::Current) => return Ok(()), + Ok(DriverStatus::Missing) => "missing", + Ok(DriverStatus::Outdated) => "outdated", + Err(error) => { + warn(opts, &error.to_string()); + "unverifiable" + } + }; + ensure!( + !opts.no_driver_build, + "Compiler driver `{}` is {status}; --no-driver-build requires a prebuilt compatible driver", + driver.display() + ); + build() } #[cfg_attr(dylint_lib = "supplementary", allow(commented_out_code))] @@ -315,13 +353,10 @@ mod tests { ); build(&opts::Dylint::default(), "nightly", tempdir.path()).unwrap(); - assert!( - !is_outdated( - &opts::Dylint::default(), - "nightly", - &tempdir.path().join("dylint-driver") - ) - .unwrap() - ); + assert!(!is_outdated("nightly", &tempdir.path().join("dylint-driver")).unwrap()); } } + +#[cfg(test)] +#[path = "driver_builder/no_driver_build_tests.rs"] +mod no_driver_build_tests; diff --git a/dylint/src/driver_builder/no_driver_build_tests.rs b/dylint/src/driver_builder/no_driver_build_tests.rs new file mode 100644 index 000000000..2a803d65f --- /dev/null +++ b/dylint/src/driver_builder/no_driver_build_tests.rs @@ -0,0 +1,99 @@ +//! Driver-build refusal tests that never run a compiler or cached driver. + +use super::*; +use std::cell::Cell; + +#[test] +fn refuses_missing_outdated_and_unverifiable_drivers_without_building() { + for (status, reason) in [ + (Ok(DriverStatus::Missing), "missing"), + (Ok(DriverStatus::Outdated), "outdated"), + (Err(anyhow!("version query failed")), "unverifiable"), + ] { + let built = Cell::new(false); + let opts = opts::Dylint { + no_driver_build: true, + ..Default::default() + }; + let result = ensure_driver(&opts, Path::new("cached-driver"), status, || { + built.set(true); + Ok(()) + }); + assert!(!built.get(), "the {reason} driver must not be built"); + let message = result + .err() + .as_ref() + .map(ToString::to_string) + .unwrap_or_default(); + assert!(message.contains(reason), "{message}"); + assert!(message.contains("cached-driver"), "{message}"); + assert!(message.contains("--no-driver-build"), "{message}"); + } +} + +#[test] +fn defaults_still_build_missing_outdated_and_unverifiable_drivers() { + for status in [ + Ok(DriverStatus::Missing), + Ok(DriverStatus::Outdated), + Err(anyhow!("version query failed")), + ] { + let built = Cell::new(false); + let result = ensure_driver( + &opts::Dylint::default(), + Path::new("cached-driver"), + status, + || { + built.set(true); + Ok(()) + }, + ); + assert!( + result.is_ok(), + "default behavior should rebuild the driver: {result:?}" + ); + assert!( + built.get(), + "default behavior must invoke the build operation" + ); + } +} + +#[test] +fn current_driver_is_reused_with_or_without_refusal() { + for no_driver_build in [false, true] { + let opts = opts::Dylint { + no_driver_build, + ..Default::default() + }; + let built = Cell::new(false); + let result = ensure_driver( + &opts, + Path::new("cached-driver"), + Ok(DriverStatus::Current), + || { + built.set(true); + Ok(()) + }, + ); + assert!( + result.is_ok(), + "a current driver must remain usable: {result:?}" + ); + assert!(!built.get(), "a current driver must not be rebuilt"); + } +} + +#[test] +fn default_mode_propagates_build_failures() { + let result = ensure_driver( + &opts::Dylint::default(), + Path::new("cached-driver"), + Ok(DriverStatus::Missing), + || Err(anyhow!("build failed")), + ); + assert_eq!( + Some("build failed"), + result.err().as_ref().map(ToString::to_string).as_deref() + ); +} diff --git a/dylint/src/opts.rs b/dylint/src/opts.rs index 99fb13d30..acb74aa8e 100644 --- a/dylint/src/opts.rs +++ b/dylint/src/opts.rs @@ -23,6 +23,12 @@ use std::sync::LazyLock; #[allow(clippy::struct_excessive_bools)] #[derive(Clone, Debug, Default)] pub struct Dylint { + /// Refuse to build a missing, outdated, or unverifiable compiler driver. + /// + /// Defaults to `false`. Existing drivers use the normal version policy. To prevent building + /// lint libraries, use [`LibrarySelection::no_build`]. + pub no_driver_build: bool, + pub pipe_stderr: Option, pub pipe_stdout: Option, diff --git a/utils/testing/src/lib.rs b/utils/testing/src/lib.rs index 5b528097d..58f9a39dd 100644 --- a/utils/testing/src/lib.rs +++ b/utils/testing/src/lib.rs @@ -295,8 +295,8 @@ fn linking_flags( static RE: LazyLock = LazyLock::new(|| Regex::new(r"^\s*Running\s*`(.*)`$").unwrap()); fn rustc_flags(_metadata: &Metadata, package: &Package, target: &Target) -> Result> { - // Previously, this function removed the example executable from `target/debug/examples` before - // running `cargo build --verbose`. With Cargo's legacy layout, that forced the example to be + // Previously, this function removed the example executable from Cargo's legacy examples + // output directory before running `cargo build --verbose`. That forced the example to be // recompiled and exposed its rustc invocation in Cargo's verbose output. With the new // build-directory layout, Cargo can restore the removed executable from its cached build // artifact without invoking rustc, making removal ineffective as a way to force recompilation.