Support specifying --store and --eval-store

nixpkgs_review/cli/__init__.py

@@ -321,6 +321,22 @@ def common_flags() -> list[CommonFlag]:
             default=None,
             help="Alternative package set to use for building, e.g. pkgsMusl, pkgsStatic, or pkgsCross.aarch64-multiplatform",
         ),
+        CommonFlag(
+            "--store",
+            type=str,
+            default=None,
+            help="Nix store to build in, passed verbatim to nix as `--store` "
+            "(e.g. `local?root=/tmp/review-store`). Defaults to the ambient store. "
+            "Useful for isolating risky/experimental builds from your main store.",
+        ),
+        CommonFlag(
+            "--eval-store",
+            type=str,
+            default=None,
+            help="Store to write evaluation artifacts (drvs, IFD results) to, "
+            "passed verbatim to nix as `--eval-store` (e.g. `auto`). Useful with "
+            "a remote/slow --store (e.g. nixbuild.net) to keep evaluation local.",
+        ),
     ]
 
 

nixpkgs_review/nix.py

@@ -29,6 +29,8 @@ class BuildConfig:
     num_eval_workers: int = 1
     max_memory_size: int = 4096
     pkgs: str | None = None
+    store: str | None = None
+    eval_store: str | None = None
 
 
 @dataclass
@@ -40,6 +42,7 @@ class Attr:
     outputs: dict[str, Path] | None
     drv_path: Path | None
     aliases: list[str] = field(default_factory=list)
+    store: str | None = None
     _path_verified: bool | None = field(init=False, default=None)
 
     def was_build(self) -> bool:
@@ -54,6 +57,7 @@ def was_build(self) -> bool:
                 "nix",
                 "--extra-experimental-features",
                 "nix-command",
+                *_store_flags(self.store),
                 "store",
                 "verify",
                 "--no-contents",
@@ -86,7 +90,20 @@ def serialize(self) -> dict:
 REVIEW_SHELL: Final[str] = str(ROOT.joinpath("nix/review-shell.nix"))
 
 
-def _nix_common_flags(allow: AllowedFeatures, nix_path: str) -> list[str]:
+def _store_flags(store: str | None) -> list[str]:
+    return ["--store", store] if store else []
+
+
+def _eval_store_flags(eval_store: str | None) -> list[str]:
+    return ["--eval-store", eval_store] if eval_store else []
+
+
+def _nix_common_flags(
+    allow: AllowedFeatures,
+    nix_path: str,
+    store: str | None = None,
+    eval_store: str | None = None,
+) -> list[str]:
     return [
         "--extra-experimental-features",
         "nix-command",
@@ -96,6 +113,8 @@ def _nix_common_flags(allow: AllowedFeatures, nix_path: str) -> list[str]:
         "--allow-import-from-derivation"
         if allow.ifd
         else "--no-allow-import-from-derivation",
+        *_store_flags(store),
+        *_eval_store_flags(eval_store),
     ]
 
 
@@ -112,6 +131,8 @@ class ShellConfig:
     run: str | None = None
     sandbox: bool = False
     pkgs: str | None = None
+    store: str | None = None
+    eval_store: str | None = None
 
 
 def nix_shell(
@@ -131,6 +152,12 @@ def nix_shell(
         nixpkgs_config=config.nixpkgs_config,
         pkgs=config.pkgs,
     )
+    if config.store:
+        warn(
+            "Using a non-default --store: the review shell may fail to launch or "
+            "run binaries built into it, since they are not at the real /nix/store. "
+            "Consider --no-shell, or `nix copy` the results to the default store first."
+        )
     if config.sandbox:
         with TemporaryDirectory(prefix="nixpkgs-review-links-") as dirname:
             bin_link = Path(dirname) / bin_name
@@ -145,6 +172,8 @@ def nix_shell(
             *shell_file_args,
             "--nix-path",
             config.nix_path,
+            *_store_flags(config.store),
+            *_eval_store_flags(config.eval_store),
             REVIEW_SHELL,
         ]
         if config.run:
@@ -240,6 +269,8 @@ def tmpfs(path: Path | str, *, is_dir: bool = True) -> list[str]:
         *shell_file_args,
         "--nix-path",
         config.nix_path,
+        *_store_flags(config.store),
+        *_eval_store_flags(config.eval_store),
         REVIEW_SHELL,
     ]
 
@@ -259,7 +290,7 @@ class NixEvalPropsExtra(TypedDict):
 NixEvalResult = list[NixEvalProps]
 
 
