Skip to content

Commit

Permalink
Test, Cargo: Use cargo-marker as a binary again in uitests
Browse files Browse the repository at this point in the history
I still like the idea of using `cargo-marker` as a library. However, this would limit the normal driver discovery mechanism for external uitests. This is not to say it can't be done, just that this is a good fix in the meantime.
  • Loading branch information
xFrednet committed Jul 11, 2023
1 parent 73a2f8b commit a0686ba
Show file tree
Hide file tree
Showing 11 changed files with 241 additions and 144 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions cargo-marker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
27 changes: 20 additions & 7 deletions cargo-marker/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,25 +63,38 @@ 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<CheckInfo, ExitStatus> {
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:");

let mut cmd = config.toolchain.cargo_command();
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")
Expand Down
46 changes: 16 additions & 30 deletions cargo-marker/src/backend/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl Toolchain {

pub fn try_find_toolchain(dev_build: bool, verbose: bool) -> Result<Toolchain, ExitStatus> {
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 `+<toolchain>` or
Expand Down Expand Up @@ -120,37 +120,23 @@ impl Toolchain {

fn search_next_to_cargo_marker(verbose: bool) -> Result<Toolchain, ExitStatus> {
if let Ok(path) = std::env::current_exe() {
return Self::search_directory(&path, verbose);
}

Err(ExitStatus::MissingDriver)
}

fn search_target_dir(verbose: bool) -> Result<Toolchain, ExitStatus> {
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<Toolchain, ExitStatus> {
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)
Expand Down
6 changes: 3 additions & 3 deletions cargo-marker/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ impl Flags {
}

pub fn collect_lint_deps(args: &ArgMatches) -> Option<HashMap<String, LintDependency>> {
if let Some(lints) = args.get_many::<&str>("lints") {
if let Some(lints) = args.get_many::<String>("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');
}
Expand Down Expand Up @@ -109,7 +109,7 @@ fn check_command_args() -> impl IntoIterator<Item = impl Into<Arg>> {
.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"),
]
}
71 changes: 0 additions & 71 deletions cargo-marker/src/lib.rs

This file was deleted.

68 changes: 62 additions & 6 deletions cargo-marker/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -82,6 +82,11 @@ fn run_check(args: &clap::ArgMatches, config: Option<Config>, 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 {
Expand All @@ -90,12 +95,20 @@ fn run_check(args: &clap::ArgMatches, config: Option<Config>, 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) {
Expand All @@ -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(())
}
1 change: 0 additions & 1 deletion marker_lints/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Loading

0 comments on commit a0686ba

Please sign in to comment.