Skip to content

Commit

Permalink
don't rely on NIX_PATH
Browse files Browse the repository at this point in the history
Nix does not respect `NIX_PATH` when the `nix-path` setting in nix.conf
is set, which breaks the `nix-store --verify-path` due to `<nixpkgs>`
diverging from the temporary nixpkgs created by nixpkgs-review.
  • Loading branch information
figsoda committed Aug 12, 2023
1 parent cd4f7a3 commit 316de1f
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 28 deletions.
8 changes: 3 additions & 5 deletions nixpkgs_review/builddir.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,10 @@ def __init__(self, name: str) -> None:

self.worktree_dir = self.path.joinpath("nixpkgs")
self.worktree_dir.mkdir()
self.worktree_dir = self.worktree_dir

os.environ["NIX_PATH"] = self.nixpkgs_path()

def nixpkgs_path(self) -> str:
return f"nixpkgs={self.worktree_dir}:nixpkgs-overlays={self.overlay.path}"
self.nix_path = (
f"nixpkgs={self.worktree_dir} nixpkgs-overlays={self.overlay.path}"
)

def __enter__(self) -> "Builddir":
return self
Expand Down
10 changes: 1 addition & 9 deletions nixpkgs_review/cli/post_result.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import argparse
import os
import subprocess
import sys
from pathlib import Path

Expand All @@ -17,14 +16,7 @@ def post_result_command(args: argparse.Namespace) -> None:
sys.exit(1)
pr = int(pr_env)

