Merge pull request #419 from GaetanLepage/multi-systems

Feat: add support for multi-systems

Author: Mic92

README.md

@@ -175,19 +175,38 @@ done
 inside the nix-shell instead of an interactive session:
 
 ```console
-$ nixpkgs-review pr --run 'jq < report.json' 113814
+$ nixpkgs-review pr --run --systems all 'jq < report.json' 340297
 # ...
 {
-  "blacklisted": [],
-  "broken": [],
-  "built": [
-    "cargo-deny"
-  ],
-  "failed": [],
-  "non-existent": [],
-  "pr": 113814,
-  "system": "x86_64-linux",
-  "tests": []
+  "checkout": "merge",
+  "extra-nixpkgs-config": null,
+  "pr": 340297,
+  "result": {
+    "aarch64-linux": {
+      "blacklisted": [],
+      "broken": [],
+      "built": [
+        "forecast"
+      ],
+      "failed": [],
+      "non-existent": [],
+      "tests": []
+    },
+    "x86_64-linux": {
+      "blacklisted": [],
+      "broken": [],
+      "built": [
+        "forecast"
+      ],
+      "failed": [],
+      "non-existent": [],
+      "tests": []
+    }
+  },
+  "systems": [
+    "x86_64-linux",
+    "aarch64-linux"
+  ]
 }
 ```
 
@@ -326,9 +345,17 @@ subcommand.
 
 ## Review changes for other operating systems/architectures
 
