Full treewide Nix format and enforcement [skip treewide] #380990
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
6.topic: flakes
infinisil
added
the
significant
github-actions
bot
added
10.rebuild-darwin: 0
infinisil
force-pushed
the
format-ikuzo
branch
from
f203d08 to
347838f
Compare
ofborg
bot
added
the
2.status: merge conflict
infinisil
changed the title
github-actions
bot
added
6.topic: python
6.topic: nixos
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
MattSturgeon
reviewed
Apr 1, 2025
Comment on lines
+27
to
+31
| treefmtNixSrc = fetchTarball { | ||
| # Master at 2025-02-12 | ||
| url = "https://github.com/numtide/treefmt-nix/archive/4f09b473c936d41582dd744e19f34ec27592c5fd.tar.gz"; | ||
| sha256 = "051vh6raskrxw5k6jncm8zbk9fhbzgm1gxpq9gm5xw1b6wgbgcna"; | ||
| }; |
There was a problem hiding this comment.
Now we have treefmt.withConfig and/or nixfmt-tree I don't think we need this dependency on treefmt-nix.
Or maybe config.build.devShell is doing something important, that adding treefmt.withConfig {} to a shell wouldn't do?
(This is minor enough to delay for another PR, especially given how late my comment is)
CONTRIBUTING.md
Outdated
| ``` | ||
|
|
||
| If you're starting your editor in `nix-shell` or `nix develop`, | ||
| you can also set it up to automatically format the file with `nixfmt` or `treefmt` on save. |
There was a problem hiding this comment.
Q: do we want to encourage running nixfmt manually or should we only document treefmt?
Introduces treefmt with a simple nixfmt-rfc-style configuration to format all Nix files. This is only practically usable with the following commit that formats all files accordingly.
This enables `nix fmt`, though it won't be practically usable without also reformatting all files, which is done in a following commit.
Changes the Nix format checking workflow to now strictly enforce formatting of all Nix files using the treefmt setup introduced in the pre-previous commit. This is in [accordance with the approved RFC 166](https://github.com/NixOS/rfcs/blob/master/rfcs/0166-nix-formatting.md#reformat-nixpkgs). Note that the "skip treewide" thing is no longer necessary, already before, because there's nothing that would fail for treewide changes. Previously the problem was that the GitHub API would be bombarded.
Format all Nix files using the officially approved formatter, making the CI check introduced in the previous commit succeed: nix-build ci -A fmt.check This is the next step of the of the [implementation](NixOS/nixfmt#153) of the accepted [RFC 166](NixOS/rfcs#166). This commit will lead to merge conflicts for a number of PRs, up to an estimated ~1100 (~33%) among the PRs with activity in the past 2 months, but that should be lower than what it would be without the previous [partial treewide format](NixOS#322537). Merge conflicts caused by this commit can now automatically be resolved while rebasing using the [auto-rebase script](https://github.com/NixOS/nixpkgs/tree/8616af08d915377bd930395f3b700a0e93d08728/maintainers/scripts/auto-rebase). If you run into any problems regarding any of this, please reach out to the [formatting team](https://nixos.org/community/teams/formatting/) by pinging @NixOS/nix-formatting.
And enable use of the auto-rebase script: https://github.com/NixOS/nixpkgs/tree/master/maintainers/scripts/auto-rebase
|
Reviewed together in a meeting with ~10 people, determined that there are no blockers, going to merge! |
MattSturgeon
approved these changes
Apr 1, 2025
dasJ
approved these changes
Apr 1, 2025
drupol
approved these changes
Apr 1, 2025
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-24.11
git worktree add -d .worktree/backport-380990-to-release-24.11 origin/release-24.11
cd .worktree/backport-380990-to-release-24.11
git switch --create backport-380990-to-release-24.11
git cherry-pick -x 398e74f70b6ef824c3891cb8889dddb851d8bab3 5a8296d74fd86de63fa441d238b10ccea0b72640 927521a6ace142da9f96ec7e87035a604fba818e 2140bf39e41767f25a395d20fb0d5698b8934b33 374e6bcc403e02a35e07b650463c01a52b13a7c8 49cf547427c17515d4b2c7a669ffdbe123d7eac8 |
|
🎈 |
13 tasks
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/formatting-team-meeting-2025-04-01/62524/1 |
|
Backport was done manually here: #395018 |
This was referenced Apr 2, 2025
13 tasks
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note
A merge party for final review and merge will be held on 2025-04-01
19:0020:00 CEST in this Jitsi, anybody is free to join! 🎉See https://discourse.nixos.org/t/nix-formatting-team-full-nixpkgs-reformat/61867 for more info
This is a huge step towards the full implementation of NixOS/rfcs#166 (ref NixOS/nixfmt#153)! Highlights:
treefmtinsidenix-shell/nix developornix fmtto get the right formatting on all files, no more "these particular files need to be formatted"nixfmton auto-save or set up a pre commit hook to runtreefmtThis PR will lead to merge conflicts for a number of PRs, up to an estimated ~1100 (~33%) among the PRs with activity in the past 2 months, but that should be lower than what it would be without the previous partial reformat (#322537).
Merge conflicts caused by this commit can now also automatically be resolved while rebasing using the auto-rebase script.
This PR should be reviewed and approved by the @NixOS/nix-formatting team.
Things done
treefmtinnix-shell/nix developnix fmt.git-blame-ignore-revs(also needs constant updating)scripts/nixfmt-mergetoolnixfmt#277 -> Implement --mergetool mode nixfmt#283pkgs.nixfmt-rfc-styleto include the above: nixfmt-rfc-style: 2024-12-04 -> 2025-03-03 #387047treefmtas an extra check.Potential follow-up: There are cases where new files could be committed with the wrong formatting. If this becomes a sufficiently annoying problem, we should have some automation to automatically fix this. But let's see about that when we get there.
This work is funded by Tweag and Antithesis ✨
Add a 👍 reaction to pull requests you find important.