Skip to content

Avoid running git diff for skipped hooks#1335

Merged
j178 merged 3 commits intomasterfrom
avoid-git-diff
Jan 12, 2026
Merged

Avoid running git diff for skipped hooks#1335
j178 merged 3 commits intomasterfrom
avoid-git-diff

Conversation

@j178
Copy link
Owner

@j178 j178 commented Jan 12, 2026

For #1327

@j178 j178 added the enhancement New feature or request label Jan 12, 2026
@j178 j178 changed the title avoid git diff Avoid running git diff for skipped hooks Jan 12, 2026
@codecov
Copy link

codecov bot commented Jan 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.05%. Comparing base (c1ec7c8) to head (4cdf2f0).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1335   +/-   ##
=======================================
  Coverage   90.05%   90.05%           
=======================================
  Files          80       80           
  Lines       15910    15915    +5     
=======================================
+ Hits        14328    14333    +5     
  Misses       1582     1582           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link

github-actions bot commented Jan 12, 2026

📦 Cargo Bloat Comparison

Binary size change: -0.60% (16.7 MiB → 16.6 MiB)

Expand for cargo-bloat output

Head Branch Results

 File  .text     Size          Crate Name
 0.7%   1.5% 115.5KiB           prek prek::languages::<impl prek::config::Language>::run::{{closure}}::{{closure}}
 0.6%   1.3% 107.3KiB          prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.3%   0.7%  54.7KiB           prek prek::archive::unpack::{{closure}}
 0.3%   0.7%  52.8KiB             h2 h2::proto::connection::Connection<T,P,B>::poll
 0.2%   0.5%  39.8KiB           prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  39.2KiB          prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.2%   0.5%  39.2KiB           prek prek::run::{{closure}}
 0.2%   0.5%  37.3KiB          hyper hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_loop
 0.2%   0.5%  36.9KiB regex_automata regex_automata::meta::strategy::new
 0.2%   0.5%  36.5KiB           prek prek::workspace::Workspace::discover
 0.2%   0.4%  30.8KiB           std? <core::marker::PhantomData<T> as serde_core::de::DeserializeSeed>::deserialize
 0.2%   0.4%  30.8KiB           prek prek::cli::run::run::run::{{closure}}
 0.2%   0.4%  29.8KiB           prek prek::languages::rust::installer::RustInstaller::install::{{closure}}
 0.2%   0.4%  28.4KiB           prek prek::languages::node::installer::NodeInstaller::install::{{closure}}
 0.2%   0.4%  28.4KiB             h2 h2::proto::connection::DynConnection<B>::recv_frame
 0.2%   0.3%  27.6KiB           prek prek::identify::by_extension::{{closure}}
 0.2%   0.3%  27.3KiB           prek prek::cli::run::run::run_hooks::{{closure}}
 0.2%   0.3%  26.2KiB           prek prek::cli::auto_update::update_repo::{{closure}}
 0.1%   0.3%  24.9KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::connect_to::{{closure}}::{{closure}}::{{closure}}
 0.1%   0.3%  24.7KiB           prek prek::hook::HookBuilder::build::{{closure}}
41.3%  88.5%   6.9MiB                And 10646 smaller methods. Use -n N to show more.
46.7% 100.0%   7.8MiB                .text section size, the file size is 16.6MiB

Base Branch Results

 File  .text     Size          Crate Name
 0.7%   1.5% 115.5KiB           prek prek::languages::<impl prek::config::Language>::run::{{closure}}::{{closure}}
 0.6%   1.3% 107.3KiB          prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.3%   0.7%  54.7KiB           prek prek::archive::unpack::{{closure}}
 0.3%   0.7%  52.8KiB             h2 h2::proto::connection::Connection<T,P,B>::poll
 0.2%   0.5%  39.8KiB           prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  39.2KiB          prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.2%   0.5%  39.2KiB           prek prek::run::{{closure}}
 0.2%   0.5%  37.3KiB          hyper hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_loop
 0.2%   0.5%  36.9KiB regex_automata regex_automata::meta::strategy::new
 0.2%   0.5%  36.5KiB           prek prek::workspace::Workspace::discover
 0.2%   0.4%  30.8KiB           std? <core::marker::PhantomData<T> as serde_core::de::DeserializeSeed>::deserialize
 0.2%   0.4%  30.8KiB           prek prek::cli::run::run::run::{{closure}}
 0.2%   0.4%  29.8KiB           prek prek::languages::rust::installer::RustInstaller::install::{{closure}}
 0.2%   0.4%  28.4KiB           prek prek::languages::node::installer::NodeInstaller::install::{{closure}}
 0.2%   0.4%  28.4KiB             h2 h2::proto::connection::DynConnection<B>::recv_frame
 0.2%   0.3%  27.6KiB           prek prek::identify::by_extension::{{closure}}
 0.2%   0.3%  27.3KiB           prek prek::cli::run::run::run_hooks::{{closure}}
 0.2%   0.3%  26.2KiB           prek prek::cli::auto_update::update_repo::{{closure}}
 0.1%   0.3%  24.9KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::connect_to::{{closure}}::{{closure}}::{{closure}}
 0.1%   0.3%  24.7KiB           prek prek::hook::HookBuilder::build::{{closure}}
41.3%  88.5%   6.9MiB                And 10644 smaller methods. Use -n N to show more.
46.7% 100.0%   7.8MiB                .text section size, the file size is 16.7MiB

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes the hook execution logic by avoiding unnecessary git diff operations when all hooks in a priority group are skipped. This improves performance by reducing file system operations.

