Skip to content

treewide: introduce formatter#314

Merged
tpwrules merged 3 commits into
mainfrom
unknown repository
Aug 2, 2025
Merged

treewide: introduce formatter#314
tpwrules merged 3 commits into
mainfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jun 29, 2025

Since this is under the nix-community banner, this project should decide on a formatter. This PR currently chooses nixfmt-rfc-style via github:numtide/treefmt-nix flake, formats all files, and adds a .git-blame-ignore-revs file to ignore this mass reformat when blaming.

This would also allow for pre-commit hooks for formatting all files as well as automatic merge checks when opening additional PRs which reduces the chance of merge conflicts occurring.

@ghost
Copy link
Copy Markdown
Author

ghost commented Jul 4, 2025

Resolved merge conflicts and rebased on main.

@ghost
Copy link
Copy Markdown
Author

ghost commented Jul 4, 2025

This is what the deadnix formatter also includes when it's run.

diff --git a/apple-silicon-support/packages/alsa-ucm-conf-asahi/default.nix b/apple-silicon-support/packages/alsa-ucm-conf-asahi/default.nix
index 6e1f324..413fffd 100644
--- a/apple-silicon-support/packages/alsa-ucm-conf-asahi/default.nix
+++ b/apple-silicon-support/packages/alsa-ucm-conf-asahi/default.nix
@@ -1,5 +1,4 @@
 {
-  lib,
   fetchFromGitHub,
   alsa-ucm-conf,
 }:
diff --git a/apple-silicon-support/packages/asahi-audio/default.nix b/apple-silicon-support/packages/asahi-audio/default.nix
index 7c0582c..553c739 100644
--- a/apple-silicon-support/packages/asahi-audio/default.nix
+++ b/apple-silicon-support/packages/asahi-audio/default.nix
@@ -1,6 +1,5 @@
 {
   stdenv,
-  lib,
   fetchFromGitHub,
   lsp-plugins,
   bankstown-lv2,
diff --git a/apple-silicon-support/packages/linux-asahi/default.nix b/apple-silicon-support/packages/linux-asahi/default.nix
index 19edef0..cd4b868 100644
--- a/apple-silicon-support/packages/linux-asahi/default.nix
+++ b/apple-silicon-support/packages/linux-asahi/default.nix
@@ -75,12 +75,11 @@ let
       stdenv,
       lib,
       fetchFromGitHub,
-      fetchpatch,
       linuxKernel,
       rustc,
       rust-bindgen,
       ...
-    }@args:
+    }:
     let
       origConfigText = builtins.readFile origConfigfile;
 
@@ -118,8 +117,6 @@ let
         builtins.listToAttrs (map makePair (lib.lists.reverseList configList));
 
       # used to fix issues when nixpkgs gets ahead of the kernel
