diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cc8e7202c..daa374ba0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,6 @@ name: CI -on: [push, pull_request] +on: [pull_request, push, workflow_dispatch] concurrency: group: ci-${{ github.ref }} diff --git a/Cargo.lock b/Cargo.lock index 9c882a0db..c7f5f1f34 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -540,6 +540,7 @@ dependencies = [ name = "dylint_internal" version = "1.0.11" dependencies = [ + "ansi_term", "anyhow", "cargo_metadata", "git2", diff --git a/cargo-dylint/src/main.rs b/cargo-dylint/src/main.rs index 4531ea9da..71c64228d 100644 --- a/cargo-dylint/src/main.rs +++ b/cargo-dylint/src/main.rs @@ -124,7 +124,8 @@ pub struct Dylint { #[clap( short, long, - help = "Do not show warnings or progress running commands besides `cargo check`" + help = "Do not show warnings or progress running commands besides `cargo check` and \ + `cargo fix`" )] pub quiet: bool, diff --git a/cargo-dylint/tests/custom_toolchain.rs b/cargo-dylint/tests/custom_toolchain.rs index ab025b71a..e82d441fd 100644 --- a/cargo-dylint/tests/custom_toolchain.rs +++ b/cargo-dylint/tests/custom_toolchain.rs @@ -26,11 +26,14 @@ mod custom_toolchain { patch_dylint_template(tempdir.path(), &custom_toolchain).unwrap(); - dylint_internal::test() - .sanitize_environment() - .current_dir(tempdir.path()) - .success() - .unwrap(); + dylint_internal::test( + &format!("with custom toolchain `{}`", custom_toolchain), + false, + ) + .sanitize_environment() + .current_dir(tempdir.path()) + .success() + .unwrap(); uninstall_toolchain(&custom_toolchain).unwrap(); } diff --git a/cargo-dylint/tests/dylint_driver_path.rs b/cargo-dylint/tests/dylint_driver_path.rs index eedb84d56..0d8a380e1 100644 --- a/cargo-dylint/tests/dylint_driver_path.rs +++ b/cargo-dylint/tests/dylint_driver_path.rs @@ -16,7 +16,7 @@ fn dylint_driver_path() { create_dir_all(&dylint_driver_path).unwrap(); - dylint_internal::test() + dylint_internal::test("dylint-template", false) .sanitize_environment() .current_dir(tempdir.path()) .envs(vec![( diff --git a/cargo-dylint/tests/list.rs b/cargo-dylint/tests/list.rs index be80eecee..b1b6a3ec2 100644 --- a/cargo-dylint/tests/list.rs +++ b/cargo-dylint/tests/list.rs @@ -27,18 +27,24 @@ fn one_name_multiple_toolchains() { dylint_internal::clone_dylint_template(tempdir.path()).unwrap(); patch_dylint_template(tempdir.path(), CHANNEL_A, CLIPPY_UTILS_TAG_A).unwrap(); - dylint_internal::build() - .sanitize_environment() - .current_dir(tempdir.path()) - .success() - .unwrap(); + dylint_internal::build( + &format!("dylint-template with channel `{}`", CHANNEL_A), + false, + ) + .sanitize_environment() + .current_dir(tempdir.path()) + .success() + .unwrap(); patch_dylint_template(tempdir.path(), CHANNEL_B, CLIPPY_UTILS_TAG_B).unwrap(); - dylint_internal::build() - .sanitize_environment() - .current_dir(tempdir.path()) - .success() - .unwrap(); + dylint_internal::build( + &format!("dylint-template with channel `{}`", CHANNEL_B), + false, + ) + .sanitize_environment() + .current_dir(tempdir.path()) + .success() + .unwrap(); std::process::Command::cargo_bin("cargo-dylint") .unwrap() @@ -81,17 +87,23 @@ fn one_name_multiple_paths() { dylint_internal::clone_dylint_template(tempdirs.0.path()).unwrap(); dylint_internal::clone_dylint_template(tempdirs.1.path()).unwrap(); - dylint_internal::build() - .sanitize_environment() - .current_dir(tempdirs.0.path()) - .success() - .unwrap(); + dylint_internal::build( + &format!("dylint-template in {:?}", tempdirs.0.path()), + false, + ) + .sanitize_environment() + .current_dir(tempdirs.0.path()) + .success() + .unwrap(); - dylint_internal::build() - .sanitize_environment() - .current_dir(tempdirs.1.path()) - .success() - .unwrap(); + dylint_internal::build( + &format!("dylint-template in {:?}", tempdirs.1.path()), + false, + ) + .sanitize_environment() + .current_dir(tempdirs.1.path()) + .success() + .unwrap(); let paths = join_paths(&[ &target_debug(tempdirs.0.path()).unwrap(), diff --git a/cargo-dylint/tests/package_options.rs b/cargo-dylint/tests/package_options.rs index 5774c0aa6..8d5dbde0f 100644 --- a/cargo-dylint/tests/package_options.rs +++ b/cargo-dylint/tests/package_options.rs @@ -20,13 +20,13 @@ fn new_package() { .assert() .success(); - dylint_internal::build() + dylint_internal::build("filled-in dylint-template", false) .sanitize_environment() .current_dir(&path) .success() .unwrap(); - dylint_internal::test() + dylint_internal::test("filled-in dylint-template", false) .sanitize_environment() .current_dir(&path) .success() @@ -62,37 +62,48 @@ fn downgrade_upgrade_package() { upgrade().args(&["--force"]).assert().success(); - dylint_internal::build() + dylint_internal::build("downgraded dylint-template", false) .sanitize_environment() .current_dir(tempdir.path()) .success() .unwrap(); - dylint_internal::test() + dylint_internal::test("downgraded dylint-template", false) .sanitize_environment() .current_dir(tempdir.path()) .success() .unwrap(); - std::process::Command::cargo_bin("cargo-dylint") - .unwrap() - .args(&["dylint", "--upgrade", &tempdir.path().to_string_lossy()]) - .assert() - .success(); - - // smoelius: Right now, dylint-template does not build with nightly-2021-10-07. So - // dylint-template's rust-toolchain file is ahead of Clippy 1.57.0's by a few days. - /* dylint_internal::build() - .sanitize_environment() - .current_dir(tempdir.path()) - .success() - .unwrap(); - - dylint_internal::test() - .sanitize_environment() - .current_dir(tempdir.path()) - .success() - .unwrap(); */ + if cfg!(not(unix)) { + std::process::Command::cargo_bin("cargo-dylint") + .unwrap() + .args(&["dylint", "--upgrade", &tempdir.path().to_string_lossy()]) + .assert() + .success(); + } else { + std::process::Command::cargo_bin("cargo-dylint") + .unwrap() + .args(&[ + "dylint", + "--upgrade", + &tempdir.path().to_string_lossy(), + "--bisect", + ]) + .assert() + .success(); + + dylint_internal::build("upgraded dylint-template", false) + .sanitize_environment() + .current_dir(tempdir.path()) + .success() + .unwrap(); + + dylint_internal::test("upgraded dylint-template", false) + .sanitize_environment() + .current_dir(tempdir.path()) + .success() + .unwrap(); + } } fn rust_version(path: &Path) -> Result { diff --git a/driver/Cargo.lock b/driver/Cargo.lock index ea45345f6..cdaa67e4b 100644 --- a/driver/Cargo.lock +++ b/driver/Cargo.lock @@ -11,6 +11,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "ansi_term" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" +dependencies = [ + "winapi", +] + [[package]] name = "anyhow" version = "1.0.53" @@ -69,6 +78,7 @@ dependencies = [ name = "dylint_internal" version = "1.0.11" dependencies = [ + "ansi_term", "anyhow", "cargo_metadata", "if_chain", diff --git a/dylint/src/driver_builder.rs b/dylint/src/driver_builder.rs index 4e36f8028..60e63af7d 100644 --- a/dylint/src/driver_builder.rs +++ b/dylint/src/driver_builder.rs @@ -10,7 +10,6 @@ use std::{ env::consts, fs::{copy, create_dir_all, write}, path::{Path, PathBuf}, - process::Stdio, }; use tempfile::tempdir; @@ -160,15 +159,11 @@ fn build(opts: &crate::Dylint, toolchain: &str, driver: &Path) -> Result<()> { toolchain_path.to_string_lossy() ); - let mut command = dylint_internal::build(); - command + dylint_internal::build(&format!("driver for toolchain `{}`", toolchain), opts.quiet) .sanitize_environment() .envs(vec![(env::RUSTFLAGS, rustflags)]) - .current_dir(&package); - if opts.quiet { - command.stderr(Stdio::null()); - } - command.success()?; + .current_dir(&package) + .success()?; let binary = metadata.target_directory.join("debug").join(format!( "dylint_driver-{}{}", diff --git a/dylint/src/lib.rs b/dylint/src/lib.rs index e4d7c3ef5..fd6112a3e 100644 --- a/dylint/src/lib.rs +++ b/dylint/src/lib.rs @@ -455,10 +455,11 @@ fn check_or_fix( let target_dir_str = target_dir.to_string_lossy(); let driver = driver_builder::get(opts, toolchain)?; let dylint_libs = serde_json::to_string(&paths)?; + let description = format!("with toolchain `{}`", toolchain); let mut command = if opts.fix { - dylint_internal::fix() + dylint_internal::fix(&description) } else { - dylint_internal::check() + dylint_internal::check(&description) }; let mut args = vec!["--target-dir", &target_dir_str]; if let Some(path) = &opts.manifest_path { diff --git a/dylint/src/metadata.rs b/dylint/src/metadata.rs index 82e4ccfc8..a52b6b495 100644 --- a/dylint/src/metadata.rs +++ b/dylint/src/metadata.rs @@ -13,10 +13,7 @@ use glob::glob; use if_chain::if_chain; use serde::Deserialize; use serde_json::{Map, Value}; -use std::{ - path::{Path, PathBuf}, - process::Stdio, -}; +use std::path::{Path, PathBuf}; #[derive(Debug, Deserialize)] struct Library { @@ -294,15 +291,14 @@ fn package_library_path( let target_dir = target_dir(metadata, package_root, package_id)?; if !opts.no_build { - let mut command = dylint_internal::build(); - command - .sanitize_environment() - .current_dir(package_root) - .args(&["--release", "--target-dir", &target_dir.to_string_lossy()]); - if opts.quiet { - command.stderr(Stdio::null()); - } - command.success()?; + dylint_internal::build( + &format!("workspace metadata entry `{}`", package_id.name()), + opts.quiet, + ) + .sanitize_environment() + .current_dir(package_root) + .args(&["--release", "--target-dir", &target_dir.to_string_lossy()]) + .success()?; } Ok(target_dir.join("release")) diff --git a/dylint/src/package_options/bisect.rs b/dylint/src/package_options/bisect.rs index ba4c26314..cd7dc547a 100644 --- a/dylint/src/package_options/bisect.rs +++ b/dylint/src/package_options/bisect.rs @@ -1,3 +1,4 @@ +use crate::Dylint; use anyhow::{Context, Result}; use dylint_internal::Command; use std::os::unix::fs::PermissionsExt; @@ -5,6 +6,7 @@ use std::{ fs::{remove_file, rename}, io::Write, path::Path, + process::Stdio, }; use tempfile::NamedTempFile; @@ -58,7 +60,7 @@ const TEMPORARY_FILES: &[&str] = &[ "successful_build_seen", ]; -pub fn bisect(path: &Path, start: &str) -> Result<()> { +pub fn bisect(opts: &Dylint, path: &Path, start: &str) -> Result<()> { Command::new("cargo") .args(&["bisect-rustc", "-V"]) .success() @@ -72,19 +74,22 @@ pub fn bisect(path: &Path, start: &str) -> Result<()> { remove_temporary_files(path); - let result = Command::new("cargo") - .args(&[ - "bisect-rustc", - "--start", - start, - "--preserve", - "--regress=success", - "--script", - &*script.path().to_string_lossy(), - "--test-dir", - &*test_dir.to_string_lossy(), - ]) - .success(); + let mut command = Command::new("cargo"); + command.args(&[ + "bisect-rustc", + "--start", + start, + "--preserve", + "--regress=success", + "--script", + &*script.path().to_string_lossy(), + "--test-dir", + &*test_dir.to_string_lossy(), + ]); + if opts.quiet { + command.stderr(Stdio::null()); + } + let result = command.success(); remove_temporary_files(path); diff --git a/dylint/src/package_options/mod.rs b/dylint/src/package_options/mod.rs index 3250193a4..49e2b6c7b 100644 --- a/dylint/src/package_options/mod.rs +++ b/dylint/src/package_options/mod.rs @@ -235,12 +235,17 @@ pub fn upgrade_package(opts: &Dylint, path: &Path) -> Result<()> { #[cfg(unix)] if opts.bisect { - dylint_internal::update() + let file_name = path + .file_name() + .ok_or_else(|| anyhow!("Could not get file name"))?; + let description = format!("`{}`", file_name.to_string_lossy()); + + dylint_internal::update(&description, opts.quiet) .sanitize_environment() .current_dir(path) .success()?; - if dylint_internal::build() + if dylint_internal::build(&description, opts.quiet) .sanitize_environment() .current_dir(path) .args(&["--tests"]) @@ -253,7 +258,7 @@ pub fn upgrade_package(opts: &Dylint, path: &Path) -> Result<()> { let start = new_nightly.format("%Y-%m-%d").to_string(); - bisect::bisect(path, &start)?; + bisect::bisect(opts, path, &start)?; } } diff --git a/examples/allow_clippy/Cargo.lock b/examples/allow_clippy/Cargo.lock index 8a875987c..f6edf0436 100644 --- a/examples/allow_clippy/Cargo.lock +++ b/examples/allow_clippy/Cargo.lock @@ -191,6 +191,7 @@ dependencies = [ name = "dylint_internal" version = "1.0.11" dependencies = [ + "ansi_term", "anyhow", "cargo_metadata", "if_chain", diff --git a/examples/clippy/Cargo.lock b/examples/clippy/Cargo.lock index bbe5a8915..575867261 100644 --- a/examples/clippy/Cargo.lock +++ b/examples/clippy/Cargo.lock @@ -236,6 +236,7 @@ dependencies = [ name = "dylint_internal" version = "1.0.11" dependencies = [ + "ansi_term", "anyhow", "cargo_metadata", "git2", diff --git a/examples/clippy/tests/ui.rs b/examples/clippy/tests/ui.rs index 3b9575378..dc366f17f 100644 --- a/examples/clippy/tests/ui.rs +++ b/examples/clippy/tests/ui.rs @@ -18,7 +18,7 @@ fn ui() { // smoelius: Try to order failures by how informative they are: failure to build the library, // failure to find the library, failure to build/find the driver. - dylint_internal::build().success().unwrap(); + dylint_internal::build("clippy", false).success().unwrap(); let tempdir = tempdir_in(env!("CARGO_MANIFEST_DIR")).unwrap(); @@ -47,7 +47,7 @@ fn ui() { // temporary target directory. let target_dir = tempdir_in(env!("CARGO_MANIFEST_DIR")).unwrap(); - dylint_internal::test() + dylint_internal::test("clippy", false) .current_dir(tempdir.path()) .envs(vec![ (env::CARGO_TARGET_DIR, &*target_dir.path().to_string_lossy()), diff --git a/examples/env_literal/Cargo.lock b/examples/env_literal/Cargo.lock index 3cda53cd4..725027386 100644 --- a/examples/env_literal/Cargo.lock +++ b/examples/env_literal/Cargo.lock @@ -181,6 +181,7 @@ dependencies = [ name = "dylint_internal" version = "1.0.11" dependencies = [ + "ansi_term", "anyhow", "cargo_metadata", "if_chain", diff --git a/examples/nonreentrant_function_in_test/Cargo.lock b/examples/nonreentrant_function_in_test/Cargo.lock index 3674e974f..1ed7f6878 100644 --- a/examples/nonreentrant_function_in_test/Cargo.lock +++ b/examples/nonreentrant_function_in_test/Cargo.lock @@ -181,6 +181,7 @@ dependencies = [ name = "dylint_internal" version = "1.0.11" dependencies = [ + "ansi_term", "anyhow", "cargo_metadata", "if_chain", diff --git a/examples/path_separator_in_string_literal/Cargo.lock b/examples/path_separator_in_string_literal/Cargo.lock index d5fc9201f..5a70ef0c0 100644 --- a/examples/path_separator_in_string_literal/Cargo.lock +++ b/examples/path_separator_in_string_literal/Cargo.lock @@ -181,6 +181,7 @@ dependencies = [ name = "dylint_internal" version = "1.0.11" dependencies = [ + "ansi_term", "anyhow", "cargo_metadata", "if_chain", diff --git a/examples/question_mark_in_expression/Cargo.lock b/examples/question_mark_in_expression/Cargo.lock index 91acfb40b..6e857ca7a 100644 --- a/examples/question_mark_in_expression/Cargo.lock +++ b/examples/question_mark_in_expression/Cargo.lock @@ -196,6 +196,7 @@ dependencies = [ name = "dylint_internal" version = "1.0.11" dependencies = [ + "ansi_term", "anyhow", "cargo_metadata", "if_chain", diff --git a/examples/try_io_result/Cargo.lock b/examples/try_io_result/Cargo.lock index c9a9e8fbf..35c2ac9d4 100644 --- a/examples/try_io_result/Cargo.lock +++ b/examples/try_io_result/Cargo.lock @@ -181,6 +181,7 @@ dependencies = [ name = "dylint_internal" version = "1.0.11" dependencies = [ + "ansi_term", "anyhow", "cargo_metadata", "if_chain", diff --git a/internal/Cargo.toml b/internal/Cargo.toml index 7bd7518d5..7aad0a6e9 100644 --- a/internal/Cargo.toml +++ b/internal/Cargo.toml @@ -8,6 +8,7 @@ license = "MIT OR Apache-2.0" repository = "https://github.com/trailofbits/dylint" [dependencies] +ansi_term = "0.12.1" anyhow = "1.0.53" cargo_metadata = "0.14.1" git2 = { version = "0.13.25", optional = true } diff --git a/internal/src/cargo.rs b/internal/src/cargo.rs index a873c4b03..2a989edcd 100644 --- a/internal/src/cargo.rs +++ b/internal/src/cargo.rs @@ -1,34 +1,48 @@ +use ansi_term::Style; use anyhow::{anyhow, ensure, Result}; use cargo_metadata::{Metadata, MetadataCommand, Package, PackageId}; +use std::{io::Write, process::Stdio}; #[must_use] -pub fn build() -> crate::Command { - cargo("build") +pub fn build(description: &str, quiet: bool) -> crate::Command { + cargo("build", "Building", description, quiet) } +// smoelius: `cargo check` and `cargo fix` are never silenced. #[must_use] -pub fn check() -> crate::Command { - cargo("check") +pub fn check(description: &str) -> crate::Command { + cargo("check", "Checking", description, false) } +// smoelius: `cargo check` and `cargo fix` are never silenced. #[must_use] -pub fn fix() -> crate::Command { - cargo("fix") +pub fn fix(description: &str) -> crate::Command { + cargo("fix", "Fixing", description, false) } #[must_use] -pub fn test() -> crate::Command { - cargo("test") +pub fn test(description: &str, quiet: bool) -> crate::Command { + cargo("test", "Testing", description, quiet) } #[must_use] -pub fn update() -> crate::Command { - cargo("update") +pub fn update(description: &str, quiet: bool) -> crate::Command { + cargo("update", "Updating", description, quiet) } -fn cargo(subcommand: &str) -> crate::Command { +fn cargo(subcommand: &str, verb: &str, description: &str, quiet: bool) -> crate::Command { + if !quiet { + // smoelius: Writing directly to `stderr` avoids capture by `libtest`. + let message = format!("{} {}", verb, description); + std::io::stderr() + .write_fmt(format_args!("{}\n", Style::new().bold().paint(message))) + .expect("Could not write to stderr"); + } let mut command = crate::Command::new("cargo"); command.args(&[subcommand]); + if quiet { + command.stderr(Stdio::null()); + } command } diff --git a/internal/src/command.rs b/internal/src/command.rs index 5b863be86..6e8544e5e 100644 --- a/internal/src/command.rs +++ b/internal/src/command.rs @@ -53,8 +53,13 @@ impl Command { self } + pub fn stdout>(&mut self, cfg: T) -> &mut Self { + self.command.stdout(cfg); + self + } + pub fn stderr>(&mut self, cfg: T) -> &mut Self { - self.command.stderr(cfg.into()); + self.command.stderr(cfg); self } diff --git a/internal/src/examples.rs b/internal/src/examples.rs index c4a7bf8b9..5c563c6cb 100644 --- a/internal/src/examples.rs +++ b/internal/src/examples.rs @@ -1,5 +1,5 @@ use crate::rustup::SanitizeEnvironment; -use anyhow::{Context, Result}; +use anyhow::{anyhow, Context, Result}; use std::{ fs::read_dir, path::{Path, PathBuf}, @@ -7,7 +7,7 @@ use std::{ pub fn build() -> Result<()> { // smoelius: The examples use `dylint-link` as the linker, so it must be built first. - crate::build() + crate::build("dylint-link", false) .sanitize_environment() .current_dir( Path::new(env!("CARGO_MANIFEST_DIR")) @@ -18,7 +18,10 @@ pub fn build() -> Result<()> { for example in iter()? { let example = example?; - crate::build() + let file_name = example + .file_name() + .ok_or_else(|| anyhow!("Could not get file name"))?; + crate::build(&format!("example `{}`", file_name.to_string_lossy()), false) .sanitize_environment() .current_dir(&example) .success()?; @@ -52,7 +55,8 @@ mod test { fn examples() { for path in iter().unwrap() { let path = path.unwrap(); - crate::test() + let file_name = path.file_name().unwrap(); + crate::test(&format!("example `{}`", file_name.to_string_lossy()), false) .sanitize_environment() .current_dir(path) .success() diff --git a/scripts/upgrade_examples.sh b/scripts/upgrade_examples.sh index 2aa9c4f17..246e4990c 100755 --- a/scripts/upgrade_examples.sh +++ b/scripts/upgrade_examples.sh @@ -43,9 +43,9 @@ for EXAMPLE in examples/*; do # `rust-toolchain` file changing. if ! git diff --exit-code "$EXAMPLE"/rust-toolchain; then PREV_VERSION="$(echo "$PREV_TAG" | sed 's/^\ Result { // smoelius: Try to order failures by how informative they are: failure to build the library, // failure to find the library, failure to build/find the driver. - dylint_internal::build().success()?; + dylint_internal::build(&format!("library `{}`", name), false).success()?; // smoelius: `DYLINT_LIBRARY_PATH` must be set before `dylint_libs` is called. // smoelius: This was true when `dylint_libs` called `name_toolchain_map`, but that is no longer @@ -183,7 +183,7 @@ fn rustc_flags(metadata: &Metadata, package: &Package, target: &Target) -> Resul remove_example(metadata, package, target)?; - cargo::build() + cargo::build(&format!("example `{}`", target.name), false) .envs(vec![(env::CARGO_TERM_COLOR, "never")]) .args(&[ "--manifest-path",