Changes:

  • Added logic to skip git diff when all hooks in a group are skipped (DryRun, NoFiles, or Unimplemented status)
  • Changed semaphore from Arc to Rc for better performance in single-threaded async context
  • Added is_skipped() helper method to RunStatus enum
  • Minor optimizations: removed redundant success = false assignment and changed to sort_unstable_by

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@j178 j178 merged commit f686b08 into master Jan 12, 2026
29 checks passed
@j178 j178 deleted the avoid-git-diff branch January 12, 2026 11:28
shaanmajid added a commit to shaanmajid/prek that referenced this pull request Jan 20, 2026
Add integration tests to verify that prek handles skipped hooks correctly.
These tests serve as regression tests for j178#1335, which fixed unnecessary
`git diff` calls when all hooks in a group are skipped.

Tests cover:
- All hooks skipped due to no matching files
- Dry run mode skipping all hooks
- Mixed scenarios with both executed and skipped hooks
- Skipped hooks across different priority groups

Refs: j178#1327, j178#1335
shaanmajid added a commit to shaanmajid/prek that referenced this pull request Jan 20, 2026
Add integration tests to verify that prek handles skipped hooks correctly.
These tests serve as regression tests for j178#1335, which fixed unnecessary
`git diff` calls when all hooks in a group are skipped.

Tests cover:
- All hooks skipped due to no matching files
- Dry run mode skipping all hooks
- Mixed scenarios with both executed and skipped hooks
- Skipped hooks across different priority groups

Refs: j178#1327, j178#1335
shaanmajid added a commit to shaanmajid/prek that referenced this pull request Jan 20, 2026
Add integration tests verifying that prek correctly identifies and
reports skipped hooks in various scenarios.

Tests cover:
- All hooks skipped due to no matching files
- Dry run mode skipping all hooks
- Mixed scenarios with both executed and skipped hooks
- Skipped hooks across different priority groups

Includes regression test for j178#1335: verifies that `git diff` is not
called to check for file modifications when all hooks in a priority
group are skipped. Uses a git wrapper script to count diff invocations.

Refs: j178#1327, j178#1335
shaanmajid added a commit to shaanmajid/prek that referenced this pull request Jan 20, 2026
Add integration tests verifying that prek correctly identifies and
reports skipped hooks in various scenarios.

Tests cover:
- All hooks skipped due to no matching files
- Dry run mode skipping all hooks
- Mixed scenarios with both executed and skipped hooks
- Skipped hooks across different priority groups

Includes regression test for j178#1335: verifies that `git diff` is not
called to check for file modifications when all hooks in a priority
group are skipped. Uses a git wrapper script to count diff invocations.

Refs: j178#1327, j178#1335
shaanmajid added a commit to shaanmajid/prek that referenced this pull request Jan 20, 2026
Add integration tests verifying that prek correctly identifies and
reports skipped hooks in various scenarios.

Tests cover:
- All hooks skipped due to no matching files
- Dry run mode skipping all hooks
- Mixed scenarios with both executed and skipped hooks
- Skipped hooks across different priority groups

Includes regression test for j178#1335: verifies that `git diff` is not
called to check for file modifications when all hooks in a priority
group are skipped. Uses a git wrapper script to count diff invocations.

Refs: j178#1327, j178#1335
shaanmajid added a commit to shaanmajid/prek that referenced this pull request Jan 21, 2026
Add integration tests verifying that prek correctly identifies and
reports skipped hooks in various scenarios.

Tests cover:
- All hooks skipped due to no matching files
- Dry run mode skipping all hooks
- Mixed scenarios with both executed and skipped hooks
- Skipped hooks across multiple priority groups

Includes regression test for j178#1335: verifies that `git diff` is only
called once (initial baseline) when all hooks skip, not once per
priority group. Uses tracing output to count `get_diff` calls.

Refs: j178#1327, j178#1335
shaanmajid added a commit to shaanmajid/prek that referenced this pull request Jan 21, 2026
Add integration tests verifying that prek correctly identifies and
reports skipped hooks in various scenarios.

Tests cover:
- All hooks skipped due to no matching files
- Dry run mode skipping all hooks
- Mixed scenarios with both executed and skipped hooks
- Skipped hooks across multiple priority groups

Includes regression test for j178#1335: verifies that `git diff` is only
called once (initial baseline) when all hooks skip, not once per
priority group. Uses tracing output to count `get_diff` calls.

Refs: j178#1327, j178#1335
shaanmajid added a commit to shaanmajid/prek that referenced this pull request Jan 21, 2026
- uses_has_worktree_changes_when_stashed: verifies optimization is used
  in normal workflow and file modifications are still detected
- uses_get_diff_when_all_files: verifies fallback to full diff comparison
  when worktree may have pre-existing changes (--all-files mode)
- Update all_hooks_skipped_multiple_priority_groups to expect 0 get_diff
  calls (combined effect of j178#1335 fix and --quiet optimization)
j178 pushed a commit that referenced this pull request Jan 22, 2026
Add integration tests verifying that prek correctly identifies and
reports skipped hooks in various scenarios.

Tests cover:
- All hooks skipped due to no matching files
- Dry run mode skipping all hooks
- Mixed scenarios with both executed and skipped hooks
- Skipped hooks across multiple priority groups

Includes regression test for #1335: verifies that `git diff` is only
called once (initial baseline) when all hooks skip, not once per
priority group. Uses tracing output to count `get_diff` calls.

Refs: #1327, #1335
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant