fix: show tag names in auto-update --freeze output#1855
fix: show tag names in auto-update --freeze output#1855deadnews wants to merge 1 commit intoj178:masterfrom
auto-update --freeze output#1855Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1855 +/- ##
=======================================
Coverage 91.93% 91.94%
=======================================
Files 107 107
Lines 21164 21188 +24
=======================================
+ Hits 19458 19481 +23
- Misses 1706 1707 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Improves the readability of prek auto-update --freeze by printing the tag name alongside the frozen commit SHA (as rev@sha) in console output, and updates snapshots/filters to match the new format.
Changes:
- Add a
Revision::display_rev()helper to format frozen revisions asrev@sha. - Update
auto-updateconsole output to use the formatted revision when reporting updates. - Adjust
auto-update --freezetest snapshots and SHA filtering regex to match SHAs after@.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/prek/src/cli/auto_update.rs | Formats the updated revision in auto-update output as rev@sha when freezing. |
| crates/prek/tests/auto_update.rs | Updates snapshot expectations and SHA filter regex for the new output format. |
📦 Cargo Bloat ComparisonBinary size change: +0.00% (25.0 MiB → 25.0 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
⚡️ Hyperfine BenchmarksSummary: 0 regressions, 0 improvements above the 10% threshold. Environment
CLI CommandsBenchmarking basic commands in the main repo:
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base --version |
2.5 ± 0.1 | 2.3 | 3.4 | 1.08 ± 0.07 |
prek-head --version |
2.3 ± 0.1 | 2.2 | 2.6 | 1.00 |
prek list
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base list |
9.0 ± 0.1 | 8.8 | 9.4 | 1.00 |
prek-head list |
9.0 ± 0.2 | 8.8 | 10.1 | 1.00 ± 0.02 |
prek validate-config .pre-commit-config.yaml
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base validate-config .pre-commit-config.yaml |
3.3 ± 0.1 | 3.1 | 3.7 | 1.03 ± 0.04 |
prek-head validate-config .pre-commit-config.yaml |
3.2 ± 0.0 | 3.1 | 3.2 | 1.00 |
prek sample-config
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base sample-config |
2.6 ± 0.0 | 2.5 | 2.7 | 1.00 ± 0.02 |
prek-head sample-config |
2.6 ± 0.0 | 2.5 | 2.7 | 1.00 |
Cold vs Warm Runs
Comparing first run (cold) vs subsequent runs (warm cache):
prek run --all-files (cold - no cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
155.1 ± 2.3 | 152.0 | 159.6 | 1.00 |
prek-head run --all-files |
156.0 ± 2.4 | 153.1 | 160.0 | 1.01 ± 0.02 |
prek run --all-files (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
156.0 ± 2.2 | 152.8 | 162.2 | 1.00 |
prek-head run --all-files |
157.5 ± 4.3 | 151.8 | 167.8 | 1.01 ± 0.03 |
Full Hook Suite
Running the builtin hook suite on the benchmark workspace:
prek run --all-files (full builtin hook suite)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
155.5 ± 2.7 | 151.4 | 163.2 | 1.00 |
prek-head run --all-files |
156.9 ± 11.2 | 150.8 | 231.6 | 1.01 ± 0.07 |
Individual Hook Performance
Benchmarking each hook individually on the test repo:
prek run trailing-whitespace --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run trailing-whitespace --all-files |
22.0 ± 0.3 | 21.2 | 22.5 | 1.01 ± 0.02 |
prek-head run trailing-whitespace --all-files |
21.7 ± 0.4 | 20.8 | 22.7 | 1.00 |
prek run end-of-file-fixer --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run end-of-file-fixer --all-files |
28.7 ± 2.6 | 25.3 | 38.1 | 1.00 |
prek-head run end-of-file-fixer --all-files |
28.8 ± 2.1 | 25.6 | 31.9 | 1.00 ± 0.12 |
prek run check-json --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-json --all-files |
13.0 ± 0.4 | 12.3 | 13.7 | 1.05 ± 0.04 |
prek-head run check-json --all-files |
12.4 ± 0.2 | 11.9 | 12.9 | 1.00 |
prek run check-yaml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-yaml --all-files |
12.2 ± 0.2 | 11.9 | 12.6 | 1.00 ± 0.02 |
prek-head run check-yaml --all-files |
12.2 ± 0.2 | 11.9 | 12.8 | 1.00 |
prek run check-toml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-toml --all-files |
12.5 ± 0.4 | 11.8 | 13.3 | 1.00 ± 0.04 |
prek-head run check-toml --all-files |
12.5 ± 0.4 | 11.9 | 13.5 | 1.00 |
prek run check-xml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-xml --all-files |
12.6 ± 0.3 | 12.0 | 13.1 | 1.01 ± 0.04 |
prek-head run check-xml --all-files |
12.5 ± 0.3 | 11.9 | 13.3 | 1.00 |
prek run detect-private-key --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run detect-private-key --all-files |
18.5 ± 1.7 | 16.4 | 25.0 | 1.00 ± 0.11 |
prek-head run detect-private-key --all-files |
18.5 ± 1.1 | 16.8 | 20.7 | 1.00 |
prek run fix-byte-order-marker --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run fix-byte-order-marker --all-files |
24.4 ± 1.4 | 21.2 | 26.8 | 1.00 |
prek-head run fix-byte-order-marker --all-files |
24.8 ± 1.7 | 21.6 | 27.4 | 1.02 ± 0.09 |
Installation Performance
Benchmarking hook installation (fast path hooks skip Python setup):
prek install-hooks (cold - no cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
5.0 ± 0.1 | 4.9 | 5.0 | 1.01 ± 0.02 |
prek-head install-hooks |
4.9 ± 0.1 | 4.8 | 5.1 | 1.00 |
prek install-hooks (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
4.9 ± 0.1 | 4.8 | 5.0 | 1.01 ± 0.03 |
prek-head install-hooks |
4.8 ± 0.1 | 4.8 | 4.9 | 1.00 |
File Filtering/Scoping Performance
Testing different file selection modes:
prek run (staged files only)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run |
53.8 ± 1.2 | 52.1 | 57.1 | 1.00 |
prek-head run |
54.0 ± 2.6 | 51.9 | 63.9 | 1.00 ± 0.05 |
prek run --files '*.json' (specific file type)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --files '*.json' |
9.3 ± 0.1 | 9.1 | 9.5 | 1.00 ± 0.02 |
prek-head run --files '*.json' |
9.3 ± 0.1 | 9.0 | 9.6 | 1.00 |
Workspace Discovery & Initialization
Benchmarking hook discovery and initialization overhead:
prek run --dry-run --all-files (measures init overhead)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --dry-run --all-files |
14.3 ± 0.4 | 13.9 | 15.9 | 1.00 |
prek-head run --dry-run --all-files |
14.3 ± 0.2 | 13.9 | 14.7 | 1.00 ± 0.03 |
Meta Hooks Performance
Benchmarking meta hooks separately:
prek run check-hooks-apply --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-hooks-apply --all-files |
13.4 ± 1.1 | 12.7 | 16.7 | 1.05 ± 0.09 |
prek-head run check-hooks-apply --all-files |
12.8 ± 0.1 | 12.6 | 12.9 | 1.00 |
prek run check-useless-excludes --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-useless-excludes --all-files |
12.9 ± 0.1 | 12.6 | 13.1 | 1.00 ± 0.02 |
prek-head run check-useless-excludes --all-files |
12.8 ± 0.2 | 12.6 | 13.5 | 1.00 |
prek run identity --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run identity --all-files |
11.4 ± 0.1 | 11.2 | 11.7 | 1.01 ± 0.02 |
prek-head run identity --all-files |
11.3 ± 0.2 | 11.1 | 11.6 | 1.00 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a1690a75b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c0ebbf737
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }; | ||
| for line in content.lines() { | ||
| if let Some(caps) = rev_re.captures(line) { | ||
| map.insert(caps[1].to_string(), caps[2].to_string()); |
There was a problem hiding this comment.
Key frozen-rev cache by repo as well as SHA
The new frozen-comment cache is keyed only by commit SHA, but auto_update later looks up the display tag using just remote_repo.rev; this means entries from different repos/projects that share the same SHA will overwrite each other and at least one update line will show the wrong tag@sha. This can happen with forks/mirrors or multiple configs in one workspace that pin identical commits with different tag names, so the output is now nondeterministic/incorrect for those cases.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The frozen comment line doesn't contain the repo URL, I'd need to also parse the repo: line above it (and differently for TOML sections). That's more parsing complexity than the edge case warrants: two different repos pinned to the same SHA with different tag names is extremely unlikely, and even then the worst case is a slightly wrong tag in a display-only message.
| let workspace = Workspace::discover(store, workspace_root, config, Some(&selectors), true)?; | ||
|
|
||
| // Parse `# frozen: <rev>` comments from config files to display old revs | ||
| let frozen_revs = parse_frozen_comments(&workspace); |
There was a problem hiding this comment.
parse_frozen_comments() is called unconditionally, and it re-reads every project config file from disk just to improve display output. In large workspaces this adds avoidable I/O on every auto-update run; consider only parsing frozen comments when needed (e.g., when --freeze is set, or when any configured rev looks like a 40-hex SHA).
| let frozen_revs = parse_frozen_comments(&workspace); | |
| let frozen_revs = if freeze { | |
| parse_frozen_comments(&workspace) | |
| } else { | |
| Default::default() | |
| }; |
There was a problem hiding this comment.
Frozen comments exist from previous runs, needed even without --freeze.
| fn parse_frozen_comments(workspace: &Workspace) -> FxHashMap<String, String> { | ||
| let rev_re = regex!(r#"rev\s*[:=]\s*['"]?([a-f0-9]{40})['"]?\s*#\s*frozen:\s*(\S+)"#); | ||
| let mut map = FxHashMap::default(); | ||
|
|
||
| for project in workspace.projects() { | ||
| let Ok(content) = fs_err::read_to_string(project.config_file()) else { | ||
| continue; | ||
| }; | ||
| for line in content.lines() { | ||
| if let Some(caps) = rev_re.captures(line) { | ||
| map.insert(caps[1].to_string(), caps[2].to_string()); |
There was a problem hiding this comment.
parse_frozen_comments() uses fs_err::read_to_string (blocking I/O) inside the async auto_update flow. To avoid blocking the Tokio runtime, consider making this async (using fs_err::tokio::read_to_string) or using tokio::task::spawn_blocking for the file reads.
| fn parse_frozen_comments(workspace: &Workspace) -> FxHashMap<String, String> { | |
| let rev_re = regex!(r#"rev\s*[:=]\s*['"]?([a-f0-9]{40})['"]?\s*#\s*frozen:\s*(\S+)"#); | |
| let mut map = FxHashMap::default(); | |
| for project in workspace.projects() { | |
| let Ok(content) = fs_err::read_to_string(project.config_file()) else { | |
| continue; | |
| }; | |
| for line in content.lines() { | |
| if let Some(caps) = rev_re.captures(line) { | |
| map.insert(caps[1].to_string(), caps[2].to_string()); | |
| async fn parse_frozen_comments(workspace: &Workspace) -> FxHashMap<String, String> { | |
| let rev_re = regex!(r#"rev\s*[:=]\s*['"]?([a-f0-9]{40})['"]?\s*#\s*frozen:\s*(\S+)"#); | |
| let mut map = FxHashMap::default(); | |
| for project in workspace.projects() { | |
| // Ignore unreadable config files, matching the original behavior. | |
| if let Ok(content) = fs_err::tokio::read_to_string(project.config_file()).await { | |
| for line in content.lines() { | |
| if let Some(caps) = rev_re.captures(line) { | |
| map.insert(caps[1].to_string(), caps[2].to_string()); | |
| } |
There was a problem hiding this comment.
Blocking fs_err::read_to_string is used throughout the codebase for config files. These are tiny files already in page cache?
Summary
rev@shainstead of raw commit SHA inauto-update --freezeconsole outputdisplay_rev()helper toRevisionthat formats asrev@shawhen frozen, plain rev otherwise# frozen:comments from config files to also show the tag on the "from" side of already-frozen repos@Closes #1854