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

Revert "NixOS Integration Tests: Enable again for darwin" #303187

Merged

Conversation

cole-h
Copy link
Member

@cole-h cole-h commented Apr 10, 2024

Reverts #303150

This broke ofborg's eval

cc @tfc

$ nix-env -qaP --no-name --out-path --arg checkMeta false --argstr path $PWD -f outpaths.nix --show-trace
error:
       … while evaluating call site

       at «none»:0: (source not available)

       … while calling anonymous lambda

       at /root/nixpkgs/outpaths.nix:48:12:

           47|   tweak = lib.mapAttrs
           48|     (name: val:
             |            ^
           49|       if name == "recurseForDerivations" then true

       … while evaluating call site

       at /root/nixpkgs/lib/attrsets.nix:1214:43:

         1213|     f:
         1214|     listToAttrs (map (n: nameValuePair n (f n)) names);
             |                                           ^
         1215|

       … while calling anonymous lambda

       at /root/nixpkgs/pkgs/top-level/release-lib.nix:145:6:

          144|   testOnCross = crossSystem: metaPatterns: f: forMatchingSystems metaPatterns
          145|     (system: hydraJob' (f (pkgsForCross crossSystem system)));
             |      ^
          146|

       … while evaluating call site

       at /root/nixpkgs/pkgs/top-level/release-lib.nix:145:14:

          144|   testOnCross = crossSystem: metaPatterns: f: forMatchingSystems metaPatterns
          145|     (system: hydraJob' (f (pkgsForCross crossSystem system)));
             |              ^
          146|

       … while calling 'hydraJob'

       at /root/nixpkgs/lib/customisation.nix:388:14:

          387|   */
          388|   hydraJob = drv:
             |              ^
          389|     let

       … while evaluating the attribute 'outPath'

       at /root/nixpkgs/lib/customisation.nix:404:13:

          403|           value = commonAttrs // {
          404|             outPath = output.outPath;
             |             ^
          405|             drvPath = output.drvPath;

       error: attribute 'out' missing

       at /root/nixpkgs/lib/customisation.nix:401:22:

          400|       makeOutput = outputName:
          401|         let output = drv.${outputName}; in
             |                      ^
          402|         { name = outputName;

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Apr 10, 2024
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: testing Tooling for automated testing of packages and modules labels Apr 10, 2024
@cole-h cole-h merged commit dbc967d into master Apr 10, 2024
8 of 9 checks passed
@cole-h cole-h deleted the revert-303150-nixos-integration-tests-macos-platforms branch April 10, 2024 19:52
@tfc
Copy link
Contributor

tfc commented Apr 10, 2024

I don't understand what the problem really is here. Do you have a link to a log file with more context?

Reverting this is bad because it breaks NixOS intefgration tests for macOS users which work just fine.

@cole-h
Copy link
Member Author

cole-h commented Apr 10, 2024

The problem is that your PR causes ofborg to be unable to evaluate Nixpkgs anymore :P

Unfortunately, there is nothing more than what I just edited into the body above. Here's a gist from an actual PR against Nixpkgs that ran into this: https://gist.github.com/GrahamcOfBorg/a5b0c39f39058d8995cf54f554f5aabf

I'm not opposed to your PR at all! I'd just like it to not break ofborg's ability to evaluate PRs against Nixpkgs 😄

(If you have more than 32GiB of RAM, you can reproduce the issue with the same command I put in the PR body, where outpaths.nix is https://github.com/NixOS/ofborg/blob/released/ofborg/src/outpaths.nix. If you don't start swapping hard, it should take about 10 minutes to complete.)

@lilyinstarlight
Copy link
Member

lilyinstarlight commented Apr 10, 2024

I don't understand what the problem really is here. Do you have a link to a log file with more context?

Reverting this is bad because it breaks NixOS intefgration tests for macOS users which work just fine.

The revert was needed because CI would have failed on initial PR and has broken eval on all PRs since it was merged (the original PR was merged before ofborg finished CI checks...)

You need to additionally remove this conditional in your original PR and replace it with inherit references;:

# VM test not supported beyond linux yet
references =
if stdenv.hostPlatform.isLinux
then references
else {};

Do please open a new PR that re-adds the systems and fixes that conditional!

@tfc
Copy link
Contributor

tfc commented Apr 11, 2024

Thank you everyone for trying to clarify. I do/did understand that the patch was blocking evaluation and would need to be reverted quickly to unblock others.

I do however not understand how the concatenation of the lists linux ++ darwin breaks evaluation. Thank you @lilyinstarlight for suggesting to use references, but i don't understand how to use this (the comment above also do not help me understand what this is supposed to do and where to place it). Can you please give me an example? Also, what do you mean with "remove this conditional" - i never added a conditional?

@lilyinstarlight
Copy link
Member

I do however not understand how the concatenation of the lists linux ++ darwin breaks evaluation. Thank you @lilyinstarlight for suggesting to use references, but i don't understand how to use this (the comment above also do not help me understand what this is supposed to do and where to place it). Can you please give me an example? Also, what do you mean with "remove this conditional" - i never added a conditional?

The conditional is the lines I posted (pkgs/build-support/trivial-builders/test/default.nix lines 23-27) and is the broken code your change triggered (and those 4 lines should just be changed to one inherit references; line). Technically adding it to platforms just uncovered an existing bug elsewhere in nixpkgs, rather than your PR inherently being the problem, but updating those lines fixes eval with your PR

I just woke up and am on mobile, but I'll send a diff when I'm at a computer in a few hours to more clearly illustrate

@lilyinstarlight
Copy link
Member

@tfc This diff should do it, but if you open a new PR, we'll watch ofborg to make sure it passes this time:

diff --git a/nixos/lib/testing/meta.nix b/nixos/lib/testing/meta.nix
index 529fe714fcf6..bdf313e5b119 100644
--- a/nixos/lib/testing/meta.nix
+++ b/nixos/lib/testing/meta.nix
@@ -36,7 +36,7 @@ in
           };
           platforms = lib.mkOption {
             type = types.listOf types.raw;
-            default = lib.platforms.linux;
+            default = lib.platforms.linux ++ lib.platforms.darwin;
             description = ''
               Sets the [`meta.platforms`](https://nixos.org/manual/nixpkgs/stable/#var-meta-platforms) attribute on the [{option}`test`](#test-opt-test) derivation.
             '';
diff --git a/pkgs/build-support/trivial-builders/test/default.nix b/pkgs/build-support/trivial-builders/test/default.nix
index f41372d922bb..e1ed0be72bf3 100644
--- a/pkgs/build-support/trivial-builders/test/default.nix
+++ b/pkgs/build-support/trivial-builders/test/default.nix
@@ -20,11 +20,7 @@ recurseIntoAttrs {
   concat = callPackage ./concat-test.nix {};
   linkFarm = callPackage ./link-farm.nix {};
   overriding = callPackage ../test-overriding.nix {};
-  # VM test not supported beyond linux yet
-  references =
-    if stdenv.hostPlatform.isLinux
-    then references
-    else {};
+  inherit references;
   writeCBin = callPackage ./writeCBin.nix {};
   writeClosure-union = callPackage ./writeClosure-union.nix {
     inherit (references) samples;

@tfc
Copy link
Contributor

tfc commented Apr 12, 2024

@lilyinstarlight Now i get the picture. Thank you very much, i will create a PR this afternoon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: testing Tooling for automated testing of packages and modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants