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

Conversation

Gabriella439
Copy link
Contributor

@Gabriella439 Gabriella439 commented Jan 20, 2024

Closes #193336
Closes #261694
Related to #108984

This pull request is a composite of the changes from #193336 and also #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:

{ inputs.nixpkgs.url = "github:Gabriella439/nixpkgs/gabriella/macOS_NixOS_test";

  outputs = { nixpkgs, ... }: {
    checks.aarch64-darwin.default =
      nixpkgs.legacyPackages.aarch64-darwin.nixosTest {
        name = "test";

        nodes.machine = { };

        testScript = ''
          print(machine.succeed('echo hello'));
        '';
      };
  };
}

… 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 #261694 than the approach in #193336. However, I still made a few changes compared to #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 Run nixosTests on darwin #193336.

  • I didn't use the node.pkgs setting to set nodes.*.nixpkgs.pkgs

    That does not work, based on my testing, but setting nixpkgs.system in nixos/lib/testing/nodes.nix does work with some supporting changes to pkgs/build-support/testers/default.nix.

Description of changes

Things done


Add a 👍 reaction to pull requests you find important.

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`.
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jan 20, 2024
@cole-h
Copy link
Member

cole-h commented Jan 20, 2024

@ofborg eval

cole-h
cole-h previously requested changes Jan 21, 2024
pkgs/top-level/stage.nix Outdated Show resolved Hide resolved
pkgs/top-level/stage.nix Outdated Show resolved Hide resolved
nixos/lib/testing/macos-host.nix Outdated Show resolved Hide resolved
nixos/lib/testing/nodes.nix Outdated Show resolved Hide resolved
nixos/lib/testing/nodes.nix Outdated Show resolved Hide resolved
nixos/lib/testing/nodes.nix Show resolved Hide resolved
nixos/lib/testing/nodes.nix Outdated Show resolved Hide resolved
@roberth
Copy link
Member

roberth commented Jan 21, 2024

  • I named the required system feature hvf instead of apple-virt

I don't see this in the code.

apple-virt is autodetected by Nix 2.19 and should have been backported to 2.18, but that hasn't been released yet.

I've chosen this name because the detection appears to be avf/hvf agnostic.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/getting-nixos-to-work-macos-m1-2-3-aarch64-darwin/39510/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/getting-nixos-to-work-macos-m1-2-3-aarch64-darwin/39510/2

Gabriella439 and others added 4 commits February 16, 2024 13:55
… as suggested by @roberth

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
… as suggested by @roberth

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
… as suggsted by @roberth
pkgs/top-level/stage.nix Outdated Show resolved Hide resolved
nixos/lib/testing/nodes.nix Outdated Show resolved Hide resolved
nixos/lib/testing/nodes.nix Outdated Show resolved Hide resolved
@Gabriella439
Copy link
Contributor Author

Yeah, I'm planning on getting to fixing the nixpkgs.pkgs = guestPkgs;. I was just puzzling through the best way to do it.

… as caught by @robert

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
… 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.
This is no longer necessary now that we're overriding `nixpkgs.system`
instead of `nixpkgs.pkgs`
@Gabriella439
Copy link
Contributor Author

@roberth: Alright, I think I finally understood what you were proposing and implemented that. The main caveat is that I had to get rid of the nixpkgs.pkgs = lib.mkDefault pkgs; in the nixosTest entry point because that breaks this (and some other stuff, too) since defining nixpkgs.pkgs (even just as a default) causes all other nixpkgs.* options to be ignored. I don't know if making that change to the nixosTest entry point will break other stuff, though.

@roberth roberth self-requested a review March 1, 2024 17:21
@Gabriella439
Copy link
Contributor Author

@roberth: Thanks! Your changes look good to me

@tfc
Copy link
Contributor

tfc commented Mar 1, 2024

Works great!

@Gabriella439
Copy link
Contributor Author

Gabriella439 commented Mar 1, 2024

I can go ahead and merge this once CI passes if nobody objects since I think all of @roberth and @tfc and I are satisfied with the final set of changes

@Gabriella439
Copy link
Contributor Author

Is it normal for ofborg-eval to take this long? I'm not sure how I can monitor its progress

@Gabriella439
Copy link
Contributor Author

@ofborg eval

# If already linux: the same package set unaltered
# Otherwise, return a natively built linux package set for the current cpu architecture string.
# (ABI and other details will be set to the default for the cpu/os pair)
pkgsLinux =
Copy link
Member

@cole-h cole-h Mar 2, 2024

Choose a reason for hiding this comment

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

I think this might be running into the same issue as when you were using pkgsLocal (#282401 (comment)). ofborg might be happier if you exclude pkgsLinux from https://github.com/NixOS/nixpkgs/blob/1b439a273dadd4c675e81a0e05abbc9258889c02/pkgs/top-level/release-attrpaths-superset.nix

(if the label comes back, that'll be confirmation, but I don't think it'd be harmful to add it to that list before that happens)

@Gabriella439 Gabriella439 merged commit b8698cd into NixOS:master Mar 2, 2024
21 checks passed
@Gabriella439
Copy link
Contributor Author

Thanks to everybody for y'all's help!

@414owen
Copy link
Contributor

414owen commented Mar 4, 2024

Hi @Gabriella439 ,

Thanks for working on this.
I've put your flake from the PR description into an empty git repo, and run nix flake check, making sure I'm on nix 2.19.2.

Doing so gives me:

$ nix flake check
warning: Git tree '/Users/owen/src/nixos-test-darwin' is dirty
warning: Ignoring setting 'auto-allocate-uids' because experimental feature 'auto-allocate-uids' is not enabled
warning: Ignoring setting 'impure-env' because experimental feature 'configurable-impure-env' is not enabled
error: a 'aarch64-linux' with features {} is required to build '/nix/store/nv3ziyvxfnyl4z0ayg9j25kvgp27ziyr-append-initrd-secrets.drv', but I am a 'aarch64-darwin' with features {apple-virt, benchmark, big-parallel, nixos-test}

I'm not sure how to go about debugging this. I can list the referrers, but it just tells me that there's a nixos-system-machine-test that needs it.

Is there a step I've missed? I got the impression that I don't need to specify pkgs in the machine configuration anymore? Is it something to do with hvf/apple-virt?

@tfc
Copy link
Contributor

tfc commented Mar 4, 2024

@414owen your mac seems to be all set to execute NixOS integration tests. It still needs a linux builder to build the NixOS guest systems. See https://nixcademy.com/2024/02/12/macos-linux-builder/

ReedClanton pushed a commit to ReedClanton/nixpkgs that referenced this pull request Mar 4, 2024
Closes NixOS#193336
Closes NixOS#261694
Related to NixOS#108984

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.
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/macos-support-for-running-nixos-tests/40801/1

@roberth
Copy link
Member

roberth commented Mar 5, 2024

Doc update, please review:

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixostest-vs-runtest/26419/4

@Gabriella439
Copy link
Contributor Author

@tfc: I was able to reproduce the file limit bug that you were originally trying to address and root cause the problem!

It turns out that the problem is caused by an old (pre-2.5) installation of multi-user Nix. In Nix 2.5 the file limit was raised for the nix-daemon to 4096 by NixOS/nix#5726 (and then later further raised to 1048576 in Nix 2.13 by NixOS/nix#6645).

The easiest way to verify the presence or absence of the problem is to check the /Library/LaunchDaemons/org.nixos.nix-daemon.plist file and to see if there is something increasing the file limit like this:

    <key>SoftResourceLimits</key>
    <dict>
      <key>NumberOfFiles</key>
      <integer>4096</integer>
    </dict>

If that's present then it means you're likely unaffected by this bug. However, if that's absent (typically because Nix was originally installed at version older than 2.5) then you're probably going to be affected by that bug.

If anyone is reading this and wondering how to fix the problem, the solution is to edit /Library/LaunchDaemons/org.nixos.nix-daemon.plist to match https://github.com/NixOS/nix/blob/master/misc/launchd/org.nixos.nix-daemon.plist.in and then restart the nix-daemon:

$ sudo launchctl kickstart -k system/org.nixos.nix-daemon

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

Successfully merging this pull request may close these issues.

6 participants