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

nixpkgs config inside machine definitions is ignored #34

Closed
Shados opened this issue Nov 29, 2018 · 11 comments · Fixed by #67
Closed

nixpkgs config inside machine definitions is ignored #34

Shados opened this issue Nov 29, 2018 · 11 comments · Fixed by #67
Labels
bug Something isn't working
Milestone

Comments

@Shados
Copy link
Contributor

Shados commented Nov 29, 2018

Problem

Here's an example deployment file to demonstrate what I mean:

let
  pkgs = import <nixpkgs> { };
in
{
  network = {
    inherit pkgs;
    description = "Example overlay/override failure";
  };

  "example.com" = { config, lib, pkgs, ...}: {
    nixpkgs.overlays = [
      (self: super: {
        pam_mount = super.pam_mount.overrideAttrs(oldAttrs: {
          name = "pam_mount overidden by overlay";
        });
      })
    ];
    nixpkgs.config.packageOverrides = pkgs: {
      pam_mount = pkgs.pam_mount.overrideAttrs(oldAttrs: {
        name = "pam_mount overidden by package overrides";
      });
    };
    system.activationScripts.testingOverlays = ''
      echo "${builtins.trace pkgs.pam_mount.name "nak"}"
    '';

    # Boilerplate
    fileSystems."/" = { label = "root"; fsType = "ext4"; };
    boot.loader.grub.device = "/dev/sda";
  };
}

If you morph build that, you'll get trace output of trace: pam_mount-2.16 or similar, rather than trace: pam_mount overridden by overlay as you would if you were building the same NixOS configuration any other way.

Cause

Morph passes pkgs from the network to the machine definitions, which is nice in terms of pinning nixpkgs versions and sharing nixpkgs config between machines, but problematic because nixpkgs' eval-config.nix will set the pkgs module argument via mkForce if pkgs is passed into it like this.

Fix

I think this would require an upstream nixpkgs change or using a vendored, modified eval-config.nix to fix*, but it's pretty near the end of a work day here and I have not fully thought it through.

*: Where 'fix' means "merge machine-specific nixpkgs elements on top of the network-specified ones"

@johanot
Copy link
Contributor

johanot commented Nov 29, 2018

You're absolutely right. This is not the first time I've looked into this issue, and it probably won't be the last. Most recent time we discussed it was at the release of NixOS 18.09, where we wanted to update the package set of only a subset of hosts in a network/group.

At that time, I dropped it while well progressed into vendoring of eval-config.nix. At that time, I didn't think it was worth maintaining our own copy of part of the module system, just to get this to work.

But.. Perhaps we should look into it again, and maybe we might even be able to upstream changes to eval-config if they are generic enough.

@johanot
Copy link
Contributor

johanot commented Nov 29, 2018

@Shados
By removing the pkgs passed to eval-config.nix, and setting nixpkgs.pkgs = mkDefault network.network.pkgs; (globally in the morph version of eval-machines.nix); I managed to make the following work:

let
  p = import <nixpkgs> {};
  p' = import <nixpkgs> { overlays = [
      (self: super: {
        pam_mount = super.pam_mount.overrideAttrs(oldAttrs: {
          name = "pam_mount overidden by overlay";
        });
      })
    ];
  };

  common = { config, pkgs, ... }: {
    system.activationScripts.testingOverlays = ''
      echo "${builtins.trace pkgs.pam_mount.name "nak"}"
    '';
    fileSystems."/" = { label = "root"; fsType = "ext4"; };
    boot.loader.grub.device = "/dev/sda";
  };
in
{
  network = {
    pkgs = p;
    description = "Example overlay/override failure";
  };

  "example.com" = { config, lib, pkgs, ...}: {
    imports = [ common ];
    nixpkgs.pkgs = p;
  };

  "example-withoverlays.com" = { config, lib, pkgs, ...}: {
    imports = [ common ];
    nixpkgs.pkgs = p';
  };

}

but for some reason (which I haven't investigated yet), still neither of nixpkgs.overlays nor nixpkgs.config.packageOverrides work as expected.

UPDATE: I investigated further, and of course nixpkgs.* doesn't work, because now I override the nixpkgs-module. I can fix by leaving the nixpkgs-module alone or by adopting the nixpkgs.pkgs default:

{
   nixpkgs.pkgs = mkDefault (import (toString pkgs.path) {
     inherit (config.nixpkgs) config overlays localSystem crossSystem;
   });
}

Have a look here: a869363 .. Perhaps test to see if this would work for you?

johanot pushed a commit that referenced this issue Nov 29, 2018
mkForced.

Instead, mkDefault the nixpkgs module to network.pkgs. This will make it
overridable in deployment files.

fixes #34
@Shados
Copy link
Contributor Author

Shados commented Nov 29, 2018

That works :). Of course, it would be nice to do something like:

