Integrate with https://pre-commit.com/ #405

Open martinvonz opened 2 years ago

Description

I don't think I had seen https://pre-commit.com/ until @chandlerc linked to it. It would be really nice to integrate with that somehow. I currently have a long command for running Clippy and rustfmt on my commits. It would be better if we could use pre-commit.com for sharing that between users and making it easy to run. I don't yet know how it would work.

Mercurial's fix extension seems closely related. IIUC, pre-commit.com runs all its checks in the working copy (possibly after stashing any unstaged changes), so that's a big difference compared to hg fix. I think it would be nice if we could run the pre-commits on all commits in parallel just like hg fix does, but that's slow to do if the working copy is large because we'd need to create a working copy for each commit (unless/until we can expose a commit in a virtual file system). That's why hg fix doesn't do it. A good start might be if we can figure out a way of running the pre-commit.com checks in the working copy.

In case its useful, a collection of useful https://pre-commit.com/ hooks is here: https://github.com/google/pre-commit-tool-hooks

Maybe some even useful for working with this repo! =D

I looked into this a bit, and I've found two potential ways to interface with pre-commit.

The first one is to use the --from-ref and --to-ref arguments of pre-commit run to specify a range of commits to use.

The other one is to pass it an explicit list of files with --files. Using this, one can run pre-commit on the file from the last revision (just after a jj commit for example), with this monstrosity (the python part extracts the list of files):

jj show @- -s| python3 -c 'import sys; print(" ".join(l[2:] for l in sys.stdin.read().split("\n\n")[-1].split("\n")))' | xargs pre-commit run --files

There's one hook that's not behaving well (darker), but that's because the program itself is looking at the git index, so I might have to find another trick to convince it to do what I want.

I think --from-ref and --to-ref implies an underlying Git repo. I think it would be nice if we could make it independent of storage backend. That's not terribly important in practice since no one uses jj's native backend yet (and they shouldn't).

A bigger problem is that the pre-submit command seems to expect to run in a Git repo, so it won't work in a repo created by jj git clone. Perhaps we should see if the pre-commit teams is open to changing that so it can be run outside of a Git repo.

PS. jj show shows the diff along with the commit message and some other metadata. If you just want the diff, you can do jj diff -s -r <revision> instead.

I would argue that the general problem here is not "running hooks" but "running commands on each commit". One other example is that you might want to run tests on all commits in your stack and get feedback on which commits are passing.

For git-branchless, I wanted to recreate https://github.com/spotify/git-test to this end, but also add some modes that amend the commit with any working copy changes. The execution strategy could be selected by the user as one of "working copy" or "use up to X worktrees" or "use up to X VFS checkouts".

EDIT: by the way, I did recreate git test with the above features.

As mentioned in Discord: https://discord.com/channels/968932220549103686/969829516539228222/1047922968543637504 I'd like to implement a jj run command which would allow integration with https://pre-commit.com, it should come with the usual set of options and should do the neat tricks from git-branchless git test and https://github.com/spotify/git-test. This should allow @arxanas to build jj test on top-off it.

Is there any status on jj run besides what's tracked here?

Sounds like quite a few steps before something like pre-commit.com integration could be ready to use. Any baby steps or prototyping that could be done while work on jj run is underway?

@PhilipMetzger has started designing the feature. There's been quite a bit of discussion on our Discord chat. We haven't quite decided how it will work yet, and we haven't talked about pre-commit.com integration specifically, but I think we've pretty much decided that the jj run command will keep semi-temporary working copies like git test does. What we haven't decided is how to handle changes made by the commands run by jj run and if we would add flags to make it not do anything (for running tests), rebase or "reparent" (rebasing by just taking the rebased commit's file contents), or if some of those should instead be separate commands. If you're interested in helping out, please join us on Discord as that's where most discussion happens.

Just checking in since I hit this lack of pre-commit support again today. Have we gotten any closer to landing a solution?

I'm slowly inching closer to the implementation of run, as most of it is in the 3rd and 4th checkmarks of #1869. But atm you'll still need to run the pre-commit hooks manually.

Awesome, as long as I can still have hope it'll land eventually!

But atm you'll still need to run the pre-commit hooks manually.

Would that generally look something like:

$ . .git/hooks/pre-commit && jj describe

? If I can find some one-liner workarounds at least I won't have to choose between using jj and having hooks applied, when I'm working in projects that have important hooks.

You'll probably need something like https://gist.github.com/thoughtpolice/8f2fd36ae17cd11b8e7bd93a70e31ad6 but instead of calling Gerrit's hooks (see the jj signoff alias), use the pre-commit driver hook . Maybe call it something like jj pre-commit.

Thanks!

And how'd you know I was dealing with Gerrit hooks, just a lucky guess?

And how'd you know I was dealing with Gerrit hooks, just a lucky guess?

It is the most common question in relation with hooks/run in Discord, so I took it as an example.

It sounds like things are going in a different direction than commit hooks but, just in case, I wanted to bring up a slightly different use case for a commit verification hook: verifying commit messages. I use my tool committed with pre-commit today. It is annoying that it rejects my commit message and I have to write it all over again. Seeing how merge conflicts work, it'd be great if a verification hook system could track the status and clear it when fixed or allow the user to explicitly force the status to be cleared.

I uses git-hooks.nix, which can generate pre-commit and pre-push hooks + run them in CI.

libgit2 appears to support pre-push hooks. So perhaps integrating with that would be feasible for jj?

martinvonz / jj

Integrate with https://pre-commit.com/ #405

Description