Enforce nixfmt on new files and changed files that were already formatted

[infinisil]

Description of changes

This is a big step towards full implementation of NixOS/rfcs#166, see also NixOS/nixfmt#153. This is split off from #322537 (the first big treewide reformatting), because there's no need to have it be in the same PR. This PR should be reviewed/merged by the @NixOS/nix-formatting team.

This PR makes the Nix format workflow check all new files and changed files (that were already formatted) to be formatted.

Things done

• Check that it works: Formatting CI test tweag/nixpkgs#95
• Optionally have a pre-commit/push hook, see Git pre-commit/push hook nixfmt#221
• Make a Discourse post announcing this: https://discourse.nixos.org/t/enforcing-nix-formatting-in-nixpkgs/49506

---

This work is sponsored by Antithesis ✨

Add a 👍 reaction to pull requests you find important.

[Pandapip1]

This PR should only be merged after the other one is merged.

Shouldn't this be the other way round? First, nixfmt is enforced for new/changed files, then the treewide PR is merged?

[infinisil]

@Pandapip1 If we first require all new/changed files to be formatted without doing the treewide format, anybody branching off master would have to format all of the files they're touching to make their PR valid.

The other way around is also not entirely great, because we shouldn't merge PRs that mess up the newly-formatted files either.

Really, these two PRs should be merged around the same time so that both of these problems can be avoided. Splitting it into two PRs is mainly done to make reviewing easier, because the other one is so big (and entirely automated), whereas this one is more tricky.

[infinisil]

Oh, just noticing a problem: If we start doing this, we need to have all-packages.nix (and similarly often updated files) formatted, because otherwise somebody else will have to format it and cause merge conflicts for everybody else. #322537 is "smart" by avoiding any conflicts, but for the case of often updated files this backfires.

My proposed solution to this is to change this PR to enforce formatting of changed files only if they were previously formatted already, so that e.g. it wouldn't be enforced for all files that aren't part of the treewide reformat, including all-packages.nix. We can deal with these special files in the future at some point instead (e.g. see here)

Edit: Now implemented, this makes this PR way more independent of the original one, there's no more need to merge them together :)

[Pandapip1]

@Pandapip1 If we first require all new/changed files to be formatted without doing the treewide format, anybody branching off master would have to format all of the files they're touching to make their PR valid.

How about this: CI checks if files that are already nixfmt-rfc-style-compliant have become non-compliant, or if a new non-compliant file has been added?

EDIT:

My proposed solution to this is to change this PR to enforce formatting of changed files only if they were previously formatted already, so that e.g. it wouldn't be enforced for all files that aren't part of the treewide reformat, including all-packages.nix. We can deal with these special files in the future at some point instead (e.g. see NixOS/nixfmt#223)

Great minds think alike, lol. +1 to this solution.

[MattSturgeon]

I think NixOS/nixfmt#179 may block this and #322537, because the /* lang */ tagging pattern is apparently used a lot in the test suite.

e.g:

nixpkgs/nixos/tests/switch-test.nix

Lines 8 to 10 in 0bf815b

	socketTest = pkgs.writeScript "socket-test.py" /* python */ ''
	#!${pkgs.python3}/bin/python3

[infinisil]

@MattSturgeon This isn't relevant for this PR, since it wouldn't enforce formatting on files that have /* foo */ comments in them. Furthermore, I don't think it's a blocker for the treewide format PR either, because treesitter also supports # foo-style comments for syntax hints.

[nixos-discourse]

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

https://discourse.nixos.org/t/formatting-team-meeting-2024-07-23/49510/1

[MattSturgeon]

Running the suggested command (see below) resolves in no changes

Are you doing this in the nix shell introduced in #322512? That should provide a nixpkgs on your PATH locked to the same version used by the CI.

Also, just to eliminate the obvious, are you running the command from the right location for those relative paths?

[felbinger]

Running the suggested command (see below) resolves in no changes

Are you doing this in the nix shell introduced in #322512? That should provide a nixpkgs on your PATH locked to the same version used by the CI.

Also, just to eliminate the obvious, are you running the command from the right location for those relative paths?

I actually didn't use the shell.nix, because I wasn't aware of it. Thanks for pointing me in the right direction
Even though i don't really understand whats the difference between it and nix-shell -p nixfmt-rfc-style - Oh: I think it might be using an older version of nixfmt 🤔

[MattSturgeon]

Even though i don't really understand whats the difference between it and nix-shell -p nixfmt-rfc-style

In most cases there should be no tangible difference, afaik. However in some cases the version used by the shell (and CI) may be different to the version in nixos-unstable.

[nixos-discourse]

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

https://discourse.nixos.org/t/how-to-use-nixfmt-for-nixpkgs/56590/2