modules/nixpkgs: complete the MVP #2738
Conversation
MattSturgeon
force-pushed
the
nixpkgs_complete
branch
from
acb3694 to
b607b1d
Compare
MattSturgeon
force-pushed
the
nixpkgs_complete
branch
from
5935e30 to
a5044b0
Compare
MattSturgeon
force-pushed
the
nixpkgs_complete
branch
4 times, most recently
from
e32c022 to
737ff93
Compare
MattSturgeon
force-pushed
the
nixpkgs_complete
branch
from
737ff93 to
1e2b758
Compare
| # 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? |
There was a problem hiding this comment.
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
| @@ -30,7 +29,8 @@ let | |||
| mkTestDerivationFromNixvimModule = | |||
| { | |||
| name ? null, | |||
| pkgs ? defaultPkgs, | |||
| pkgs ? null, | |||
There was a problem hiding this comment.
We do we keep the pkgs argument? For backwards compatibility? You should be able to do the same thing by setting nixpkgs.pkgs right?
There was a problem hiding this comment.
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.
| # TODO: Only do this when `args?pkgs` | ||
| # Consider deprecating the `pkgs` arg too... |
There was a problem hiding this comment.
I think it's a good idea to deprecate that :D
| nixpkgs = { | ||
| # Use global packages by default in nixvim's submodule | ||
| # FIXME: probably doesn't need to be mkForce | ||
| # What should happen if the user configures both `nixpkgs.pkgs` and `nixpkgs.useGlobalPackages` ? |
There was a problem hiding this comment.
I think that we should either raise a warning and ignore one of them (I don't know which ...) or raise an error
| > [!CAUTION] | ||
| > This default will be removed in a future version of nixvim | ||
| ''; |
There was a problem hiding this comment.
Should probably move this admonition to the useGlobalPackages option.
| nixpkgs.config = { | ||
| allowUnfree = true; | ||
| }; |
There was a problem hiding this comment.
In a future PR, this should be moved to whichever tests actually require it.
| # Test that a nixpkgs revision can be specified using `nixpkgs.source` | ||
| source = |
There was a problem hiding this comment.
Let's also test that config has an effect
|
|
||
| > [!TIP] | ||
| > The host configuration is usually home-manager, nixos, or nix-darwin. |
There was a problem hiding this comment.
Instead of a generic "tip", this could actually use a context option to dynamically produce platform-specific docs. This is possible because we actually show the per-platform options separately.
Probably out of scope for this PR though.
configoptionhostPlatform&buildPlatformoptionsnixpkgs.sourceuseGlobalPackagesoptionsystemas anevalNixvimargpkgsarg optional, allow specifyingsystemThis PR finally gets the
nixpkgsmodule to a fully functional state. Ifnixpkgs.pkgsis not defined, then an instance of nixpkgs is constructed fromnixpkgs.source.Constructing nixpkgs also requires knowing which system to target, so the
hostPlatformandbuildPlatformoptions are included.I also included the
nixpkgs.configoption. This could just beattrsOf anything, but the upstream otpion-type is a bit more complex because it allows definitions to be either attrs or functions-to-attrs.Directly based on https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/misc/nixpkgs.nix
Follow up to #2670, #2430, and #2301
Closes #2022
Closes #2437
Fixes #1784
Marking as a draft for now, still need to:
nixpkgs.*option docs, new & existingFuture work
This will allow us to have a non-per-system replacement for
makeNixvimWithModuleand somewhat simplify the standalone templateIn the future we will also be able to change the default value of
nixpkgs.useGlobalPackages. I'm not 100% sure I know a good way to warn about that planned transition though.There's also a bunch of assertions in the upstream module that I have not ported over yet.