[WIP] Feat: add support for multi-systems

README.md

@@ -175,19 +175,40 @@ done
 inside the nix-shell instead of an interactive session:
 
 ```console
-$ nixpkgs-review pr --run 'jq < report.json' 113814
+$ nixpkgs-review pr --run '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",
+        "forecast"
+      ],
+      "failed": [],
+      "non-existent": [],
+      "tests": []
+    },
+    "x86_64-linux": {
+      "blacklisted": [],
+      "broken": [],
+      "built": [
+        "forecast",
+        "forecast"
+      ],
+      "failed": [],
+      "non-existent": [],
+      "tests": []
+    }
+  },
+  "systems": [
+    "x86_64-linux",
+    "aarch64-linux"
+  ]
 }
 ```
 
@@ -324,10 +345,15 @@ 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 `--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
@@ -21,7 +21,7 @@
 try:
     import argcomplete
 except ImportError:
-    argcomplete = None
+    argcomplete = None  # type: ignore
 
 
 def regex_type(s: str) -> Pattern[str]:
@@ -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",

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,7 +79,7 @@ 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,

nixpkgs_review/nix.py

@@ -11,7 +11,7 @@
 
 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 +56,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 +71,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(
@@ -267,27 +270,32 @@ def nix_eval(
 
 
 def nix_build(
-    attr_names: set[str],
+    attr_names_per_system: dict[System, set[str]],
     args: str,
     cache_directory: Path,
-    system: str,
+    systems: set[str],
     allow: AllowedFeatures,
     build_graph: str,
     nix_path: str,
     nixpkgs_config: Path,
-) -> list[Attr]:
-    if not attr_names:
+) -> 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)
+    # TODO parallelize evaluation
+    attrs_per_system: dict[System, list[Attr]] = {}
+    filtered_per_system: dict[System, list[str]] = {}
+    for system, attr_names in attr_names_per_system.items():
+        attrs_per_system[system] = nix_eval(attr_names, system, allow, nix_path)
 
-    if len(filtered) == 0:
-        return attrs
+        filtered_per_system[system] = []
+        for attr in attrs_per_system[system]:
+            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,
@@ -305,6 +313,8 @@ def nix_build(
         else "--no-allow-import-from-derivation",
     ]
 
+    # TODO what should we do if we build for both darwin and linux ?
+    # Should this only be affected by the host ?
     if platform == "linux":
         command += [
             # only matters for single-user nix and trusted users
@@ -314,26 +324,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=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,31 @@
-{ 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;
+
+  pathStringToPath = lib.splitString ".";
+  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;