From 5ea7b97147744580bc5d5f7d07f764dfc2cc175b Mon Sep 17 00:00:00 2001 From: Jeremy Fleischman Date: Tue, 22 Apr 2025 17:07:08 -0700 Subject: [PATCH 1/5] refactor: create a new `nix formatter run` command alias for `nix fmt` 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. --- maintainers/flake-module.nix | 4 +- src/nix/fmt.cc | 55 ----------- src/nix/{fmt.md => formatter-run.md} | 2 +- src/nix/formatter.cc | 98 +++++++++++++++++++ src/nix/meson.build | 2 +- tests/functional/{fmt.sh => formatter.sh} | 0 .../{fmt.simple.sh => formatter.simple.sh} | 0 tests/functional/meson.build | 2 +- 8 files changed, 103 insertions(+), 60 deletions(-) delete mode 100644 src/nix/fmt.cc rename src/nix/{fmt.md => formatter-run.md} (90%) create mode 100644 src/nix/formatter.cc rename tests/functional/{fmt.sh => formatter.sh} (100%) rename tests/functional/{fmt.simple.sh => formatter.simple.sh} (100%) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index de71be0d7..2dbeaf889 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -605,8 +605,8 @@ ''^tests/functional/flakes/prefetch\.sh$'' ''^tests/functional/flakes/run\.sh$'' ''^tests/functional/flakes/show\.sh$'' - ''^tests/functional/fmt\.sh$'' - ''^tests/functional/fmt\.simple\.sh$'' + ''^tests/functional/formatter\.sh$'' + ''^tests/functional/formatter\.simple\.sh$'' ''^tests/functional/gc-auto\.sh$'' ''^tests/functional/gc-concurrent\.builder\.sh$'' ''^tests/functional/gc-concurrent\.sh$'' diff --git a/src/nix/fmt.cc b/src/nix/fmt.cc deleted file mode 100644 index dc270fb8c..000000000 --- a/src/nix/fmt.cc +++ /dev/null @@ -1,55 +0,0 @@ -#include "nix/cmd/command.hh" -#include "nix/cmd/installable-value.hh" -#include "nix/expr/eval.hh" -#include "run.hh" - -using namespace nix; - -struct CmdFmt : SourceExprCommand { - std::vector args; - - CmdFmt() { expectArgs({.label = "args", .handler = {&args}}); } - - std::string description() override { - return "reformat your code in the standard style"; - } - - std::string doc() override { - return - #include "fmt.md" - ; - } - - Category category() override { return catSecondary; } - - Strings getDefaultFlakeAttrPaths() override { - return Strings{"formatter." + settings.thisSystem.get()}; - } - - Strings getDefaultFlakeAttrPathPrefixes() override { return Strings{}; } - - void run(ref store) override - { - auto evalState = getEvalState(); - auto evalStore = getEvalStore(); - - auto installable_ = parseInstallable(store, "."); - auto & installable = InstallableValue::require(*installable_); - auto app = installable.toApp(*evalState).resolve(evalStore, store); - - Strings programArgs{app.program}; - - // Propagate arguments from the CLI - for (auto &i : args) { - programArgs.push_back(i); - } - - // Release our references to eval caches to ensure they are persisted to disk, because - // we are about to exec out of this process without running C++ destructors. - evalState->evalCaches.clear(); - - execProgramInStore(store, UseLookupPath::DontUse, app.program, programArgs); - }; -}; - -static auto r2 = registerCommand("fmt"); diff --git a/src/nix/fmt.md b/src/nix/formatter-run.md similarity index 90% rename from src/nix/fmt.md rename to src/nix/formatter-run.md index b4693eb65..a01b52a9d 100644 --- a/src/nix/fmt.md +++ b/src/nix/formatter-run.md @@ -2,7 +2,7 @@ R""( # Description -`nix fmt` calls the formatter specified in the flake. +`nix fmt` (an alias for `nix formatter run`) calls the formatter specified in the flake. Flags can be forwarded to the formatter by using `--` followed by the flags. diff --git a/src/nix/formatter.cc b/src/nix/formatter.cc new file mode 100644 index 000000000..80ce64d28 --- /dev/null +++ b/src/nix/formatter.cc @@ -0,0 +1,98 @@ +#include "nix/cmd/command.hh" +#include "nix/cmd/installable-value.hh" +#include "nix/expr/eval.hh" +#include "run.hh" + +using namespace nix; + +struct CmdFormatter : NixMultiCommand +{ + CmdFormatter() + : NixMultiCommand("formatter", RegisterCommand::getCommandsFor({"formatter"})) + { + } + + std::string description() override + { + return "build or run the formatter"; + } + + Category category() override + { + return catSecondary; + } +}; + +static auto rCmdFormatter = registerCommand("formatter"); + +struct CmdFormatterRun : SourceExprCommand +{ + std::vector args; + + CmdFormatterRun() + { + expectArgs({.label = "args", .handler = {&args}}); + } + + std::string description() override + { + return "reformat your code in the standard style"; + } + + std::string doc() override + { + return +#include "formatter-run.md" + ; + } + + Category category() override + { + return catSecondary; + } + + Strings getDefaultFlakeAttrPaths() override + { + return Strings{"formatter." + settings.thisSystem.get()}; + } + + Strings getDefaultFlakeAttrPathPrefixes() override + { + return Strings{}; + } + + void run(ref store) override + { + auto evalState = getEvalState(); + auto evalStore = getEvalStore(); + + auto installable_ = parseInstallable(store, "."); + auto & installable = InstallableValue::require(*installable_); + auto app = installable.toApp(*evalState).resolve(evalStore, store); + + Strings programArgs{app.program}; + + // Propagate arguments from the CLI + for (auto & i : args) { + programArgs.push_back(i); + } + + // Release our references to eval caches to ensure they are persisted to disk, because + // we are about to exec out of this process without running C++ destructors. + evalState->evalCaches.clear(); + + execProgramInStore(store, UseLookupPath::DontUse, app.program, programArgs); + }; +}; + +static auto rFormatterRun = registerCommand2({"formatter", "run"}); + +struct CmdFmt : CmdFormatterRun +{ + void run(ref store) override + { + CmdFormatterRun::run(store); + } +}; + +static auto rFmt = registerCommand("fmt"); diff --git a/src/nix/meson.build b/src/nix/meson.build index 901021330..11c30914b 100644 --- a/src/nix/meson.build +++ b/src/nix/meson.build @@ -78,7 +78,7 @@ nix_sources = [config_priv_h] + files( 'env.cc', 'eval.cc', 'flake.cc', - 'fmt.cc', + 'formatter.cc', 'hash.cc', 'log.cc', 'ls.cc', diff --git a/tests/functional/fmt.sh b/tests/functional/formatter.sh similarity index 100% rename from tests/functional/fmt.sh rename to tests/functional/formatter.sh diff --git a/tests/functional/fmt.simple.sh b/tests/functional/formatter.simple.sh similarity index 100% rename from tests/functional/fmt.simple.sh rename to tests/functional/formatter.simple.sh diff --git a/tests/functional/meson.build b/tests/functional/meson.build index f2d6a64ea..b2005d9d9 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -132,7 +132,7 @@ suites = [ 'nix-copy-ssh-ng.sh', 'post-hook.sh', 'function-trace.sh', - 'fmt.sh', + 'formatter.sh', 'eval-store.sh', 'why-depends.sh', 'derivation-json.sh', From ba6b617e755e229f44f8f1d37fe969de1de6e32c Mon Sep 17 00:00:00 2001 From: Jeremy Fleischman Date: Wed, 23 Apr 2025 16:54:01 -0700 Subject: [PATCH 2/5] Add `nix formatter build` command `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: https://github.com/nvimtools/none-ls.nvim/pull/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`. --- src/nix/formatter-build.md | 23 +++++++++++ src/nix/formatter.cc | 78 ++++++++++++++++++++++++++++++++++- tests/functional/formatter.sh | 30 ++++++++++++-- 3 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 src/nix/formatter-build.md diff --git a/src/nix/formatter-build.md b/src/nix/formatter-build.md new file mode 100644 index 000000000..fbb6adb1c --- /dev/null +++ b/src/nix/formatter-build.md @@ -0,0 +1,23 @@ +R""( + +# Description + +`nix formatter build` builds the formatter specified in the flake. + +Similar to [`nix build`](@docroot@/command-ref/new-cli/nix3-build.md), +unless `--no-link` is specified, after a successful +build, it creates a symlink to the store path of the formatter. This symlink is +named `./result` by default; this can be overridden using the +`--out-link` option. + +It always prints the command to standard output. + +# Examples + +* Build the formatter: + + ```console + # nix formatter build + /nix/store/cb9w44vkhk2x4adfxwgdkkf5gjmm856j-treefmt/bin/treefmt + ``` +)"" diff --git a/src/nix/formatter.cc b/src/nix/formatter.cc index 80ce64d28..0f915e7c9 100644 --- a/src/nix/formatter.cc +++ b/src/nix/formatter.cc @@ -1,6 +1,8 @@ #include "nix/cmd/command.hh" #include "nix/cmd/installable-value.hh" #include "nix/expr/eval.hh" +#include "nix/store/local-fs-store.hh" +#include "nix/cmd/installable-derived-path.hh" #include "run.hh" using namespace nix; @@ -25,7 +27,7 @@ struct CmdFormatter : NixMultiCommand static auto rCmdFormatter = registerCommand("formatter"); -struct CmdFormatterRun : SourceExprCommand +struct CmdFormatterRun : SourceExprCommand, MixJSON { std::vector args; @@ -87,6 +89,80 @@ struct CmdFormatterRun : SourceExprCommand static auto rFormatterRun = registerCommand2({"formatter", "run"}); +struct CmdFormatterBuild : SourceExprCommand +{ + Path outLink = "result"; + + CmdFormatterBuild() + { + addFlag({ + .longName = "out-link", + .shortName = 'o', + .description = "Use *path* as prefix for the symlink to the build result. It defaults to `result`.", + .labels = {"path"}, + .handler = {&outLink}, + .completer = completePath, + }); + + addFlag({ + .longName = "no-link", + .description = "Do not create symlink to the build results.", + .handler = {&outLink, Path("")}, + }); + } + + std::string description() override + { + return "build the current flake's formatter"; + } + + std::string doc() override + { + return +#include "formatter-build.md" + ; + } + + Category category() override + { + return catSecondary; + } + + Strings getDefaultFlakeAttrPaths() override + { + return Strings{"formatter." + settings.thisSystem.get()}; + } + + Strings getDefaultFlakeAttrPathPrefixes() override + { + return Strings{}; + } + + void run(ref store) override + { + auto evalState = getEvalState(); + auto evalStore = getEvalStore(); + + auto installable_ = parseInstallable(store, "."); + auto & installable = InstallableValue::require(*installable_); + auto unresolvedApp = installable.toApp(*evalState); + auto app = unresolvedApp.resolve(evalStore, store); + + Installables installableContext; + for (auto & ctxElt : unresolvedApp.unresolved.context) + installableContext.push_back(make_ref(store, DerivedPath{ctxElt})); + auto buildables = Installable::build(evalStore, store, Realise::Outputs, installableContext); + + if (outLink != "") + if (auto store2 = store.dynamic_pointer_cast()) + createOutLinks(outLink, toBuiltPaths(buildables), *store2); + + logger->cout("%s", app.program); + }; +}; + +static auto rFormatterBuild = registerCommand2({"formatter", "build"}); + struct CmdFmt : CmdFormatterRun { void run(ref store) override diff --git a/tests/functional/formatter.sh b/tests/functional/formatter.sh index e9bff50d5..ea6f9e1ce 100755 --- a/tests/functional/formatter.sh +++ b/tests/functional/formatter.sh @@ -7,11 +7,14 @@ TODO_NixOS # Provide a `shell` variable. Try not to `export` it, perhaps. clearStoreIfPossible rm -rf "$TEST_HOME"/.cache "$TEST_HOME"/.config "$TEST_HOME"/.local -cp ./simple.nix ./simple.builder.sh ./fmt.simple.sh "${config_nix}" "$TEST_HOME" +cp ./simple.nix ./simple.builder.sh ./formatter.simple.sh "${config_nix}" "$TEST_HOME" cd "$TEST_HOME" -nix fmt --help | grep "forward" +nix formatter --help | grep "build or run the formatter" +nix fmt --help | grep "reformat your code" +nix fmt run --help | grep "reformat your code" +nix fmt build --help | grep "build" cat << EOF > flake.nix { @@ -23,16 +26,37 @@ cat << EOF > flake.nix buildCommand = '' mkdir -p \$out/bin echo "#! ${shell}" > \$out/bin/formatter - cat \${./fmt.simple.sh} >> \$out/bin/formatter + cat \${./formatter.simple.sh} >> \$out/bin/formatter chmod +x \$out/bin/formatter ''; }; }; } EOF + # No arguments check [[ "$(nix fmt)" = "Formatting(0):" ]] +[[ "$(nix formatter run)" = "Formatting(0):" ]] + # Argument forwarding check nix fmt ./file ./folder | grep 'Formatting(2): ./file ./folder' +nix formatter run ./file ./folder | grep 'Formatting(2): ./file ./folder' + +# Build checks +## Defaults to a ./result. +nix formatter build | grep ".\+/bin/formatter" +[[ -L ./result ]] +rm result + +## Can prevent the symlink. +nix formatter build --no-link +[[ ! -e ./result ]] + +## Can change the symlink name. +nix formatter build --out-link my-result | grep ".\+/bin/formatter" +[[ -L ./my-result ]] +rm ./my-result + +# Flake outputs check. nix flake check nix flake show | grep -P "package 'formatter'" From e14346c7daad22782eb872ab5daf42e812090104 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 28 Apr 2025 15:08:15 +0200 Subject: [PATCH 3/5] Refactor, dedup nix formatter attribute methods --- src/nix/formatter.cc | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/src/nix/formatter.cc b/src/nix/formatter.cc index 0f915e7c9..31275eb35 100644 --- a/src/nix/formatter.cc +++ b/src/nix/formatter.cc @@ -27,7 +27,21 @@ struct CmdFormatter : NixMultiCommand static auto rCmdFormatter = registerCommand("formatter"); -struct CmdFormatterRun : SourceExprCommand, MixJSON +/** Common implementation bits for the `nix formatter` subcommands. */ +struct MixFormatter : SourceExprCommand +{ + Strings getDefaultFlakeAttrPaths() override + { + return Strings{"formatter." + settings.thisSystem.get()}; + } + + Strings getDefaultFlakeAttrPathPrefixes() override + { + return Strings{}; + } +}; + +struct CmdFormatterRun : MixFormatter, MixJSON { std::vector args; @@ -53,16 +67,6 @@ struct CmdFormatterRun : SourceExprCommand, MixJSON return catSecondary; } - Strings getDefaultFlakeAttrPaths() override - { - return Strings{"formatter." + settings.thisSystem.get()}; - } - - Strings getDefaultFlakeAttrPathPrefixes() override - { - return Strings{}; - } - void run(ref store) override { auto evalState = getEvalState(); @@ -89,7 +93,7 @@ struct CmdFormatterRun : SourceExprCommand, MixJSON static auto rFormatterRun = registerCommand2({"formatter", "run"}); -struct CmdFormatterBuild : SourceExprCommand +struct CmdFormatterBuild : MixFormatter { Path outLink = "result"; @@ -128,16 +132,6 @@ struct CmdFormatterBuild : SourceExprCommand return catSecondary; } - Strings getDefaultFlakeAttrPaths() override - { - return Strings{"formatter." + settings.thisSystem.get()}; - } - - Strings getDefaultFlakeAttrPathPrefixes() override - { - return Strings{}; - } - void run(ref store) override { auto evalState = getEvalState(); From 7df7bde3065e3ffbadf8ef6c9c3d83f7f868209f Mon Sep 17 00:00:00 2001 From: Jeremy Fleischman Date: Tue, 29 Apr 2025 18:36:56 -0700 Subject: [PATCH 4/5] Refactor, extract some shared code into `UnresolvedApp::build` --- src/libcmd/include/nix/cmd/installable-value.hh | 1 + src/nix/app.cc | 15 ++++++++++----- src/nix/formatter.cc | 6 +----- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/libcmd/include/nix/cmd/installable-value.hh b/src/libcmd/include/nix/cmd/installable-value.hh index 9c8f1a9fb..e65c199a5 100644 --- a/src/libcmd/include/nix/cmd/installable-value.hh +++ b/src/libcmd/include/nix/cmd/installable-value.hh @@ -21,6 +21,7 @@ struct App struct UnresolvedApp { App unresolved; + std::vector build(ref evalStore, ref store); App resolve(ref evalStore, ref store); }; diff --git a/src/nix/app.cc b/src/nix/app.cc index 75ef874ba..c9a9f9caf 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -129,18 +129,23 @@ UnresolvedApp InstallableValue::toApp(EvalState & state) throw Error("attribute '%s' has unsupported type '%s'", cursor->getAttrPathStr(), type); } -// FIXME: move to libcmd -App UnresolvedApp::resolve(ref evalStore, ref store) +std::vector UnresolvedApp::build(ref evalStore, ref store) { - auto res = unresolved; - Installables installableContext; for (auto & ctxElt : unresolved.context) installableContext.push_back( make_ref(store, DerivedPath { ctxElt })); - auto builtContext = Installable::build(evalStore, store, Realise::Outputs, installableContext); + return Installable::build(evalStore, store, Realise::Outputs, installableContext); +} + +// FIXME: move to libcmd +App UnresolvedApp::resolve(ref evalStore, ref store) +{ + auto res = unresolved; + + auto builtContext = build(evalStore, store); res.program = resolveString(*store, unresolved.program, builtContext); if (!store->isInStore(res.program)) throw Error("app program '%s' is not in the Nix store", res.program); diff --git a/src/nix/formatter.cc b/src/nix/formatter.cc index 31275eb35..f4cde7d68 100644 --- a/src/nix/formatter.cc +++ b/src/nix/formatter.cc @@ -141,11 +141,7 @@ struct CmdFormatterBuild : MixFormatter auto & installable = InstallableValue::require(*installable_); auto unresolvedApp = installable.toApp(*evalState); auto app = unresolvedApp.resolve(evalStore, store); - - Installables installableContext; - for (auto & ctxElt : unresolvedApp.unresolved.context) - installableContext.push_back(make_ref(store, DerivedPath{ctxElt})); - auto buildables = Installable::build(evalStore, store, Realise::Outputs, installableContext); + auto buildables = unresolvedApp.build(evalStore, store); if (outLink != "") if (auto store2 = store.dynamic_pointer_cast()) From 5089f1292dabd01e9d27c5752e498d9a00f13b0d Mon Sep 17 00:00:00 2001 From: Jeremy Fleischman Date: Tue, 29 Apr 2025 18:37:20 -0700 Subject: [PATCH 5/5] Refactor, use `MixOutLinkByDefault` --- src/nix/formatter.cc | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/src/nix/formatter.cc b/src/nix/formatter.cc index f4cde7d68..8b171b244 100644 --- a/src/nix/formatter.cc +++ b/src/nix/formatter.cc @@ -93,27 +93,9 @@ struct CmdFormatterRun : MixFormatter, MixJSON static auto rFormatterRun = registerCommand2({"formatter", "run"}); -struct CmdFormatterBuild : MixFormatter +struct CmdFormatterBuild : MixFormatter, MixOutLinkByDefault { - Path outLink = "result"; - - CmdFormatterBuild() - { - addFlag({ - .longName = "out-link", - .shortName = 'o', - .description = "Use *path* as prefix for the symlink to the build result. It defaults to `result`.", - .labels = {"path"}, - .handler = {&outLink}, - .completer = completePath, - }); - - addFlag({ - .longName = "no-link", - .description = "Do not create symlink to the build results.", - .handler = {&outLink, Path("")}, - }); - } + CmdFormatterBuild() {} std::string description() override { @@ -142,10 +124,7 @@ struct CmdFormatterBuild : MixFormatter auto unresolvedApp = installable.toApp(*evalState); auto app = unresolvedApp.resolve(evalStore, store); auto buildables = unresolvedApp.build(evalStore, store); - - if (outLink != "") - if (auto store2 = store.dynamic_pointer_cast()) - createOutLinks(outLink, toBuiltPaths(buildables), *store2); + createOutLinksMaybe(buildables, store); logger->cout("%s", app.program); };