Add `nix formatter build` and `nix formatter run` commands #13063

jfly · 2025-04-23T00:21:23Z

nix formatter run is an alias for nix fmt. Nothing new there.

nix formatter build is sort of like nix build: it builds, links, and
prints a path to the formatter program:

$ nix formatter build
/nix/store/cb9w44vkhk2x4adfxwgdkkf5gjmm856j-treefmt/bin/treefmt

Note that unlike nix build, this prints the full path to the program,
not just the store path (in the example above that would be
/nix/store/cb9w44vkhk2x4adfxwgdkkf5gjmm856j-treefmt).

Motivation

I maintain a vim plugin that automatically runs nix fmt on files on
save. Since nix fmt can be quite slow due to nix evaluation, I choose
to cache the nix fmt entrypoint. This was very awkward to do, see the
implementation for details:
https://github.com/nvimtools/none-ls.nvim/blob/786460723170bda9e9f95c55a382d21436575297/lua/null-ls/builtins/formatting/nix_flake_fmt.lua#L83-L110.

I recently discovered that my implementation was buggy (it didn't handle
flakes that expose a formatter package, such as nixpkgs), so I had to
rework the implementation:
nvimtools/none-ls.nvim#272.

With the new nix formatter build command, I can delete all this akward
code, and it will be easier for other folks to build performant editor
integrations for nix fmt.

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

jfly · 2025-04-23T00:27:04Z

One problem with this PR is that it doesn't provide any mechanism for users to protect against garbage collection between running nix fmt --print-command and then running the command. I think fix would be to implement similar behavior to nix build, namely:

By default, create a result symlink.
Opt out with --no-link, or configure the location with --out-link/-o.

I'm happy to implement something like this, but would appreciate some direction from a maintainer before I go to the effort!

Mic92 · 2025-04-23T03:44:37Z

Are you not missing program args here for a correct implementation? I think this might make quoting a bit awkward. So I am wondering if we should have a JSON output option instead --json or --print-json (the latter one could indicates that we are not running any command maybe better, unsure)

roberth · 2025-04-23T11:07:03Z

nix run should support the same functionality. Some code could be factored into a superclass/mix-in I think.

For the out link flags, look at src/nix/build.cc.
Perhaps a superclass could be factored out of that too, for the flags, but I don't know if a nice helper method can be factored out to consume those flags. If not, that's also fine.

or --print-json

The behavior and the format should be separate.

--print-only --json?
Printing a bash statement with shellEscape by default seems sensible.

roberth · 2025-04-23T11:17:20Z

shellEscape

Rename shellEscape -> escapeShellArgAlways #13065

edolstra · 2025-04-23T15:18:48Z

Since nix fmt can be quite slow due to nix evaluation

It would probably be better to fix the root issue here. Maybe evaluation caching is broken for nix fmt?

Mic92 · 2025-04-23T16:01:44Z

Since nix fmt can be quite slow due to nix evaluation

It would probably be better to fix the root issue here. Maybe evaluation caching is broken for nix fmt?

I don't think the evaluation cache can help here, because it would get invalidated on every file change. For context nix fmt is called here from an editor, so we expect file changes to happen basically on every save of the editor and also whenever nix fmt actually reformatted a file.

jfly · 2025-04-23T16:14:28Z

@Mic92 already made the point more succinctly, but I went to the effort to capture some numbers, so here they are if it's helpful:

Some concrete timing

