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 12 commits into
base: main
Choose a base branch
from

Conversation

MattSturgeon
Copy link
Member

@MattSturgeon MattSturgeon commented Dec 23, 2024

This PR finally gets the nixpkgs module to a fully functional state. If nixpkgs.pkgs is not defined, then an instance of nixpkgs is constructed from nixpkgs.source.

Constructing nixpkgs also requires knowing which system to target, so the hostPlatform and buildPlatform options are included.

I also included the nixpkgs.config option. This could just be attrsOf 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 #2740, #2670, #2430, and #2301
Partially split into #2835

Closes #2022
Closes #2437
Fixes #1784

Marking as a draft for now, still need to:

  • add some test cases
  • proof-read all nixpkgs.* option docs, new & existing

Future work

This will allow us to have a non-per-system replacement for makeNixvimWithModule and somewhat simplify the standalone template

In 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.

@MattSturgeon MattSturgeon marked this pull request as ready for review December 23, 2024 16:19
@MattSturgeon MattSturgeon requested a review from a team December 23, 2024 16:39
@MattSturgeon MattSturgeon force-pushed the nixpkgs_complete branch 4 times, most recently from e32c022 to 737ff93 Compare December 23, 2024 17:16
@MattSturgeon MattSturgeon requested a review from a team December 23, 2024 17:37
lib/modules.nix Outdated Show resolved Hide resolved
lib/tests.nix Show resolved Hide resolved
lib/tests.nix Show resolved Hide resolved
wrappers/_shared.nix Outdated Show resolved Hide resolved
modules/top-level/nixpkgs.nix Show resolved Hide resolved
tests/main.nix Outdated Show resolved Hide resolved
tests/test-sources/modules/nixpkgs.nix Outdated Show resolved Hide resolved
Comment on lines 13 to 15

> [!TIP]
> The host configuration is usually home-manager, nixos, or nix-darwin.
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Trivial with the existing meta.config.name option that was added for the per-platform option-docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

image
image
image

modules/top-level/nixpkgs.nix Show resolved Hide resolved
tests/nixpkgs-module.nix Show resolved Hide resolved
@khaneliman
Copy link
Contributor

khaneliman commented Jan 16, 2025

Added some comments to matrix that I'll move here:

while calling the 'intersectAttrs' builtin
         at /nix/store/4hpdrd3qvj7nks3rrimqm2jdmcga8isc-source/lib/customisation.nix:219:17:
          218|       # This includes automatic ones and ones passed explicitly
          219|       allArgs = intersectAttrs fargs autoArgs // args;
             |                 ^
          220|in the right operand of the update (//) operator
         at /nix/store/4hpdrd3qvj7nks3rrimqm2jdmcga8isc-source/pkgs/top-level/splice.nix:152:54:
          151|
          152|   newScope = extra: lib.callPackageWith (pkgsForCall // extra);
             |                                                      ^
          153|in the left operand of the update (//) operator
         at /nix/store/4hpdrd3qvj7nks3rrimqm2jdmcga8isc-source/lib/customisation.nix:649:32:
          648|       };
          649|       spliced = extra spliced0 // spliced0 // keep self;
             |                                ^
          650|       self = f self // {

       … from call site
         at /nix/store/4hpdrd3qvj7nks3rrimqm2jdmcga8isc-source/lib/customisation.nix:649:17:
          648|       };
          649|       spliced = extra spliced0 // spliced0 // keep self;
             |                 ^
          650|       self = f self // {

       … while calling 'extra'
         at /nix/store/4hpdrd3qvj7nks3rrimqm2jdmcga8isc-source/pkgs/top-level/darwin-packages.nix:48:11:
           47|   otherSplices = generateSplicesForMkScope "darwin";
           48|   extra = spliced: spliced.apple_sdk.frameworks;
             |           ^
           49|   f = lib.extends aliases (

       … while evaluating the attribute 'apple_sdk.frameworks'
         at /nix/store/4hpdrd3qvj7nks3rrimqm2jdmcga8isc-source/pkgs/top-level/darwin-packages.nix:90:13:
           89|           inherit
           90|             apple_sdk
             |             ^
           91|             apple_sdk_10_12

       … while calling the 'throw' builtin
         at /nix/store/4hpdrd3qvj7nks3rrimqm2jdmcga8isc-source/pkgs/top-level/darwin-packages.nix:85:15:
           84|         .${stdenv.hostPlatform.darwinSdkVersion}
           85|           or (throw "Unsupported sdk: ${stdenv.hostPlatform.darwinSdkVersion}");
             |               ^
           86|

       error: Unsupported sdk: 11.3

The line of code is present in the sourced nixpkgs archive in the test

    # Test that a nixpkgs revision can be specified using `nixpkgs.source`
  (testModule "nixpkgs-source" (
    { pkgs, ... }:
    {     
      nixpkgs.source = builtins.fetchTarball {
        url = "https://github.com/NixOS/nixpkgs/archive/1807c2b91223227ad5599d7067a61665c52d1295.tar.gz";
        sha256 = "0xnj86751780hg1zhx9rjkbjr0sx0wps4wxz7zryvrj6hgwrng1z";
      };

      nixpkgs.hostPlatform = {
        inherit (stdenv.hostPlatform) system;
      };

      assertions = [
        {
          assertion = pkgs.lib.version == "24.11pre-git";
          message = ''
            Expected `pkgs.lib.version` to be "24.11pre-git", but found "${pkgs.lib.version}"
          '';
        }
      ];
    }
  ))

Is it a mismatch between the pkgs coming in for the test case and the nixpkgs.source being used in the test? Does it show an issue with the impelmentation? Or would that be an expected failure?

The earliest commit that would avoid this error:

      nixpkgs.source = builtins.fetchTarball {
        url = "https://github.com/NixOS/nixpkgs/archive/0f9067f865a22aa6c1fe94eed7f218287b3e9425.tar.gz";
        sha256 = "0bq40zax9s3hcw901qm91cxh7z6q5czvyz1814gvg3bgii0ndavv";
      };

Rather than getting `runCommand` from the pkgs instance under test, get
the function from a fixed/consistent pkgs instance.
Instead of importing the full set of top-level nixvim modules, only
import the nixpkgs-module and its direct dependencies.
@MattSturgeon
Copy link
Member Author

Thanks for the help @khaneliman

I've reduced flakyness by testing with a mock nixpkgs source rather than downloading a real instance.

I've also narrowed the scope of the tests to focus on only the nixpkgs module and its dependencies. That way we aren't accidentally using the pkgs instance elsewhere in unrelated modules.

The testWrappers tests still test against a full nixvim configuration with all modules, due to them using the actual wrapper modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

modules/misc: nixpkgs module
3 participants