Reformat Nixpkgs

[infinisil]

The recent python reformatting has shown that people are eager to have a uniform style already. Even though nixfmt isn't fully stable yet, we can already go ahead and use it to reformat all of Nixpkgs. With future nixfmt changes it can be reformatted again.

• shell.nix/direnv integration: Development shell with a pinned nixfmt #322512

• Make sure --verify succeeds: Use --verify in Nixpkgs diff CI nixfmt#215

• Change the workflow to only enforce formatting of new/changed files, and do the first formatting pass

  • This prevents the problem of breaking master when merging an older PR that introduces a new files and succeeded CI from before the formatting was enforced by CI
  • Enforce nixfmt on new files and changed files that were already formatted #326407

• Fix [Tracking issue] remove all src = ./. entries that unintentionally include their own .nix files #301014

• Wait until after the current release, minimise the number of conflicts (by picking a time with minimal open PRs)

• [ ] Have better integration

  • Make treefmt, nix fmt work
  • Fix Remove recursive mode nixfmt#151 beforehand
  • We don't need this for now, see flake: configure nix fmt #325575 (review)

• [ ] Have an easy maintainer script to rebase PRs with formatting: Script to rebase with formatting nixfmt#159
  Not a blocker

• Open an issue/discourse post with information on the nixfmt reformatting

• Format all actively developed branches (master, release-, staging-? (ask staging maintainers)) at the same time to avoid all backporting problems

• [ ] Figure out how to do future reformats and nixfmt updates (pin it, probably separately from the main package)
  Not a blocker

• [ ] Make sure there's no problems with generated files

  CI will complain when they get updated without being formatted, which gives a good signal to the PR author that they need to either manually format, or implement the formatting in the generator

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/formatting-team-meeting-2024-05-28/46126/1

[infinisil]

First step towards this, shell.nix integration: #322512

[infinisil]

After the above PR, here's the next one with first formatting pass! #322537 🎉

[myclevorname]

Should we include formatter = forAllSystems (system: self.legacyPackages.${system}.nixfmt-rfc-style); in flake.nix?

[infinisil]

Discussed in todays meeting:

• @infinisil: Potential problem after full treewide reformat:
  • Open a PR introducing a new nix file, CI passes
  • We update the nixfmt version on the base branch
  • Merge the PR, CI broken for all other PRs, because the formatter changed
• @infinisil: Proposed solution: Check formatting on pushes, if non-conformant, auto-commit to master a reformat
  • @dasJ: +1

I'll implement this together with the upcoming full treewide reformat

[Atemu]

Proposed solution: Check formatting on pushes, if non-conformant, auto-commit to master a reformat

This is only relevant for pushes regarding a change in the definition of the formatter used for CI, right?

Wouldn't it be better to just be more diligent to reformat nixpkgs (and/or have CI verify that fact) on the few occasions that the formatter definition is actually changed, without any auto-commit business?

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/formatting-team-meeting-2024-12-10-and-2025-01-07/58466/1

[infinisil]

This is only relevant for pushes regarding a change in the definition of the formatter used for CI, right?

No it's for any random PR that introduces a new Nix file. The problem is that nothing forces CI to rerun before the PR can be merged, even if the formatter version has changed on master. So to the merger it looks like CI is green, but if you ran it again it would fail. This would be fixed by merge queues, but that needs some work. This is only a problem with PRs introducing new files, because all existing files will have to be reformatted to pass CI already in the PR that updates the formatter version.

[MattSturgeon]

The problem is that nothing forces CI to rerun before the PR can be merged, even if the formatter version has changed on master.

This could also be solved by a merge-queue, that runs CI on the merge-commit before actually merging. E.g. GitHub's or Mergify's queues.

But that's its own can of worms. And as we already saw in NixOS/org#39, it would need to be approached with caution.

EDIT: I just realised you'd already said this yourself in the message I was replying to...