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

macOS support for NixOS tests #282401

Merged
merged 21 commits into from
Mar 2, 2024

Commits on Jan 20, 2024

  1. macOS support for NixOS tests

    Closes NixOS#193336
    Closes NixOS#261694
    Related to 108984
    
    This pull request is a composite of the changes from NixOS#193336 and
    also NixOS#261694, with some changes of my own that I made.
    
    The goal here was to get the following flake to build and run on
    `aarch64-darwin`:
    
    ```nix
    { inputs.nixpkgs.url = <this branch>;
    
      outputs = { nixpkgs, ... }: {
        checks.aarch64-darwin.default =
          nixpkgs.legacyPackages.aarch64-darwin.nixosTest {
            name = "test";
    
            nodes.machine = { };
    
            testScript = "";
          };
      };
    }
    ```
    
    … and after this change it does.  There's no longer a need for the
    user to set `nodes.*.nixpkgs.pkgs` or
    `nodes.*.virtualisation.host.pkgs` as the correct values are inferred
    from the host system.
    
    This change is spiritually much closer to the approach in NixOS#261694
    than the approach in NixOS#193336.  However, I still made a few changes
    compared to NixOS#261694:
    
    - I didn't include the change to increase `ulimit`
    
      I think this change was questionable because I feel like a script
      provided by Nixpkgs shouldn't be tinkering with the user's ambient
      `ulimit` settings.
    
    - I named the required system feature `hvf` instead of `apple-virt`
    
      I preferred the `hvf` system feature name chosen by NixOS#193336.
    
    - I didn't use the `node.pkgs` setting to set `nodes.*.nixpkgs.pkgs`
    
      That does not work, based on my testing, but setting the same thing
      in `nixos/lib/testing/nodes.nix` does work.
    
    - I created a `pkgsLocal` helper
    
      This is based on the feedback from @roberth here:
    
      https://github.com/NixOS/nixpkgs/pull/261694/files#r1365443955
    
      … and is similar in spirit to `pkgsCross`.
    Gabriella439 committed Jan 20, 2024
    Configuration menu
    Copy the full SHA
    b0ac7f2 View commit details
    Browse the repository at this point in the history

Commits on Feb 16, 2024

  1. Improve error message

    … as suggested by @roberth
    
    Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
    Gabriella439 and roberth committed Feb 16, 2024
    Configuration menu
    Copy the full SHA
    6c013e3 View commit details
    Browse the repository at this point in the history
  2. Remove unused pkgs parameter

    … as suggested by @roberth
    
    Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
    Gabriella439 and roberth committed Feb 16, 2024
    Configuration menu
    Copy the full SHA
    9aa8994 View commit details
    Browse the repository at this point in the history
  3. s/pkgsLocal/pkgsNative/

    … as suggsted by @roberth
    Gabriella439 committed Feb 16, 2024
    Configuration menu
    Copy the full SHA
    ccbfcca View commit details
    Browse the repository at this point in the history
  4. Exclude pkgsNative from build

    … as caught by @cole-h
    Gabriella439 committed Feb 16, 2024
    Configuration menu
    Copy the full SHA
    2bc4a79 View commit details
    Browse the repository at this point in the history
  5. Fix typo

    … as caught by @robert
    
    Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
    Gabriella439 and roberth committed Feb 16, 2024
    Configuration menu
    Copy the full SHA
    21f534a View commit details
    Browse the repository at this point in the history

Commits on Feb 21, 2024

  1. Specify guest *system* instead of guest *packages*

    … based on feedback from @roberth
    
    This plays nicer with other changes the user might want to make and
    also works in conjunction with the `pkgsReadOnly`.
    
    Note that this required deleting the `nixpkgs.pkgs` default because
    that's not compatible with setting `nixpkgs.system`.  If `nixpkgs.pkgs`
    is defined (even as a default-valued option) then `nixpkgs.system` is
    ignored.
    Gabriella439 committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    90679ea View commit details
    Browse the repository at this point in the history
  2. Revert the addition of pkgsNative

    This is no longer necessary now that we're overriding `nixpkgs.system`
    instead of `nixpkgs.pkgs`
    Gabriella439 committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    03f2f87 View commit details
    Browse the repository at this point in the history
  3. Remove macos-host.nix

    Based on my local testing these options seem unnecessary and the VM
    runs fine without them being set.  I originally cargo-culted these
    from NixOS#261694 but they don't seem necessary.
    Gabriella439 committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    0870201 View commit details
    Browse the repository at this point in the history