output = subprocess.run(
["nix-instantiate", "--find-file", "nixpkgs"],
check=True,
stdout=subprocess.PIPE,
text=True,
).stdout.strip()
nixpkgs_path = Path(output)
report = nixpkgs_path.parent.joinpath("report.md")
report = Path(os.environ["NIXPKGS_REVIEW_ROOT"]) / "report.md"
if not report.exists():
warn(f"Report not found in {report}. Are you in a nixpkgs-review nix-shell?")
sys.exit(1)
Expand Down
19 changes: 14 additions & 5 deletions nixpkgs_review/nix.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def nix_shell(
cache_directory: Path,
system: str,
build_graph: str,
nix_path: str,
nixpkgs_config: Path,
run: Optional[str] = None,
sandbox: bool = False,
Expand All @@ -57,15 +58,17 @@ def nix_shell(
shell = cache_directory.joinpath("shell.nix")
write_shell_expression(shell, attrs, system, nixpkgs_config)
if sandbox:
args = _nix_shell_sandbox(nix_shell, shell, nixpkgs_config)
args = _nix_shell_sandbox(nix_shell, shell, nix_path, nixpkgs_config)
else:
args = [nix_shell, str(shell)]
args = [nix_shell, str(shell), "--nix-path", nix_path]
if run:
args.extend(["--run", run])
sh(args, cwd=cache_directory, check=False)


def _nix_shell_sandbox(nix_shell: str, shell: Path, nixpkgs_config: Path) -> List[str]:
def _nix_shell_sandbox(
nix_shell: str, shell: Path, nix_path: str, nixpkgs_config: Path
) -> List[str]:
if platform != "linux":
raise RuntimeError("Sandbox mode is only available on Linux platforms.")

Expand Down Expand Up @@ -137,7 +140,7 @@ def tmpfs(path: Union[Path, str], dir: bool = True) -> List[str]:
*bind(hub_config, try_=True),
*bind(gh_config, try_=True),
]
return [bwrap, *bwrap_args, "--", nix_shell, str(shell)]
return [bwrap, *bwrap_args, "--", nix_shell, str(shell), "--nix-path", nix_path]


def _nix_eval_filter(json: Dict[str, Any]) -> List[Attr]:
Expand Down Expand Up @@ -185,6 +188,7 @@ def nix_eval(
attrs: Set[str],
system: str,
allow: AllowedFeatures,
nix_path: str,
) -> List[Attr]:
attr_json = NamedTemporaryFile(mode="w+", delete=False)
delete = True
Expand All @@ -199,6 +203,8 @@ def nix_eval(
"--system",
system,
"eval",
"--nix-path",
nix_path,
"--json",
"--impure",
"--allow-import-from-derivation"
Expand Down Expand Up @@ -233,13 +239,14 @@ def nix_build(
system: str,
allow: AllowedFeatures,
build_graph: str,
nix_path: str,
nixpkgs_config: Path,
) -> List[Attr]:
if not attr_names:
info("Nothing to be built.")
return []

attrs = nix_eval(attr_names, system, allow)
attrs = nix_eval(attr_names, system, allow, nix_path)
filtered = []
for attr in attrs:
if not (attr.broken or attr.blacklisted):
Expand All @@ -254,6 +261,8 @@ def nix_build(
command = [
build_graph,
"build",
"--nix-path",
nix_path,
"--extra-experimental-features",
"nix-command" if allow.url_literals else "nix-command no-url-literals",
"--no-link",
Expand Down
24 changes: 17 additions & 7 deletions nixpkgs_review/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def build_commit(
self.git_worktree(base_commit)

base_packages = list_packages(
str(self.worktree_dir()),
self.builddir.nix_path,
self.system,
self.allow,
)
Expand All @@ -162,7 +162,7 @@ def build_commit(
self.git_merge(reviewed_commit)

merged_packages = list_packages(
str(self.worktree_dir()),
self.builddir.nix_path,
self.system,
self.allow,
check_meta=True,
Expand Down Expand Up @@ -192,6 +192,7 @@ def build(self, packages: Set[str], args: str) -> List[Attr]:
self.skip_packages_regex,
self.system,
self.allow,
self.builddir.nix_path,
)
return nix_build(
packages,
Expand All @@ -200,6 +201,7 @@ def build(self, packages: Set[str], args: str) -> List[Attr]:
self.system,
self.allow,
self.build_graph,
self.builddir.nix_path,
self.nixpkgs_config,
)

Expand Down Expand Up @@ -252,7 +254,7 @@ def start_review(
print_result: bool = False,
) -> None:
os.environ.pop("NIXPKGS_CONFIG", None)
os.environ["NIX_PATH"] = path.as_posix()
os.environ["NIXPKGS_REVIEW_ROOT"] = str(path)
if pr:
os.environ["PR"] = str(pr)
report = Report(self.system, attr, self.extra_nixpkgs_config)
Expand All @@ -273,6 +275,7 @@ def start_review(
path,
self.system,
self.build_graph,
self.builddir.nix_path,
self.nixpkgs_config,
self.run,
self.sandbox,
Expand Down Expand Up @@ -342,7 +345,7 @@ def parse_packages_xml(stdout: IO[str]) -> List[Package]:


def list_packages(
path: str,
nix_path: str,
system: str,
allow: AllowedFeatures,
check_meta: bool = False,
Expand All @@ -355,7 +358,9 @@ def list_packages(
"system",
system,
"-f",
path,
"<nixpkgs>",
"--nix-path",
nix_path,
"-qaP",
"--xml",
"--out-path",
Expand All @@ -378,13 +383,14 @@ def package_attrs(
package_set: Set[str],
system: str,
allow: AllowedFeatures,
nix_path: str,
ignore_nonexisting: bool = True,
) -> Dict[str, Attr]:
attrs: Dict[str, Attr] = {}

nonexisting = []

for attr in nix_eval(package_set, system, allow):
for attr in nix_eval(package_set, system, allow, nix_path):
if not attr.exists:
nonexisting.append(attr.name)
elif not attr.broken:
Expand All @@ -403,12 +409,14 @@ def join_packages(
specified_packages: Set[str],
system: str,
allow: AllowedFeatures,
nix_path: str,
) -> Set[str]:
changed_attrs = package_attrs(changed_packages, system, allow)
changed_attrs = package_attrs(changed_packages, system, allow, nix_path)
specified_attrs = package_attrs(
specified_packages,
system,
allow,
nix_path,
ignore_nonexisting=False,
)

Expand Down Expand Up @@ -439,6 +447,7 @@ def filter_packages(
skip_package_regexes: List[Pattern[str]],
system: str,
allow: AllowedFeatures,
nix_path: str,
) -> Set[str]:
packages: Set[str] = set()

Expand All @@ -456,6 +465,7 @@ def filter_packages(
specified_packages,
system,
allow,
nix_path,
)

for attr in changed_packages:
Expand Down
6 changes: 4 additions & 2 deletions tests/test_github_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
@patch("urllib.request.urlopen")
def test_post_result(mock_urlopen: MagicMock, helpers: Helpers) -> None:
with helpers.nixpkgs() as nixpkgs:
root = nixpkgs.path.parent

os.environ["PR"] = "1"
os.environ["NIX_PATH"] = f"nixpkgs={nixpkgs.path}"
os.environ["GITHUB_TOKEN"] = "foo"
os.environ["NIXPKGS_REVIEW_ROOT"] = str(root)
mock_urlopen.side_effect = [mock_open(read_data="{}")()]

report = nixpkgs.path.joinpath("..", "report.md")
report = root / "report.md"
with open(report, "w") as f:
f.write("")

Expand Down

0 comments on commit 316de1f

Please sign in to comment.