-      rustAtLeast = version: withRust && (lib.versionAtLeast rustc.version version);
-      bindgenAtLeast = version: withRust && (lib.versionAtLeast rust-bindgen.unwrapped.version version);
     in
     linuxKernel.manualConfig rec {
       inherit stdenv lib;
diff --git a/apple-silicon-support/packages/m1n1/default.nix b/apple-silicon-support/packages/m1n1/default.nix
index 7628ed5..d117d5a 100644
--- a/apple-silicon-support/packages/m1n1/default.nix
+++ b/apple-silicon-support/packages/m1n1/default.nix
@@ -29,7 +29,7 @@ let
     stdenv = lib.recursiveUpdate buildPackages.stdenv stdenvOpts;
   };
   rustPackages = rust.packages.stable.overrideScope (
-    f: p: {
+    _f: p: {
       rustc-unwrapped = p.rustc-unwrapped.override {
         stdenv = lib.recursiveUpdate p.rustc-unwrapped.stdenv stdenvOpts;
       };
diff --git a/iso-configuration/default.nix b/iso-configuration/default.nix
index 1ae8e42..9397e01 100644
--- a/iso-configuration/default.nix
+++ b/iso-configuration/default.nix
@@ -1,7 +1,6 @@
 # configuration that is specific to the ISO
 {
   config,
-  pkgs,
   lib,
   ...
 }:
diff --git a/iso-configuration/installer-configuration.nix b/iso-configuration/installer-configuration.nix
index 4970784..129de68 100644
--- a/iso-configuration/installer-configuration.nix
+++ b/iso-configuration/installer-configuration.nix
@@ -98,13 +98,13 @@
   };
 
   nixpkgs.overlays = [
-    (final: prev: {
+    (_final: prev: {
       # disabling pcsclite avoids the need to cross-compile gobject
       # introspection stuff which works now but is slow and unnecessary
       libfido2 = prev.libfido2.override {
         withPcsclite = false;
       };
-      openssh = prev.openssh.overrideAttrs (old: {
+      openssh = prev.openssh.overrideAttrs (_old: {
         # we have to cross compile openssh ourselves for whatever reason
         # but the tests take quite a long time to run
         doCheck = false;

@ghost
Copy link
Copy Markdown
Author

ghost commented Jul 4, 2025

I can optionally add deadnix as a formatter.

@ghost ghost mentioned this pull request Jul 4, 2025
@ghost
Copy link
Copy Markdown
Author

ghost commented Jul 15, 2025

I've added deadnix as a formatter, this has the effect of introducing a '_' character to functions whose parameters are unused as well as cleaning up any unused code.

I've also reworded some commits and updated the ignore revision.

@ghost ghost mentioned this pull request Jul 18, 2025
13 tasks
@tpwrules
Copy link
Copy Markdown
Collaborator

tpwrules commented Jul 20, 2025

My personal thoughts:

  • It seems reasonable to format the code and I'd be okay with keeping it consistent with NixOS organization decisions. I will withhold complaints about their style choices :)
  • I would even be okay with running it automatically in CI.
  • Can we do this without an extra flake input? Is the formatter not already in nixpkgs? Is the code to hook it up worth the extra input?
  • The extra flake brings in an extra nixpkgs, would much prefer to override to use the same one as the main flake.
  • I don't like the dead code removal. It's not smart enough to remove the associated comments and it's just going to introduce churn on things that are used or not depending on exact project state.

@ghost
Copy link
Copy Markdown
Author

ghost commented Jul 20, 2025

Responding to @tpwrules

  • Any formatter can be chosen, since the issue at hand is that without a formatter, formatting related merge conflicts become an issue. Since nixfmt-rfc-style is now being standardized into nixfmt[1] and we are under nix-community this seems like the best choice.
  • A simple lint workflow in PRs would be fine, nothing too fancy.
  • If we do this without the additional flake input, then we'd also have to track a treefmt.toml and do the glue code ourselves. I just set it so that treefmt-nix follows this flake's nixpkgs. If having the treefmt-nix input removed is more desirable, I can rework the code to do so.
  • Removed deadnix as a default formatter.

1: https://github.com/NixOS/nixfmt/releases/tag/v1.0.0

@ghost
Copy link
Copy Markdown
Author

ghost commented Jul 20, 2025

Resolved merge conflicts and rebased on current main.

Jasi added 3 commits July 26, 2025 15:16
Enables formatting support via `nix fmt` and `nix flake check` for Nix
code.
@ghost
Copy link
Copy Markdown
Author

ghost commented Jul 26, 2025

Resolved merge conflicts and rebased on main.

@flokli
Copy link
Copy Markdown
Member

flokli commented Jul 30, 2025

@normalcea I'm ok with nixfmt-rfc-style, it seems everyone converges to it.

Let's add CI soon, to prevent something accidentially being merged unformatted, but that can be a followup. Thanks!

Copy link
Copy Markdown
Collaborator

@tpwrules tpwrules left a comment

Choose a reason for hiding this comment

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

Tested that nix fmt works and makes the same set of changes. Also checked that it doesn't change the system derivation.

@tpwrules tpwrules merged commit 9c936c2 into nix-community:main Aug 2, 2025
@ghost ghost deleted the formatting branch August 2, 2025 17:02
@tpwrules
Copy link
Copy Markdown
Collaborator

tpwrules commented Aug 3, 2025

@normalcea

I upgraded nixpkgs to a two month newer revision and the new nix fmt makes a 180 line diff. Upstream nixfmt just did a v1.0.0 release and they say "just think of it as an entirely new formatting". This is a disappointing outcome especially so soon.

Can you help me understand why the difference is so big? I would have expected the nixfmt-rfc-style you say you used would be approximately the same format. Surely nixpkgs is not about to redo all of its formatting effort?

Maybe the wrong version got picked when I requested that we follow this flake's nixpkgs?

@tpwrules
Copy link
Copy Markdown
Collaborator

tpwrules commented Aug 3, 2025

It looks like there was actually that much difference between nixfmt-rfc-style unstable-2025-04-04 and nixfmt v1.0.0. Nixpkgs just did another massive reformatting commit to adapt as part of NixOS/nixpkgs#427437 .

Are other users just doing the same?

@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 3, 2025

Yes there should be another treewide re-format similar to this PR when the flake inputs are upgraded.

@tpwrules
Copy link
Copy Markdown
Collaborator

tpwrules commented Aug 3, 2025

Okay, I didn't realize that was a necessary step. We'll see how that goes. I guess it's good that we can control exactly when the formatter changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants