BurntSushi
Support .jj as well as .git
- Allow
.jjdirs to count as vcs directories. - Simplify the control flow around resolving info/exclude
I totally understand if you don't want to expand things to support every oddball VCS, but so many tools I like (cargo watch, rg, fzf, fd) are all built on this common library that adding a bit of support at the core gets a lot of things to just work.
I think my main concerns here are:
- I don't even know what
.jjis referring to. You didn't give a link or explain anything. - This patch seems to assume that
.jjand.githave identical semantics. Do they? There are literally no differences? - This doubles the number of stat calls for every directory ripgrep searches. This is a rather annoying hurdle because I don't want this to be used as a bludgeon to forever fix ourselves to
git. But at the same time, there is a cost here that needs to be weighed with the benefit.
Sorry about the lack of link, that is my bad. https://martinvonz.github.io/jj/latest/ is the best spot to read. It is a version control system like git, so .jj is a directory that contains metadata for it (just like .git).
As an alternate suggestion, I would recommend respecting .gitignore files even when there is no .git directory. Then you can cut the stat calls down from 1 to zero.
As an alternate suggestion, I would recommend respecting
.gitignorefiles even when there is no.gitdirectory. Then you can cut the stat calls down from 1 to zero.
That already exists today with --no-require-git. But vcs directory detection is in general required for correctness so that you don't let gitignore files cross repository boundaries.
The problem that I run into with --no-require-git is that every tool built on the library exposes it differently (and some don't). Is there a way to make that controlled via env variable or something?
No. I personally think that would be wildly inappropriate. ignore isn't a "standard," and trying to reach out into the environment in a library to do an end-runaround on the CLI interface just seems really bad to me. Like, there would, by construction, be no coupling between the environment variable and the CLI, which means there would be no way for ignore to know when to respect the environment variable versus some other option (which might override the environment variable). The only way to solve that would be to introduce more API machinery to deal with it. Sounds awful.
I'm not saying No to this PR, but it's going to require that I or someone goes and understands jj to ensure this is implemented correctly. At minimum. And I'll also need to make a judgment of whether the cost is worth it.
I checked with folks on the jj discord to make sure that I was getting all the corners. I have updated the PR with that and also moved things around a tiny bit to minimize the number of stat calls as well as the number of allocations.
I absolutely understand that this is a judgement call on your end about complexity versus general utility. If there is any data I can provide to make that case, I would be happy to try, but I recognize that jj is a niche VCS. (I have hopes it will grow, but realistically I am not sure it will.)
Following up, I have verified this PR with jj's owner (and I am also a committer on it). So I think this is ready for review (pending your actual acceptance of the direction).
Thanks!
Some musings:
Jujutsu is a neat VCS, and is quite git compatible. It has a lot of positives for it, and the biggest negative for it I've found in the past ~2 months I've been using it come from the (understandable, given it being relatively newer and more niche) lack of recognition of it by some tools, so I was considering making a PR to ripgrep to add support, and lo and behold, this PR already exists ❤️
As a quick note: jj's only way (currently?) to ignore files is to use a .gitignore (it even looks at $GIT_DIR/info/exclude as git would), and while the ignore format is itself not a standard, I would be hard-pressed to accept that any tool that uses .gitignore (i.e., with the git in the file name) with a departure in semantics from what git already uses to not be considered a bug in the tool. Thus, I would expect jj to maintain the same semantics as git for their usage of .gitignore.
Personally, I was using ripgrep suboptimally for some time, because I wasn't aware of --no-require-git, since at least as of ripgrep 14.1.1, it is not easy to realize that .gitignores are ignored unless a .git exists. Obviously, once I found --no-require-git (via this PR, funnily), I took a gander through rg --help, and the only mention of the .git directory being necessary is in the --no-require-git option's docs (which does mention the defaults). However, I wonder if people might be happier with the opposite default (I am somewhat biased here, so maybe I am not accounting for some other common use case where a .gitignore file should be ignored). I will note that literally the second line of --help states:
By default, ripgrep will respect gitignore rules and automatically skip hidden files/directories and binary files.
So I don't think people would be annoyed by a default that respects gitignore rules (even if .git is absent).
If the flipped version becomes the default, then we would not need to double the number of stat calls (indeed, number of stat calls would drop for most people), at which point, we might actually be able to afford having some --require-vcs git,jj,...-style flag (again, this is just me musing) where the user is opting in for which VCSs they want the requirements checked, allowing for more stat calls (obviously, --require-git is already a flag right now, so more thought would be needed into such a future flag), but nonetheless, a flipped default would probably fix the issue, and not even need a .jj/ check.
Sidenote: I now have --no-require-git as part of my CLI default for rg now; happy to report back if it'd help with (one person) anecdotal data if you find it useful.
Another alternative to consider: if a .gitignore file is found, but a .git is not, telling the user that --no-require-git exists could be nice. Clearly I am not the only one who missed that this option exists :)
With all the above, I also want to add: I don't want to increase maintainer burden, and I understand things like defaults are a hard decision. The existence of --no-require-git is great, and I am very glad that @BurntSushi you place such a laser focus on having a fast and awesome tool! :heart:
So I don't think people would be annoyed by a default that respects gitignore rules (even if .git is absent).
That's a problem for globally-defined .gitignores that people tend to place in ~.
Ping on this, it would be awesome to get this in, although for now --no-require-git is a fine temporary solution :)
And yes, there's no way that --no-require-git is going to become a ripgrep default. That would allow gitignore files to cross pollinate into other repos. And nesting git repos inside other git repos is a very common pattern.
The next step here is for me to try this out and do some ad hoc perf testing to ensure the extra stat call isn't too big of a perf hit. If it is, we can brainstorm how to mitigate it (or just accept it). My guess is that it will be acceptable.