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.

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

WIP: feat(fmt): use nixfmt-rfc-style #11252

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libcmd/installable-flake.hh
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ struct InstallableFlake : InstallableValue
* have their own Nixpkgs input, or other installables.
*
* It is a layer violation for Nix to know about Nixpkgs; currently just
* `nix develop` does. Be wary of using this /
* `nix develop` and `nix fmt` does. Be wary of using this /
* `InstallableFlake::nixpkgsFlakeRef` more places.
*/
static inline FlakeRef defaultNixpkgsFlakeRef()
Expand Down
87 changes: 83 additions & 4 deletions src/nix/fmt.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "command.hh"
#include "installable-value.hh"
#include "installable-flake.hh"
#include "eval.hh"
#include "run.hh"

Expand Down Expand Up @@ -33,11 +34,89 @@ struct CmdFmt : SourceExprCommand {
auto evalState = getEvalState();
auto evalStore = getEvalStore();

Path nixfmt = "nixfmt";
FlakeRef nixpkgs = defaultNixpkgsFlakeRef();
nixpkgs = nixpkgs.resolve(evalStore);
auto nixpkgsLockFlags = lockFlags;
auto installable_ = parseInstallable(store, ".");
auto & installable = InstallableValue::require(*installable_);
auto app = installable.toApp(*evalState).resolve(evalStore, store);

Strings programArgs{app.program};
// Check for "formatters.SYSTEM", too slow on Nixpkgs until lazy-trees
/*
try {
if (auto * i = dynamic_cast<const InstallableFlake *>(&*installable_)) {
auto & installable = InstallableFlake::require(*installable_);
auto app = installable.toApp(*evalState).resolve(evalStore, store);
nixfmt = app.program;
}
} catch (Error &) {
// ignoreException();
}
*/

// Check for nixpkgs input, too slow on Nixpkgs until lazy-trees
/*
if (nixfmt == "nixfmt") {
try {
nixpkgsLockFlags.inputOverrides = {};
nixpkgsLockFlags.inputUpdates = {};

// Hard code the nixpkgs revision from <nixpkgs>/ci/pinned-nixpgs.json
if (auto * i = dynamic_cast<const InstallableFlake *>(&*installable_))
nixpkgs = i->nixpkgsFlakeRef();
} catch (Error &) {
// ignoreException();
}
}
*/

// Check for <nixpkgs>/ci/pinned-nixpkgs.json and resolve it
if (nixfmt == "nixfmt") {
try {
auto res = nixpkgs.fetchTree(store);
auto s = store->printStorePath(res.first) + "/ci/pinned-nixpkgs.json";
Copy link
Member

Choose a reason for hiding this comment

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

This elevates a very recent Nixpkgs implementation detail to a new Nix interface that is not consistent with anything that came before in Nix. It would be more appropriate to do this in a Nixpkgs CLI.

What if we could just load the shell instead, or just stick to the plan and finish lazy trees?

Copy link
Contributor Author

@tomberek tomberek Aug 6, 2024

Choose a reason for hiding this comment

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

Agreed, this is quite fragile. What about having a version pinned in Nix itself? A reference to nixpkgs or nixfmt repos. Benefit is thst it is simple and predictable. Downside is that you don't get an automatic upgrade.

Nixpkgs-specific tooling can do additional work to match the Nixpkgs to the fmt.

if (pathExists(s)) {
nlohmann::json pinned_json = nlohmann::json::parse(readFile(s));
auto pinned_rev = getString(pinned_json["rev"]);
nixpkgs = FlakeRef::fromAttrs(fetchSettings, {{"type","indirect"}, {"id", "nixpkgs"},{"rev", pinned_rev}});
nixpkgs = nixpkgs.resolve(evalStore);
}
} catch (Error &) {
// ignoreException();
}
}
// Check for nixfmt, otherwise use PATH
if (nixfmt == "nixfmt") {
try {
auto nixfmtInstallable = make_ref<InstallableFlake>(
this,
evalState,
std::move(nixpkgs),
"nixfmt-rfc-style",
ExtendedOutputsSpec::Default(),
Strings{},
Strings{"legacyPackages." + settings.thisSystem.get() + "."},
nixpkgsLockFlags);

bool found = false;

for (auto & path : Installable::toStorePathSet(getEvalStore(), store, Realise::Outputs, OperateOn::Output, {nixfmtInstallable})) {
auto s = store->printStorePath(path) + "/bin/nixfmt";
if (pathExists(s)) {
nixfmt = s;
found = true;
break;
}
}

if (!found)
throw Error("package 'nixpkgs#nixfmt-rfc-style' does not provide a 'bin/nixfmt'");

} catch (Error &) {
ignoreException();
}
}

Strings programArgs{nixfmt};

// Propagate arguments from the CLI
if (args.empty()) {
Expand All @@ -54,7 +133,7 @@ struct CmdFmt : SourceExprCommand {
// we are about to exec out of this process without running C++ destructors.
evalState->evalCaches.clear();

execProgramInStore(store, UseLookupPath::DontUse, app.program, programArgs);
execProgramInStore(store, UseLookupPath::DontUse, nixfmt, programArgs);
};
};

Expand Down
30 changes: 3 additions & 27 deletions src/nix/fmt.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,48 +2,24 @@ R""(

# Examples

With [nixpkgs-fmt](https://github.com/nix-community/nixpkgs-fmt):

```nix
# flake.nix
{
outputs = { nixpkgs, self }: {
formatter.x86_64-linux = nixpkgs.legacyPackages.x86_64-linux.nixpkgs-fmt;
};
}
```
With [nixfmt](https://github.com/NixOS/nixfmt):

- Format the current flake: `$ nix fmt`

- Format a specific folder or file: `$ nix fmt ./folder ./file.nix`

With [nixfmt](https://github.com/serokell/nixfmt):

```nix
# flake.nix
{
outputs = { nixpkgs, self }: {
formatter.x86_64-linux = nixpkgs.legacyPackages.x86_64-linux.nixfmt;
};
}
```

- Format specific files: `$ nix fmt ./file1.nix ./file2.nix`

With [Alejandra](https://github.com/kamadorueda/alejandra):

## **disabled** Overriding the formatter used
```nix
# flake.nix
{
outputs = { nixpkgs, self }: {
formatter.x86_64-linux = nixpkgs.legacyPackages.x86_64-linux.alejandra;
formatter.x86_64-linux = nixpkgs.legacyPackages.x86_64-linux.nixfmt-rfc-style;
};
}
```

- Format the current flake: `$ nix fmt`

- Format a specific folder or file: `$ nix fmt ./folder ./file.nix`

# Description

Expand Down
Loading