{
  nixpkgs.pkgs = mkDefault (import (toString pkgs.path) {
    inherit (config.nixpkgs) localSystem crossSystem;
    config = pkgs.lib.recursiveUpdate pkgs.config config.nixpkgs.config;
    overlays = pkgs.config.overlaysApplied ++ config.nixpkgs.overlays;
  });
}

So that you could merge machine-specific nixpkgs changes on top of the network-wide ones, but nixpkgs doesn't appear to actually keep an attribute listing the overlays used to build it.

I think this would require only a trivial change to <nixpkgs/pkgs/top-level/stage.nix>, so I'll have a play with the idea on a local checkout and see if there are any issues I have not anticipated.

Edit: nixpkgs.config has a somewhat more complicated merge function in reality, to handle packageOverrides correctly, but you get the idea.

@Shados
Copy link
Contributor Author

Shados commented Nov 30, 2018

Well, adding such an attribute to the nixpkgs set doesn't appear to cause any problems, and does work for this purpose.

I also took a look into what could be done without changes to upstream nixpkgs, this is what I came up with:

{
  nixpkgs.pkgs = mkDefault (pkgs.appendOverlays (
    pkgs.lib.optional
      (config.nixpkgs.config ? packageOverrides)
      (self: super: config.nixpkgs.config.packageOverrides super)
    ++ config.nixpkgs.overlays));
}

But it has some differences compared to re-importing nixpkgs and passing in a merged overlays and config:

  1. Doesn't handle any other per-machine nixpkgs.config changes (allowUnfree, perlPackageOverrides, etc.)
  2. Doing things this way does not allow for setting a per-machine crossSystem, which I imagine could be useful (FWIU localSystem should always be inherited from the actual local platform type, unless/until Morph starts doing distributed builds)

@johanot
Copy link
Contributor

johanot commented Nov 30, 2018

@Shados Yeah.. I tried to make it simple and transparent by not merging anything. In my example, you could deliberately decide that the network pkgs is for bootstrapping only; i.e. overlays and other package args are ignored by design. You'd then have to set nixpkgs.pkgs , nixpkgs.overlays etc. explicitly in your config. But you could of course do that in a common module, in case you want it to apply for all hosts in a network.

That said; I agree that it would still be more ergonomic and beautiful to get the merging to work. :-) Let me try to play around with the snippets you posted and test with our setup. Thanks so far! I'll get back to you in the beginning of next week.

@johanot johanot added the bug Something isn't working label Nov 30, 2018
@johanot johanot added this to the 1.2 milestone Nov 30, 2018
@Shados
Copy link
Contributor Author

Shados commented Nov 30, 2018

Sounds good. Have a nice weekend :).

@johanot
Copy link
Contributor

johanot commented Dec 7, 2018

@Shados I promised to get back to you in the beginning of this week. That... didn't happen, sorry :). I actually tried to reach you on IRC but nevermind. Basically; I'm good with the snippet you proposed, except pkgs.appendOverlays seems to be a new feature, which is not in nixpkgs 18.09? Am I right? If so, I'd be slightly reluctant to implement a solution entirely based on that before 19.03 becomes stable.

@Shados
Copy link
Contributor Author

Shados commented Dec 9, 2018

All good, I did get a message from you on IRC, but not I think your initial one. And yeah, it looks like appendOverlays is only in unstable at this point.

@joepie91
Copy link

What's the current status of this? And now that 19.03 is stable, perhaps it can be reevaluated? :)

@delroth
Copy link
Contributor

delroth commented Jul 22, 2019

Another failure mode: I have a machine definition in which I use nonfree packages, so I put nixpkgs.config.allowUnfree = true; in there. With morph, it gets "ignored".

I'd also like to have a cross-built buildOnly node in my deployment using nixpkgs.crossSystem. This is something that I definitely can't apply to all nodes (unlike allowUnfree which would be harmless).

@leotaku
Copy link

leotaku commented Aug 6, 2019

I have a similar problem, which is blocking me from switching to morph.

My “fleet” is made up of machines with different architectures, which I can achieve using NixOps with:

nixpkgs.system = "aarch64-linux";

This option ofc unfortunately gets ignored by morph, causing the deployment to fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants