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

modules/nixpkgs: complete the MVP #2738

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
4 changes: 3 additions & 1 deletion flake-modules/lib.nix
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
{ pkgs, system, ... }:
{
# NOTE: this is the publicly documented flake output we've had for a while
check = pkgs.callPackage ../lib/tests.nix { inherit self; };
check = pkgs.callPackage ../lib/tests.nix {
inherit lib self system;
};

# NOTE: no longer needs to be per-system
helpers = lib.warn "nixvim: `<nixvim>.lib.${system}.helpers` has been moved to `<nixvim>.lib.nixvim` and no longer depends on a specific system" self.lib.nixvim;
Expand Down
12 changes: 10 additions & 2 deletions flake-modules/wrappers.nix
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
{ inputs, self, ... }:
{
inputs,
self,
lib,
...
}:
{
perSystem =
{ system, pkgs, ... }:
{
_module.args = {
makeNixvimWithModule = import ../wrappers/standalone.nix pkgs self;
makeNixvimWithModule = import ../wrappers/standalone.nix {
inherit lib self;
defaultSystem = system;
};
};

checks =
Expand Down
10 changes: 10 additions & 0 deletions lib/modules.nix
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,29 @@ in
{
modules ? [ ],
extraSpecialArgs ? { },
system ? null, # Can also be defined using the `nixpkgs.hostPlatform` option
}:
# Ensure a suitable `lib` is used
# TODO: offer a lib overlay that end-users could use to apply nixvim's extensions to their own `lib`
assert lib.assertMsg (extraSpecialArgs ? lib -> extraSpecialArgs.lib ? nixvim) ''
Nixvim requires a lib that includes some custom extensions, however the `lib` from `specialArgs` does not have a `nixvim` attr.
Remove `lib` from nixvim's `specialArgs` or ensure you apply nixvim's extensions to your `lib`.'';
assert lib.assertMsg (system != null -> lib.isString system) ''
When `system` is supplied to `evalNixvim`, it must be a string.
To define a more complex system, please use nixvim's `nixpkgs.hostPlatform` option.'';
lib.evalModules {
modules = modules ++ [
../modules/top-level
{
_file = "<nixvim-flake>";
flake = lib.mkOptionDefault flake;
}
(lib.optionalAttrs (lib.isString system) {
_file = "evalNixvim";
# FIXME: what priority should this have?
# If the user has set it as an evalNixvim arg _and_ a module option what behaviour would they expect?
Comment on lines +43 to +44
Copy link
Member

Choose a reason for hiding this comment

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

I think the implemented behaviour is the correct one, as the module options seems the best place to implement dynamic behaviour, whereas evalNixvim would likely be quite static. In that sense I would say that the more dynamic way should override the more static one if that makes sens

nixpkgs.hostPlatform = lib.mkOptionDefault { inherit system; };
})
];
specialArgs = {
inherit lib;
Expand Down
20 changes: 12 additions & 8 deletions lib/tests.nix
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
{
self,
pkgs,
lib ? pkgs.lib,
...
system,
lib,
}:
let
defaultPkgs = pkgs;
defaultSystem = system;

# Create a nix derivation from a nixvim executable.
# The build phase simply consists in running the provided nvim binary.
Expand All @@ -30,7 +29,8 @@ let
mkTestDerivationFromNixvimModule =
{
name ? null,
pkgs ? defaultPkgs,
pkgs ? null,
Copy link
Member

Choose a reason for hiding this comment

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

We do we keep the pkgs argument? For backwards compatibility? You should be able to do the same thing by setting nixpkgs.pkgs right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, for backwards compatibility. I've tried to make this a non-breaking change.

IMO rather than deprecating specific args, it makes more sense to deprecate all functions in lib/tests.nix and wrappers/standalone.nix. That way they can be replaced by functions that have no baggage, do not need to be per-system in the flake output, and can more closely wrap the underlying evalNixvim.

I'm not ready to do that in this PR, and it's already large enough as it is. Before deprecating the per-system functions, I think we should move the "standalone package" into the module configuration; modules/top-level/output.nix.

system ? defaultSystem,
module,
extraSpecialArgs ? { },
}:
Expand All @@ -42,14 +42,18 @@ let
_nixvimTests = true;
};

systemMod =
if pkgs == null then
{ nixpkgs.hostPlatform = lib.mkDefault { inherit system; }; }
else
{ nixpkgs.pkgs = lib.mkDefault pkgs; };

result = helpers.modules.evalNixvim {
modules = [
module
(lib.optionalAttrs (name != null) { test.name = name; })
{ wrapRc = true; }
# TODO: Only do this when `args?pkgs`
# Consider deprecating the `pkgs` arg too...
Comment on lines -50 to -51
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a good idea to deprecate that :D

{ nixpkgs.pkgs = lib.mkDefault pkgs; }
systemMod
];
inherit extraSpecialArgs;
};
Expand Down
175 changes: 128 additions & 47 deletions modules/top-level/nixpkgs.nix
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,46 @@
config,
options,
lib,
pkgs,
...
}:
let
cfg = config.nixpkgs;
opt = options.nixpkgs;

isConfig = x: builtins.isAttrs x || lib.isFunction x;

mergeConfig =
lhs_: rhs_:
let
optCall = maybeFn: x: if lib.isFunction maybeFn then maybeFn x else maybeFn;
lhs = optCall lhs_ { inherit pkgs; };
rhs = optCall rhs_ { inherit pkgs; };
in
lib.recursiveUpdate lhs rhs
// lib.optionalAttrs (lhs ? packageOverrides) {
packageOverrides =
pkgs:
optCall lhs.packageOverrides pkgs // optCall (lib.attrByPath [ "packageOverrides" ] { } rhs) pkgs;
}
// lib.optionalAttrs (lhs ? perlPackageOverrides) {
perlPackageOverrides =
pkgs:
optCall lhs.perlPackageOverrides pkgs
// optCall (lib.attrByPath [ "perlPackageOverrides" ] { } rhs) pkgs;
};
in
{
options.nixpkgs = {
pkgs = lib.mkOption {
# TODO:
# defaultText = lib.literalExpression ''
# import "''${nixos}/.." {
# inherit (cfg) config overlays localSystem crossSystem;
# }
# '';
defaultText = lib.literalMD ''
The `pkgs` inherited from your host config (i.e. NixOS, home-manager, or nix-darwin),
or the `pkgs` supplied to `makeNixvimWithModule` when building a standalone nixvim.

> [!CAUTION]
> This default will be removed in a future version of nixvim
'';
Comment on lines -24 to -26
Copy link
Member Author

Choose a reason for hiding this comment

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

Should probably move this admonition to the useGlobalPackages option.

type = lib.types.pkgs // {
description = "An evaluation of Nixpkgs; the top level attribute set of packages";
};
example = lib.literalExpression "import <nixpkgs> { }";
description = ''
If set, the `pkgs` argument to all Nixvim modules is the value of this option.

<!-- TODO: remove -->
If unset, an assertion will trigger. In the future a `pkgs` instance will be constructed.

<!--
TODO:
If unset, the pkgs argument is determined as shown in the default value for this option.

TODO:
The default value imports the Nixpkgs input specified in Nixvim's `flake.lock`.
The `config`, `overlays`, `localSystem`, and `crossSystem` come from this option's siblings.
-->
If unset, the `pkgs` argument is determined by importing `nixpkgs.source`.

This option can be used by external applications to increase the performance of evaluation,
or to create packages that depend on a container that should be built with the exact same
Expand All @@ -56,6 +56,29 @@ in
'';
};

config = lib.mkOption {
default = { };
example = {
allowBroken = true;
allowUnfree = true;
};
type = lib.mkOptionType {
name = "nixpkgs-config";
description = "nixpkgs config";
check = x: isConfig x || lib.traceSeqN 1 x false;
merge = args: lib.foldr (def: mergeConfig def.value) { };
};
description = ''
Global configuration for Nixpkgs.
The complete list of [Nixpkgs configuration options] is in the [Nixpkgs manual section on global configuration].

Ignored when {option}`nixpkgs.pkgs` is set.

[Nixpkgs configuration options]: https://nixos.org/manual/nixpkgs/unstable/#sec-config-options-reference
[Nixpkgs manual section on global configuration]: https://nixos.org/manual/nixpkgs/unstable/#chap-packageconfig
'';
};

overlays = lib.mkOption {
type =
let
Expand Down Expand Up @@ -114,15 +137,55 @@ in

For details, see the [Overlays chapter in the Nixpkgs manual](https://nixos.org/manual/nixpkgs/stable/#chap-overlays).

<!-- TODO: Remove -->
Overlays specified using the {option}`nixpkgs.overlays` option will be
applied after the overlays that were already included in `nixpkgs.pkgs`.
If the {option}`nixpkgs.pkgs` option is set, overlays specified using `nixpkgs.overlays`
will be applied after the overlays that were already included in `nixpkgs.pkgs`.
'';
};

hostPlatform = lib.mkOption {
type = with lib.types; either str attrs;
example = {
system = "aarch64-linux";
};
apply = lib.systems.elaborate;
defaultText = lib.literalMD ''
Inherited from the "host" configuration's `pkgs`.
Or the `evalNixvim`'s `system` argument.
Otherwise, must be specified manually.
'';
description = ''
Specifies the platform where the Nixvim configuration will run.

To cross-compile, also set `nixpkgs.buildPlatform`.

<!--
TODO:
If the {option}`nixpkgs.pkgs` option is set, overlays specified using `nixpkgs.overlays`
will be applied after the overlays that were already included in `nixpkgs.pkgs`.
-->
Ignored when `nixpkgs.pkgs` is set.
'';
};

buildPlatform = lib.mkOption {
type = with lib.types; either str attrs;
default = cfg.hostPlatform;
example = {
system = "x86_64-linux";
};
apply =
value:
let
elaborated = lib.systems.elaborate value;
in
# If equivalent to `hostPlatform`, make it actually identical so that `==` can be used
# See https://github.com/NixOS/nixpkgs/issues/278001
if lib.systems.equals elaborated cfg.hostPlatform then cfg.hostPlatform else elaborated;
defaultText = lib.literalMD ''
Inherited from the "host" configuration's `pkgs`.
Or `config.nixpkgs.hostPlatform` when building a standalone nixvim.
'';
description = ''
Specifies the platform on which Nixvim should be built.
By default, Nixvim is built on the system where it runs, but you can change where it's built.
Setting this option will cause Nixvim to be cross-compiled.

Ignored when `nixpkgs.pkgs` is set.
'';
};

Expand All @@ -136,26 +199,33 @@ in

Ignored when `nixpkgs.pkgs` is set.
'';

# FIXME: This is a stub option for now
internal = true;
};
};

config =
let
# TODO: construct a default pkgs instance from pkgsPath and cfg options
# https://github.com/nix-community/nixvim/issues/1784

finalPkgs =
if opt.pkgs.isDefined then
cfg.pkgs.appendOverlays cfg.overlays
else
# TODO: Remove once pkgs can be constructed internally
throw ''
nixvim: `nixpkgs.pkgs` is not defined. In the future, this option will be optional.
Currently a pkgs instance must be evaluated externally and assigned to `nixpkgs.pkgs` option.
'';
let
args = {
inherit (cfg) config overlays;
};

# Configure `localSystem` and `crossSystem` as required
systemArgs =
if cfg.buildPlatform == cfg.hostPlatform then
{
localSystem = cfg.hostPlatform;
}
else
{
localSystem = cfg.buildPlatform;
crossSystem = cfg.hostPlatform;
};
in
import cfg.source (args // systemArgs);
in
{
# We explicitly set the default override priority, so that we do not need
Expand All @@ -166,9 +236,20 @@ in
# don't need to evaluate `finalPkgs`.
_module.args.pkgs = lib.mkOverride lib.modules.defaultOverridePriority finalPkgs.__splicedPackages;

# FIXME: This is a stub option for now
warnings = lib.optional (
opt.source.isDefined && opt.source.highestPrio < (lib.mkOptionDefault null).priority
) "Defining the option `nixpkgs.source` currently has no effect";
assertions = [
{
assertion = opt.pkgs.isDefined -> cfg.config == { };
message = ''
Your system configures nixpkgs with an externally created instance.
`nixpkgs.config` options should be passed when creating the instance instead.

Current value:
${lib.generators.toPretty { multiline = true; } cfg.config}

Defined in:
${lib.concatMapStringsSep "\n" (file: " - ${file}") opt.config.files}
'';
}
];
};
}
4 changes: 2 additions & 2 deletions templates/simple/flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
];

perSystem =
{ pkgs, system, ... }:
{ system, ... }:
let
nixvimLib = nixvim.lib.${system};
nixvim' = nixvim.legacyPackages.${system};
nixvimModule = {
inherit pkgs;
inherit system; # or alternatively, set `pkgs`
module = import ./config; # import the module directly
# You can use `extraSpecialArgs` to pass additional arguments to your module files
extraSpecialArgs = {
Expand Down
9 changes: 6 additions & 3 deletions tests/main.nix
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
lib ? pkgs.lib,
linkFarm,
pkgs,
pkgsUnfree,
self,
system,
}:
let
fetchTests = callTest ./fetch-tests.nix { };
test-derivation = callPackage ../lib/tests.nix { inherit self; };
test-derivation = callPackage ../lib/tests.nix {
inherit lib self system;
};
inherit (test-derivation) mkTestDerivationFromNixvimModule;

moduleToTest =
Expand All @@ -27,8 +28,10 @@ let
inherit self system;
inherit (self) inputs;
};
nixpkgs.config = {
allowUnfree = true;
};
Comment on lines +31 to +33
Copy link
Member Author

Choose a reason for hiding this comment

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

In a future PR, this should be moved to whichever tests actually require it.

};
pkgs = pkgsUnfree;
};

# List of files containing configurations
Expand Down
Loading