nixos-option: rewrite as a nix script

[FireyFly]

This ports the functionality of the C++ nixos-option to a nix script (with a tiny shellscript for argument processing and invoking the nix script). The main impetus for this is that the current C++ version seems a bit verbose, unnecessarily tightly coupled to cppnix internals, fragile (with the occasional segfaults if you hold it slightly wrong).

While I'm rewriting it anyway, I opted to improve the output format slightly to make it easier on the eyes (there's no point in the type/description strings and declared/defined by lists as nix values; instead just make sure each output is indented with two spaces).

There's some changes in behaviour especially when it comes to errors whilst evaluating options. In recursive mode, errors that can be caught with builtins.tryEval render as just «error» whereas fatal errors bubble to the top and stop the recursion (the old C++ version prints the full stacktrace inline in place of the value for each option where evaluating the value errors).

The new version supports descending into attrsOf/listOf submodule options (as long as they're defined in the user's config)--the previous version doesn't seem to understand them (and segfaults in the recursive case, e.g. nixos-option -r users.users.youruser).

The new version (well, the shell wrapper part of it) supports --show-trace for full error traces.

Performance is basically unchanged vs the existing version (I think most of the time is spent evaluating nixos anyway).

Fixes #293543
Fixes #97855

---

Before this is merged, it'd be nice if it was a bit more widely tested (so far I've just tested it a bit on random options/paths locally).

The manual page should probably also be updated, and release notes added, but I figured it makes sense to get some feedback on the derivation changes first.

Description of changes

• C++ implementation of nixos-option removed
• nixlang implementation added, with a small shellscript wrapper to invoke it
  • (this uses nix-instantiate --eval --json | jq -r as a hack to output raw strings; suggestions for improvements welcome)
• new default.nix; I took the liberty to change the license since it's a fresh implementation anyway and doesn't link against nix (and if anything I took some notes from the nixos-rebuild derivation, which is also MIT licensed)

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[K900]

Have not looked closely yet, but yes please.

[stuebinm]

I really like this rewrite, and would love to see it in nixpkgs.

There are some slight regressions compared to the existing implementation:

• the old implementation supported a couple more flags (notably --help), though these were apparently entirely undocumented
• the old version had a marginally nicer error message for nonexisting option paths; the new version currently just throws a (Nix) error (see below)

Additionally, I think it'd be nice to update the man page as well — it contains some example output which has changed slightly, and the --show-trace option is not mentioned there.

[stuebinm]

perhaps it would be worth it not to throw here, and instead return some structured error which the wrapper script can display nicely?

The output for nonexistent config options is the only thing which feels worse than in the old version to me.

[FireyFly]

hmm, at first I just returned strings in case of errors, but then figured we probably want nonzero exit codes and so opted to throw instead

Maybe it'd be an idea to still throw but format the errors from nixos-option itself with a specific prefix that we can look for in stderr in the wrapper script, and if present cut off the stacktrace and only show the error message (plus perhaps also improve the message a bit)

I like sticking to throwing in the .nix script since it makes it less strictly coupled to being used with the wrapper script

Good point re --help--we could handle that and show the manpage in the wrapper script easily enough. The manpage & release notes should be updated but I wanted feedback on the functionality first ^^

[r-vdp]

I guess we can also drop the version pin for nix in all-packges.nix then, and in nixos/modules/installer/tools/tools.nix we can then override the nix input of nixos-option with config.nix.package.

[azuwis]

I've added flake support in this commit azuwis@1bd0fb3, feel free to merge it into this PR.

Or should I open another PR?

[eclairevoyant]

Let's keep that as a separate PR; I'd personally say this PR should be for establishing parity with the original tool (as much as possible), which is already a fair bit of work. Keeping new functionality in a separate PR would be easier for contributors and reviewers IMO.

[AndersonTorres]

Stylistic issue:

instead of if !P then A else B
use if P then B else A.

[FireyFly]

Hm, that was intentional since I found it easier to read (kind of as an "early return" of sorts, taking care of the error case first) but I'm happy to rephrase it if it's easier to read without the negation

[FireyFly]

@azuwis:

I've added flake support in this commit azuwis@1bd0fb3, feel free to merge it into this PR.

I appreciate covering flake support and think it's something that would be nice to have, but I think I agree with @eclairevoyant to first focus this PR on essentially parity with the existing C++ version.. that said, after that it'd be nice to add such support.

---

Apart from that, I got a bit distracted by other things around me, but i should be able to get back to this PR now. I appreciate all the feedback and I'll try to incorporate them into this PR soon.

I didn't do thorough tests of running my tool on every possible option subtree, but @philiptaron poked me on Matrix as well with some feedback taking that kind of route, hence also the linked PRs opened referencing this one. Appreciate this as well!

[philiptaron]

I'm actively testing this out. There are definitely some sharp edges, some of them issues in this tool and some of them issues in the codebase.

• nixos/systemd-boot: guard assertion messages against eval failures #320850
• nixos/filesystems: guard assertion messages against eval failures #320846 (but this will be likely rejected)
• nixos/bluemap: fix defaults issue with services.bluemap.host #320836
• nixos/nvidia: move TOPOLOGY_FILE_PATH and DATABASE_PATH into nvidia-fabricmanager service definition #320830

There are likely many others. This was sufficient to get most options printed.

As I do use flakes to configure my NixOS system, I am using a mildly modified version as testing fodder, where I pass --arg nixos (builtins.getFlake "github:philiptaron/flock.nix/eeeeeee").nixosConfigurations.zebul to interrogate that flake's configuration.

[azuwis]

Compared to the cpp version of nixos-option, the nix script version is 3 times faster when showing options that display derivations like programs.sway.package:

