Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cargo-dylint/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -258,6 +265,7 @@ impl From<Dylint> for dylint::opts::Dylint {
fix,
keep_going,
no_deps,
no_driver_build,
packages,
workspace,
args,
Expand Down Expand Up @@ -304,6 +312,7 @@ impl From<Dylint> for dylint::opts::Dylint {
}),
};
Self {
no_driver_build,
pipe_stderr,
pipe_stdout,
quiet,
Expand Down Expand Up @@ -425,3 +434,7 @@ mod tests {
Opts::command().debug_assert();
}
}

#[cfg(test)]
#[path = "no_driver_build_tests.rs"]
mod no_driver_build_tests;
65 changes: 65 additions & 0 deletions cargo-dylint/src/no_driver_build_tests.rs
Original file line number Diff line number Diff line change
@@ -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::<Vec<_>>()
.join(" ");
assert!(help.contains("--no-driver-build"), "{help}");
assert!(help.contains("unverifiable compiler drivers"), "{help}");
assert!(help.contains("Do not build metadata entries"), "{help}");
}
58 changes: 58 additions & 0 deletions cargo-dylint/tests/no_driver_build.rs
Original file line number Diff line number Diff line change
@@ -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(())
}
101 changes: 68 additions & 33 deletions dylint/src/driver_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,20 @@ pub fn get(opts: &opts::Dylint, toolchain: &str) -> Result<PathBuf> {
}

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)
}
Expand Down Expand Up @@ -88,28 +99,55 @@ fn dylint_drivers() -> Result<PathBuf> {
}
}

fn is_outdated(opts: &opts::Dylint, toolchain: &str, driver: &Path) -> Result<bool> {
(|| -> Result<bool> {
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<bool> {
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<DriverStatus>,
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))]
Expand Down Expand Up @@ -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;
99 changes: 99 additions & 0 deletions dylint/src/driver_builder/no_driver_build_tests.rs
Original file line number Diff line number Diff line change
@@ -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()
);
}
6 changes: 6 additions & 0 deletions dylint/src/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,

pub pipe_stdout: Option<String>,
Expand Down
Loading
Loading