Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 27 additions & 21 deletions modules/nixpkgs.nix
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think additions to this module are necessary.

Original file line number Diff line number Diff line change
@@ -1,26 +1,32 @@
#
# Nixpkgs module. The only exception to the rule.
#
# Provides a `pkgs` argument in `perSystem`.
#
# Arguably, this shouldn't be in flake-parts, but in nixpkgs.
# Nixpkgs could define its own module that does this, which would be
# a more consistent UX, but for now this will do.
#
# The existence of this module does not mean that other flakes' logic
# will be accepted into flake-parts, because it's against the
# spirit of Flakes.
#
topLevel@{ config, options, inputs, lib, flake-parts-lib, ... }:
let
inherit (flake-parts-lib) mkPerSystemOption;

in
{
config = {
perSystem = { inputs', lib, ... }: {
config = {
_module.args.pkgs = lib.mkOptionDefault (
builtins.seq
(inputs'.nixpkgs or (throw "flake-parts: The flake does not have a `nixpkgs` input. Please add it, or set `perSystem._module.args.pkgs` yourself."))
inputs'.nixpkgs.legacyPackages
);
};
};
imports = [ inputs.nixpkgs.flakeModules.default ];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We shouldn't make assumptions about the inputs, especially not in the core of flake parts.


options = {
perSystem = mkPerSystemOption ({ config, system, ... }:
let
finalOverlay = lib.composeManyExtensions
# Last element has priority, so `perSystem` overlays rule
(topLevel.config.nixpkgs.overlays ++ config.nixpkgs.overlays);

finalPkgs = config.nixpkgs.mkPkgsFromArgs {
localSystem = { inherit system; };
overlays = [ finalOverlay ];
};

in
{
imports = [ inputs.nixpkgs.flakeModules.default ];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As a general rule, modules are imported at the top level only. This makes importing more consistent, not having to wonder where to import it.
Maybe this was added by mistake? Maybe it doesn't even raise an error, as the laziness can mask problems sometimes; a blessing and a curse.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I thought this would be needed to expose the nixpkgs option to the perSystem evaluation scope. Will experiment with removing it now.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see you've avoided the perSystem option there. I think users would expect the Nixpkgs to be in perSystem.

See my comment above. Am I wrong? I'm using nixpkgs option inside the perSystem scope here

_file = ./nixpkgs.nix;
config = {
_module.args.pkgs = lib.mkForce finalPkgs;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would give this responsibility to nixpkgs.flakeModules.default, and leave the original default in place for the time being.

};
});
};
}