Commits on Feb 22, 2024

  1. Remove unused pkgsNative

    Gabriella439 committed Feb 22, 2024
    Configuration menu
    Copy the full SHA
    ba4e0dd View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    0474c0a View commit details
    Browse the repository at this point in the history

Commits on Feb 28, 2024

  1. Fix test failure

    Specifically, the `nixosTest-example` test correctly flagged that the
    change from 90679ea broke the user's
    expectation that the `pkgs` in `pkgs.nixosTest` would be reused as the
    package set for the NixOS test system.
    
    However, if we restore the original code removed by that commit:
    
    ```nix
    nixpkgs.pkgs = lib.mkDefault pkgs;
    ```
    
    … then that won't work because then we can't override `nixpkgs.system`
    and reinstantiate that package set.  The whole *point* of this set of
    changes is that we DON'T want to use the user's `pkgs` set as is
    because then the wrong system is used for the NixOS test (on macOS).
    Specifically, on Apple Silicon the `pkgs` in `pkgs.nixosTest` is going
    to be instantiated with `system = aarch64-darwin;` but we want the
    `nixpkgs.pkgs` that the NixOS machine uses to be that same package set
    reinstantiated with `system = aarch64-linux;`.
    
    Then I realized we can strike a balance by doing this instead:
    
    ```nix
    nixpkgs.overlays = lib.mkDefault pkgs.overlays;
    nixpkgs.config = lib.mkDefault pkgs.config;
    ```
    
    … then we can get something which splits the difference: we preserve
    the user's original package overrides but now we can reinstantiate
    `nixpkgs.pkgs` with the new (correct) `nixpkgs.system`.
    
    This isn't a foolproof solution, though, because `pkgs.overlays` and
    `pkgs.config` do not necessarily contain all the information to
    recreate the original `pkgs` that the user supplied.  As a concrete
    example, if the user were to extend the package set "artificially" like
    this:
    
    ```nix
    (pkgs // { foo = …; }).nixosTest …
    ```
    
    … then that `pkgs.foo` attribute would no longer be passed along to
    `nixpgs.pkgs`.  However, in practice I believe most users are going to
    be overriding `pkgs` using either the `config.packageOverrides` or
    `overlays` arguments to the Nixpkgs function, so I think this is a
    reasonable compromise.  Moreover, the benefit of this compromise is
    that the NixOS test framework is much more "fire and forget" (we don't
    need to instruct the users to instantiate Nixpkgs themselves twice for
    the host and guest architecture).
    Gabriella439 committed Feb 28, 2024
    Configuration menu
    Copy the full SHA
    ab45d31 View commit details
    Browse the repository at this point in the history

Commits on Mar 1, 2024

  1. nixosTest: Make similar to previous implementation again

    I'd considered suggesting to add a conditional here, but that would
    create VM-host-dependent portability issues for no good reason.
    roberth committed Mar 1, 2024
    Configuration menu
    Copy the full SHA
    5da6236 View commit details
    Browse the repository at this point in the history
  2. pkgs.pkgsLinux: init

    roberth committed Mar 1, 2024
    Configuration menu
    Copy the full SHA
    724e021 View commit details
    Browse the repository at this point in the history
  3. nixosTest: Use memoized pkgsLinux

    This solves the previously introduced evaluation performance
    regression when evaluating multiple tests at once.
    roberth committed Mar 1, 2024
    Configuration menu
    Copy the full SHA
    2ef8ac2 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    315229b View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    b915914 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    ddab359 View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    3ef5de9 View commit details
    Browse the repository at this point in the history

Commits on Mar 2, 2024

  1. Exclude pkgsLinux

    … as suggested by @cole-h
    Gabriella439 committed Mar 2, 2024
    Configuration menu
    Copy the full SHA
    c73be48 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    39b7626 View commit details
    Browse the repository at this point in the history