Repository navigation
Add Jujutsu (jj) workspace support - #2393
nikosavola wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2393 +/- ##
==========================================
+ Coverage 94.32% 94.40% +0.07%
==========================================
Files 142 144 +2
Lines 30771 31470 +699
==========================================
+ Hits 29026 29710 +684
- Misses 1745 1760 +15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1b143c382
ℹ️ 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.
Pull request overview
Adds first-class support for running prek inside Jujutsu (jj) workspaces (including secondary/non-colocated workspaces without a .git directory) by introducing a repository-backend abstraction and routing file selection + related logic through it, while keeping existing Git behavior intact.
Changes:
- Introduces
repo+jjmodules to detect the active repo backend once and provide backend-specific implementations for root detection, changed/added file queries, tracked-file listing, and conflict detection. - Updates
prek runand relevant built-in hooks to use the backend abstraction (and skips Git-only behaviors like index-stash / staged-config checks under jj). - Adds jj-focused integration tests and updates CI to install jj on Linux/macOS; updates documentation to mention jj behavior.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/workspace.md | Updates workspace discovery boundary to treat .git or .jj as the repo boundary. |
| docs/quickstart.md | Documents that default prek run works with jj working-copy changes. |
| docs/faq.md | Adds FAQ entry describing how prek behaves in jj workspaces. |
| crates/prek/tests/run.rs | Adds integration tests for prek run behavior in jj colocated/non-colocated/nested/conflict/diff modes. |
| crates/prek/tests/builtin_hooks.rs | Adds jj-backed test coverage for built-in hooks in jj workspaces. |
| crates/prek/src/workspace.rs | Skips staged-config checks when the backend doesn’t have a staging area (jj). |
| crates/prek/src/repo.rs | New backend routing layer (Git vs jj) for root/files/conflicts/stash behavior. |
| crates/prek/src/main.rs | Registers new jj and repo modules. |
| crates/prek/src/jj.rs | New jj adapter for workspace discovery, backing Git dir resolution, and file queries. |
| crates/prek/src/hooks/pre_commit_hooks/check_case_conflict.rs | Uses backend-agnostic tracked/added file queries (git vs jj). |
| crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs | Uses backend-agnostic “added files” selection (git index vs jj working-copy adds). |
| crates/prek/src/git.rs | Integrates repo-context root detection + per-command git env injection; improves env isolation for try-repo. |
| crates/prek/src/cli/try_repo.rs | Ensures try-repo Git operations are isolated from ambient repo env (esp. jj backing-store env). |
| crates/prek/src/cli/run/run.rs | Adjusts run flow to avoid Git-only stash/merge-index behaviors in jj mode. |
| crates/prek/src/cli/run/filter.rs | Routes file selection (--all-files, diff modes, default selection) through the repo backend. |
| crates/prek/src/cli/install.rs | Updates messaging to include jj repositories as “inside a supported repo”. |
| crates/prek/src/cli/hook_impl.rs | Uses repo backend root instead of Git-only root. |
| .github/workflows/ci.yml | Installs a pinned/checksummed jj binary on Linux/macOS to run the new jj tests. |
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:414
- The jj installer step selects the macOS target solely by
RUNNER_OSand hard-codesaarch64-apple-darwin.macos-latestis currently ARM64, but selecting byRUNNER_ARCHavoids breakage if the runner architecture changes or the matrix adds x86_64 macOS.
case "$RUNNER_OS" in
Linux)
target="x86_64-unknown-linux-musl"
sha256="$JJ_SHA256_LINUX_X86_64"
;;
macOS)
target="aarch64-apple-darwin"
sha256="$JJ_SHA256_MACOS_AARCH64"
;;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📦 Cargo Bloat Comparison
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.4 ± 0.1 | 2.2 | 2.6 | 1.09 ± 0.06 |
prek-head --version |
2.1 ± 0.1 | 2.0 | 2.6 | 1.00 |
prek list
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base list |
9.5 ± 0.5 | 9.1 | 12.0 | 1.02 ± 0.07 |
prek-head list |
9.3 ± 0.4 | 8.9 | 11.7 | 1.00 |
prek validate-config .pre-commit-config.yaml
⏭️ Skipped: .pre-commit-config.yaml not found
prek sample-config
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base sample-config |
2.8 ± 0.3 | 2.6 | 4.7 | 1.08 ± 0.13 |
prek-head sample-config |
2.6 ± 0.1 | 2.4 | 3.4 | 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 |
17.7 ± 0.6 | 17.0 | 18.9 | 1.03 ± 0.04 |
prek-head run --all-files |
17.2 ± 0.4 | 16.8 | 18.1 | 1.00 |
prek run --all-files (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
17.2 ± 0.3 | 16.5 | 17.7 | 1.00 |
prek-head run --all-files |
17.4 ± 0.7 | 16.7 | 19.8 | 1.01 ± 0.04 |
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 |
17.4 ± 0.4 | 16.5 | 18.9 | 1.02 ± 0.04 |
prek-head run --all-files |
17.1 ± 0.6 | 16.4 | 19.3 | 1.00 |
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 |
8.8 ± 0.2 | 8.5 | 9.2 | 1.00 |
prek-head run trailing-whitespace --all-files |
8.9 ± 0.6 | 8.5 | 11.7 | 1.01 ± 0.07 |
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 |
8.1 ± 0.4 | 7.8 | 9.7 | 1.04 ± 0.06 |
prek-head run end-of-file-fixer --all-files |
7.8 ± 0.1 | 7.6 | 8.2 | 1.00 |
prek run check-json --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-json --all-files |
6.9 ± 0.1 | 6.8 | 7.1 | 1.01 ± 0.09 |
prek-head run check-json --all-files |
6.9 ± 0.6 | 6.6 | 9.6 | 1.00 |
prek run check-yaml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-yaml --all-files |
7.2 ± 0.4 | 7.0 | 9.1 | 1.02 ± 0.10 |
prek-head run check-yaml --all-files |
7.1 ± 0.6 | 6.8 | 9.8 | 1.00 |
prek run check-toml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-toml --all-files |
6.9 ± 0.1 | 6.8 | 7.2 | 1.04 ± 0.02 |
prek-head run check-toml --all-files |
6.7 ± 0.1 | 6.6 | 6.8 | 1.00 |
prek run check-xml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-xml --all-files |
7.0 ± 0.2 | 6.8 | 8.2 | 1.02 ± 0.10 |
prek-head run check-xml --all-files |
6.9 ± 0.7 | 6.6 | 10.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 |
7.4 ± 0.2 | 7.2 | 8.5 | 1.02 ± 0.07 |
prek-head run detect-private-key --all-files |
7.3 ± 0.4 | 7.0 | 9.5 | 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 |
7.3 ± 0.1 | 7.1 | 7.6 | 1.00 |
prek-head run fix-byte-order-marker --all-files |
7.3 ± 0.3 | 7.1 | 8.5 | 1.00 ± 0.04 |
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 |
4.7 ± 0.1 | 4.6 | 4.8 | 1.05 ± 0.02 |
prek-head install-hooks |
4.4 ± 0.1 | 4.4 | 4.5 | 1.00 |
prek install-hooks (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
4.8 ± 0.1 | 4.7 | 4.9 | 1.04 ± 0.02 |
prek-head install-hooks |
4.6 ± 0.1 | 4.5 | 4.6 | 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 |
10.4 ± 0.1 | 10.2 | 10.7 | 1.00 |
prek-head run |
11.4 ± 0.4 | 11.1 | 13.2 | 1.09 ± 0.05 |
prek run --files '*.json' (specific file type)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --files '*.json' |
5.3 ± 0.1 | 5.2 | 5.6 | 1.03 ± 0.04 |
prek-head run --files '*.json' |
5.2 ± 0.2 | 5.0 | 5.8 | 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 |
7.0 ± 0.2 | 6.8 | 7.8 | 1.03 ± 0.03 |
prek-head run --dry-run --all-files |
6.8 ± 0.1 | 6.6 | 7.0 | 1.00 |
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 |
8.2 ± 0.2 | 8.0 | 8.7 | 1.02 ± 0.02 |
prek-head run check-hooks-apply --all-files |
8.0 ± 0.1 | 7.9 | 8.2 | 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 |
8.5 ± 0.6 | 8.2 | 10.8 | 1.05 ± 0.08 |
prek-head run check-useless-excludes --all-files |
8.0 ± 0.1 | 8.0 | 8.2 | 1.00 |
prek run identity --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run identity --all-files |
7.1 ± 0.1 | 7.0 | 7.3 | 1.02 ± 0.01 |
prek-head run identity --all-files |
7.0 ± 0.1 | 6.9 | 7.2 | 1.00 |
fbb16a2 to
2e9e61f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
crates/prek/src/jj.rs:278
get_conflicted_files()treats any non-zero exit status fromjj resolve --listas “no conflicts”. That’s correct for the “no conflicts” case, but it will also silently swallow real failures (e.g., running outside a jj repo, incompatible jj version, or other command errors), causing prek to proceed without surfacing the underlying problem.
pub(crate) async fn get_conflicted_files(root: &Path) -> Result<Vec<PathBuf>, Error> {
let output = jj_cmd()?
.current_dir(root)
.arg("resolve")
.arg("--list")
.check(false)
.output()
.await?;
if !output.status.success() {
return Ok(Vec::new());
}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e9e61f4cb
ℹ️ 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".
2e9e61f to
4b775dc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b775dc551
ℹ️ 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".
723deea to
ca7e4fa
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
crates/prek/src/repo.rs:224
changed_files_between()takesworkspace_root, but the Jujutsu branch ignores it and diffs the full repository (repo.root()), relying on later filtering. In large monorepos with nested prek workspaces this can make--from-ref/--to-refnoticeably slower than the Git backend, which scopes the diff toworkspace_root. Consider adding a workspace-root scope to the jj diff call (fileset/path filter) so only relevant paths are computed.
pub(crate) async fn changed_files_between(
old: &str,
new: &str,
workspace_root: &Path,
) -> Result<Vec<PathBuf>> {
let repo = current()?;
match repo.kind() {
RepoKind::Git => git::get_changed_files(old, new, workspace_root)
.await
.map_err(Into::into),
RepoKind::Jujutsu => jj::get_changed_files_between(old, new, repo.root())
.await
.map_err(Into::into),
crates/prek/tests/builtin_hooks.rs:26
jj_cmd()is duplicated across multiple integration test files (this one andcrates/prek/tests/run.rs). Duplicated helpers tend to drift (e.g., env vars, defaults), which can make future jj-related test changes inconsistent. Consider moving this helper intocrates/prek/tests/common/mod.rs(or a dedicated test helper module) and reusing it from both test files.
// Give jj a deterministic identity so `jj commit` does not depend on host config.
cmd.env("JJ_USER", "prek test");
cmd.env("JJ_EMAIL", "prek-test@example.com");
Some(cmd)
}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca7e4fabb5
ℹ️ 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".
f1869ac to
b876181
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-backend repository discovery and subprocess environment changes warrant human review before approval.
7 open findings
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
1dced90 to
734f739
Compare
There was a problem hiding this comment.
🟡 Changes recommended
jj output formatting and fileset escaping can still break hook file selection.
2 open findings
8 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🟡 Changes recommended
Repository metadata decoding still rejects valid Unix paths and alters backing-store paths ending in whitespace.
2 open findings
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-backend discovery, Git environment handling, and file selection require human validation, with correctness issues still unresolved.
4 open findings
3 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Repository discovery has an unresolved error path, and cross-platform hook environments still need human validation.
1 open finding
4 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Repository discovery and subprocess environment changes need human validation, with correctness issues still unresolved.
3 open findings
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Revision selection needs correction, and cross-platform repository discovery and Git environment handling require final human validation.
3 open findings
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
The cross-cutting repository discovery and Git environment changes still need corrections and human validation.
4 open findings
2 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved jj conflicts can cause other working-copy changes to bypass hooks.
3 open findings
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🟡 Changes recommended
Hook-time jj snapshots can falsely report file modifications and fail read-only runs.
1 open finding
3 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Repository discovery and hook environment changes need human validation, and backend-specific correctness issues remain.
2 open findings
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-cutting repository discovery and hook Git-environment changes require final human validation.
4 open findings
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-platform repository discovery and Git environment changes warrant human validation alongside the unresolved correctness issues.
2 open findings
4 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🟡 Changes recommended
Repository discovery and hook revision handling have unresolved correctness issues.
3 open findings
2 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Repository discovery and Git environment changes need maintainer validation across supported workspace configurations.
3 open findings
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-platform changes to repository discovery, Git environments, and hook execution require final human validation.
1 open finding
3 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Detect Jujutsu workspaces and route the VCS queries that differ by backend, so prek works in colocated workspaces and in secondary ones created with `jj workspace add`, including workspaces with no `.git` of their own. - Detect a jj workspace during Git discovery, resolve its backing Git store from `.jj/repo/store/git_target`, and point Git at it when the workspace has no `.git` entry of its own, including for a hook's own Git commands. - Add a `repo` module dispatching default/added/changed/conflicted file selection, stashing, and the staged-config rule per backend. - Query jj through NUL-delimited templates and byte-exact path decoding, so paths with spaces, newlines, or non-UTF-8 bytes survive; a rename also reports the removed source path, and deleted paths keep their file mode. - Skip `no-commit-to-branch` under jj: jj has no current branch that maps to Git's HEAD, and a non-colocated store's unborn HEAD blocked every run. - Install a pinned, checksum-verified jj in CI and run the jj integration tests there, and document jj support and its limits. Co-Authored-By: Martijn Berger <mbr@veo.co>
| if last_commit { | ||
| return Self::Diff { | ||
| from_ref: "HEAD~1".to_string(), | ||
| to_ref: "HEAD".to_string(), | ||
| }; | ||
| return Self::LastCommit; |
| - a working-copy path that is not valid UTF-8 does not reach file selection, so hooks | ||
| never receive it; a path holding a newline does. |
Adds Jujutsu (jj) workspace support so
prek runworks in jj workspaces, including secondary ones created withjj workspace addthat have no.git.Rebased onto current
masterand squashed; @martijnberger's original work is preserved as aCo-Authored-Bytrailer on the commit..jj/repo/store/git_target, and point Git at it only when the workspace has no.gitentry of its own, including for hooks' own Git commands.prek runuses the working-copy changeset;--all-files,--files,--from-ref/--to-ref, and conflict detection use jj equivalents.no-commit-to-branchfor jj.Supersedes #1677.