Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Full treewide Nix format and enforcement [skip treewide] #380990

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Full treewide Nix format and enforcement [skip treewide] #380990

wants to merge 6 commits into from
+1,010,246 −531,017

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Feb 10, 2025 •
edited
Loading

This is a huge step towards the full implementation of NixOS/rfcs#166 (ref NixOS/nixfmt#153)! Highlights:

  • You can now regularly run treefmt inside nix-shell/nix develop or nix fmt to get the right formatting on all files, no more "these particular files need to be formatted"
  • You can even hook up your editor's to run nixfmt on auto-save or set up a pre commit hook to run treefmt
  • CI will enforce this on all PRs going forward, ensuring that from now on, all Nix files consistently use the same formatting

This 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. Once ready, I'd like to schedule a merge party for this PR for the final review and merge. 🚀 🎉

Things done

  • Tested treefmt in nix-shell/nix develop
  • Tested nix fmt
  • Update/add docs
  • Do the treewide reformat (but needs constant updating as master gets updated)
  • Added the reformat commit to .git-blame-ignore-revs (also needs constant updating)
    • Check that the auto-rebase script works
    • Check that GitHub ignores the commit (it's highlighted in bold red, a bad sign..)
  • Tested the GHA workflow to check Nix formatting
  • Investigated the causes for the changed paths
  • Create an issue to describe how to resolve merge conflicts, pin it
  • Announce it on Discourse (can reuse https://discourse.nixos.org/t/nix-formatting-team-treewide-nixpkgs-formatting/56666)
  • Prepare for 24.11 backport
  • Make sure staging isn't a problem

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.

@github-actions github-actions bot added 6.topic: flakes The experimental Nix feature 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: policy discussion backport release-24.11 Backport PR automatically labels Feb 10, 2025
@infinisil infinisil mentioned this pull request Feb 10, 2025
Open
7 tasks
@infinisil infinisil changed the title treefmt and nix fmt Fully enforce nixfmt treewide Feb 10, 2025
@infinisil infinisil added the significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. label Feb 10, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Feb 10, 2025
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 12, 2025
@infinisil infinisil changed the title Fully enforce nixfmt treewide Fully enforce nixfmt treewide [skip treewide] Feb 12, 2025
@github-actions github-actions bot added 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell 6.topic: qt/kde 6.topic: kernel The Linux kernel 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: emacs Text editor 6.topic: printing 6.topic: rust 6.topic: golang 6.topic: ruby 6.topic: vim 6.topic: erlang 6.topic: ocaml 6.topic: xfce The Xfce Desktop Environment 6.topic: fetch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Feb 12, 2025
@github-actions github-actions bot added 6.topic: module system About "NixOS" module system internals 6.topic: docker tools 6.topic: crystal Programming language - https://crystal-lang.org/ 6.topic: systemd 6.topic: java Including JDK, tooling, other languages, other VMs 6.topic: R 6.topic: LXQt The Lightweight Qt Desktop Environment 6.topic: lib The Nixpkgs function library 6.topic: games 6.topic: rocm 6.topic: julia 6.topic: php 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 6.topic: dotnet Language: .NET 6.topic: nvidia 6.topic: xen-project The Xen Project hypervisor 6.topic: tcl labels Feb 12, 2025
@infinisil
shell: Introduce treefmt
8791351 
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.
@infinisil
flake.nix: Set formatter
41003a8 
This enables `nix fmt`, though it won't be practically usable without
also reformatting all files, which is done in a following commit.
@infinisil
workflows/check-nix-format: Enforce formatting on all files
2aee856 
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.
@infinisil
CONTRIBUTING: Improve and update formatting docs
9461679
@infinisil
treewide: Format all Nix files
0136356 
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.
@infinisil
git-blame-ignore-revs: Add previous commit
240326d 
And enable use of the auto-rebase script: https://github.com/NixOS/nixpkgs/tree/master/maintainers/scripts/auto-rebase
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Feb 12, 2025
@infinisil infinisil changed the title Fully enforce nixfmt treewide [skip treewide] Full treewide Nix format and enforcement [skip treewide] Feb 12, 2025
@infinisil infinisil mentioned this pull request Feb 12, 2025
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: cinnamon Desktop environment 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: coq "A formal proof management system" 6.topic: crystal Programming language - https://crystal-lang.org/ 6.topic: docker tools 6.topic: dotnet Language: .NET 6.topic: emacs Text editor 6.topic: erlang 6.topic: fetch 6.topic: flakes The experimental Nix feature 6.topic: games 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: golang 6.topic: hardware 6.topic: haskell 6.topic: java Including JDK, tooling, other languages, other VMs 6.topic: julia 6.topic: kernel The Linux kernel 6.topic: lib The Nixpkgs function library 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 6.topic: lua 6.topic: LXQt The Lightweight Qt Desktop Environment 6.topic: module system About "NixOS" module system internals 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: nixos-container Imperative and declarative systemd-nspawn containers 6.topic: nodejs 6.topic: nvidia 6.topic: ocaml 6.topic: pantheon The Pantheon desktop environment 6.topic: php 6.topic: policy discussion 6.topic: printing 6.topic: python 6.topic: qt/kde 6.topic: R 6.topic: rocm 6.topic: ruby 6.topic: rust 6.topic: stdenv Standard environment 6.topic: systemd 6.topic: tcl 6.topic: testing Tooling for automated testing of packages and modules 6.topic: TeX Issues regarding texlive and TeX in general 6.topic: vim 6.topic: xen-project The Xen Project hypervisor 6.topic: xfce The Xfce Desktop Environment 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 backport release-24.11 Backport PR automatically significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc.
Projects
Nix formatting
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@infinisil