feat: respect .gitignore in Jujutsu (.jj) repositories#2016
Conversation
Jujutsu repos use a `.jj` directory instead of `.git` but still rely on `.gitignore` files. Automatically disable require_git when searching inside such repositories, matching the behavior of --no-require-git. Fixes #1817 Co-authored-by: Cursor <cursoragent@cursor.com>
![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: ce5b739316
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| !search_paths | ||
| .iter() | ||
| .any(|path| path.ancestors().any(|ancestor| ancestor.join(".jj").is_dir())) |
There was a problem hiding this comment.
.jj
path.ancestors() only walks lexical components of the provided search path, so for the common default ./ (or other relative paths) it never reaches parent directories above the current working directory. That means running fd from a subdirectory inside a Jujutsu repo (where .jj is at the repo root) will not be detected here, and .gitignore rules still won’t apply unless the user is at the root or passes an absolute path, which defeats the new feature for many real invocations.
Useful? React with 👍 / 👎.
| if no_require_git { | ||
| return false; | ||
| } | ||
|
|
||
| !search_paths | ||
| .iter() | ||
| .any(|path| path.ancestors().any(|ancestor| ancestor.join(".jj").is_dir())) |
There was a problem hiding this comment.
--require-git in Jujutsu repositories
This helper cannot distinguish “default behavior” from an explicit --require-git, because it only receives no_require_git; as soon as .jj is found it always returns false (do not require git). In a Jujutsu repo this makes --require-git ineffective, even though the intended behavior and existing flag semantics rely on explicit override precedence.
Useful? React with 👍 / 👎.
tmccombs
left a comment
tmccombs
left a comment
There was a problem hiding this comment.
I think it would be better to address this upstream in the ignore crate.
Also this doesn't handle if the search paths are relative, and if one search path has a .jj directory but others don't, it disables requiring a .git (or .jj) directory for all of them, which probably isn't desirable.
|
Addressed the review feedback and CI failure in commit 3267318. The .jj detection now resolves relative search paths against the current directory and only relaxes require_git when all search paths are inside a Jujutsu repo, so a mixed set of .jj and non-.jj search paths no longer disables the git requirement globally. Also ran cargo fmt to clear the formatter check. Local verification:
|
|
This now has the opposite problem where it will only use gitignore if all the base paths have .jj directories. And furthermore, doesn't handle cases where a subdirectory has a .jj dir, or has a .gitignore file, but not a .git OR .jj directory. I really think this would be better handled by looking for .jj directories in the ignore crate itself, and would suggest you instead open a PR with ripgrep. In lieu of that, along with my suspicion that this is AI slop that hasn't had human review. I am closing this PR. |
2 participants
Summary
Fixes #1817.
Jujutsu repositories use a
.jjdirectory and still rely on.gitignorefiles, but fd only applies gitignore rules without a.gitdirectory when--no-require-gitis passed.When any search path is inside a Jujutsu repository (a
.jjdirectory exists in an ancestor path), automatically treat the search like--no-require-gitunless the user explicitly passes--require-git.Test plan
cargo test --locked jujutsutest_jujutsu_repo_respects_gitignoreshould_require_git_in_jujutsu_repo