`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:
7864607231/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`.
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.
Before the change `nix` was stripping warning flags
reported by `gcc-14` too eagerly:
$ nix build -f. texinfo4
error: builder for '/nix/store/i9948l91s3df44ip5jlpp6imbrcs646x-texinfo-4.13a.drv' failed with exit code 2;
last 25 log lines:
> 1495 | info_tag (mbi_iterator_t iter, int handle, size_t *plen)
> | ~~~~~~~~^~~~
> window.c:1887:39: error: passing argument 4 of 'printed_representation' from incompatible pointer type []
> 1887 | &replen);
> | ^~~~~~~
> | |
> | int *
After the change the compiler flag remains:
$ ~/patched.nix build -f. texinfo4
error: builder for '/nix/store/i9948l91s3df44ip5jlpp6imbrcs646x-texinfo-4.13a.drv' failed with exit code 2;
last 25 log lines:
> 1495 | info_tag (mbi_iterator_t iter, int handle, size_t *plen)
> | ~~~~~~~~^~~~
> window.c:1887:39: error: passing argument 4 of 'printed_representation' from incompatible pointer type [-Wincompatible-pointer-types]
> 1887 | &replen);
> | ^~~~~~~
> | |
> | int *
Note the difference in flag rendering around the warning.
https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda has a
good sumamry of why it happens. Befomre the change `nix` was handling
just one form or URL separator:
$ printf '\e]8;;http://example.com\e\\This is a link\e]8;;\e\\\n'
Now it also handled another for (used by gcc-14`):
printf '\e]8;;http://example.com\aThis is a link\e]8;;\a\n'
While at it fixed accumulation of trailing escape `\e\\` symbol.
The explicit include is needed for clangd to not get confused somehow,
which is also what threw me off initially and made me pick the wrong
constructor.
The (pointer, number, number) constructor first constructs a C string
and then takes a substring from that, but we didn't specify that the
buffer needs to be NUL-terminated, and then what would be the point of
the size argument anyway...
basic_string.h:
> basic_string(const _Tp& __t, size_type __pos, size_type __n,
> const _Alloc& __a = _Alloc())
> : basic_string(_S_to_string_view(__t).substr(__pos, __n), __a) { }
Valgrind on nixops4/rust/nix-flake tests:
==1344422== Conditional jump or move depends on uninitialised value(s)
==1344422== at 0x48513E8: strlen (vg_replace_strmem.c:505)
==1344422== by 0x488E941: UnknownInlinedFun (char_traits.h:391)
==1344422== by 0x488E941: UnknownInlinedFun (string_view:141)
==1344422== by 0x488E941: UnknownInlinedFun (basic_string.h:790)
==1344422== by 0x488E941: nix_flake_reference_and_fragment_from_string (nix_api_flake.cc:81)
==1344422== by 0x127332: nix_flake::FlakeReference::parse_with_fragment (lib.rs:123)
It seems that the intention was to format a number in base 8 (as
suggested by the %o format specifier), but `perms` is a `std::string`
and not a number. Looks like `rawMode` is the correct thing to use here.
This name is close to the Nixpkgs lib function `escapeShellArg`,
making it easier to find.
A friendlier function with the same behavior as lib could be added
later.
Fixes
../src/libstore/unix/build/derivation-builder.cc:1130:86: warning: the compiler can assume that the address of ‘nix::DerivationBuilderParams::drv’ will never be NULL [-Waddress]
1130 | if (useChroot && settings.preBuildHook != "" && dynamic_cast<const Derivation *>(&drv)) {
| ^~~~
Assuming this check was left over from the time `drv` could be a
`BasicDerivation`.
I split it out before to try to separate the building logic, but now we
have the much better `DerivationBuilder` abstraction for that. With that
change, I think `LocalDerivationGoal` has outlived its usefulness.
We just inline it back into `DerivationGoal`, and do so with minimal
`#ifdef` for Windows.
Note that the order of statements in `~DerivationGoal` is different than
it was after the `~LocalDerivationGoal` split, but it is *restored* to
the way it original was before --- evidently I did the split slightly
wrong, but nobody noticed, probably because the order doesn't actually
matter.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Now that `DerivationBuilder` is created after the underlying data has
already been initialized, we can just refer this data normally, with a
direct reference.
Only `parsedDrv` takes a (borrowing) pointer, because independent of
initialization the derivation may or may not have structured attrs.
