Re-implement ratchet checks on single checkouts

[wolfgangwalther]

Currently, the ratchet-type checks require two checkouts to work with and compare. If it fails in one, it's OK to fail in the other. This is a bit annoying to set up, especially when trying to run nixpkgs-vet on a local branch. nix-build ci -A nixpkgs-vet can't be branch-aware, thus we have a helper script in ci/nixpkgs-vet.sh.

I propose a different approach, which could be used for treefmt as well, when introducing new formatters, which might not pass on all files, yet:

• Add something to ignore specific files (Add some way to ignore files #138)
• Keep a list of "known failures" in nixpkgs and pass the ignore list to the check
• Throw an error if any of the ignored files actually passes (to then nudge the user towards removing the file from the list)
• Bonus points: Provide a --fix mode for that error, so that the ignore file is written to automatically.

This would allow step-by-step introduction of new rules in single checkouts, too. It would allow to easily run the same nixpkgs-vet locally as in CI. And it would allow to see the list of violating files for each tool quickly.

Especially the latter is not easy to do right now. The only way I came up with for nixpkgs-vet would be to compare your current checkout against a mostly empty checkout or so?

[pbsds]

Can't the tool checkout a worktree inside .git of the common ancestor of HEAD and upstream master? Or even just ask git to print the files of interest at that ancestor commit and do the checks in memory? Keeping the ratchet check mechanism opens the door to add a lot more checks without having to think about and be careful about the rest of the machinery around the lints.

[philiptaron]

Can't the tool checkout a worktree

This is the direction I think we should go -- bring git inside the tool, instead of asking the user to manage the worktree, or have the nixpkgs CI manage it. That does run the risk of making the checkout CI uses slow, but every time I've run into a Git problem like this, there is a strange melange of command line flags which fix it.

[pbsds]

• Keep a list of "known failures" in nixpkgs and pass the ignore list to the check

Checking an index of known lint errors into nixpkgs would risk being too massive. Here is a treewide run i did with nixpkgs-hammer a great while ago as an example: https://pvv.ntnu.no/~pederbs/tmp/merged-issues-report-nixpkgs/0396d3b0fb7f/report-hammering.html

[wolfgangwalther]

This is the direction I think we should go -- bring git inside the tool, instead of asking the user to manage the worktree, or have the nixpkgs CI manage it.

This brings another challenge, though: How can I do that inside the nix sandbox?

Currently, we run all those CI tools inside nix. But I didn't find a way to bring the local git repo including history / .git in there.

[philiptaron]

How can I do that inside the nix sandbox?

For nixpkgs CI, we don't actually need to do that. We can run in the bare GitHub actions environment. And for developers running the tool locally (which will be more-and-more a usecase as it gains functionality) it never ran in the Nix sandbox.

[wolfgangwalther]

For nixpkgs CI, we don't actually need to do that. We can run in the bare GitHub actions environment.

The reason I did that, was to give a consistent interface for running CI locally. Leaving Eval out for a moment, the idea is to be able to run nix-build ci once - and have it run all the checks that CI runs, too. We lose that, if we require running it outside of nix again.

[pbsds]

Maybe not all checks should be beaten into being fully heretic? A simple ./ci/run-checks.sh could setup the worktree and invoke a couple of nix-builds for you. Keeping it under 50 LoC would even let users vet the script to decide if they want to trust it.

(If flakes were non-experimental one could use something like nix build .?rev=HEAD^#ci grumble grumble...)

[wolfgangwalther]

A simple ./ci/run-checks.sh could setup the worktree and invoke a couple of nix-builds for you.

You would at least need to provide a target branch to compare against. And then you'd need to make sure that target branch is up2date. And then you'd still not be guaranteed to get the same result as CI, because it creates a different merge commit.

This makes it much harder to reason about, especially for a random drive-by contributor. I really just want to give them a tool which gives consistent results about their work. And that's the state of the head branch.

Also, any "this was X before this commit and Y after this commit" logic is fundamentally better put in git hooks. There you can really look at change. But looking at the consistency of a certain state (checkout) should not depend on other commits or even branches.

[pbsds]

With gum you can give them an interactive choice

https://gist.github.com/pbsds/99efaef277e3e3d75904c898065497fc

[infinisil]

This already exists: https://github.com/NixOS/nixpkgs/blob/master/ci/nixpkgs-vet.sh

[pbsds]

Whelp, at least i got to play around with and learn git merge-tree and git commit-tree 🤡

[infinisil]

Oh I wish I linked it earlier 😅
Wanna be code owner of it in Nixpkgs? :D

[wolfgangwalther]

Of course, a generic entrypoint for CI (similar to the already existing nixpkgs-vet one) would have one benefit that surely won't work with any of my other proposals: You could actually run full Eval, including rebuilds, locally.

I'm not sure whether that's something people would do, though. I can see running all static checks locally, but eval not so much.