diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3eaf64c85..ecdfec4fb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -173,6 +173,9 @@ jobs: echo /Applications/Xcode* | sort | head -n1 | xargs rm -rf # du -sh /*/* 2>/dev/null | sort -h || true + - name: Preinstall toolchains + run: cargo run -p dylint_internal --bin preinstall-toolchains + - name: Test run: | if [[ '${{ matrix.package }}' =~ ^cargo-dylint ]]; then diff --git a/cargo-dylint/tests/depinfo_dylint_libs.rs b/cargo-dylint/tests/integration/depinfo_dylint_libs.rs similarity index 100% rename from cargo-dylint/tests/depinfo_dylint_libs.rs rename to cargo-dylint/tests/integration/depinfo_dylint_libs.rs diff --git a/cargo-dylint/tests/dylint_driver_path.rs b/cargo-dylint/tests/integration/dylint_driver_path.rs similarity index 94% rename from cargo-dylint/tests/dylint_driver_path.rs rename to cargo-dylint/tests/integration/dylint_driver_path.rs index f01a86f17..6b5d11063 100644 --- a/cargo-dylint/tests/dylint_driver_path.rs +++ b/cargo-dylint/tests/integration/dylint_driver_path.rs @@ -7,6 +7,7 @@ use dylint_internal::{ use std::fs::create_dir_all; use tempfile::tempdir_in; +#[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))] #[test] fn dylint_driver_path() { let tempdir = tempdir_in(env!("CARGO_MANIFEST_DIR")).unwrap(); diff --git a/cargo-dylint/tests/fix.rs b/cargo-dylint/tests/integration/fix.rs similarity index 97% rename from cargo-dylint/tests/fix.rs rename to cargo-dylint/tests/integration/fix.rs index fe8f436d2..70a24409d 100644 --- a/cargo-dylint/tests/fix.rs +++ b/cargo-dylint/tests/integration/fix.rs @@ -35,6 +35,7 @@ fn main() { } "#; +#[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))] #[test] fn fix() { let tempdir = tempdir().unwrap(); diff --git a/cargo-dylint/tests/library_packages.rs b/cargo-dylint/tests/integration/library_packages.rs similarity index 100% rename from cargo-dylint/tests/library_packages.rs rename to cargo-dylint/tests/integration/library_packages.rs diff --git a/cargo-dylint/tests/list.rs b/cargo-dylint/tests/integration/list.rs similarity index 100% rename from cargo-dylint/tests/list.rs rename to cargo-dylint/tests/integration/list.rs diff --git a/cargo-dylint/tests/integration/main.rs b/cargo-dylint/tests/integration/main.rs new file mode 100644 index 000000000..81becfbc5 --- /dev/null +++ b/cargo-dylint/tests/integration/main.rs @@ -0,0 +1,9 @@ +mod depinfo_dylint_libs; +mod dylint_driver_path; +mod fix; +mod library_packages; +mod list; +mod nightly_toolchain; +mod no_deps; +mod package_options; +mod warn; diff --git a/cargo-dylint/tests/nightly_toolchain.rs b/cargo-dylint/tests/integration/nightly_toolchain.rs similarity index 100% rename from cargo-dylint/tests/nightly_toolchain.rs rename to cargo-dylint/tests/integration/nightly_toolchain.rs diff --git a/cargo-dylint/tests/no_deps.rs b/cargo-dylint/tests/integration/no_deps.rs similarity index 100% rename from cargo-dylint/tests/no_deps.rs rename to cargo-dylint/tests/integration/no_deps.rs diff --git a/cargo-dylint/tests/package_options.rs b/cargo-dylint/tests/integration/package_options.rs similarity index 100% rename from cargo-dylint/tests/package_options.rs rename to cargo-dylint/tests/integration/package_options.rs diff --git a/cargo-dylint/tests/warn.rs b/cargo-dylint/tests/integration/warn.rs similarity index 100% rename from cargo-dylint/tests/warn.rs rename to cargo-dylint/tests/integration/warn.rs diff --git a/driver/Cargo.lock b/driver/Cargo.lock index b65204abf..f41c621ea 100644 --- a/driver/Cargo.lock +++ b/driver/Cargo.lock @@ -2,6 +2,15 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "aho-corasick" +version = "1.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e60d3430d3a69478ad0993f19238d2df97c507009a52b3c10addcd7f6bcb916" +dependencies = [ + "memchr", +] + [[package]] name = "anyhow" version = "1.0.89" @@ -34,6 +43,8 @@ version = "3.2.0" dependencies = [ "anyhow", "log", + "once_cell", + "regex", ] [[package]] @@ -70,6 +81,12 @@ version = "2.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" +[[package]] +name = "once_cell" +version = "1.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" + [[package]] name = "proc-macro2" version = "1.0.86" @@ -88,6 +105,35 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "regex" +version = "1.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4219d74c6b67a3654a9fbebc4b419e22126d13d2f3c4a07ee0cb61ff79a79619" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38caf58cc5ef2fed281f89292ef23f6365465ed9a41b7a7754eb4e26496c92df" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a66a03ae7c801facd77a29370b4faec201768915ac14a721ba36f20bc9c209b" + [[package]] name = "rustc_version" version = "0.4.1" diff --git a/dylint/src/driver_builder.rs b/dylint/src/driver_builder.rs index dd3d2907d..64636d403 100644 --- a/dylint/src/driver_builder.rs +++ b/dylint/src/driver_builder.rs @@ -9,10 +9,10 @@ use dylint_internal::{ use semver::Version; use std::{ env::consts, - fs::{copy, create_dir_all, write}, + fs::{copy, create_dir_all, rename, write}, path::{Path, PathBuf}, }; -use tempfile::tempdir; +use tempfile::{tempdir, NamedTempFile}; include!(concat!(env!("OUT_DIR"), "/dylint_driver_manifest_dir.rs")); @@ -87,7 +87,7 @@ 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)?; + build(opts, toolchain, &driver_dir)?; } Ok(driver) @@ -142,7 +142,7 @@ fn is_outdated(opts: &opts::Dylint, toolchain: &str, driver: &Path) -> Result Result<()> { +fn build(opts: &opts::Dylint, toolchain: &str, driver_dir: &Path) -> Result<()> { let tempdir = tempdir().with_context(|| "`tempdir` failed")?; let package = tempdir.path(); @@ -182,14 +182,30 @@ fn build(opts: &opts::Dylint, toolchain: &str, driver: &Path) -> Result<()> { .target_directory .join("debug") .join(format!("dylint_driver-{toolchain}{}", consts::EXE_SUFFIX)); + + let named_temp_file = NamedTempFile::new_in(driver_dir) + .with_context(|| "Could not create temporary directory")?; + #[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))] - copy(&binary, driver).with_context(|| { + copy(&binary, &named_temp_file).with_context(|| { format!( "Could not copy `{binary}` to `{}`", - driver.to_string_lossy() + named_temp_file.path().to_string_lossy() ) })?; + let driver = driver_dir.join("dylint-driver"); + + // smoelius: Windows requires that the old file be moved out of the way first. + if cfg!(target_os = "windows") { + let temp_path = NamedTempFile::new_in(driver_dir) + .map(NamedTempFile::into_temp_path) + .with_context(|| "Could not create temporary directory")?; + rename(&driver, &temp_path).unwrap_or_default(); + } + + named_temp_file.persist(&driver)?; + Ok(()) } @@ -228,17 +244,44 @@ fn initialize(toolchain: &str, package: &Path) -> Result<()> { #[cfg(test)] mod test { use super::*; + use std::process::{Command, ExitStatus}; // smoelius: `tempdir` is a temporary directory. So there should be no race here. #[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))] #[test] fn nightly() { let tempdir = tempdir().unwrap(); - build( - &opts::Dylint::default(), - "nightly", - &tempdir.path().join("dylint-driver"), - ) - .unwrap(); + build(&opts::Dylint::default(), "nightly", tempdir.path()).unwrap(); + } + + // smoelius: As mentioned above, `tempdir` is a temporary directory. So there should be no race + // here. + #[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))] + #[test] + fn can_install_while_driver_is_running() { + const WHICH: &str = if cfg!(target_os = "windows") { + "where" + } else { + "which" + }; + + let tempdir = tempdir().unwrap(); + let driver = tempdir.path().join("dylint-driver"); + + // Set tmpdir/dylint-driver to `sleep` and call it with `infinity`. + let stdout = Command::new(WHICH) + .arg("sleep") + .logged_output(false) + .unwrap() + .stdout; + let sleep_path = String::from_utf8(stdout).unwrap(); + copy(sleep_path.trim_end(), &driver).unwrap(); + let mut child = Command::new(driver).arg("infinity").spawn().unwrap(); + + // Install should not fail with "text file busy". + build(&opts::Dylint::default(), "nightly", tempdir.path()).unwrap(); + + child.kill().unwrap(); + let _: ExitStatus = child.wait().unwrap(); } } diff --git a/examples/general/crate_wide_allow/src/lib.rs b/examples/general/crate_wide_allow/src/lib.rs index 2b04417ef..06f1f76db 100644 --- a/examples/general/crate_wide_allow/src/lib.rs +++ b/examples/general/crate_wide_allow/src/lib.rs @@ -72,13 +72,28 @@ mod test { use cargo_metadata::MetadataCommand; use dylint_internal::env; use predicates::prelude::*; - use std::{env::consts, sync::Mutex}; + use std::{ + env::consts, + sync::{Mutex, MutexGuard}, + }; - static MUTEX: Mutex<()> = Mutex::new(()); + fn mutex>>() -> T::Output { + static MUTEX: Mutex<()> = Mutex::new(()); + + let lock = MUTEX.lock().unwrap(); + + // smoelius: Ensure the `clippy` component is installed. + Command::new("rustup") + .args(["component", "add", "clippy"]) + .assert() + .success(); + + T::maybe_return(lock) + } #[test] fn ui() { - let _lock = MUTEX.lock().unwrap(); + let _lock = mutex::(); dylint_testing::ui_test_example(env!("CARGO_PKG_NAME"), "ui"); } @@ -115,7 +130,7 @@ mod test { fn test(rustflags: &str, assert: impl Fn(Assert) -> Assert) { const MANIFEST_DIR: &str = "../../.."; - let _lock = MUTEX.lock().unwrap(); + let _lock = mutex::(); Command::new("cargo") .current_dir(MANIFEST_DIR) @@ -159,6 +174,8 @@ mod test { #[test] fn premise_manifest_sanity() { + mutex::(); + let mut command = Command::new("cargo"); command.args(["clippy"]); command.current_dir("ui_manifest"); @@ -171,6 +188,8 @@ mod test { /// Verify that `allow`ing a lint in the manifest does not silently override `--warn`. #[test] fn premise_manifest_warn() { + mutex::(); + let mut command = Command::new("cargo"); command.args(["clippy", "--", "--warn=clippy::assertions-on-constants"]); command.current_dir("ui_manifest"); @@ -183,6 +202,8 @@ mod test { /// Verify that `allow`ing a lint in the manifest does not silently override `--deny`. #[test] fn premise_manifest_deny() { + mutex::(); + let mut command = Command::new("cargo"); command.args(["clippy", "--", "--deny=clippy::assertions-on-constants"]); command.current_dir("ui_manifest"); @@ -191,4 +212,27 @@ mod test { .failure() .stderr(predicate::str::contains(ASSERTIONS_ON_CONSTANTS_WARNING)); } + + mod maybe_return { + pub trait MaybeReturn { + type Output; + fn maybe_return(value: T) -> Self::Output; + } + + pub struct Yes; + + pub struct No; + + impl MaybeReturn for Yes { + type Output = T; + fn maybe_return(value: T) -> Self::Output { + value + } + } + + impl MaybeReturn for No { + type Output = (); + fn maybe_return(_value: T) -> Self::Output {} + } + } } diff --git a/examples/testing/marker/Cargo.lock b/examples/testing/marker/Cargo.lock index 660694a3e..03dff224e 100644 --- a/examples/testing/marker/Cargo.lock +++ b/examples/testing/marker/Cargo.lock @@ -140,6 +140,7 @@ dependencies = [ "is-terminal", "log", "once_cell", + "regex", "serde", "thiserror", "toml", diff --git a/internal/Cargo.toml b/internal/Cargo.toml index f5bb5dcfa..3f6834adc 100644 --- a/internal/Cargo.toml +++ b/internal/Cargo.toml @@ -9,6 +9,8 @@ repository = "https://github.com/trailofbits/dylint" [dependencies] anyhow = "1.0" +once_cell = "1.19" +regex = "1.10" ansi_term = { version = "0.12", optional = true } bitflags = { version = "2.6", optional = true } @@ -21,8 +23,6 @@ home = { version = "0.5", optional = true } if_chain = { version = "1.0", optional = true } is-terminal = { version = "0.4", optional = true } log = { version = "0.4", optional = true } -once_cell = { version = "1.19", optional = true } -regex = { version = "1.10", optional = true } rust-embed = { version = "8.5", features = [ "include-exclude", ], optional = true } @@ -44,7 +44,6 @@ cargo = [ "command", "home", "is-terminal", - "once_cell", ] clippy_utils = ["semver", "toml_edit"] command = ["log"] @@ -53,7 +52,7 @@ examples = ["cargo", "cargo-util", "rustup", "walkdir"] git = ["command", "git2", "if_chain"] packaging = ["cargo", "rust-embed"] rustup = ["command"] -sed = ["regex"] +sed = [] testing = ["ctor", "env_logger", "packaging"] [lints] diff --git a/internal/src/bin/preinstall-toolchains.rs b/internal/src/bin/preinstall-toolchains.rs new file mode 100644 index 000000000..6def88099 --- /dev/null +++ b/internal/src/bin/preinstall-toolchains.rs @@ -0,0 +1,105 @@ +use anyhow::{anyhow, ensure, Context, Result}; +use once_cell::sync::Lazy; +use regex::Regex; +use std::{ + fs::File, + io::{BufRead, BufReader}, + path::Path, + process::{Command, Stdio}, + thread, +}; + +fn main() -> Result<()> { + let toolchains = collect_toolchains(&["cargo-dylint", "internal"])?; + + println!("{:#?}", &toolchains); + + let handles = std::iter::once("nightly".to_owned()) + .chain(toolchains) + .map(|toolchain| (thread::spawn(move || install_toolchain(&toolchain)))); + + for handle in handles { + let () = handle + .join() + .map_err(|error| anyhow!("{error:?}")) + .and_then(std::convert::identity)?; + } + + Ok(()) +} + +fn collect_toolchains(dirs: &[&str]) -> Result> { + let mut toolchains = Vec::new(); + + for dir in dirs { + let toolchains_for_dir = collect_toolchains_for_dir(dir)?; + toolchains.extend(toolchains_for_dir); + } + + toolchains.sort(); + toolchains.dedup(); + + Ok(toolchains) +} + +fn collect_toolchains_for_dir(dir: &str) -> Result> { + let mut ls_files = Command::new("git") + .args(["ls-files", dir]) + .stdout(Stdio::piped()) + .spawn() + .with_context(|| "Could not spawn `git ls-files`")?; + + let stdout = ls_files.stdout.take().unwrap(); + BufReader::new(stdout) + .lines() + .try_fold(Vec::new(), |mut toolchains, result| -> Result<_> { + let path = result.with_context(|| "Could not read from `git ls-files`")?; + let toolchains_for_path = collect_toolchains_for_path(path)?; + toolchains.extend(toolchains_for_path); + Ok(toolchains) + }) +} + +static RE: Lazy = + Lazy::new(|| Regex::new(r"\").unwrap()); + +fn collect_toolchains_for_path(path: impl AsRef) -> Result> { + let file = File::open(&path).with_context(|| format!("Could not open {:?}", path.as_ref()))?; + BufReader::new(file) + .lines() + .try_fold(Vec::new(), |mut toolchains, result| -> Result<_> { + let line = + result.with_context(|| format!("Could not read from {:?}", path.as_ref()))?; + let n = line.find("//").unwrap_or(line.len()); + toolchains.extend(RE.find_iter(&line[..n]).map(|m| m.as_str().to_owned())); + Ok(toolchains) + }) +} + +fn install_toolchain(toolchain: &str) -> Result<()> { + let status = Command::new("rustup") + .args([ + "install", + toolchain, + "--profile=minimal", + "--no-self-update", + ]) + .status() + .with_context(|| format!("Could not install {toolchain} with `rustup`"))?; + ensure!(status.success()); + + let status = Command::new("rustup") + .args([ + "component", + "add", + "llvm-tools-preview", + "rustc-dev", + "--toolchain", + toolchain, + ]) + .status() + .with_context(|| format!("Could not add components to {toolchain} with `rustup`"))?; + ensure!(status.success()); + + Ok(()) +} diff --git a/utils/linting/Cargo.lock b/utils/linting/Cargo.lock index bb5a6126e..629f38db0 100644 --- a/utils/linting/Cargo.lock +++ b/utils/linting/Cargo.lock @@ -2,6 +2,15 @@ # It is not intended for manual editing. version = 3 +[[package]] +name = "aho-corasick" +version = "1.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e60d3430d3a69478ad0993f19238d2df97c507009a52b3c10addcd7f6bcb916" +dependencies = [ + "memchr", +] + [[package]] name = "anstyle" version = "1.0.8" @@ -103,6 +112,8 @@ version = "3.2.0" dependencies = [ "anyhow", "cargo_metadata", + "once_cell", + "regex", "serde", "thiserror", "toml", @@ -243,11 +254,34 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "regex" +version = "1.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4219d74c6b67a3654a9fbebc4b419e22126d13d2f3c4a07ee0cb61ff79a79619" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + [[package]] name = "regex-automata" version = "0.4.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "38caf58cc5ef2fed281f89292ef23f6365465ed9a41b7a7754eb4e26496c92df" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a66a03ae7c801facd77a29370b4faec201768915ac14a721ba36f20bc9c209b" [[package]] name = "rustc_version"