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

flake.nix: NixOS nixpkgs registry pin as nixos #197424

Closed
wants to merge 1 commit into from

Conversation

nrdxp
Copy link
Contributor

@nrdxp nrdxp commented Oct 23, 2022

Description of changes

On a given NixOS system, if a user discovers the nix.registry option, they may wish to set the nixpkgs of their system as a pin for convenience, and rightfully so.

However, pinning the global nixpkgs to a /nix/store path can wreak havoc with the lockfile of any other flake with an indirect reference to nixpkgs, borking it for anyone who tries to call it remotely.

This is simply bad UX. It should be hard to do the wrong thing, but this is nearly inevitable.

Instead of expecting the user to discover this on their own, we should offer them the convenience of a pinning of their system's nixpkgs as nixos and encourage them to use it as their "goto" nixpkgs, analogous to <nixpkgs> in "pre-flake" material, but more precise.

Since we can expect this reference to exist we can always point to it in documentation and reference material.

Anecdotally, I don't think we want a users first impression of a flakes based NixOS system to be that nix shell nixpkgs#* always seems to redownload the world, and if they are keen enough to quickly discover the root cause and attempt to address it with nix.registry.nixpkgs.flake they will run into an even trickier, opaque breakage.

This is also consistent with the previous channels semantic, where on nixos the system channel was labeled nixos, and on other systems it was nixpkgs.

Things done
  • Always pin a nixos flake reference in the NixOS system registry, pointing to the nixpkgs used to evaluate the system.
  • Documentation

On a given NixOS system, if a user discovers the `nix.registry` option,
they may wish to set the nixpkgs of their system as a pin for
convenience, and rightfully so.

However, pinning the global nixpkgs to a `/nix/store` path can wreak
havoc with the lockfile of any other flake with an indirect reference
to nixpkgs, borking it for anyone who tries to call it remotely.

This is simply bad UX. It should be hard to do the wrong thing, but
this is nearly inevitable.

Instead of expecting the user to discover this on their own, we should
offer them the convenience of a pinning of their system's nixpkgs as
`nixos` and encourage them to use it as their "goto" nixpkgs, analogous
to `<nixpkgs>` in "pre-flake" material, but more precise.

Since we can expect this reference to exist we can always point to it
in documentation and reference material.
@nrdxp
Copy link
Contributor Author

nrdxp commented Oct 23, 2022

cc @NixOS/documentation-team

@@ -26,6 +26,7 @@
system.nixos.versionSuffix =
".${final.substring 0 8 (self.lastModifiedDate or self.lastModified or "19700101")}.${self.shortRev or "dirty"}";
system.nixos.revision = final.mkIf (self ? rev) self.rev;
nix.registry.nixos.flake = "${self}";
Copy link
Member

Choose a reason for hiding this comment

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

  • The nix registry (flake registry?) just reinvents NIX_PATH for the CLI. Both are extra namespaces that are set implicitly through the environment, whether that's an env variable or a non-local file. Instead, we could remove the registry indirection by adding a /etc/nixpkgs/flake.{nix,lock}, so that we can do nix shell /etc/nixpkgs#foo? This makes the dependency obvious.

The other issues are of secondary importance, but I'll list them anyway because they correspond to requirements for a /etc/nixpkgs solution as well.

  • This line defeats lazy trees by unpacking the archive.
  • It does not seem to preserve flake metadata such as sourceInfo.
  • Closure size: this is only relevant for systems that need to be capable of evaluating. Many deployment tools do not require this feature. This includes nixos-rebuild with --target-host.
  • By depending on the Nixpkgs revision, we'll be creating more system generations where nothing changes except the Nixpkgs revision. This could lead to huge boot menus and unnecessary remote activations.

Copy link
Contributor Author

@nrdxp nrdxp Oct 24, 2022

Choose a reason for hiding this comment

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

  • so that we can do nix shell /etc/nixpkgs#foo

That's fine and all, but not quite as convenient as a nixos#foo. I am admittedly bias on this point though, as I was also in favour of NixOS/nix#4438. It would be nice to have a solution for the underlying "local ref in flake.lock breaking everything" from Nix itself though. Not sure what we could do there, maybe a fallback remote uri?

  • This line defeats lazy trees by unpacking the archive.

True, I realized that. I guess this should only be active for interactive installations of NixOS, since in that case, I can't really see a time when a user wouldn't want a local reference to nixpkgs to avoid superflous fetching, and building.

Although I'm not sure there is a nice way to detect that automatically unfortunately. Maybe we need a whole new module for sane interactive defaults. We could also add nix.nixPath = [ "nixpkgs=${pkgs.path}" ] to such a modue as well, for example.

Even so, NixOS has always had a local copy of nixpkgs (i.e. channels) in the past, and it seems like a generally good idea, especially with the eval cache available. For example, nix search nixos from a pinned nixpkgs is typically a lot faster after the first invocation, and is just an all around better experience.

  • It does not seem to preserve flake metadata such as sourceInfo.

I took the recommended workaround mentioned in the lazy-tree's PR, which advises against just flake = self.

  • Closure size: this is only relevant for systems that need to be capable of evaluating. Many deployment tools do not require this feature. This includes nixos-rebuild with --target-host.

see my response to the first point

  • By depending on the Nixpkgs revision, we'll be creating more system generations where nothing changes except the Nixpkgs revision. This could lead to huge boot menus and unnecessary remote activations.

Is this really a risk in practice? nixpkgs changes so often, I've never really ran into a situation where I bumped my pin and nothing changed, but I suppose it's feasible on very minimal systems.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is a reasonable default value, but I have following issues:

  1. The closure size of a NixOS raises by the size of nixpkgs, which is not always wanted.
  2. Default values should not defined in flake.nix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. The closure size of a NixOS raises by the size of nixpkgs, which is not always wanted.

That is true. We might want a toggle, but I didn't do this originally since then we have the same, non-default behavior as before and the user has to going searching for the option, so I figured there isn't much improvement there.

2. Default values should not defined in flake.nix.

Right, but I wanted to only make this the case with flake based configurations, which are the only configs that call this function. Of course, I don't think it would hurt anything for non-flake configs so perhaps we should move this to the module after all.

@@ -26,6 +26,7 @@
system.nixos.versionSuffix =
".${final.substring 0 8 (self.lastModifiedDate or self.lastModified or "19700101")}.${self.shortRev or "dirty"}";
system.nixos.revision = final.mkIf (self ? rev) self.rev;
nix.registry.nixos.flake = "${self}";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is a reasonable default value, but I have following issues:

  1. The closure size of a NixOS raises by the size of nixpkgs, which is not always wanted.
  2. Default values should not defined in flake.nix.

@@ -26,6 +26,7 @@
system.nixos.versionSuffix =
".${final.substring 0 8 (self.lastModifiedDate or self.lastModified or "19700101")}.${self.shortRev or "dirty"}";
system.nixos.revision = final.mkIf (self ? rev) self.rev;
nix.registry.nixos.flake = "${self}";
Copy link
Member

Choose a reason for hiding this comment

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

We're supposed to be the distro with hermeticity and no mutable global state magic. The registry can't be fixed. Do not use it.

Suggested change
nix.registry.nixos.flake = "${self}";

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be my response: NixOS/nix#7422 (comment)

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 18, 2023
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@ck3d ck3d closed this Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants