diff --git a/.config/flakebox/id b/.config/flakebox/id index c3ac39a..402fe1c 100644 --- a/.config/flakebox/id +++ b/.config/flakebox/id @@ -1 +1 @@ -6e5c13b823dbc2228a5144d5e86d92a0e3a92e873c9f2a3adedd2e7669f8b1bf2863d328c89e6e3d3ef57ec095899c1671085bbd8d183c513b85eaa165a803c7 +b3c9ea32bd238c00ccfd8b74c3b0c683441ebb2d67d4c0a47a0065404d27dbf5e33ade808310e029032c69e0d5418ac285a19dee4c331484b51a0924b27fcb50 diff --git a/checks/git-hooks.nix b/checks/git-hooks.nix index 9b5f999..9e5c92d 100644 --- a/checks/git-hooks.nix +++ b/checks/git-hooks.nix @@ -41,6 +41,11 @@ let while [ ! -e "$FLAKEBOX_TEST_LOG.a-ready" ]; do sleep 0.01; done sleep 2 ''; + "d-slow-nul" = '' + printf 'd-slow-nul\n' >> "$FLAKEBOX_TEST_LOG" + printf '\0' >&2 + sleep 2 + ''; }; queueHooks = mkHookFixture { @@ -52,6 +57,13 @@ let ''; }; + backgroundHooks = mkHookFixture { + background = '' + sleep 30 /dev/null 2>/dev/null & + printf '%s\n' "$!" > "$FLAKEBOX_BACKGROUND_PID" + ''; + }; + stashHooks = mkHookFixture { stash-probe = '' grep -qx staged tracked.txt @@ -92,6 +104,29 @@ let ''; }; + usabilityHooks = + (mkLib pkgs { + config.git = { + pre-commit = { + trailing_newline = false; + trailing_whitespace = false; + hooks = pkgs.lib.mkForce { + fail-pre-commit = '' + printf 'pre-commit ran\n' >> "$FLAKEBOX_TEST_LOG" + return 41 + ''; + }; + }; + commit-msg.hooks = pkgs.lib.mkForce { + fail-commit-msg = '' + printf 'commit-msg ran\n' >> "$FLAKEBOX_TEST_LOG" + exit 42 + ''; + }; + commit-template.enable = false; + }; + }).root; + emptyHooks = mkHookFixture { }; in assert pkgs.lib.elem timer flakeboxLib.config.env.shellPackages; @@ -127,7 +162,7 @@ pkgs.runCommand "git-hooks-tests" bash -n "$pre_commit" ! grep -q check_nothing "$pre_commit" - grep -q '^ flakebox-git-hook-check-timer "\$1"$' "$pre_commit" + grep -q '^ flakebox-git-hook-check-timer "\$1"$' "$pre_commit" ! grep -q '/nix/store/.*flakebox-git-hook-check-timer' "$pre_commit" ! grep -q 'Skipping semgrep check' "$pre_commit" @@ -167,6 +202,20 @@ pkgs.runCommand "git-hooks-tests" cmp expected-timer.stdout timer.stdout cmp expected-timer.stderr timer.stderr + function check_slow_newline() { + printf 'terminated stderr\n' >&2 + sleep 1.2 + } + export -f check_slow_newline + flakebox-git-hook-check-timer check_slow_newline 2> slow-newline.stderr + sed -E 's/took [0-9]+\.[0-9]{3}s/took DURATION/' \ + slow-newline.stderr > normalized-slow-newline.stderr + # Standalone timer use conservatively separates warnings because it cannot + # inspect arbitrary inherited stderr. Generated hooks compact this output. + printf 'terminated stderr\n\nflakebox: warning: check_slow_newline took DURATION (>1s)\n' \ + > expected-slow-newline.stderr + cmp expected-slow-newline.stderr normalized-slow-newline.stderr + function check_slow_output() { printf slow-stderr-without-newline >&2 sleep 1.2 @@ -236,17 +285,21 @@ pkgs.runCommand "git-hooks-tests" NO_STASH=1 bash ${timedHooks}/misc/git-hooks/pre-commit \ > hook.stdout 2> hook.stderr - printf 'a-slow\nb-fast\nc-slow\n' | sort > expected-hooks + printf 'a-slow\nb-fast\nc-slow\nd-slow-nul\n' | sort > expected-hooks sort hooks.log > actual-hooks cmp expected-hooks actual-hooks - [ "$(grep -c '^flakebox: warning: check_.* took [0-9]*\.[0-9]\{3\}s (>1s)$' hook.stderr)" -eq 2 ] - [ "$(grep -c '^flakebox: warning: check_a_slow took ' hook.stderr)" -eq 1 ] - [ "$(grep -c '^flakebox: warning: check_c_slow took ' hook.stderr)" -eq 1 ] - ! grep -q 'warning: check_b_fast took' hook.stderr - grep -q 'slow stderr without newline' hook.stderr - grep -q 'fast stderr without newline' hook.stderr + [ "$(grep -a -c '^flakebox: warning: check_.* took [0-9]*\.[0-9]\{3\}s (>1s)$' hook.stderr)" -eq 3 ] + [ "$(grep -a -c '^flakebox: warning: check_a_slow took ' hook.stderr)" -eq 1 ] + [ "$(grep -a -c '^flakebox: warning: check_c_slow took ' hook.stderr)" -eq 1 ] + [ "$(grep -a -c '^flakebox: warning: check_d_slow_nul took ' hook.stderr)" -eq 1 ] + ! grep -a -q 'warning: check_b_fast took' hook.stderr + grep -a -q 'slow stderr without newline' hook.stderr + grep -a -q 'fast stderr without newline' hook.stderr grep -q 'slow stdout' hook.stdout grep -q 'fast stdout without newline' hook.stdout + ! grep -a -q '^$' hook.stderr + od -An -t u1 hook.stderr | tr -s ' ' '\n' | grep -A1 -x 0 | + grep -qx 10 # GNU Parallel's aggregate status and complete coverage are unchanged when # checks return distinct non-zero statuses. @@ -274,6 +327,21 @@ pkgs.runCommand "git-hooks-tests" [ "$(grep -c '^flakebox: warning:' queue.stderr)" -eq 1 ] grep -q '^flakebox: warning: check_a_blocker took ' queue.stderr ! grep -q 'warning: check_b_fast_after_queue took' queue.stderr + ! grep -q '^$' queue.stderr + cd .. + + # The formatter descriptor is closed in checks, so a detached descendant + # with redirected standard streams cannot keep the hook alive. + mkdir background-repo + cd background-repo + git init -q + touch tracked + git add tracked + export FLAKEBOX_BACKGROUND_PID="$PWD/background.pid" + timeout 10 env NO_STASH=1 \ + bash ${backgroundHooks}/misc/git-hooks/pre-commit + background_pid="$(cat "$FLAKEBOX_BACKGROUND_PID")" + kill "$background_pid" 2>/dev/null || true cd .. # Private patch restoration exposes indexed content to checks and restores @@ -352,6 +420,119 @@ pkgs.runCommand "git-hooks-tests" [ ! -s jj.stderr ] cd .. + # Pre-commit failures happen before Git has opened the message editor, so + # they must not suggest recovering COMMIT_EDITMSG. A commit-msg failure does + # provide the exact retry command once Git has populated that file. + mkdir usability-repo + cd usability-repo + git init -q + git config user.email flakebox@example.invalid + git config user.name Flakebox + touch tracked + git add tracked + mkdir -p .git/hooks + cp ${usabilityHooks}/misc/git-hooks/pre-commit .git/hooks/pre-commit.flakebox + cp ${usabilityHooks}/misc/git-hooks/commit-msg .git/hooks/commit-msg.flakebox + cat > .git/hooks/pre-commit <<'EOF' + #!${pkgs.bash}/bin/bash + exec ${pkgs.bash}/bin/bash .git/hooks/pre-commit.flakebox "$@" + EOF + cat > .git/hooks/commit-msg <<'EOF' + #!${pkgs.bash}/bin/bash + exec ${pkgs.bash}/bin/bash .git/hooks/commit-msg.flakebox "$@" + EOF + chmod +x .git/hooks/pre-commit .git/hooks/commit-msg + export FLAKEBOX_TEST_LOG="$PWD/hooks.log" + + set +e + git commit -m rejected-by-pre-commit > pre-commit.stdout 2> pre-commit.stderr + pre_commit_status=$? + set -e + [ "$pre_commit_status" -ne 0 ] + grep -qx 'pre-commit ran' hooks.log + ! grep -q -- '-eF .git/COMMIT_EDITMSG' pre-commit.stderr + + : > hooks.log + mv .git/hooks/pre-commit .git/hooks/pre-commit.disabled + set +e + git commit -m rejected-by-commit-msg > commit-msg.stdout 2> commit-msg.stderr + commit_msg_status=$? + set -e + [ "$commit_msg_status" -ne 0 ] + grep -qx 'commit-msg ran' hooks.log + grep -qx 'flakebox: commit message checks failed; your message is saved in .git/COMMIT_EDITMSG' commit-msg.stderr + grep -qx 'flakebox: rerun your original git commit command, replacing any message options with:' commit-msg.stderr + grep -qx -- '-eF .git/COMMIT_EDITMSG' commit-msg.stderr + [ -s .git/COMMIT_EDITMSG ] + + # Alternate commit-msg input paths do not support the advertised recovery + # command and therefore receive no guidance. + printf 'message\n' > alternate-message + set +e + bash .git/hooks/commit-msg.flakebox alternate-message \ + > alternate.stdout 2> alternate.stderr + alternate_status=$? + set -e + [ "$alternate_status" -eq 42 ] + ! grep -q -- '-eF .git/COMMIT_EDITMSG' alternate.stderr + + # Only the exact value 1 bypasses each generated hook. Unset and other + # values preserve normal failure behavior. + set +e + NO_STASH=1 bash .git/hooks/pre-commit.flakebox > /dev/null 2> /dev/null + unset_pre_commit_status=$? + FLAKEBOX_SKIP_GIT_HOOKS=yes NO_STASH=1 \ + bash .git/hooks/pre-commit.flakebox > /dev/null 2> /dev/null + other_pre_commit_status=$? + bash .git/hooks/commit-msg.flakebox alternate-message > /dev/null 2> /dev/null + unset_commit_msg_status=$? + FLAKEBOX_SKIP_GIT_HOOKS=yes \ + bash .git/hooks/commit-msg.flakebox alternate-message > /dev/null 2> /dev/null + other_commit_msg_status=$? + set -e + [ "$unset_pre_commit_status" -ne 0 ] + [ "$other_pre_commit_status" -ne 0 ] + [ "$unset_commit_msg_status" -eq 42 ] + [ "$other_commit_msg_status" -eq 42 ] + + : > hooks.log + FLAKEBOX_SKIP_GIT_HOOKS=1 NO_STASH=1 \ + bash .git/hooks/pre-commit.flakebox + FLAKEBOX_SKIP_GIT_HOOKS=1 \ + bash .git/hooks/commit-msg.flakebox alternate-message + [ ! -s hooks.log ] + + mv .git/hooks/pre-commit.disabled .git/hooks/pre-commit + FLAKEBOX_SKIP_GIT_HOOKS=1 git commit -m bypassed -q + [ ! -s hooks.log ] + git rev-parse --verify HEAD + + # Operations whose original options matter must be rerun with those options + # plus the saved-message flags, rather than as a plain new commit. + mv .git/hooks/pre-commit .git/hooks/pre-commit.disabled + printf 'changed\n' > tracked + set +e + git commit -a -m rejected-a > commit-a.stdout 2> commit-a.stderr + commit_a_status=$? + set -e + [ "$commit_a_status" -ne 0 ] + grep -qx -- '-eF .git/COMMIT_EDITMSG' commit-a.stderr + FLAKEBOX_SKIP_GIT_HOOKS=1 GIT_EDITOR=true \ + git commit -a -eF .git/COMMIT_EDITMSG -q + [ "$(git rev-list --count HEAD)" -eq 2 ] + + set +e + git commit --amend -m rejected-amend \ + > commit-amend.stdout 2> commit-amend.stderr + commit_amend_status=$? + set -e + [ "$commit_amend_status" -ne 0 ] + grep -qx -- '-eF .git/COMMIT_EDITMSG' commit-amend.stderr + FLAKEBOX_SKIP_GIT_HOOKS=1 GIT_EDITOR=true \ + git commit --amend -eF .git/COMMIT_EDITMSG -q + [ "$(git rev-list --count HEAD)" -eq 2 ] + cd .. + # A configuration with no hooks remains a valid no-op without a sentinel. empty_pre_commit="${emptyHooks}/misc/git-hooks/pre-commit" bash -n "$empty_pre_commit" diff --git a/docs/technical-details.md b/docs/technical-details.md index 2aa9a72..58f7a63 100644 --- a/docs/technical-details.md +++ b/docs/technical-details.md @@ -48,6 +48,25 @@ barely-over-threshold result is displayed as `1.001s` rather than a misleading `1.000s`. Timing is diagnostic only: checks still run with the same concurrency, output, exit status, and signal behavior. +The generated hook keeps consecutive warnings compact and adds a separator only +when check output lacks a final newline. The internal timer helper's standalone +mode cannot inspect arbitrary inherited standard error portably, so it +conservatively starts its warning with a newline instead. + +### Skipping generated commit hooks + +Set `FLAKEBOX_SKIP_GIT_HOOKS=1` to bypass Flakebox's generated `pre-commit` +and `commit-msg` checks for one command or environment: + +```console +FLAKEBOX_SKIP_GIT_HOOKS=1 git commit +``` + +Only the exact value `1` enables the bypass; an unset variable or any other +value runs the checks normally. This does not affect hooks not generated by +Flakebox. Git's `git commit --no-verify` is also available as a one-shot +alternative. + ## Flakebox `lib` output Flakebox's Flake exposes a `lib` flake output which allows: diff --git a/lib/modules/git.nix b/lib/modules/git.nix index 2f89419..d9d984c 100644 --- a/lib/modules/git.nix +++ b/lib/modules/git.nix @@ -123,6 +123,27 @@ in # not a hardcoded one, as we copy these files into the repo source = pkgs.writeScript "commit-msg" '' #!/usr/bin/env bash + if [ "''${FLAKEBOX_SKIP_GIT_HOOKS:-}" = 1 ]; then + exit 0 + fi + + flakebox_commit_message_path="''${1:-}" + function flakebox_commit_message_failure_guidance() { + local hook_status=$? + + if [ "$hook_status" -ne 0 ] && + [ -d .git ] && + [ "$flakebox_commit_message_path" = ".git/COMMIT_EDITMSG" ] && + [ -f .git/COMMIT_EDITMSG ]; then + >&2 echo "flakebox: commit message checks failed; your message is saved in .git/COMMIT_EDITMSG" + >&2 echo "flakebox: rerun your original git commit command, replacing any message options with:" + >&2 echo "-eF .git/COMMIT_EDITMSG" + fi + + return "$hook_status" + } + trap flakebox_commit_message_failure_guidance EXIT + ${content} ''; }; @@ -195,15 +216,25 @@ in ) config.git.pre-commit.hooks; runHooks = lib.optionalString (hookNames != [ ]) '' function flakebox_run_check() { - flakebox-git-hook-check-timer "$1" + FLAKEBOX_COMPACT_GIT_HOOK_WARNINGS=1 \ + flakebox-git-hook-check-timer "$1" } export -f flakebox_run_check + set +e + exec 9> >(flakebox-git-hook-check-timer --compact-warnings >&2) + formatter_pid=$! parallel \ --nonotice \ flakebox_run_check \ ::: \ - ${builtins.concatStringsSep " \\\n " hookNames} + ${builtins.concatStringsSep " \\\n " hookNames} \ + 2>&9 9>&- + parallel_status=$? + exec 9>&- + wait "$formatter_pid" + set -e + exit "$parallel_status" ''; in @@ -215,6 +246,10 @@ in # # NOTE: THIS FILE IS AUTO-GENERATED BY FLAKEBOX # + if [ "''${FLAKEBOX_SKIP_GIT_HOOKS:-}" = 1 ]; then + exit 0 + fi + ${builtins.readFile ./git/pre-commit.body.bash} ${hooksFns} ${runHooks} diff --git a/lib/pkgs/git-hook-check-timer-tests.rs b/lib/pkgs/git-hook-check-timer-tests.rs index cbe82da..54ca90c 100644 --- a/lib/pkgs/git-hook-check-timer-tests.rs +++ b/lib/pkgs/git-hook-check-timer-tests.rs @@ -33,6 +33,69 @@ fn rounds_display_to_millisecond_precision() { ); } +#[test] +fn warning_formatter_compacts_only_redundant_newlines() { + let input = [ + b"terminated\n".as_slice(), + WARNING_MARKER, + b"flakebox: warning: check_one took 1.200s (>1s)\n", + WARNING_MARKER, + b"flakebox: warning: check_two took 1.300s (>1s)\n", + ] + .concat(); + let mut output = Vec::new(); + + compact_warning_spacing(&input[..], &mut output).unwrap(); + + assert_eq!( + output, + b"terminated\nflakebox: warning: check_one took 1.200s (>1s)\n\ + flakebox: warning: check_two took 1.300s (>1s)\n" + ); +} + +#[test] +fn warning_formatter_preserves_unterminated_and_binary_output() { + let input = [ + b"unterminated\0".as_slice(), + WARNING_MARKER, + b"flakebox: warning: check_slow took 1.200s (>1s)\n", + ] + .concat(); + let mut output = Vec::new(); + + compact_warning_spacing(&input[..], &mut output).unwrap(); + + assert_eq!( + output, + b"unterminated\0\nflakebox: warning: check_slow took 1.200s (>1s)\n" + ); +} + +#[test] +fn warning_formatter_preserves_ordinary_output_byte_exactly() { + let input = b"\0ordinary\n\noutput without newline"; + let mut output = Vec::new(); + + compact_warning_spacing(&input[..], &mut output).unwrap(); + + assert_eq!(output, input); +} + +#[test] +fn warning_formatter_does_not_prefix_first_warning() { + let input = [ + WARNING_MARKER, + b"flakebox: warning: check_slow took 1.200s (>1s)\n", + ] + .concat(); + let mut output = Vec::new(); + + compact_warning_spacing(&input[..], &mut output).unwrap(); + + assert_eq!(output, b"flakebox: warning: check_slow took 1.200s (>1s)\n"); +} + #[cfg(unix)] #[test] fn signal_before_child_publication_is_queued_for_publisher() { diff --git a/lib/pkgs/git-hook-check-timer.rs b/lib/pkgs/git-hook-check-timer.rs index a9b12a8..24babad 100644 --- a/lib/pkgs/git-hook-check-timer.rs +++ b/lib/pkgs/git-hook-check-timer.rs @@ -15,6 +15,9 @@ use std::time::{Duration, Instant}; const WARNING_THRESHOLD: Duration = Duration::from_secs(1); const TIMER_ERROR_EXIT_CODE: i32 = 125; +const COMPACT_WARNINGS_ARG: &str = "--compact-warnings"; +const COMPACT_WARNINGS_ENV: &str = "FLAKEBOX_COMPACT_GIT_HOOK_WARNINGS"; +const WARNING_MARKER: &[u8] = b"\x1eflakebox-git-hook-warning\x1e"; #[cfg(unix)] const HANGUP_SIGNAL: i32 = 1; @@ -140,6 +143,15 @@ fn main() { let Some(check_name) = args.next() else { timer_error("missing check name"); }; + if check_name == OsStr::new(COMPACT_WARNINGS_ARG) { + if args.next().is_some() { + timer_error("warning formatter does not accept arguments"); + } + if compact_warning_spacing(io::stdin().lock(), io::stdout().lock()).is_err() { + process::exit(TIMER_ERROR_EXIT_CODE); + } + return; + } if args.next().is_some() { timer_error("expected exactly one check name"); } @@ -152,9 +164,16 @@ fn main() { if let Ok(check_result) = &check_result { if let Some(warning) = slow_check_warning(&check_name, check_result.elapsed) { // A diagnostic write failure must never replace the check's exit status. - // The leading newline keeps this record complete even if this or - // another concurrent check left stderr unterminated. - let warning_record = format!("\n{warning}\n"); + let separator = if env::var_os(COMPACT_WARNINGS_ENV).as_deref() == Some(OsStr::new("1")) + { + std::str::from_utf8(WARNING_MARKER).expect("warning marker is UTF-8") + } else { + // Standalone use cannot inspect arbitrary inherited stderr + // portably. Prefer a complete record after unterminated output; + // generated hooks use the formatter to avoid blank lines. + "\n" + }; + let warning_record = format!("{separator}{warning}\n"); let _ = io::stderr().lock().write_all(warning_record.as_bytes()); } } @@ -165,6 +184,37 @@ fn main() { } } +/// Streams hook stderr while replacing timer markers with a needed separator. +fn compact_warning_spacing(mut input: impl io::Read, mut output: impl io::Write) -> io::Result<()> { + let mut pending = Vec::with_capacity(WARNING_MARKER.len()); + let mut buffer = [0; 8192]; + let mut last_output_byte = None; + + loop { + let bytes_read = input.read(&mut buffer)?; + if bytes_read == 0 { + output.write_all(&pending)?; + return output.flush(); + } + + for &byte in &buffer[..bytes_read] { + pending.push(byte); + while !WARNING_MARKER.starts_with(&pending) { + output.write_all(&pending[..1])?; + last_output_byte = pending.first().copied(); + pending.remove(0); + } + if pending == WARNING_MARKER { + if last_output_byte.is_some() && last_output_byte != Some(b'\n') { + output.write_all(b"\n")?; + last_output_byte = Some(b'\n'); + } + pending.clear(); + } + } + } +} + fn run_check(check_name: &OsStr) -> io::Result { let mut command = Command::new("bash"); command.arg("-c").arg(check_name); diff --git a/misc/git-hooks/commit-msg b/misc/git-hooks/commit-msg index 922aa4c..770d05e 100755 --- a/misc/git-hooks/commit-msg +++ b/misc/git-hooks/commit-msg @@ -1,4 +1,25 @@ #!/usr/bin/env bash +if [ "${FLAKEBOX_SKIP_GIT_HOOKS:-}" = 1 ]; then + exit 0 +fi + +flakebox_commit_message_path="${1:-}" +function flakebox_commit_message_failure_guidance() { + local hook_status=$? + + if [ "$hook_status" -ne 0 ] && + [ -d .git ] && + [ "$flakebox_commit_message_path" = ".git/COMMIT_EDITMSG" ] && + [ -f .git/COMMIT_EDITMSG ]; then + >&2 echo "flakebox: commit message checks failed; your message is saved in .git/COMMIT_EDITMSG" + >&2 echo "flakebox: rerun your original git commit command, replacing any message options with:" + >&2 echo "-eF .git/COMMIT_EDITMSG" + fi + + return "$hook_status" +} +trap flakebox_commit_message_failure_guidance EXIT + # Sanitize file first, by removing leading lines that are empty or start with a hash, # as `convco` currently does not do it automatically (but git will) # TODO: next release of convco should be able to do it automatically diff --git a/misc/git-hooks/pre-commit b/misc/git-hooks/pre-commit index dea34f3..7b50aec 100755 --- a/misc/git-hooks/pre-commit +++ b/misc/git-hooks/pre-commit @@ -2,6 +2,10 @@ # # NOTE: THIS FILE IS AUTO-GENERATED BY FLAKEBOX # +if [ "${FLAKEBOX_SKIP_GIT_HOOKS:-}" = 1 ]; then + exit 0 +fi + set -euo pipefail set +e @@ -172,10 +176,14 @@ function check_typos() { export -f check_typos function flakebox_run_check() { - flakebox-git-hook-check-timer "$1" + FLAKEBOX_COMPACT_GIT_HOOK_WARNINGS=1 \ + flakebox-git-hook-check-timer "$1" } export -f flakebox_run_check +set +e +exec 9> >(flakebox-git-hook-check-timer --compact-warnings >&2) +formatter_pid=$! parallel \ --nonotice \ flakebox_run_check \ @@ -187,7 +195,13 @@ parallel \ check_trailing_newline \ check_trailing_whitespace \ check_treefmt \ - check_typos + check_typos \ + 2>&9 9>&- +parallel_status=$? +exec 9>&- +wait "$formatter_pid" +set -e +exit "$parallel_status" # # NOTE: THIS FILE IS AUTO-GENERATED BY FLAKEBOX