-def _nix_eval_filter(packages: NixEvalResult) -> list[Attr]:
+def _nix_eval_filter(packages: NixEvalResult, store: str | None) -> list[Attr]:
     # workaround https://github.com/NixOS/ofborg/issues/269
     blacklist = {
         "appimage-run-tests",
@@ -293,6 +324,7 @@ def _nix_eval_filter(packages: NixEvalResult) -> list[Attr]:
             blacklisted=name in blacklist,
             outputs=outputs,
             drv_path=drv_path,
+            store=store,
         )
         if attr.drv_path is not None:
             if (other := attr_by_path.get(attr.drv_path)) is None:
@@ -327,7 +359,12 @@ def multi_system_eval(
             "--max-memory-size",
             str(build_config.max_memory_size),
             "--no-instantiate",
-            *_nix_common_flags(build_config.allow, build_config.nix_path),
+            *_nix_common_flags(
+                build_config.allow,
+                build_config.nix_path,
+                build_config.store,
+                build_config.eval_store,
+            ),
             "--expr",
             f"(import {eval_script} {{ attr-json = {attr_json.name}; }})",
             "--apply",
@@ -361,7 +398,7 @@ def multi_system_eval(
             systems_packages[system].append(eval_result)
 
         return {
-            system: _nix_eval_filter(packages)
+            system: _nix_eval_filter(packages, build_config.store)
             for system, packages in systems_packages.items()
         }
     finally:
@@ -399,7 +436,12 @@ def nix_build(
         "build",
         "--file",
         REVIEW_SHELL,
-        *_nix_common_flags(build_config.allow, build_config.nix_path),
+        *_nix_common_flags(
+            build_config.allow,
+            build_config.nix_path,
+            build_config.store,
+            build_config.eval_store,
+        ),
         "--no-link",
         "--keep-going",
     ]
@@ -424,6 +466,8 @@ def nix_build(
         shell_file_args=shell_file_args,
         allow=build_config.allow,
         nix_path=build_config.nix_path,
+        store=build_config.store,
+        eval_store=build_config.eval_store,
     )
 
     command += shell_file_args + shlex.split(args)
@@ -466,17 +510,19 @@ def build_shell_file_args(
     ]
 
 
-def _write_review_shell_drv(
+def _write_review_shell_drv(  # noqa: PLR0913
     cache_directory: Path,
     shell_file_args: list[str],
     allow: AllowedFeatures,
     nix_path: str,
+    store: str | None = None,
+    eval_store: str | None = None,
 ) -> None:
     review_drv_link: Path = cache_directory / "review-shell.drv"
 
     cmd: list[str] = [
         "nix-instantiate",
-        *_nix_common_flags(allow, nix_path),
+        *_nix_common_flags(allow, nix_path, store, eval_store),
         *shell_file_args,
         REVIEW_SHELL,
     ]

nixpkgs_review/report.py

@@ -214,12 +214,14 @@ def _write_log_for_attr(
     logs: LazyDirectory,
     extra_nix_log_args: list[str],
 ) -> None:
+    store_flags = ["--store", attr.store] if attr.store else []
     with logs.ensure().joinpath(get_log_filename(attr, system)).open("w+") as f:
         subprocess.run(
             [
                 "nix",
                 "--extra-experimental-features",
                 "nix-command",
+                *store_flags,
                 "log",
                 f"{attr.drv_path}^*",
                 *extra_nix_log_args,

nixpkgs_review/review.py

@@ -420,6 +420,8 @@ def build_commit(
             self.systems,
             self.build_config.allow,
             self.build_config.pkgs,
+            self.build_config.store,
+            self.build_config.eval_store,
         )
 
         if head_commit is None:
@@ -434,6 +436,8 @@ def build_commit(
             self.systems,
             self.build_config.allow,
             self.build_config.pkgs,
+            self.build_config.store,
+            self.build_config.eval_store,
             check_meta=True,
         )
 
@@ -687,6 +691,8 @@ def start_review(
                 run=self.shell_options.run,
                 sandbox=self.shell_options.sandbox,
                 pkgs=self.build_config.pkgs,
+                store=self.build_config.store,
+                eval_store=self.build_config.eval_store,
             )
             nix_shell(report.built_packages(), shell_config)
 
@@ -764,11 +770,13 @@ def parse_packages_xml(stdout: IO[str]) -> list[Package]:
     return packages
 
 
-def _list_packages_system(
+def _list_packages_system(  # noqa: PLR0913
     system: System,
     nix_path: str,
     allow: AllowedFeatures,
     pkgs: str | None = None,
+    store: str | None = None,
+    eval_store: str | None = None,
     *,
     check_meta: bool = False,
 ) -> list[Package]:
@@ -789,6 +797,8 @@ def _list_packages_system(
         "--allow-import-from-derivation"
         if allow.ifd
         else "--no-allow-import-from-derivation",
+        *(["--store", store] if store else []),
+        *(["--eval-store", eval_store] if eval_store else []),
         *(["-A", pkgs] if pkgs else []),
     ]
     if check_meta:
@@ -804,11 +814,13 @@ def _list_packages_system(
             return parse_packages_xml(f)
 
 
-def list_packages(
+def list_packages(  # noqa: PLR0913
     nix_path: str,
     systems: set[System],
     allow: AllowedFeatures,
     pkgs: str | None = None,
+    store: str | None = None,
+    eval_store: str | None = None,
     *,
     check_meta: bool = False,
 ) -> dict[System, list[Package]]:
@@ -820,6 +832,8 @@ def list_packages(
             allow=allow,
             check_meta=check_meta,
             pkgs=pkgs,
+            store=store,
+            eval_store=eval_store,
         )
 
     return results
@@ -1018,6 +1032,8 @@ def build_config_from_args(
         num_eval_workers=args.num_eval_workers,
         max_memory_size=args.max_memory_size,
         pkgs=args.pkgs,
+        store=args.store,
+        eval_store=args.eval_store,
     )
 
 

tests/test_store_flag.py

@@ -0,0 +1,90 @@
+from __future__ import annotations
+
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+from nixpkgs_review.allow import AllowedFeatures
+from nixpkgs_review.cli import parse_args
+from nixpkgs_review.nix import (
+    BuildConfig,
+    _eval_store_flags,
+    _nix_common_flags,
+    _store_flags,
+)
+from nixpkgs_review.review import build_config_from_args
+
+
+def test_store_flag_defaults_to_none() -> None:
+    args = parse_args("nixpkgs-review", ["rev", "HEAD"])
+    assert args.store is None
+    assert args.eval_store is None
+
+
+def test_store_flag_parsed_for_pr_rev_wip() -> None:
+    for subcommand, extra in (
+        ("pr", ["1"]),
+        ("rev", ["HEAD"]),
+        ("wip", []),
+    ):
+        args = parse_args(
+            "nixpkgs-review", [subcommand, *extra, "--store", "local?root=/tmp/x"]
+        )
+        assert args.store == "local?root=/tmp/x"
+
+
+def test_eval_store_flag_parsed_for_pr_rev_wip() -> None:
+    for subcommand, extra in (
+        ("pr", ["1"]),
+        ("rev", ["HEAD"]),
+        ("wip", []),
+    ):
+        args = parse_args(
+            "nixpkgs-review", [subcommand, *extra, "--eval-store", "auto"]
+        )
+        assert args.eval_store == "auto"
+
+
+def test_store_flags_helper() -> None:
+    assert _store_flags(None) == []
+    assert _store_flags("local?root=/tmp/x") == ["--store", "local?root=/tmp/x"]
+
+
+def test_eval_store_flags_helper() -> None:
+    assert _eval_store_flags(None) == []
+    assert _eval_store_flags("auto") == ["--eval-store", "auto"]
+
+
+def test_nix_common_flags_includes_store_when_set() -> None:
+    allow = AllowedFeatures([])
+    flags = _nix_common_flags(allow, "", "local?root=/tmp/x")
+    assert flags[-2:] == ["--store", "local?root=/tmp/x"]
+
+
+def test_nix_common_flags_includes_eval_store_when_set() -> None:
+    allow = AllowedFeatures([])
+    flags = _nix_common_flags(allow, "", "local?root=/tmp/x", "auto")
+    assert flags[-4:] == ["--store", "local?root=/tmp/x", "--eval-store", "auto"]
+
+
+def test_nix_common_flags_omits_store_when_unset() -> None:
+    allow = AllowedFeatures([])
+    flags = _nix_common_flags(allow, "")
+    assert "--store" not in flags
+    assert "--eval-store" not in flags
+
+
+@patch("nixpkgs_review.review.current_system", return_value="x86_64-linux")
+def test_build_config_from_args_carries_store(mock_current_system: MagicMock) -> None:
+    # current_system() shells out to `nix eval`; mocked so this test doesn't
+    # depend on ambient Nix state (e.g. a writable profile dir), which isn't
+    # available in a Nix build sandbox.
+    args = parse_args(
+        "nixpkgs-review",
+        ["rev", "HEAD", "--store", "local?root=/tmp/x", "--eval-store", "auto"],
+    )
+    build_config: BuildConfig = build_config_from_args(
+        args, AllowedFeatures([]), nix_path="", nixpkgs_config=Path("/dev/null")
+    )
+    assert build_config.store == "local?root=/tmp/x"
+    assert build_config.eval_store == "auto"
+    assert build_config.local_system == mock_current_system.return_value