-The `--system` flag allows setting a system different from the current one. Note
-that the result nix-shell may not be able to execute all hooks correctly since
-the architecture/operating system mismatches.
+The `--systems` flag allows setting a system different from the current one.
+Note that the result nix-shell may not be able to execute all hooks correctly
+since the architecture/operating system mismatches.
+
+By default, `nixpkgs-review` targets only the current system
+(`--systems current`). You can also explicitly provide one or several systems to
+target (`--systems "x86_64-linux aarch64-darwin"`). The `--systems all` value
+will build for the four major platforms supported by hydra (`--x86_64-linux`,
+`aarch64-linux`, `x86_64-darwin` and `aarch64-darwin`). Ensure that your system
+is capable of building for the specified architectures, either locally or
+through the remote builder protocol.
 
 ```console
 $ nixpkgs-review pr --system aarch64-linux 98734

nixpkgs_review/cli/__init__.py

@@ -9,7 +9,7 @@
 from shutil import which
 from typing import Any, cast
 
-from ..utils import current_system, nix_nom_tool
+from ..utils import nix_nom_tool
 from .approve import approve_command
 from .comments import show_comments
 from .merge import merge_command
@@ -213,11 +213,17 @@ def common_flags() -> list[CommonFlag]:
             type=regex_type,
             help="Regular expression that package attributes have not to match (can be passed multiple times)",
         ),
+        CommonFlag(
+            "--systems",
+            type=str,
+            default="current",
+            help="Nix 'systems' to evaluate and build packages for",
+        ),
         CommonFlag(
             "--system",
             type=str,
-            default=current_system(),
-            help="Nix 'system' to evaluate and build packages for",
+            default="",
+            help="[DEPRECATED] use `--systems` instead",
         ),
         CommonFlag(
             "--token",
@@ -243,6 +249,12 @@ def common_flags() -> list[CommonFlag]:
             default="{ }",
             help="Extra nixpkgs config to pass to `import <nixpkgs>`",
         ),
+        CommonFlag(
+            "--num-procs-eval",
+            type=int,
+            default=1,
+            help="Number of parallel `nix-env` processes to run simultaneously (warning, can imply heavy RAM usage)",
+        ),
     ]
 
 

nixpkgs_review/cli/pr.py

@@ -2,13 +2,15 @@
 import re
 import sys
 from contextlib import ExitStack
+from pathlib import Path
 
 from ..allow import AllowedFeatures
 from ..builddir import Builddir
 from ..buildenv import Buildenv
 from ..errors import NixpkgsReviewError
+from ..nix import Attr
 from ..review import CheckoutOption, Review
-from ..utils import warn
+from ..utils import System, warn
 from .utils import ensure_github_token
 
 
@@ -32,16 +34,28 @@ def parse_pr_numbers(number_args: list[str]) -> list[int]:
 
 
 def pr_command(args: argparse.Namespace) -> str:
-    prs = parse_pr_numbers(args.number)
+    prs: list[int] = parse_pr_numbers(args.number)
     use_ofborg_eval = args.eval == "ofborg"
     checkout_option = (
         CheckoutOption.MERGE if args.checkout == "merge" else CheckoutOption.COMMIT
     )
 
     if args.post_result:
         ensure_github_token(args.token)
+    if args.system:
+        warn("Warning: The `--system` is deprecated. Use `--systems` instead.")
+        args.systems = args.system
 
-    contexts = []
+    contexts: list[
+        tuple[
+            # PR number
+            int,
+            # builddir path
+            Path,
+            # Attrs to build for each system
+            dict[System, list[Attr]],
+        ]
+    ] = []
 
     allow = AllowedFeatures(args.allow)
 
@@ -65,13 +79,14 @@ def pr_command(args: argparse.Namespace) -> str:
                     package_regexes=args.package_regex,
                     skip_packages=set(args.skip_package),
                     skip_packages_regex=args.skip_package_regex,
-                    system=args.system,
+                    systems=args.systems.split(" "),
                     allow=allow,
                     checkout=checkout_option,
                     sandbox=args.sandbox,
                     build_graph=args.build_graph,
                     nixpkgs_config=nixpkgs_config,
                     extra_nixpkgs_config=args.extra_nixpkgs_config,
+                    n_procs_eval=args.num_procs_eval,
                 )
                 contexts.append((pr, builddir.path, review.build_pr(pr)))
             except NixpkgsReviewError as e:

nixpkgs_review/nix.py

@@ -1,17 +1,19 @@
 import json
+import multiprocessing as mp
 import os
 import shlex
 import shutil
 import subprocess
 from dataclasses import dataclass, field
+from functools import partial
 from pathlib import Path
 from sys import platform
 from tempfile import NamedTemporaryFile
 from typing import Any, Final
 
 from .allow import AllowedFeatures
 from .errors import NixpkgsReviewError
-from .utils import ROOT, escape_attr, info, sh, warn
+from .utils import ROOT, System, info, sh, warn
 
 
 @dataclass
@@ -56,9 +58,9 @@ def is_test(self) -> bool:
 
 
 def nix_shell(
-    attrs: list[str],
+    attrs_per_system: dict[System, list[str]],
     cache_directory: Path,
-    system: str,
+    local_system: str,
     build_graph: str,
     nix_path: str,
     nixpkgs_config: Path,
@@ -71,7 +73,10 @@ def nix_shell(
         raise RuntimeError(f"{build_graph} not found in PATH")
 
     shell_file_args = build_shell_file_args(
-        cache_directory, attrs, system, nixpkgs_config
+        cache_dir=cache_directory,
+        attrs_per_system=attrs_per_system,
+        local_system=local_system,
+        nixpkgs_config=nixpkgs_config,
     )
     if sandbox:
         args = _nix_shell_sandbox(
@@ -266,28 +271,66 @@ def nix_eval(
             os.unlink(attr_json.name)
 
 
-def nix_build(
+def nix_eval_thread(
+    system: System,
     attr_names: set[str],
+    allow: AllowedFeatures,
+    nix_path: str,
+) -> tuple[System, list[Attr]]:
+    return system, nix_eval(attr_names, system, allow, nix_path)
+
+
+def multi_system_eval(
+    attr_names_per_system: dict[System, set[str]],
+    allow: AllowedFeatures,
+    nix_path: str,
+    n_procs: int,
+) -> dict[System, list[Attr]]:
+    nix_eval_partial = partial(
+        nix_eval_thread,
+        allow=allow,
+        nix_path=nix_path,
+    )
+
+    args: list[tuple[System, set[str]]] = list(attr_names_per_system.items())
+    with mp.Pool(n_procs) as pool:
+        results: list[tuple[System, list[Attr]]] = pool.starmap(nix_eval_partial, args)
+
+    return {system: attrs for system, attrs in results}
+
+
+def nix_build(
+    attr_names_per_system: dict[System, set[str]],
     args: str,
     cache_directory: Path,
-    system: str,
+    systems: set[System],
+    local_system: System,
     allow: AllowedFeatures,
     build_graph: str,
     nix_path: str,
     nixpkgs_config: Path,
-) -> list[Attr]:
-    if not attr_names:
+    n_procs_eval: int,
+) -> dict[System, list[Attr]]:
+    if not attr_names_per_system:
         info("Nothing to be built.")
-        return []
+        return {}
 
-    attrs = nix_eval(attr_names, system, allow, nix_path)
-    filtered = []
-    for attr in attrs:
-        if not (attr.broken or attr.blacklisted):
-            filtered.append(attr.name)
+    attrs_per_system: dict[System, list[Attr]] = multi_system_eval(
+        attr_names_per_system,
+        allow,
+        nix_path,
+        n_procs=n_procs_eval,
+    )
 
-    if len(filtered) == 0:
-        return attrs
+    filtered_per_system: dict[System, list[str]] = {}
+    for system, attrs in attrs_per_system.items():
+        filtered_per_system[system] = []
+        for attr in attrs:
+            if not (attr.broken or attr.blacklisted):
+                filtered_per_system[system].append(attr.name)
+
+    if all(len(filtered) == 0 for filtered in filtered_per_system.values()):
+        return attrs_per_system
 
     command = [
         build_graph,
@@ -314,26 +357,37 @@ def nix_build(
         ]
 
     command += build_shell_file_args(
-        cache_directory, filtered, system, nixpkgs_config
+        cache_dir=cache_directory,
+        attrs_per_system=filtered_per_system,
+        local_system=local_system,
+        nixpkgs_config=nixpkgs_config,
     ) + shlex.split(args)
 
     sh(command)
-    return attrs
+    return attrs_per_system
 
 
 def build_shell_file_args(
-    cache_dir: Path, attrs: list[str], system: str, nixpkgs_config: Path
+    cache_dir: Path,
+    attrs_per_system: dict[System, list[str]],
+    local_system: str,
+    nixpkgs_config: Path,
 ) -> list[str]:
     attrs_file = cache_dir.joinpath("attrs.nix")
     with open(attrs_file, "w+", encoding="utf-8") as f:
-        f.write("pkgs: with pkgs; [\n")
-        f.write("\n".join(f"  {escape_attr(a)}" for a in attrs))
-        f.write("\n]")
+        f.write("{\n")
+        for system, attrs in attrs_per_system.items():
+            f.write(f"  {system} = [\n")
+            for attr in attrs:
+                f.write(f'    "{attr}"\n')
+            f.write("  ];\n")
+        f.write("}")
+        print(f.read())
 
     return [
         "--argstr",
-        "system",
-        system,
+        "local-system",
+        local_system,
         "--argstr",
         "nixpkgs-path",
         str(cache_dir.joinpath("nixpkgs/")),

nixpkgs_review/nix/review-shell.nix

@@ -1,14 +1,32 @@
-{ system
-, nixpkgs-config-path # Path to Nix file containing the Nixpkgs config
-, attrs-path # Path to Nix file containing a list of attributes to build
-, nixpkgs-path # Path to this review's nixpkgs
-, pkgs ? import nixpkgs-path { inherit system; config = import nixpkgs-config-path; }
-, lib ? pkgs.lib
+{ local-system
+, nixpkgs-config-path
+, # Path to Nix file containing the Nixpkgs config
+  attrs-path
+, # Path to Nix file containing a list of attributes to build
+  nixpkgs-path
+, # Path to this review's nixpkgs
+  local-pkgs ? import nixpkgs-path {
+    system = local-system;
+    config = import nixpkgs-config-path;
+  }
+, lib ? local-pkgs.lib
+,
 }:
 
 let
-  attrs = import attrs-path pkgs;
-  env = pkgs.buildEnv {
+
+  nixpkgs-config = import nixpkgs-config-path;
+  extractPackagesForSystem =
+    system: system-attrs:
+    let
+      system-pkg = import nixpkgs-path {
+        inherit system;
+        config = nixpkgs-config;
+      };
+    in
+    map (attrString: lib.attrByPath (lib.splitString "." attrString) null system-pkg) system-attrs;
+  attrs = lib.flatten (lib.mapAttrsToList extractPackagesForSystem (import attrs-path));
+  env = local-pkgs.buildEnv {
     name = "env";
     paths = attrs;
     ignoreCollisions = true;