flake: fix nixpkgs config

[Gerg-L]

Description

Closes #2942

Checklist

• Change is backwards compatible.

• Code formatted with ./format.

• Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

  See CONTRIBUTING for more information and recent commit messages for examples.

• If this PR adds a new module

  • Added myself as module maintainer. See example.

Maintainer CC

[NotAShelf]

lol

[justinlime]

Finally 👌

[eclairevoyant]

test case, adapted from the linked issue:

{
  inputs.home-manager = {
    url = "github:gerg-l/home-manager";
    inputs.nixpkgs.follows = "nixpkgs";
  };

  outputs = {
    self,
    nixpkgs,
    home-manager,
  }: let
    system = "x86_64-linux";
    allowUnfree = {nixpkgs.config.allowUnfree = true;};
  in {
    homeConfigurations.test = home-manager.lib.homeManagerConfiguration {
      pkgs = nixpkgs.legacyPackages.${system};
      modules = [
        {
          home = {
            username = "test";
            homeDirectory = "/home/test";
            stateVersion = "22.11";
          };
        }

        allowUnfree

        ({pkgs, ...}: {
          home.packages = [pkgs.unrar];
        })
      ];
    };
  };
}

nix run github:nix-community/home-manager -- --flake .#test build completes successfully

[sumnerevans]

Looks promising. Please make sure to follow the checklist, especially the commit message.

[cvoges12]

Finally!

Edit: (why the thumbs down? I'm so confused)

[Gerg-L]

Looks promising. Please make sure to follow the checklist, especially the commit message.

yup, dealing with it now

[Gerg-L]

alright ignore my struggling with naming all should be good now

[Gerg-L]

It should be noted that this fix is still a hack, and the idiomatic solution IMO is to change the pkgs argument in lib.homeManagerConfiguration to a nixpkgs argument and a system argument

[emilazy]

I'm curious why homeManagerConfiguration requires a pkgs option rather than having it set modularly like NixOS and nix-darwin do and just requiring lib? In particular I would expect passing pkgs to override the package set and disable the nixpkgs module, so defaulting the nixpkgs.* options based on it seems strange to me.

[emilazy]

Linking LnL7/nix-darwin@f9724c4 just in case it's useful to anyone; Home Manager doesn't seem to have quite the brokenness that plagued nix-darwin at the time, but hopefully it offers some food for thought on how external users of the module system should handle specifying a package set and the kind of interface I ended up settling on.

[Gerg-L]

that's basically what I'm saying, I could easily make a PR to switch it. but it'd obviously be a very breaking change

[cvoges12]

I think this leaves two options for what we could do:

• merge the PR as is and wait for the next breaking change to merge a better solution
• close the PR and wait patiently for a better solution in the next breaking change

I'm not sure what's the best decision to go with here. But in my head, that's kind of what it comes down to. We could get something working and clean it up after. Or we could wait it out before properly implementing it. So what do you guys think is the right move?

[eclairevoyant]

Wait what out? The issue is over a year old. There's no reason to hold back a 1 LOC nonintrusive change for something that should never have been in HM to begin with.

As far as I know there is no "schedule" for breaking changes in HM either, only that it needs to follow the guidelines in the manual. The general consensus for such types of changes would be deprecate (warn) first whenever possible. If you want stability you would stick to the release branch, so this change would only be merged to the release branch until at least 23.11.

PS the thumbs down on your first comment is because +1 comments are basically spam and add nothing to the conversation.

[emilazy]

There is no need to break compatibility to support specifying a package set in other ways.

[Gerg-L]

There is no need to break compatibility to support specifying a package set in other ways.

that's not really what I mean,

if you're using the current idiomatic solution:

pkgs = inputs.nixpkgs.legacyPackages.<system>

inline everything it looks like this:

_module.args.pkgs = import inputs.nixpkgs.legacyPackages.<system>.path {
  system = "<system>";
  inherit (config.nixpkgs) config overlays ; 
}

which is not that bad

but if you're using

pkgs = import inputs.nixpkgs {
    system = "<system>";
    overlays = [...];
    config = {...};
}

then you're instantiating nixpkgs twice!
which inline looks something like this!

_module.args.pkgs = import (import inputs.nixpkgs {
    system = "<system>";
    overlays = [...];
    config = {...};
}).path {
  system = "<system>";
  inherit (config.nixpkgs) config overlays; 
};

I'm very active in the unofficial NixOS discord server and I see this ALL. THE. TIME.

My thoughts are deprecate passing pkgs with warnings and add a nixpkgs argument

IMO the idiomatic solution looks like this

nixpkgs.system = "<system>";

_module.args.pkgs = import nixpkgs {
  inherit (config.nixpkgs) config overlays;
}

[emilazy]

Sure, I agree the current behaviour is bad, just that a better behaviour can be supported without breaking existing use.

(That said, passing a package set in directly is useful if you want to immutably override the package set and use nixpkgs-disabled.nix; otherwise you would need to accept lib or import Nixpkgs twice. This is how the Home Manager NixOS/nix-darwin module useGlobalPkgs works, as well as the nix-darwin pkgs argument. But this would indeed be a breaking change, unfortunately.)

[Gerg-L]

That said, passing a package set in directly is useful if you want to immutably override the package set

that can easily be done with a nixpkgs.pkgs option exactly how Nixpkgs does it

[emilazy]

It's not quite that simple; Home Manager needs lib.evalModules from Nixpkgs and it's very bad for the lib and pkgs to mismatch. So you either need to take lib, or take pkgs and get lib from it. nix-darwin implements this: if pkgs is specified, it uses pkgs.lib and also injects { _module.args.pkgs = lib.mkForce pkgs; }.

[Gerg-L]

tracked down the commits to blame for this:
0c236e1
NixOS/nixpkgs@1c49b81

[ambroisie]

Should a test be added, to check that it won't break in the future?

[Gerg-L]

Should a test be added, to check that it won't break in the future?

What would break? nixpkgs., config, and overlays are no longer being overridden if they have default values set.

The actual fix to how pkgs is handled will require breaking change. I'm currently working on that.

[Gerg-L]

Turns out overlays needs to not be set mkDefault as lists merge

[schuelermine]

Is anybody looking to merge this?

[Gerg-L]

Is anybody looking to merge this?

I don't really know what else could be done...

[rycee]

Thanks! Looks good to me so finally merged now. Apologies for the long delay.

[PaulGrandperrin]

This might have closed #2954 too