diff --git a/Cargo.lock b/Cargo.lock index b2165a71..de155c7c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -460,7 +460,6 @@ dependencies = [ name = "marker_lints" version = "0.1.0" dependencies = [ - "cargo_marker", "marker_api", "tempfile", "ui_test", @@ -480,7 +479,6 @@ dependencies = [ name = "marker_uitest" version = "0.1.0" dependencies = [ - "cargo_marker", "marker_api", "tempfile", "ui_test", diff --git a/cargo-marker/Cargo.toml b/cargo-marker/Cargo.toml index f8e6ae04..3ccd34a5 100644 --- a/cargo-marker/Cargo.toml +++ b/cargo-marker/Cargo.toml @@ -14,11 +14,6 @@ description = "Marker's CLI interface to automatically compile and run lint crat [[bin]] name = "cargo-marker" path = "src/main.rs" -doc = false - -[lib] -name = "cargo_marker" -path = "src/lib.rs" [dependencies] clap = { version = "4.0", features = ["string"] } diff --git a/cargo-marker/src/backend.rs b/cargo-marker/src/backend.rs index f6434be5..387dadf5 100644 --- a/cargo-marker/src/backend.rs +++ b/cargo-marker/src/backend.rs @@ -63,16 +63,29 @@ impl Config { } } -pub fn run_check(config: &Config, additional_cargo_args: &[String]) -> Result<(), ExitStatus> { - // If this is a dev build, we want to rebuild the driver before checking - if config.dev_build { - driver::install_driver(false, config.dev_build, &config.build_rustc_flags)?; - } +/// This struct contains all information to use rustc as a driver. +pub struct CheckInfo { + pub env: Vec<(&'static str, OsString)>, +} +pub fn prepare_check(config: &Config) -> Result { println!(); println!("Compiling Lints:"); let lints = lints::build_lints(config)?; + #[rustfmt::skip] + let mut env = vec![ + ("RUSTC_WORKSPACE_WRAPPER", config.toolchain.driver_path.as_os_str().to_os_string()), + ("MARKER_LINT_CRATES", to_marker_lint_crates_env(&lints)), + ]; + if let Some(toolchain) = &config.toolchain.toolchain { + env.push(("RUSTUP_TOOLCHAIN", toolchain.into())); + } + + Ok(CheckInfo { env }) +} + +pub fn run_check(config: &Config, info: CheckInfo, additional_cargo_args: &[String]) -> Result<(), ExitStatus> { println!(); println!("Start linting:"); @@ -80,8 +93,8 @@ pub fn run_check(config: &Config, additional_cargo_args: &[String]) -> Result<() cmd.arg("check"); cmd.args(additional_cargo_args); - cmd.env("RUSTC_WORKSPACE_WRAPPER", config.toolchain.driver_path.as_os_str()); - cmd.env("MARKER_LINT_CRATES", to_marker_lint_crates_env(&lints)); + cmd.envs(info.env); + let exit_status = cmd .spawn() .expect("could not run cargo") diff --git a/cargo-marker/src/backend/toolchain.rs b/cargo-marker/src/backend/toolchain.rs index 5c0b5fa0..fe0693dc 100644 --- a/cargo-marker/src/backend/toolchain.rs +++ b/cargo-marker/src/backend/toolchain.rs @@ -78,7 +78,7 @@ impl Toolchain { pub fn try_find_toolchain(dev_build: bool, verbose: bool) -> Result { if dev_build { - Self::search_target_dir(verbose) + Self::search_next_to_cargo_marker(verbose) } else { // First check if there is a rustc driver for the current toolchain. This // allows the used to override the used toolchain with `+` or @@ -120,37 +120,23 @@ impl Toolchain { fn search_next_to_cargo_marker(verbose: bool) -> Result { if let Ok(path) = std::env::current_exe() { - return Self::search_directory(&path, verbose); - } - - Err(ExitStatus::MissingDriver) - } - - fn search_target_dir(verbose: bool) -> Result { - let metadata = MetadataCommand::new() - .exec() - .map_err(|_| ExitStatus::BadConfiguration)?; - let path = metadata.target_directory.as_std_path(); - Self::search_directory(&path.join("debug").join("dummy_file_name"), verbose) - } - - fn search_directory(path: &Path, verbose: bool) -> Result { - let driver_path = path.with_file_name(MARKER_DRIVER_BIN_NAME); - if verbose { - println!("Searching for driver at '{}'", driver_path.to_string_lossy()); - } - - if driver_path.exists() && driver_path.is_file() { + let driver_path = path.with_file_name(MARKER_DRIVER_BIN_NAME); if verbose { - println!("Found driver at '{}'", driver_path.to_string_lossy()); + println!("Searching for driver at '{}'", driver_path.to_string_lossy()); + } + + if driver_path.exists() && driver_path.is_file() { + if verbose { + println!("Found driver at '{}'", driver_path.to_string_lossy()); + } + return Ok(Toolchain { + driver_path, + cargo_path: PathBuf::from( + std::env::var_os("CARGO").expect("expected environment value `CARGO` to be set"), + ), + toolchain: None, + }); } - return Ok(Toolchain { - driver_path, - cargo_path: PathBuf::from( - std::env::var_os("CARGO").expect("expected environment value `CARGO` to be set"), - ), - toolchain: None, - }); } Err(ExitStatus::MissingDriver) diff --git a/cargo-marker/src/cli.rs b/cargo-marker/src/cli.rs index ff4e7c2a..a10ef0b4 100644 --- a/cargo-marker/src/cli.rs +++ b/cargo-marker/src/cli.rs @@ -36,9 +36,9 @@ impl Flags { } pub fn collect_lint_deps(args: &ArgMatches) -> Option> { - if let Some(lints) = args.get_many::<&str>("lints") { + if let Some(lints) = args.get_many::("lints") { let mut virtual_manifest = "[workspace.metadata.marker.lints]\n".to_string(); - for dep in lints.copied() { + for dep in lints { virtual_manifest.push_str(dep); virtual_manifest.push('\n'); } @@ -109,7 +109,7 @@ fn check_command_args() -> impl IntoIterator> { .short('l') .long("lints") .num_args(1..) - .value_parser(ValueParser::os_string()) + .value_parser(ValueParser::string()) .help("Defines a set of lint crates that should be used"), ] } diff --git a/cargo-marker/src/lib.rs b/cargo-marker/src/lib.rs deleted file mode 100644 index 079e446b..00000000 --- a/cargo-marker/src/lib.rs +++ /dev/null @@ -1,71 +0,0 @@ -#![warn(clippy::pedantic)] -#![allow(clippy::module_name_repetitions)] -#![allow(clippy::manual_let_else)] // Rustfmt doesn't like `let ... else {` rn - -#[allow(dead_code)] // Only check for dead code when the binary gets compiled -mod backend; -#[allow(dead_code)] // Only check for dead code when the binary gets compiled -mod config; -mod exit; -#[allow(dead_code)] // Only check for dead code when the binary gets compiled -mod utils; - -use std::{ - collections::HashMap, - ffi::OsString, - path::{Path, PathBuf}, -}; - -pub use exit::ExitStatus; - -use crate::{backend::Config, config::LintDependencyEntry}; - -#[derive(Debug)] -pub struct TestSetup { - pub rustc_path: PathBuf, - /// The environment values that should be set. The first element is the - /// value name, the second is the value the it should be set to. - pub env_vars: HashMap, -} - -#[allow(clippy::missing_panics_doc, clippy::missing_errors_doc)] -pub fn test_setup(krate_name: String, krate_dir: &Path) -> Result { - let dev_build = cfg!(feature = "dev-build"); - - let toolchain; - if dev_build { - let lint_dir = std::env::current_dir().unwrap(); - std::env::set_current_dir(lint_dir.parent().unwrap()).unwrap(); - backend::driver::install_driver(false, dev_build, "")?; - - toolchain = backend::toolchain::Toolchain::try_find_toolchain(dev_build, true)?; - std::env::set_current_dir(lint_dir).unwrap(); - } else { - toolchain = backend::toolchain::Toolchain::try_find_toolchain(dev_build, true)?; - } - - let mut config = Config::try_base_from(toolchain)?; - config.lints.insert( - krate_name, - LintDependencyEntry { - source: config::Source::Path { - path: krate_dir.to_str().unwrap().to_string(), - }, - package: None, - default_features: None, - features: None, - }, - ); - - println!(); - println!("Compiling Lints:"); - let lints = backend::lints::build_lints(&config)?; - let env_vars = HashMap::from([( - "MARKER_LINT_CRATES".to_string(), - backend::to_marker_lint_crates_env(&lints), - )]); - Ok(TestSetup { - rustc_path: config.toolchain.driver_path.clone(), - env_vars, - }) -} diff --git a/cargo-marker/src/main.rs b/cargo-marker/src/main.rs index ee237721..a984ef07 100644 --- a/cargo-marker/src/main.rs +++ b/cargo-marker/src/main.rs @@ -9,7 +9,7 @@ mod config; mod exit; mod utils; -use std::collections::HashMap; +use std::{collections::HashMap, ffi::OsString}; use cli::{get_clap_config, Flags}; use config::Config; @@ -82,6 +82,11 @@ fn run_check(args: &clap::ArgMatches, config: Option, flags: &Flags) -> return Err(ExitStatus::NoLints); } + // If this is a dev build, we want to rebuild the driver before checking + if flags.dev_build { + backend::driver::install_driver(false, flags.dev_build, "")?; + } + // Configure backend let toolchain = backend::toolchain::Toolchain::try_find_toolchain(flags.dev_build, flags.verbose)?; let backend_conf = backend::Config { @@ -90,12 +95,20 @@ fn run_check(args: &clap::ArgMatches, config: Option, flags: &Flags) -> ..backend::Config::try_base_from(toolchain)? }; + // Prepare backend + let info = backend::prepare_check(&backend_conf)?; + // Run backend - let additional_cargo_args: Vec<_> = std::env::args() - .skip_while(|c| c != CARGO_ARGS_SEPARATOR) - .skip(1) - .collect(); - backend::run_check(&backend_conf, &additional_cargo_args) + if flags.test_build { + print_env(&info.env).unwrap(); + Ok(()) + } else { + let additional_cargo_args: Vec<_> = std::env::args() + .skip_while(|c| c != CARGO_ARGS_SEPARATOR) + .skip(1) + .collect(); + backend::run_check(&backend_conf, info, &additional_cargo_args) + } } fn print_version(flags: &Flags) { @@ -105,3 +118,46 @@ fn print_version(flags: &Flags) { backend::driver::print_driver_version(flags.dev_build); } } + +#[allow(clippy::unnecessary_wraps)] +fn print_env(env: &[(&'static str, OsString)]) -> std::io::Result<()> { + // Operating systems are fun... So, this function prints out the environment + // values to the standard output. For Unix systems, this requires `OsStr` + // objects, as file names are just bytes and don't need to be valid UTF-8. + // Windows, on the other hand, restricts file names, but uses UTF-16. The + // restriction only makes it slightly better, since windows `OsString` version + // doesn't have a `bytes()` method. Rust additionally has a restriction on the + // stdout of windows, that it has to be valid UTF-8, which means more conversion. + // + // This would be so much easier if everyone followed the "UTF-8 Everywhere Manifesto" + + #[cfg(any(target_os = "linux", target_os = "macos"))] + { + use std::io::Write; + use std::os::unix::prelude::OsStrExt; + + // stdout is used directly, to print the `OsString`s without requiring + // them to be valid UTF-8 + let mut lock = std::io::stdout().lock(); + for (name, value) in env { + write!(lock, "env:")?; + lock.write_all(name.as_bytes())?; + write!(lock, "=")?; + lock.write_all(value.as_bytes())?; + writeln!(lock)?; + } + } + + #[cfg(target_os = "windows")] + { + for (name, value) in env { + if let Some(value) = value.to_str() { + println!("env:{name}={value}"); + } else { + unreachable!("Windows requires it's file path to be valid UTF-16 AFAIK"); + } + } + } + + Ok(()) +} diff --git a/marker_lints/Cargo.toml b/marker_lints/Cargo.toml index 24e16b01..6b75581b 100644 --- a/marker_lints/Cargo.toml +++ b/marker_lints/Cargo.toml @@ -18,4 +18,3 @@ marker_api = { path = "../marker_api" } marker_api = { path = "../marker_api" } ui_test = "0.11.5" tempfile = "3.5.0" -cargo_marker = { path = "../cargo-marker", features = ["dev-build"] } diff --git a/marker_lints/tests/uitest.rs b/marker_lints/tests/uitest.rs index 8f7cd119..3773756d 100644 --- a/marker_lints/tests/uitest.rs +++ b/marker_lints/tests/uitest.rs @@ -1,15 +1,17 @@ -use std::{env, num::NonZeroUsize, path::PathBuf}; +use std::{ + collections::HashMap, + env, fs, + num::NonZeroUsize, + path::{Path, PathBuf}, + process::Command, +}; use ui_test::*; #[test] fn ui_test() -> ui_test::color_eyre::Result<()> { let path = "../target"; - let setup = cargo_marker::test_setup( - env!("CARGO_PKG_NAME").to_string(), - &PathBuf::from(env!("CARGO_MANIFEST_DIR")), - ) - .unwrap(); + let setup = run_test_setup(); for (key, val) in setup.env_vars { env::set_var(key, val); } @@ -23,16 +25,21 @@ fn ui_test() -> ui_test::color_eyre::Result<()> { }; config.program.program = PathBuf::from(setup.rustc_path); + config.program.args.push("-Aunused".into()); let bless = std::env::var("BLESS").eq(&Ok("1".to_string())); if bless { config.output_conflict_handling = OutputConflictHandling::Bless } - config.stderr_filter("in ([0-9]m )?[0-9\\.]+s", ""); - config.stdout_filter("in ([0-9]m )?[0-9\\.]+s", ""); - config.stderr_filter(r"[^ ]*/\.?cargo/registry/.*/", "$$CARGO_REGISTRY"); - config.path_stderr_filter(&std::path::Path::new(path), "$DIR"); + let filters = [ + // Normalization for windows... + (r"ui//", "ui/"), + ]; + for (pat, repl) in filters { + config.stderr_filter(pat, repl); + config.stdout_filter(pat, repl); + } // hide binaries generated for successfully passing tests let tmp_dir = tempfile::tempdir_in(path)?; @@ -40,11 +47,77 @@ fn ui_test() -> ui_test::color_eyre::Result<()> { config.out_dir = tmp_dir.into(); config.path_stderr_filter(tmp_dir, "$TMP"); + let test_name_filter = test_name_filter(); run_tests_generic( config, - default_file_filter, + move |path| default_file_filter(path) && test_name_filter(path), default_per_file_config, - // Avoid github actions, as these would end up showing up in `Cargo.stderr` status_emitter::Text, ) } + +// Gracefully stolen from Clippy, thank you! +fn test_name_filter() -> Box bool + Sync> { + if let Ok(filters) = env::var("TESTNAME") { + let filters: Vec<_> = filters.split(',').map(ToString::to_string).collect(); + Box::new(move |path| { + filters.is_empty() + || filters + .iter() + .any(|f| path.file_stem().map_or(false, |stem| stem == f.as_str())) + }) + } else { + Box::new(|_| true) + } +} + +struct TestSetup { + rustc_path: String, + /// The environment values that should be set. The first element is the + /// value name, the second is the value the it should be set to. + env_vars: HashMap, +} + +/// This function calls `cargo-marker` for the basic test setup. For normal linting +/// crates this will need to be adjusted to run the installed `cargo-marker` version +/// +/// In the future it would be nice to have a nice wrapper library as well. +fn run_test_setup() -> TestSetup { + const CARGO_MARKER_INVOCATION: &[&str] = &["run", "--bin", "cargo-marker", "--features", "dev-build", "--"]; + + // ../marker/marker_uitest + let current_dir = env::current_dir().unwrap(); + let lint_crate_src = fs::canonicalize(¤t_dir).unwrap(); + let lint_spec = format!( + r#"{} = {{ path = "{}" }}"#, + env!("CARGO_PKG_NAME"), + lint_crate_src.display() + ); + let mut cmd = Command::new("cargo"); + let output = cmd + .current_dir(current_dir.parent().unwrap()) + .args(CARGO_MARKER_INVOCATION) + .arg("-l") + .arg(lint_spec) + .arg("--test-setup") + .output() + .expect("Unable to run the test setup using `cargo-marker`"); + let stdout = String::from_utf8(output.stdout).unwrap(); + + if !output.status.success() { + let stderr = String::from_utf8(output.stderr).unwrap(); + panic!("Test setup failed:\n\n===STDOUT===\n{stdout}\n\n===STDERR===\n{stderr}\n"); + } + + let mut env_vars: HashMap<_, _> = stdout + .lines() + .filter_map(|line| line.strip_prefix("env:")) + .filter_map(|line| line.split_once('=')) + .map(|(var, val)| (var.to_string(), val.to_string())) + .collect(); + + TestSetup { + rustc_path: env_vars.remove("RUSTC_WORKSPACE_WRAPPER").unwrap(), + env_vars, + } +} diff --git a/marker_uitest/Cargo.toml b/marker_uitest/Cargo.toml index 8792c2b9..69b2b0b4 100644 --- a/marker_uitest/Cargo.toml +++ b/marker_uitest/Cargo.toml @@ -16,4 +16,3 @@ marker_api = { path = "../marker_api" } [dev-dependencies] ui_test = "0.11.5" tempfile = "3.5.0" -cargo_marker = { path = "../cargo-marker", features = ["dev-build"] } diff --git a/marker_uitest/tests/uitest.rs b/marker_uitest/tests/uitest.rs index c6b0c76a..3773756d 100644 --- a/marker_uitest/tests/uitest.rs +++ b/marker_uitest/tests/uitest.rs @@ -1,7 +1,9 @@ use std::{ - env, + collections::HashMap, + env, fs, num::NonZeroUsize, path::{Path, PathBuf}, + process::Command, }; use ui_test::*; @@ -9,11 +11,7 @@ use ui_test::*; fn ui_test() -> ui_test::color_eyre::Result<()> { let path = "../target"; - let setup = cargo_marker::test_setup( - env!("CARGO_PKG_NAME").to_string(), - &PathBuf::from(env!("CARGO_MANIFEST_DIR")), - ) - .unwrap(); + let setup = run_test_setup(); for (key, val) in setup.env_vars { env::set_var(key, val); } @@ -72,3 +70,54 @@ fn test_name_filter() -> Box bool + Sync> { Box::new(|_| true) } } + +struct TestSetup { + rustc_path: String, + /// The environment values that should be set. The first element is the + /// value name, the second is the value the it should be set to. + env_vars: HashMap, +} + +/// This function calls `cargo-marker` for the basic test setup. For normal linting +/// crates this will need to be adjusted to run the installed `cargo-marker` version +/// +/// In the future it would be nice to have a nice wrapper library as well. +fn run_test_setup() -> TestSetup { + const CARGO_MARKER_INVOCATION: &[&str] = &["run", "--bin", "cargo-marker", "--features", "dev-build", "--"]; + + // ../marker/marker_uitest + let current_dir = env::current_dir().unwrap(); + let lint_crate_src = fs::canonicalize(¤t_dir).unwrap(); + let lint_spec = format!( + r#"{} = {{ path = "{}" }}"#, + env!("CARGO_PKG_NAME"), + lint_crate_src.display() + ); + let mut cmd = Command::new("cargo"); + let output = cmd + .current_dir(current_dir.parent().unwrap()) + .args(CARGO_MARKER_INVOCATION) + .arg("-l") + .arg(lint_spec) + .arg("--test-setup") + .output() + .expect("Unable to run the test setup using `cargo-marker`"); + let stdout = String::from_utf8(output.stdout).unwrap(); + + if !output.status.success() { + let stderr = String::from_utf8(output.stderr).unwrap(); + panic!("Test setup failed:\n\n===STDOUT===\n{stdout}\n\n===STDERR===\n{stderr}\n"); + } + + let mut env_vars: HashMap<_, _> = stdout + .lines() + .filter_map(|line| line.strip_prefix("env:")) + .filter_map(|line| line.split_once('=')) + .map(|(var, val)| (var.to_string(), val.to_string())) + .collect(); + + TestSetup { + rustc_path: env_vars.remove("RUSTC_WORKSPACE_WRAPPER").unwrap(), + env_vars, + } +}