$ nix shell nixpkgs#hyperfine nixpkgs#nixos-option
$ hyperfine 'nixos-option -I nixos-config=/etc/nixos/configuration.nix programs.sway.package'
Benchmark 1: nixos-option -I nixos-config=/etc/nixos/configuration.nix programs.sway.package
  Time (mean ± σ):      2.667 s ±  0.024 s    [User: 2.417 s, System: 0.237 s]
  Range (min … max):    2.636 s …  2.713 s    10 runs

$ nix shell nixpkgs#hyperfine .#nixos-option
$ hyperfine 'nixos-option programs.sway.package'                                          
Benchmark 1: nixos-option programs.sway.package
  Time (mean ± σ):     787.8 ms ±  32.6 ms    [User: 642.2 ms, System: 124.7 ms]
  Range (min … max):   767.7 ms … 878.1 ms    10 runs

[Aleksanaa]

Compared to the cpp version of nixos-option, the nix script version is 3 times faster when showing options that display derivations like programs.sway.package:

Thanks for testing, but this may have some eval caching involved?

[eclairevoyant]

(it was in the wrong if block and missing a /)

[lf-]

That git detection is broken. You cannot assume the flake dir itself is a git repo because the flake could well be in a subdirectory. I suggested git rev-parse --show-toplevel because it avoids this bug.

[eclairevoyant]

I think we should just drop flake support. It wasn't in the original script anyway and it needs a full design in a separate PR.

[lf-]

I also don't think we've at all exhausted the ways to not use getFlake, like --apply. So yeah I think we should merge this without flake support and figure that out in a separate change.

[Mindavi]

I agree with this conceptually, haven't really used the tool before so I find it hard to give in-depth comments.

[lf-]

I think we should just drop flake support. It wasn't in the original script anyway and it needs a full design in a separate PR.

I mean, flakes are very widely used, so I don't know if that would be the wisest decision? Idk.

The reason we should do this is that it has taken three months to merge this, which is very silly given that it was in a state to be an incremental improvement (minus new flake functionality) a month or two ago. Thus the flake functionality should be split out and worked on separately.

[nyabinary]

What further work is needed to get this in?

[lf-]

What further work is needed to get this in?

The flakes pieces need to be removed and then it can be merged with no other changes.

[Mic92]

How are you planning to support flakes? With the C++ version we wouldn't have to rely on builtins.getFlake

[lf-]

How are you planning to support flakes? With the C++ version we wouldn't have to rely on builtins.getFlake

There are a few ways to do this and get around the fact that running nix code in the context of a flake is way too hard for no reason. One way to do this is using --applyon nix eval. Being involved in maintaining the Nix implementations, this is our problem, and if there is no viable option, we will have to fix the CLI to be able to do this kind of obvious thing that has been broken for years.

The flake support in nixos-option is already nonexistent, and I think it is a super bad example to be setting to have this kind of Nix language inspection tooling be impossible to write except in C++.

My immediate answer is "don't worry about it", and my longer-term answer is "this must be possible [or made to be possible] because it would be fundamentally unserious of Nix/Lix to not be able to build this".

We need to get a nixos-option that is not a long-term maintenance burden merged, and then we can discuss how to get the flakes to work. It is no regression to not support flakes, since nixos-option already does not support flakes, and it is much more important IMO to have it in a maintainable state where it does not break every single Nix release (and completely not support Lix either!).

[aviallon]

I used your PR to debug my NixOS config using Flakes, and it works marvelously. Thank you.

[thiagokokada]

We probably don't want this so late in the 24.11 cycle, so I would move this to the 25.05 release notes instead.

[thiagokokada]

I am also interested in seeing this merged. IMO right now I already have a wrapper for nixos-option to make it work with flakes:

    nixos-option =
      let
        prefix = ''
          (import ${inputs.flake-compat} {
            src = ${flake};
          }).defaultNix.nixosConfigurations.\$(hostname)
        '';
      in
      prev.runCommand "nixos-option"
        {
          buildInputs = with prev; [
            makeWrapper
            installShellFiles
          ];
        }
        ''
          makeWrapper ${prev.nixos-option}/bin/nixos-option $out/bin/nixos-option \
          --add-flags --config_expr \
          --add-flags "\"${prefix}.config\"" \
          --add-flags --options_expr \
          --add-flags "\"${prefix}.options\""

          installManPage ${prev.nixos-option}/share/man/**/*
        '';

I assume this will continue working with zero or minimal changes. So I concur that not supporting Flakes right now is fine, we can have another PR adding support for Flakes later.

[Mic92]

@thiagokokada maybe just make a new pull request.

[thiagokokada]

@thiagokokada maybe just make a new pull request.

I have little to no context about this change, but maybe one of the maintainers can create another PR?

[benley]

@thiagokokada maybe just make a new pull request.

I have little to no context about this change, but maybe one of the maintainers can create another PR?

According to github it should be possible for anyone with commit privileges in nixpkgs to amend the existing PR, fwiw:

(though it may be more polite to open a new one? @FireyFly, what is your preference?)

[thiagokokada]

@thiagokokada maybe just make a new pull request.

I have little to no context about this change, but maybe one of the maintainers can create another PR?

According to github it should be possible for anyone with commit privileges in nixpkgs to amend the existing PR, fwiw:

(though it may be more polite to open a new one? @FireyFly, what is your preference?)

I know it is possible, but it would be better done by the people that understand what is happening to fix possible issues found in the review.

I may open another PR anyway if there is no one else interested, but would prefer to leave it to the people that already invested in the source code.

[thiagokokada]

Opened a new PR: #369151.

[Mic92]

Closing in favor of #369151