First time: cache miss (that's OK, nothing we can do about this):

$ time nix fmt flake.nix
2025/04/23 08:58:46 INFO using config file: /nix/store/hhzsjb1ydnrrkc178naf5nqgdw06n05f-treefmt.toml
traversed 1 files
emitted 1 files for processing
formatted 0 files (0 changed) in 24ms

________________________________________________________
Executed in   12.43 secs    fish           external
   usr time   10.28 secs    0.00 millis   10.28 secs
   sys time    0.98 secs    1.71 millis    0.97 secs

Second time: cache hit. Much better, but still causes one core CPU to spike for
~700ms. I wouldn't want to run this every time I save a file:

$ time nix fmt flake.nix
2025/04/23 08:58:57 INFO using config file: /nix/store/hhzsjb1ydnrrkc178naf5nqgdw06n05f-treefmt.toml
traversed 1 files
emitted 1 files for processing
formatted 0 files (0 changed) in 6ms

________________________________________________________
Executed in  705.00 millis    fish           external
   usr time  348.87 millis    0.32 millis  348.54 millis
   sys time  286.94 millis    1.14 millis  285.81 millis

Now, if I make a change to flake.nix, I'm paying a pretty
price for what looks like some sort of a cache hit:

$ echo >> flake.nix
$ time nix fmt flake.nix
warning: Git tree '/home/jeremy/src/github.com/NixOS/nixpkgs' is dirty
2025/04/23 09:04:07 INFO using config file: /nix/store/hhzsjb1ydnrrkc178naf5nqgdw06n05f-treefmt.toml
INFO formatter | nixfmt: 1 file(s) processed in 12.494733ms
traversed 1 files
emitted 1 files for processing
formatted 1 files (1 changed) in 29ms

________________________________________________________
Executed in    3.78 secs    fish           external
   usr time    1.84 secs  615.00 micros    1.84 secs
   sys time    0.79 secs  896.00 micros    0.78 secs

This results in a pretty terrible experience when editing your flake.nix if
you do this every time you change it.

I personally like @roberth's suggestion to add 2 flags to both nix run and nix fmt:

--print-only
--json

I'll give this a shot. I don't know if my C++ is up to the task of trying to share this code between the implementations, though.

Mic92 · 2025-04-23T16:16:03Z

You don't necessarily need to add the --json flag in the first iteration. You could also move this into a second pull request. It was just an idea how to export a shell command safely.

edolstra · 2025-04-23T19:40:35Z

Notes from team meeting:

Turn nix fmt into nix formatter run (with nix fmt as a convenience alias).
--print-command would become a new command nix formatter build (or something like that). It should support --out-link to avoid GC issues.

jfly · 2025-04-23T23:41:35Z

I've updated this PR in light of today's discussion at the nix team meeting. Thanks for the help, everyone!

jfly

Urg, this diff is impossible to see read with the file renames. I'll split the renames into a separate commit, standby...

This refactor shouldn't change much except add a new `nix formatter run` command. This creates space for the new `nix formatter build` command, which I'll be introducing in the next commit.

jfly · 2025-04-24T00:25:16Z

Ok, this should be ready to review now.

jfly · 2025-04-24T00:29:13Z

src/nix/formatter.cc

+        Installables installableContext;
+        for (auto & ctxElt : unresolvedApp.unresolved.context)
+            installableContext.push_back(make_ref<InstallableDerivedPath>(store, DerivedPath{ctxElt}));
+        auto buildables = Installable::build(evalStore, store, Realise::Outputs, installableContext);


I'm not sure if there's a less awkward way of doing this. I basically copied this logic from here: https://github.com/jfly/nix/blob/7706cff51871449d27b96e528b784a4eb630031d/src/nix/app.cc#L137-L143

`nix formatter build` is sort of like `nix build`: it builds, links, and prints a path to the formatter program: $ nix formatter build /nix/store/cb9w44vkhk2x4adfxwgdkkf5gjmm856j-treefmt/bin/treefmt Note that unlike `nix build`, this prints the full path to the program, not just the store path (in the example above that would be `/nix/store/cb9w44vkhk2x4adfxwgdkkf5gjmm856j-treefmt`). Motivation ---------- I maintain a vim plugin that automatically runs `nix fmt` on files on save. Since `nix fmt` can be quite slow due to nix evaluation, I choose to cache the `nix fmt `entrypoint. This was very awkward to do, see the implementation for details: https://github.com/nvimtools/none-ls.nvim/blob/786460723170bda9e9f95c55a382d21436575297/lua/null-ls/builtins/formatting/nix_flake_fmt.lua#L83-L110. I recently discovered that my implementation was buggy (it didn't handle flakes that expose a `formatter` package, such as nixpkgs), so I had to rework the implementation: nvimtools/none-ls.nvim#272. With the new `nix formatter build` command, I can delete all this akward code, and it will be easier for other folks to build performant editor integrations for `nix fmt`.

jfly requested a review from edolstra as a code owner April 23, 2025 00:21

github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Apr 23, 2025

jfly mentioned this pull request Apr 23, 2025

fix(nix_flake_fmt): handle flakes with a formatter package nvimtools/none-ls.nvim#272

Open

jfly requested a review from Mic92 April 23, 2025 00:22

roberth mentioned this pull request Apr 23, 2025

Rename shellEscape -> escapeShellArgAlways #13065

Merged

edolstra added this to Nix team Apr 23, 2025

github-project-automation bot moved this to To triage in Nix team Apr 23, 2025

edolstra closed this Apr 23, 2025

edolstra reopened this Apr 23, 2025

edolstra added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Apr 23, 2025

jfly force-pushed the add-nix-fmt-print-command-option branch from 22c2578 to 79390c9 Compare April 23, 2025 23:41

github-actions bot added the documentation label Apr 23, 2025

jfly changed the title ~~Add a --print-command option to nix fmt~~ Add nix formatter build and nix formatter run commands Apr 23, 2025

jfly commented Apr 23, 2025

View reviewed changes

jfly force-pushed the add-nix-fmt-print-command-option branch 2 times, most recently from fcd24f7 to 3978cd2 Compare April 23, 2025 23:57

refactor: create a new nix formatter run command alias for nix fmt

7706cff

This refactor shouldn't change much except add a new `nix formatter run` command. This creates space for the new `nix formatter build` command, which I'll be introducing in the next commit.

jfly force-pushed the add-nix-fmt-print-command-option branch 2 times, most recently from 0847316 to 67d22c7 Compare April 24, 2025 00:24

jfly force-pushed the add-nix-fmt-print-command-option branch from 67d22c7 to c292904 Compare April 24, 2025 00:28

jfly commented Apr 24, 2025

View reviewed changes

jfly force-pushed the add-nix-fmt-print-command-option branch from c292904 to e3ed704 Compare April 24, 2025 01:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `nix formatter build` and `nix formatter run` commands #13063

Add `nix formatter build` and `nix formatter run` commands #13063

jfly commented Apr 23, 2025 •

edited

Loading

jfly commented Apr 23, 2025

Mic92 commented Apr 23, 2025 •

edited

Loading

roberth commented Apr 23, 2025

roberth commented Apr 23, 2025

edolstra commented Apr 23, 2025

Mic92 commented Apr 23, 2025 •

edited

Loading

jfly commented Apr 23, 2025 •

edited

Loading

Mic92 commented Apr 23, 2025 •

edited

Loading

edolstra commented Apr 23, 2025 •

edited

Loading

jfly commented Apr 23, 2025

jfly left a comment

jfly commented Apr 24, 2025

jfly Apr 24, 2025

Add nix formatter build and nix formatter run commands #13063

Are you sure you want to change the base?

Add nix formatter build and nix formatter run commands #13063

Conversation

jfly commented Apr 23, 2025 • edited Loading

Motivation

jfly commented Apr 23, 2025

Mic92 commented Apr 23, 2025 • edited Loading

roberth commented Apr 23, 2025

roberth commented Apr 23, 2025

edolstra commented Apr 23, 2025

Mic92 commented Apr 23, 2025 • edited Loading

jfly commented Apr 23, 2025 • edited Loading

Mic92 commented Apr 23, 2025 • edited Loading

edolstra commented Apr 23, 2025 • edited Loading

jfly commented Apr 23, 2025

jfly left a comment

Choose a reason for hiding this comment

jfly commented Apr 24, 2025

jfly Apr 24, 2025

Choose a reason for hiding this comment

Add `nix formatter build` and `nix formatter run` commands #13063

Add `nix formatter build` and `nix formatter run` commands #13063

jfly commented Apr 23, 2025 •

edited

Loading

Mic92 commented Apr 23, 2025 •

edited

Loading

Mic92 commented Apr 23, 2025 •

edited

Loading

jfly commented Apr 23, 2025 •

edited

Loading

Mic92 commented Apr 23, 2025 •

edited

Loading

edolstra commented Apr 23, 2025 •

edited

Loading