From 568a10d463e37ef68796604d245f2bd933c82438 Mon Sep 17 00:00:00 2001 From: PerchunPak Date: Fri, 31 Jan 2025 17:11:26 +0100 Subject: [PATCH] Add support for building `namespace.*` without eval'ing nixpkgs E.g. `nixpkgs-revew pr/rev --package vimPlugins` will build all packages that are in `vimPlugins` namespace` --- nixpkgs_review/cli/pr.py | 9 +++++---- nixpkgs_review/nix/evalAttrs.nix | 6 ++++++ nixpkgs_review/report.py | 5 +++++ nixpkgs_review/review.py | 26 +++++++++++++++++++++++++- tests/test_pr.py | 31 +++++++++++++++++++++++++++++++ tests/test_rev.py | 30 ++++++++++++++++++++++++++++++ tests/test_wip.py | 29 +++++++++++++++++++++++++++++ 7 files changed, 131 insertions(+), 5 deletions(-) diff --git a/nixpkgs_review/cli/pr.py b/nixpkgs_review/cli/pr.py index b0b70089..42e90416 100644 --- a/nixpkgs_review/cli/pr.py +++ b/nixpkgs_review/cli/pr.py @@ -48,10 +48,11 @@ def pr_command(args: argparse.Namespace) -> str: if args.token: args.eval = "github" else: - warn( - "No GitHub token provided via GITHUB_TOKEN variable. Falling back to local evaluation.\n" - "Tip: Install the `gh` command line tool and run `gh auth login` to authenticate." - ) + if not args.package: + warn( + "No GitHub token provided via GITHUB_TOKEN variable. Falling back to local evaluation.\n" + "Tip: Install the `gh` command line tool and run `gh auth login` to authenticate." + ) args.eval = "local" case "github": if not args.token: diff --git a/nixpkgs_review/nix/evalAttrs.nix b/nixpkgs_review/nix/evalAttrs.nix index 90eb64a5..64b8f8bd 100644 --- a/nixpkgs_review/nix/evalAttrs.nix +++ b/nixpkgs_review/nix/evalAttrs.nix @@ -27,6 +27,12 @@ let drvPath = null; }) ] + else if !lib.isDerivation pkg then + if builtins.typeOf pkg != "set" then + # if it is not a package, ignore it (it is probably something like overrideAttrs) + [ ] + else + lib.flatten (lib.mapAttrsToList (name': _: getProperties ("${name}.${name'}")) pkg) else lib.flip map pkg.outputs or [ "out" ] ( output: diff --git a/nixpkgs_review/report.py b/nixpkgs_review/report.py index acb1297c..da508917 100644 --- a/nixpkgs_review/report.py +++ b/nixpkgs_review/report.py @@ -149,6 +149,7 @@ def __init__( self, attrs_per_system: dict[str, list[Attr]], extra_nixpkgs_config: str, + only_packages: set[str], show_header: bool = True, *, checkout: Literal["merge", "commit"] = "merge", @@ -156,6 +157,7 @@ def __init__( self.show_header = show_header self.attrs = attrs_per_system self.checkout = checkout + self.only_packages = only_packages if extra_nixpkgs_config != "{ }": self.extra_nixpkgs_config: str | None = extra_nixpkgs_config @@ -190,6 +192,7 @@ def json(self, pr: int | None) -> str: "pr": pr, "checkout": self.checkout, "extra-nixpkgs-config": self.extra_nixpkgs_config, + "only_packages": list(self.only_packages), "result": { system: report.serialize() for system, report in self.system_reports.items() @@ -212,6 +215,8 @@ def markdown(self, pr: int | None) -> str: cmd += f" --extra-nixpkgs-config '{self.extra_nixpkgs_config}'" if self.checkout != "merge": cmd += f" --checkout {self.checkout}" + if self.only_packages: + cmd += " --package " + " --package ".join(self.only_packages) msg += f"Command: `{cmd}`\n" for system, report in self.system_reports.items(): diff --git a/nixpkgs_review/review.py b/nixpkgs_review/review.py index ad14e582..90af163e 100644 --- a/nixpkgs_review/review.py +++ b/nixpkgs_review/review.py @@ -122,7 +122,7 @@ def __init__( self.run = run self.remote = remote self.github_client = GithubClient(api_token) - self.use_github_eval = use_github_eval + self.use_github_eval = use_github_eval and not only_packages self.checkout = checkout self.only_packages = only_packages self.package_regex = package_regexes @@ -216,6 +216,22 @@ def build_commit( """ self.git_worktree(base_commit) + if self.only_packages: + if reviewed_commit is None: + self.apply_unstaged(staged) + elif self.checkout == CheckoutOption.MERGE: + self.git_checkout(reviewed_commit) + else: + self.git_merge(reviewed_commit) + + changed_attrs = {} + for system in self.systems: + changed_attrs[system] = set() + for package in self.only_packages: + changed_attrs[system].add(package) + + return self.build(changed_attrs, self.build_args) + print("Local evaluation for computing rebuilds") # TODO: nix-eval-jobs ? @@ -330,6 +346,13 @@ def build_pr(self, pr_number: int) -> dict[System, list[Attr]]: raise NixpkgsReviewError(msg) base_rev = run.stdout.strip() + if self.only_packages: + packages_per_system = {} + for system in self.systems: + packages_per_system[system] = set() + for package in self.only_packages: + packages_per_system[system].add(package) + if packages_per_system is None: return self.build_commit(base_rev, pr_rev) @@ -356,6 +379,7 @@ def start_review( attrs_per_system, self.extra_nixpkgs_config, checkout=self.checkout.name.lower(), # type: ignore[arg-type] + only_packages=self.only_packages, show_header=self.show_header, ) report.print_console(pr) diff --git a/tests/test_pr.py b/tests/test_pr.py index 253e63d0..1171bd0c 100644 --- a/tests/test_pr.py +++ b/tests/test_pr.py @@ -241,3 +241,34 @@ def test_pr_github_action_eval( ], ) helpers.assert_built(pkg_name="pkg1", path=path) + + +@patch("nixpkgs_review.review._list_packages_system") +def test_pr_only_packages_does_not_trigger_an_eval( + mock_eval: MagicMock, + helpers: Helpers, +) -> None: + mock_eval.side_effect = RuntimeError + with helpers.nixpkgs() as nixpkgs: + nixpkgs.path.joinpath("pkg1.txt").write_text("foo") + subprocess.run(["git", "add", "."], check=True) + subprocess.run(["git", "commit", "-m", "example-change"], check=True) + subprocess.run(["git", "checkout", "-b", "pull/363128/merge"], check=True) + subprocess.run( + ["git", "push", str(nixpkgs.remote), "pull/363128/merge"], check=True + ) + + path = main( + "nixpkgs-review", + [ + "pr", + "--remote", + str(nixpkgs.remote), + "--run", + "exit 0", + "--package", + "pkg1", + "363128", + ], + ) + helpers.assert_built(pkg_name="pkg1", path=path) diff --git a/tests/test_rev.py b/tests/test_rev.py index 65b4cc11..3f665d47 100644 --- a/tests/test_rev.py +++ b/tests/test_rev.py @@ -1,5 +1,6 @@ import shutil import subprocess +from unittest.mock import MagicMock, patch import pytest @@ -40,3 +41,32 @@ def test_rev_command_without_nom(helpers: Helpers) -> None: ], ) helpers.assert_built(pkg_name="pkg1", path=path) + + +@patch("nixpkgs_review.review._list_packages_system") +def test_rev_only_packages_does_not_trigger_an_eval( + mock_eval: MagicMock, + helpers: Helpers, +) -> None: + mock_eval.side_effect = RuntimeError + with helpers.nixpkgs() as nixpkgs: + nixpkgs.path.joinpath("pkg1.txt").write_text("foo") + subprocess.run(["git", "add", "."], check=True) + subprocess.run(["git", "commit", "-m", "example-change"], check=True) + + path = main( + "nixpkgs-review", + [ + "rev", + "HEAD", + "--remote", + str(nixpkgs.remote), + "--run", + "exit 0", + "--build-graph", + "nix", + "--package", + "pkg1", + ], + ) + helpers.assert_built(pkg_name="pkg1", path=path) diff --git a/tests/test_wip.py b/tests/test_wip.py index 9574f474..9db856c1 100644 --- a/tests/test_wip.py +++ b/tests/test_wip.py @@ -1,4 +1,5 @@ import shutil +from unittest.mock import MagicMock, patch import pytest @@ -40,3 +41,31 @@ def test_wip_command_without_nom( helpers.assert_built(pkg_name="pkg1", path=path) captured = capfd.readouterr() assert "$ nix build" in captured.out + + +@patch("nixpkgs_review.review._list_packages_system") +def test_wip_only_packages_does_not_trigger_an_eval( + mock_eval: MagicMock, + helpers: Helpers, + capfd: pytest.CaptureFixture, +) -> None: + mock_eval.side_effect = RuntimeError + with helpers.nixpkgs() as nixpkgs: + nixpkgs.path.joinpath("pkg1.txt").write_text("foo") + path = main( + "nixpkgs-review", + [ + "wip", + "--remote", + str(nixpkgs.remote), + "--run", + "exit 0", + "--build-graph", + "nix", + "--package", + "pkg1", + ], + ) + helpers.assert_built(pkg_name="pkg1", path=path) + captured = capfd.readouterr() + assert "$ nix build" in captured.out