Skip to content

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

Merged
infinisil merged 6 commits intoNixOS:masterfrom
tweag:format-ikuzo
Apr 1, 2025
Merged

Full treewide Nix format and enforcement [skip treewide]#380990
infinisil merged 6 commits intoNixOS:masterfrom
tweag:format-ikuzo

Conversation

@infinisil
Copy link
Member

@infinisil infinisil commented Feb 10, 2025 •
edited
Loading

Note

A merge party for final review and merge will be held on 2025-04-01 19:00 20: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:

  • 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.

Things done

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 Discuss policies to work in and around Nixpkgs backport release-24.11 labels Feb 10, 2025
@infinisil infinisil mentioned this pull request Feb 10, 2025
Closed
7 tasks
@infinisil infinisil changed the title treefmt and nix fmt Fully enforce nixfmt treewide Feb 10, 2025
@infinisil infinisil added the 1.severity: 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 Python is a high-level, general-purpose programming language. 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 General-purpose, statically typed, purely functional programming language 6.topic: qt/kde Object-oriented framework for GUI creation 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 Drivers, CUPS & Co. 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: ruby A dynamic, open source programming language with a focus on simplicity and productivity. 6.topic: vim Advanced text editor 6.topic: erlang General-purpose, concurrent, functional high-level programming language 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: xfce The Xfce Desktop Environment 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 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 the 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. label Mar 31, 2025
MattSturgeon
MattSturgeon reviewed Apr 1, 2025
View reviewed changes
ci/default.nix Outdated
Comment on lines +27 to +31
Copy link
Contributor

@MattSturgeon MattSturgeon Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

@MattSturgeon MattSturgeon Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: do we want to encourage running nixfmt manually or should we only document treefmt?

infinisil and others added 6 commits April 1, 2025 20:09
@infinisil
shell: Introduce treefmt
398e74f 
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
5a8296d 
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
927521a 
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
2140bf3
@infinisil
treewide: Format all Nix files
374e6bc 
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
49cf547 
And enable use of the auto-rebase script: https://github.com/NixOS/nixpkgs/tree/master/maintainers/scripts/auto-rebase
@infinisil infinisil mentioned this pull request Apr 1, 2025
Open
@infinisil infinisil marked this pull request as ready for review April 1, 2025 18:33
@infinisil
Copy link
Member Author

infinisil commented Apr 1, 2025

Reviewed together in a meeting with ~10 people, determined that there are no blockers, going to merge!

jfly
jfly approved these changes Apr 1, 2025
View reviewed changes
Copy link
Contributor

@jfly jfly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@infinisil infinisil merged commit 38ccab2 into NixOS:master Apr 1, 2025
45 of 47 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Nix formatting Apr 1, 2025
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Apr 1, 2025

Backport failed for release-24.11, because it was unable to cherry-pick the commit(s).

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

@mightyiam
Copy link

mightyiam commented Apr 1, 2025

🎈

@gepbird gepbird mentioned this pull request Apr 1, 2025
Closed
13 tasks
@nixos-discourse
Copy link

nixos-discourse commented Apr 1, 2025

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

@infinisil
Copy link
Member Author

infinisil commented Apr 1, 2025

Backport was done manually here: #395018

This was referenced Apr 2, 2025
Merged
Merged
@PerchunPak PerchunPak mentioned this pull request Apr 2, 2025
Open
13 tasks
@gepbird gepbird mentioned this pull request Apr 3, 2025
Merged
13 tasks
@wolfgangwalther wolfgangwalther mentioned this pull request Apr 3, 2025
Merged
@adamcstephens adamcstephens mentioned this pull request Apr 10, 2025
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@drupol drupol drupol approved these changes

@dasJ dasJ dasJ approved these changes

@MattSturgeon MattSturgeon MattSturgeon approved these changes

+1 more reviewer

@jfly jfly jfly approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

1.severity: significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. 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 Open-source software for deploying and running of containerized applications 6.topic: dotnet Language: .NET 6.topic: emacs Text editor 6.topic: erlang General-purpose, concurrent, functional high-level programming language 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 6.topic: flakes The experimental Nix feature 6.topic: games Gaming on NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: hardware Drivers, Firmware and Kernels 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: java Including JDK, tooling, other languages, other VMs 6.topic: julia Julia is a high-level, high-performance dynamic language for technical computing. 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 Lua is a powerful, efficient, lightweight, embeddable scripting language. 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 Node.js is a free, open-source, cross-platform JavaScript runtime environment 6.topic: nvidia Nvidia-specific issues and fixes 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: pantheon The Pantheon desktop environment 6.topic: php PHP is a general-purpose scripting language geared towards web development. 6.topic: policy discussion Discuss policies to work in and around Nixpkgs 6.topic: printing Drivers, CUPS & Co. 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: R R is a programming language for statistical computing and data visualization. 6.topic: rocm ROCm is an Advanced Micro Devices software stack for graphics processing unit programming. 6.topic: ruby A dynamic, open source programming language with a focus on simplicity and productivity. 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: stdenv Standard environment 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 6.topic: tcl Dynamic, multi-paradigm programming language 6.topic: testing Tooling for automated testing of packages and modules 6.topic: TeX Issues regarding texlive and TeX in general 6.topic: vim Advanced text editor 6.topic: xen-project Issues and PRs related to 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 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

Nix formatting
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@infinisil @SuperSandro2000 @nixos-discourse @mightyiam @drupol @jfly @dasJ @MattSturgeon