Conversation
Enable prek to work inside jj workspaces, including secondary workspaces created with `jj workspace add` where there is no `.git` directory. - Add new `jj` module that detects jj workspaces and resolves the backing git directory from `.jj/repo/store/git_target` - Set GIT_DIR/GIT_WORK_TREE env vars early in startup so all git commands work transparently in jj workspaces - Use `jj diff --name-only` instead of `git diff --staged` for file collection, since jj doesn't use git's staging area - Disable git-specific stashing/work-tree-cleaning for jj workspaces - Short-circuit merge conflict detection for jj (handles conflicts differently)
- Use EnvVars constants instead of raw string literals for GIT_DIR/GIT_WORK_TREE - Use EnvVars::is_set() instead of disallowed std::env::var_os() - Use let...else pattern per clippy::manual_let_else - Add #[instrument(level = "trace")] to get_changed_files() matching git.rs - Add doc comments to JJ static and jj_cmd() matching git.rs conventions - Import prek_consts::env_vars::EnvVars for consistency with rest of codebase - Add unit tests for find_jj_dir, colocated workspace, and secondary workspace resolution
- Add FAQ entry explaining how prek detects and works with jj workspaces - Mention jj working copy in quickstart guide's "run hooks on demand" section - Update workspace discovery docs to reference .jj alongside .git as a repository boundary
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1677 +/- ##
==========================================
- Coverage 91.64% 91.45% -0.20%
==========================================
Files 96 97 +1
Lines 18669 18818 +149
==========================================
+ Hits 17110 17210 +100
- Misses 1559 1608 +49 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| let files = git::get_staged_files(workspace_root).await?; | ||
| debug!("Staged files: {}", files.len()); | ||
| let files = if *crate::jj::IS_JJ_WORKSPACE { |
There was a problem hiding this comment.
Rest of the project does not reference crates this way.
There was a problem hiding this comment.
as in the crate:: prefix ?
| if all_files { | ||
| let files = git::ls_files(git_root, workspace_root).await?; | ||
| debug!("All files in the workspace: {}", files.len()); | ||
| return Ok(files); | ||
| } |
There was a problem hiding this comment.
Yeah I think I should take another pass and push this deeper into prek rather then patch the surface
There was a problem hiding this comment.
Pull request overview
This PR adds support for Jujutsu (jj) workspaces to prek, enabling the tool to work seamlessly with both primary (colocated) and secondary jj workspaces. The implementation detects jj workspaces automatically and adapts prek's behavior to work with jj's different model (no staging area, different conflict handling).
Changes:
- Adds new
jj.rsmodule with workspace detection, git directory resolution, and changed files collection - Modifies startup sequence to set
GIT_DIRandGIT_WORK_TREEenvironment variables for jj workspaces - Disables git-index-specific features (stashing, merge conflict detection) when in jj workspaces
- Updates documentation to explain jj support
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/prek/src/jj.rs | New module implementing jj workspace detection, git directory resolution for colocated/secondary workspaces, and changed files collection via jj diff --name-only |
| crates/prek/src/main.rs | Calls setup_git_env_for_jj() early in startup to configure environment variables before any git commands run |
| crates/prek/src/cli/run/run.rs | Disables stashing for jj workspaces by adding !*crate::jj::IS_JJ_WORKSPACE to the should_stash condition |
| crates/prek/src/cli/run/filter.rs | Uses jj::get_changed_files() instead of git::get_staged_files() when IS_JJ_WORKSPACE is true |
| crates/prek/src/git.rs | Short-circuits is_in_merge_conflict() to return false for jj workspaces since jj handles conflicts differently |
| docs/faq.md | Adds FAQ entry explaining jj support, automatic detection, and behavioral differences |
| docs/quickstart.md | Updates prek run description to mention jj working copy alongside git staging area |
| docs/workspace.md | Updates repository boundary description to include .jj directory |
| /// Whether the current working directory is inside a jj workspace. | ||
| pub(crate) static IS_JJ_WORKSPACE: LazyLock<bool> = LazyLock::new(|| { | ||
| let Ok(cwd) = std::env::current_dir() else { | ||
| return false; | ||
| }; | ||
| find_jj_dir(&cwd).is_some() | ||
| }); |
There was a problem hiding this comment.
The IS_JJ_WORKSPACE LazyLock is initialized based on the current working directory at the time it's first accessed. If the working directory changes before this LazyLock is initialized (e.g., via the --cd option), the detection may be incorrect. However, looking at main.rs, setup_git_env_for_jj() is called before the --cd option is processed (line 207), so accessing IS_JJ_WORKSPACE later will use the original cwd. This is correct but fragile - consider documenting this dependency or restructuring to make it more explicit.
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn find_jj_dir_returns_none_for_non_jj_directory() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| assert!(find_jj_dir(dir.path()).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn find_jj_dir_finds_jj_in_current_directory() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| std::fs::create_dir(dir.path().join(".jj")).unwrap(); | ||
| let result = find_jj_dir(dir.path()); | ||
| assert_eq!(result, Some(dir.path().join(".jj"))); | ||
| } | ||
|
|
||
| #[test] | ||
| fn find_jj_dir_finds_jj_in_parent_directory() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| std::fs::create_dir(dir.path().join(".jj")).unwrap(); | ||
| let child = dir.path().join("subdir"); | ||
| std::fs::create_dir(&child).unwrap(); | ||
| let result = find_jj_dir(&child); | ||
| assert_eq!(result, Some(dir.path().join(".jj"))); | ||
| } | ||
|
|
||
| #[test] | ||
| fn detect_jj_git_dir_returns_none_without_jj_workspace() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| // No .jj dir at all — detection should return None. | ||
| // We can't easily test this without changing CWD, so just verify | ||
| // the helper function returns None. | ||
| assert!(find_jj_dir(dir.path()).is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn detect_jj_git_dir_returns_none_without_git_target() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| let jj_dir = dir.path().join(".jj"); | ||
| let repo_dir = jj_dir.join("repo"); | ||
| let store_dir = repo_dir.join("store"); | ||
| std::fs::create_dir_all(&store_dir).unwrap(); | ||
| // No git_target file — should not resolve. | ||
| // detect_jj_git_dir() reads CWD, so we test the building blocks. | ||
| assert!(find_jj_dir(dir.path()).is_some()); | ||
| assert!(!store_dir.join("git_target").exists()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn detect_jj_git_dir_resolves_colocated_workspace() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| let root = dir.path(); | ||
|
|
||
| // Set up a colocated jj workspace structure: | ||
| // .jj/repo/store/git_target → "../../../.git" | ||
| // .git/ | ||
| let jj_dir = root.join(".jj"); | ||
| let store_dir = jj_dir.join("repo").join("store"); | ||
| std::fs::create_dir_all(&store_dir).unwrap(); | ||
| std::fs::write(store_dir.join("git_target"), "../../../.git").unwrap(); | ||
| let git_dir = root.join(".git"); | ||
| std::fs::create_dir(&git_dir).unwrap(); | ||
|
|
||
| // Since detect_jj_git_dir uses std::env::current_dir, we test the | ||
| // resolution logic directly. | ||
| let repo_dir = jj_dir.join("repo"); | ||
| let git_target = | ||
| std::fs::read_to_string(repo_dir.join("store").join("git_target")).unwrap(); | ||
| let resolved = repo_dir.join("store").join(git_target.trim()); | ||
| let resolved = resolved.canonicalize().unwrap(); | ||
| assert_eq!(resolved, git_dir.canonicalize().unwrap()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn detect_jj_git_dir_resolves_secondary_workspace() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| let main_root = dir.path().join("main"); | ||
| let secondary_root = dir.path().join("secondary"); | ||
|
|
||
| // Set up main workspace: | ||
| // main/.jj/repo/store/git_target → "../../../.git" | ||
| // main/.git/ | ||
| let main_jj = main_root.join(".jj"); | ||
| let main_store = main_jj.join("repo").join("store"); | ||
| std::fs::create_dir_all(&main_store).unwrap(); | ||
| std::fs::write(main_store.join("git_target"), "../../../.git").unwrap(); | ||
| let main_git = main_root.join(".git"); | ||
| std::fs::create_dir(&main_git).unwrap(); | ||
|
|
||
| // Set up secondary workspace: | ||
| // secondary/.jj/repo → file pointing to main/.jj/repo (absolute path) | ||
| let secondary_jj = secondary_root.join(".jj"); | ||
| std::fs::create_dir_all(&secondary_jj).unwrap(); | ||
| let main_repo_abs = main_jj.join("repo").canonicalize().unwrap(); | ||
| std::fs::write(secondary_jj.join("repo"), main_repo_abs.to_str().unwrap()).unwrap(); | ||
|
|
||
| // Verify secondary workspace resolves to the same git dir. | ||
| let repo_content = std::fs::read_to_string(secondary_jj.join("repo")).unwrap(); | ||
| let repo_dir = PathBuf::from(repo_content.trim()); | ||
| assert!(repo_dir.is_dir()); | ||
|
|
||
| let git_target = | ||
| std::fs::read_to_string(repo_dir.join("store").join("git_target")).unwrap(); | ||
| let resolved = repo_dir.join("store").join(git_target.trim()); | ||
| let resolved = resolved.canonicalize().unwrap(); | ||
| assert_eq!(resolved, main_git.canonicalize().unwrap()); |
There was a problem hiding this comment.
There are no integration tests for jj workspace functionality. The PR only includes unit tests for the jj.rs module itself. Consider adding integration tests that verify end-to-end behavior, such as: running hooks in a simulated jj workspace, verifying that --all-files works correctly, and ensuring that stashing is properly disabled. Without integration tests, it's difficult to verify that all the pieces work together correctly.
| let workspace_root = find_jj_dir(&cwd) | ||
| .and_then(|jj_dir| jj_dir.parent().map(Path::to_path_buf)) | ||
| .unwrap_or(cwd); |
There was a problem hiding this comment.
On line 116, if find_jj_dir(&cwd) returns None (which shouldn't happen since we already validated it exists in detect_jj_git_dir()), or if .parent() returns None (which would be unusual for a .jj directory), the code falls back to using cwd as the workspace root. This fallback might lead to incorrect behavior where GIT_WORK_TREE points to a subdirectory instead of the actual workspace root. Consider whether this fallback is necessary or if it should be an error condition.
| let workspace_root = find_jj_dir(&cwd) | |
| .and_then(|jj_dir| jj_dir.parent().map(Path::to_path_buf)) | |
| .unwrap_or(cwd); | |
| let Some(workspace_root) = find_jj_dir(&cwd) | |
| .and_then(|jj_dir| jj_dir.parent().map(Path::to_path_buf)) | |
| else { | |
| debug!( | |
| "jj workspace detected, but failed to determine workspace root from cwd {}; not setting git env vars", | |
| cwd.display() | |
| ); | |
| return; | |
| }; |
| }; | ||
|
|
||
| let git_target_file = repo_dir.join("store").join("git_target"); | ||
| let git_target = std::fs::read_to_string(&git_target_file).ok()?; |
There was a problem hiding this comment.
The new jj.rs module uses std::fs directly instead of fs_err which is the standard throughout the codebase. Using fs_err provides better error messages with file paths included automatically. Replace all instances of std::fs::read_to_string, std::fs::create_dir, etc. with their fs_err:: equivalents.
| // Detect jj workspace and set GIT_DIR/GIT_WORK_TREE if needed. | ||
| jj::setup_git_env_for_jj(); |
There was a problem hiding this comment.
The setup_git_env_for_jj function is called before logging is fully initialized. If there are issues in jj workspace detection, the debug messages on lines 118-122 will not be captured in the log file. Consider moving the call after logging is set up in main.rs, or document why early execution is required.
| let repo_dir_candidate = jj_dir.join("repo"); | ||
| let repo_dir = if repo_dir_candidate.is_file() { | ||
| // Secondary workspace: file contains the path to the main repo dir. | ||
| let content = std::fs::read_to_string(&repo_dir_candidate).ok()?; |
There was a problem hiding this comment.
The new jj.rs module uses std::fs directly instead of fs_err which is the standard throughout the codebase. Using fs_err provides better error messages with file paths included automatically. Replace all instances of std::fs::read_to_string, std::fs::create_dir, etc. with their fs_err:: equivalents.
| let should_stash = | ||
| !all_files && files.is_empty() && directories.is_empty() && !*crate::jj::IS_JJ_WORKSPACE; |
There was a problem hiding this comment.
The should_stash condition correctly excludes jj workspaces from stashing, but the code on lines 85-87 (not shown in diff) still calls workspace.check_configs_staged() when should_stash is true. Since this function uses git::files_not_staged() which checks git's staging area, it will work correctly for git repos but may need jj-specific handling in the future. Consider verifying this works as expected in jj workspaces where configs are modified.
| When running inside a jj workspace, prek: | ||
|
|
||
| - Resolves the backing Git directory from `.jj/repo/store/git_target`, so all internal git commands work even when there is no `.git` directory. | ||
| - Uses `jj diff --name-only` instead of `git diff --staged` to collect changed files, since jj does not use Git's staging area. |
There was a problem hiding this comment.
The documentation states that prek "Uses jj diff --name-only instead of git diff --staged", but jj diff --name-only shows changes in the current changeset (working copy changes), not just the equivalent of staged files. This is conceptually different from git's staging area. Consider clarifying that jj operates on the working copy changeset, which includes all modified files, not just a subset like git's staging area.
| - Uses `jj diff --name-only` instead of `git diff --staged` to collect changed files, since jj does not use Git's staging area. | |
| - Uses `jj diff --name-only` to collect files changed in the current working-copy changeset (jj does not have a separate staging area like `git diff --staged`). |
| let files = crate::jj::get_changed_files(workspace_root) | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!(e))?; |
There was a problem hiding this comment.
The error from jj::get_changed_files() is converted to anyhow using map_err(|e| anyhow::anyhow!(e)). This is redundant since jj::Error already implements Display and std::error::Error (via thiserror), so it can be automatically converted using the ? operator or .context(). The current git code on line 397 uses ? directly without explicit error conversion.
| let files = crate::jj::get_changed_files(workspace_root) | |
| .await | |
| .map_err(|e| anyhow::anyhow!(e))?; | |
| let files = crate::jj::get_changed_files(workspace_root).await?; |
| let files = if *crate::jj::IS_JJ_WORKSPACE { | ||
| let files = crate::jj::get_changed_files(workspace_root) | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!(e))?; |
There was a problem hiding this comment.
When IS_JJ_WORKSPACE is true but the jj command is not available in PATH, calling get_changed_files() will fail with a "Failed to find jj" error. This could happen if someone checks out a jj workspace on a machine without jj installed. Consider providing a more helpful error message in this scenario, or falling back to a different behavior. The error handling in filter.rs (line 393) will show the raw error which may be confusing to users.
| .map_err(|e| anyhow::anyhow!(e))?; | |
| .map_err(|e| { | |
| let msg = e.to_string(); | |
| if msg.contains("Failed to find jj") { | |
| anyhow::anyhow!( | |
| "Detected a jj workspace, but the `jj` command was not found in PATH.\n\ | |
| Install `jj` (https://github.com/martinvonz/jj), ensure it is on your PATH,\n\ | |
| or convert this workspace to use git instead.\n\ | |
| Underlying error: {msg}" | |
| ) | |
| } else { | |
| anyhow::anyhow!(e) | |
| } | |
| })?; |
📦 Cargo Bloat ComparisonBinary size change: +0.00% (23.8 MiB → 23.8 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
|
Thanks! I definitely want to support Let’s mark this as a draft for now. Once I have a better understanding of |
Agreed, ill take a proper pass at this tomorrow and do it by hand in the artisanal way |
Motivation
I'm a happy jj (Jujutsu) user and I love prek. I just want the two to work together. Right now, prek fails in jj workspaces because:
jj workspace add) have no.gitdirectory, only.jj/. Every git command fails with "fatal: not a git repository".git diff --stagedreturns empty because jj doesn't use git's staging area. This makes the defaultprek runmode (which checks staged files) non-functional.This PR makes prek detect jj workspaces transparently and do the right thing, with no configuration required.
Changes
New:
crates/prek/src/jj.rsA new module for all jj-related logic, mirroring
git.rspatterns:IS_JJ_WORKSPACEdetects.jj/by walking up from CWD.detect_jj_git_dir()resolves the backing.gitdirectory from.jj/repo/store/git_target, handling both primary (colocated) and secondary workspaces.setup_git_env_for_jj()setsGIT_DIRandGIT_WORK_TREEenv vars early in startup so all git commands work transparently.get_changed_files()usesjj diff --name-onlyto collect changed files in the working copy.Modified files
main.rscallssetup_git_env_for_jj()before anyGIT_DIRhandling.cli/run/run.rsdisables stashing for jj workspaces (stashing,check_configs_staged(),has_unmerged_paths(), andWorkTreeKeeper::clean()are all git-index-specific).cli/run/filter.rsusesjj::get_changed_files()instead ofgit::get_staged_files()when in a jj workspace.git.rsshort-circuitsis_in_merge_conflict()for jj (jj handles conflicts differently).Documentation
docs/faq.mdnew FAQ entry: "Does prek work with Jujutsu (jj)?"docs/quickstart.mdmentions jj working copy alongside git staging area.docs/workspace.mdupdated repository boundary description to include.jj.Test plan
find_jj_dir, colocated workspace resolution, and secondary workspace resolutionprek runprek run --all-filesIS_JJ_WORKSPACE = falsewhen no.jj/exists)