NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.08k stars 14.06k forks source link

`nixos-rebuild` does not properly support subattributes #336082

Open CryoMyst opened 2 months ago

CryoMyst commented 2 months ago

Describe the bug

When running nixos-rebuild --flake .#group.system, the command refers to nix build .#nixosConfigurations."group.system".config.system.build.toplevel, which does not follow the expected behavior of other Nix commands. Specifically, the command does not seem to treat the path as a typical attribute path, and it is unable to use subattributes like other Nix commands do.

Steps To Reproduce

  1. Run nixos-rebuild --flake .#group.system.
  2. Observe that the command refers to nix build .#nixosConfigurations."group.system".config.system.build.toplevel.

Expected behavior

The expected behavior is that nixos-rebuild should handle subattribute paths similarly to other Nix commands and not quote everything on the right hand side, allowing for proper grouping of machines and states under the same attribute.

Additional context

I had two use cases for this:

  1. General grouping of machines based on their purpose.
  2. For my homelab, I modified the Terraform NixOS provider to have both a base and deployed state for bare metal machines. I wanted to group both states of that machine under the same attribute.

It seems like a possible oversight in the CLI, as it should allow for the usage of subattributes or nonstandard attribute paths.

Notify maintainers

Not really sure who to ping here.

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.8.12, NixOS, 24.11 (Vicuna), 24.11.20240725.5ad6a14`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.5`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`
CryoMyst commented 2 months ago

Might need some discussion since hostnames can have periods in them.

Aleksanaa commented 2 months ago

It seems like a possible oversight in the CLI, as it should allow for the usage of subattributes or nonstandard attribute paths.

I should note that it is commonly expected to have certain attribute structure inside flake outputs. This expectation is also a part of nix flake check.

eclairevoyant commented 2 months ago

The other thing to note is that the current behaviour is okay as long as we fall back to checking the attribute itself. But whether we quote or not can turn into a footgun, and I would guess that's part of why it wasn't implemented as such originally.

Aleksanaa commented 2 months ago

Also if you want to have both your ideal attribute structure, and something compatible with nixos-rebuild, you can write a function to translate your set to nixosConfigurations. You may take some inspiration from the famous flattenTree function.

CryoMyst commented 2 months ago

I should note that it is commonly expected to have certain attribute structure inside flake outputs. This expectation is also a part of nix flake check.

This is fair enough and would definitely break something out there if this was changed.

I did ask @NobbZ about this behaviour in discord

Interesting. Yeah, probably an oversight, but I am not aware of an issue reported. Even though I consider it a misconception on your side to have the subattribures or nonstandard attr paths, the CLI definitely should allow for it.

We can probably close this ticket if it's undesirable and I can use some workarounds for now.

roberth commented 2 months ago

I believe it was designed this way intentionally, matching the packages attribute for instance, though it has to be noted that the same argument does not apply: determining the tree structure of nested attribute sets of packages is expensive, because it requires evaluating all packages. For configurations, this is not expensive: (inputs.nixpkgs.lib.nixosSystem { modules = throw "unused"; })._type or null == "configuration" evaluates to true, showing that determining a leaf of the tree of configs only depends on a few files in nixpkgs (and not passing a non-existent argument to nixosSystem). Not entirely trivial, but very cheap and sufficiently robust that it's generally not a problem for commands like nix flake show or check.

So in my opinion, this is worth considering as a feature, even if it's not a bug.

Might need some discussion since hostnames can have periods in them.

One possibility is to preserve the meaning of foo#bar.baz to be the hostname bar.baz, and only look for nested attributes when a more explicit path syntax is used, such as

foo#group."system"

This is a valid path selection in the Nix language, but not a valid hostname, and it feels safe to assume nobody in their right mind uses config names with quotes in them ("I'm \"confused\"" = nixosSystem ...;). It also allows hostnames to be grouped:

foo#group."system.lan"

CryoMyst commented 1 month ago

I just noticed https://github.com/NixOS/nixpkgs/pull/320462

'--flake' cannot be used with '--file' or '--attr'"

Would it make sense to extend it so this only works when --flake and --attr are specified together?

nixos-rebuild --flake . --attr nixosConfigurations.group.system