From b2efeb124343f672c271cf33bde439a74a2344f3 Mon Sep 17 00:00:00 2001 From: Jo <10510431+j178@users.noreply.github.com> Date: Wed, 30 Oct 2024 17:32:11 +0800 Subject: [PATCH] Support `log-file` (#4) * Add log file * Merge stderr into stdout --- .github/workflows/ci.yml | 6 ++--- src/cli/run.rs | 36 ++++++++++++++++---------- src/fs.rs | 10 +++----- src/languages/python.rs | 1 + tests/run.rs | 55 ++++++++++++++++++++++++++++++++++++++-- 5 files changed, 84 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fba82121..feca33e6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -100,7 +100,7 @@ jobs: run: | cargo nextest run \ --workspace \ - --status-level skip --failure-output immediate-final --no-fail-fast -j 8 --final-status-level slow + --status-level skip --failure-output immediate --no-fail-fast -j 8 --final-status-level slow cargo-test-macos: timeout-minutes: 10 @@ -128,7 +128,7 @@ jobs: run: | cargo nextest run \ --workspace \ - --status-level skip --failure-output immediate-final --no-fail-fast -j 8 --final-status-level slow + --status-level skip --failure-output immediate --no-fail-fast -j 8 --final-status-level slow cargo-test-windows: timeout-minutes: 15 @@ -168,4 +168,4 @@ jobs: run: | # use only 1 job for now # uv concurrently writes to cache, which causes `Access is denied` errors - cargo nextest run --workspace --status-level skip --failure-output immediate-final --no-fail-fast -j 1 --final-status-level slow + cargo nextest run --workspace --status-level skip --failure-output immediate --no-fail-fast -j 1 --final-status-level slow diff --git a/src/cli/run.rs b/src/cli/run.rs index 653bfe72..ddfd9191 100644 --- a/src/cli/run.rs +++ b/src/cli/run.rs @@ -83,7 +83,7 @@ pub(crate) async fn run( } let skips = get_skips(); - let non_skipped = hooks + let to_install = hooks .iter() .filter(|h| !skips.contains(&h.id) && !skips.contains(&h.alias)) .cloned() @@ -91,16 +91,15 @@ pub(crate) async fn run( debug!( "Hooks going to run: {:?}", - non_skipped.iter().map(|h| &h.id).collect::>() + to_install.iter().map(|h| &h.id).collect::>() ); - install_hooks(&non_skipped, printer).await?; + install_hooks(&to_install, printer).await?; drop(lock); - let filenames = all_filenames(hook_stage, from_ref, to_ref, all_files, files) - .await? - .into_iter() - .map(normalize_path) - .collect::>(); + let mut filenames = all_filenames(hook_stage, from_ref, to_ref, all_files, files).await?; + for filename in &mut filenames { + normalize_path(filename); + } let filenames = filter_filenames( filenames.par_iter(), @@ -440,10 +439,22 @@ async fn run_hook( "- files were modified by this hook".dimmed() )?; } - if !(output.stdout.is_empty() && output.stderr.is_empty()) { - // TODO: write to log file - std::io::stdout().write_all(&output.stdout)?; - std::io::stdout().write_all(&output.stderr)?; + + // To be consistent with pre-commit, merge stderr into stdout. + let stdout = output.stdout.trim_ascii(); + if !stdout.is_empty() { + if let Some(file) = hook.log_file.as_deref() { + fs_err::OpenOptions::new() + .create(true) + .append(true) + .open(file) + .and_then(|mut f| { + f.write_all(stdout)?; + Ok(()) + })?; + } else { + writeln!(printer.stdout(), "{}", String::from_utf8_lossy(stdout))?; + }; } } @@ -455,7 +466,6 @@ fn filter_filenames<'a>( include: Option<&str>, exclude: Option<&str>, ) -> Result, regex::Error> { - // TODO: normalize path separators let include = include.map(Regex::new).transpose()?; let exclude = exclude.map(Regex::new).transpose()?; diff --git a/src/fs.rs b/src/fs.rs index 4bbb136f..33e48b8d 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -185,25 +185,23 @@ pub fn copy_dir_all(src: impl AsRef, dst: impl AsRef) -> std::io::Re /// Normalizes a path to use `/` as a separator everywhere, even on platforms /// that recognize other characters as separators. #[cfg(unix)] -pub(crate) fn normalize_path(path: String) -> String { +pub(crate) fn normalize_path(_path: &mut str) { // UNIX only uses /, so we're good. - path } /// Normalizes a path to use `/` as a separator everywhere, even on platforms /// that recognize other characters as separators. #[cfg(not(unix))] -pub(crate) fn normalize_path(path: String) -> String { +pub(crate) fn normalize_path(path: &mut str) { use std::path::is_separator; - let mut bytes = path.into_bytes(); - for c in &mut bytes { + let bytes = unsafe { path.as_bytes_mut() }; + for c in bytes { if *c == b'/' || !is_separator(char::from(*c)) { continue; } *c = b'/'; } - String::from_utf8(bytes).expect("Path is valid UTF-8") } /// Compute a path describing `path` relative to `base`. diff --git a/src/languages/python.rs b/src/languages/python.rs index c838440f..afa76b02 100644 --- a/src/languages/python.rs +++ b/src/languages/python.rs @@ -85,6 +85,7 @@ impl LanguageImpl for Python { .env("VIRTUAL_ENV", &env) .env("PATH", new_path) .env_remove("PYTHONHOME") + .stderr(std::process::Stdio::inherit()) .output() .await?; diff --git a/tests/run.rs b/tests/run.rs index 4ffac1aa..55ebe2d3 100644 --- a/tests/run.rs +++ b/tests/run.rs @@ -1,8 +1,8 @@ +use crate::common::{cmd_snapshot, TestContext}; use anyhow::Result; use assert_cmd::Command; use assert_fs::prelude::*; - -use crate::common::{cmd_snapshot, TestContext}; +use insta::assert_snapshot; mod common; @@ -475,3 +475,54 @@ fn subdirectory() -> Result<()> { Ok(()) } + +/// Test hook `log_file` option. +#[test] +fn log_file() -> Result<()> { + let context = TestContext::new(); + + context.init_project(); + + let cwd = context.workdir(); + cwd.child("file.txt").write_str("Hello, world! ")?; + + // Global files and exclude. + context + .workdir() + .child(".pre-commit-config.yaml") + .write_str(indoc::indoc! {r" + repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: trailing-whitespace + log_file: log.txt + " + })?; + + Command::new("git") + .arg("add") + .arg(".") + .current_dir(cwd) + .assert() + .success(); + + cmd_snapshot!(context.filters(), context.run(), @r#" + success: true + exit_code: 0 + ----- stdout ----- + Cloning https://github.com/pre-commit/pre-commit-hooks@v5.0.0 + Installing environment for https://github.com/pre-commit/pre-commit-hooks@v5.0.0 + trim trailing whitespace.................................................Failed + - hook id: trailing-whitespace + - exit code: 1 + - files were modified by this hook + + ----- stderr ----- + "#); + + let log = context.read("log.txt"); + assert_snapshot!(log, @"Fixing file.txt"); + + Ok(()) +}