Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 35 additions & 91 deletions nixpkgs_review/nix.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class BuildConfig:

allow: AllowedFeatures
nix_path: str
local_system: str
nixpkgs_config: Path
num_eval_workers: int = 1
max_memory_size: int = 4096
Expand Down Expand Up @@ -99,17 +98,14 @@ class ShellConfig:
"""Configuration for launching a nix-shell."""

cache_directory: Path
local_system: str
build_graph: str
nix_path: str
nixpkgs_config: Path
nixpkgs_overlay: Path
run: str | None = None
sandbox: bool = False


def nix_shell(
attrs_per_system: dict[System, list[str]],
attrs_per_system: dict[System, list[Attr]],
config: ShellConfig,
) -> None:
bin_name = f"{config.build_graph}-shell"
Expand All @@ -121,8 +117,6 @@ def nix_shell(
shell_file_args = build_shell_file_args(
cache_dir=config.cache_directory,
attrs_per_system=attrs_per_system,
local_system=config.local_system,
nixpkgs_config=config.nixpkgs_config,
)
if config.sandbox:
args = _nix_shell_sandbox(nix_shell_bin, shell_file_args, config)
Expand Down Expand Up @@ -200,9 +194,6 @@ def tmpfs(path: Path | str, *, is_dir: bool = True) -> list[str]:
*bind("/"),
*bind("/dev", dev=True),
*tmpfs("/tmp"), # noqa: S108
# Required for evaluation
*bind(config.nixpkgs_config),
*bind(config.nixpkgs_overlay),
# /run (also cover sockets for wayland/pulseaudio and pipewires)
*bind(Path("/run/user").joinpath(uid), dev=True, try_=True),
# HOME
Expand Down Expand Up @@ -295,6 +286,8 @@ def _nix_eval_filter(packages: NixEvalResult) -> list[Attr]:
def multi_system_eval(
attr_names_per_system: dict[System, set[str]],
build_config: BuildConfig,
*,
instantiate: bool = False,
) -> dict[System, list[Attr]]:
attr_json = NamedTemporaryFile(mode="w+", delete=False) # noqa: SIM115
delete = True
Expand All @@ -311,7 +304,7 @@ def multi_system_eval(
str(build_config.num_eval_workers),
"--max-memory-size",
str(build_config.max_memory_size),
"--no-instantiate",
*(() if instantiate else ("--no-instantiate",)),
*_nix_common_flags(build_config.allow, build_config.nix_path),
"--expr",
f"(import {eval_script} {{ attr-json = {attr_json.name}; }})",
Expand Down Expand Up @@ -369,24 +362,26 @@ def nix_build(
attrs_per_system: dict[System, list[Attr]] = multi_system_eval(
attr_names_per_system,
build_config,
instantiate=True,
)

filtered_per_system = {
system: [attr.name for attr in attrs if not (attr.broken or attr.blacklisted)]
for system, attrs in attrs_per_system.items()
}
filtered_drv_paths = [
f"{attr.drv_path}^*"
for attrs in attrs_per_system.values()
for attr in attrs
if not (attr.broken or attr.blacklisted)
]

if all(len(filtered) == 0 for filtered in filtered_per_system.values()):
if len(filtered_drv_paths) == 0:
return attrs_per_system

command = [
build_graph,
"build",
"--file",
REVIEW_SHELL,
*_nix_common_flags(build_config.allow, build_config.nix_path),
"--no-link",
"--keep-going",
"--stdin",
]

if platform == "linux":
Expand All @@ -397,88 +392,37 @@ def nix_build(
"relaxed",
]

shell_file_args = build_shell_file_args(
cache_dir=cache_directory,
attrs_per_system=filtered_per_system,
local_system=build_config.local_system,
nixpkgs_config=build_config.nixpkgs_config,
)
_write_review_shell_drv(
cache_directory=cache_directory,
shell_file_args=shell_file_args,
allow=build_config.allow,
nix_path=build_config.nix_path,
)
command += shlex.split(args)

command += shell_file_args + shlex.split(args)
rebuilds_file = cache_directory / "rebuilds.txt"
with rebuilds_file.open("w+") as f:
f.write("".join(f"{p}\n" for p in filtered_drv_paths))
f.flush()
f.seek(0, os.SEEK_SET)

sh(command, stdin=f)

sh(command)
return attrs_per_system


def build_shell_file_args(
cache_dir: Path,
attrs_per_system: dict[System, list[str]],
local_system: str,
nixpkgs_config: Path,
attrs_per_system: dict[System, list[Attr]],
) -> list[str]:
attrs_file = cache_dir.joinpath("attrs.nix")
with attrs_file.open("w+") as f:
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("}")
outputs_file = cache_dir.joinpath("outputs.json")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need a gcroot for derivations on top, if we go with this. Both nix-eval-jobs and nix-instantiate have flags for gcroots for derivations.

with outputs_file.open("w+") as f:
json.dump(
[
str(output)
for attrs in attrs_per_system.values()
for attr in attrs
for output in (attr.outputs or {}).values()
],
f,
)

return [
"--argstr",
"local-system",
local_system,
"--argstr",
"nixpkgs-path",
str(cache_dir.joinpath("nixpkgs/")),
"--argstr",
"nixpkgs-config-path",
str(nixpkgs_config),
"--argstr",
"attrs-path",
str(attrs_file),
]


def _write_review_shell_drv(
cache_directory: Path,
shell_file_args: list[str],
allow: AllowedFeatures,
nix_path: str,
) -> None:
review_drv_link: Path = cache_directory / "review-shell.drv"

cmd: list[str] = [
"nix-instantiate",
*_nix_common_flags(allow, nix_path),
*shell_file_args,
REVIEW_SHELL,
"outputs-path",
str(outputs_file),
]
res = subprocess.run(
cmd,
capture_output=True,
text=True,
check=False,
)
if res.returncode != 0:
msg = "Failed to instantiate review shell derivation for caching"
if res.stderr:
msg = f"{msg}: {res.stderr.strip()}"
raise NixpkgsReviewError(msg)

drv_lines = [line.strip() for line in res.stdout.splitlines() if line.strip()]
if not drv_lines:
msg = "No review shell derivation path produced for caching"
raise NixpkgsReviewError(msg)

drv_path = drv_lines[-1]
review_drv_link.unlink(missing_ok=True)
review_drv_link.symlink_to(drv_path)
37 changes: 9 additions & 28 deletions nixpkgs_review/nix/review-shell.nix
Original file line number Diff line number Diff line change
@@ -1,48 +1,29 @@
{
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,
# Path to json file containing the outputs of all built packages
outputs-path,
}:

let
inherit (import <nixpkgs> { }) lib buildEnv mkShell;

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));
supportIgnoreSingleFileOutputs = (lib.functionArgs local-pkgs.buildEnv) ? ignoreSingleFileOutputs;
env = local-pkgs.buildEnv (
outputs = map builtins.storePath (builtins.fromJSON (builtins.readFile outputs-path));
supportIgnoreSingleFileOutputs = (lib.functionArgs buildEnv) ? ignoreSingleFileOutputs;
env = buildEnv (
{
name = "env";
paths = attrs;
paths = outputs;
ignoreCollisions = true;
}
// lib.optionalAttrs supportIgnoreSingleFileOutputs {
ignoreSingleFileOutputs = true;
}
);
in
(import nixpkgs-path { }).mkShell {
mkShell {
name = "review-shell";
preferLocalBuild = true;
allowSubstitutes = false;
dontWrapQtApps = true;
# see test_rev_command_with_pkg_count
packages = if builtins.length attrs > 50 then [ env ] else attrs;
packages = if builtins.length outputs > 50 then [ env ] else outputs;
}
7 changes: 2 additions & 5 deletions nixpkgs_review/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,8 @@ def __init__(
reports[system] = SystemReport(attrs)
self.system_reports: dict[System, SystemReport] = order_reports(reports)

def built_packages(self) -> dict[System, list[str]]:
return {
system: [a.name for a in report.built]
for system, report in self.system_reports.items()
}
def built_packages(self) -> dict[System, list[Attr]]:
return {system: report.built for system, report in self.system_reports.items()}

def write(self, directory: Path, pr: int | None) -> None:
# write logs first because snippets from them may be needed for the report
Expand Down
4 changes: 0 additions & 4 deletions nixpkgs_review/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -638,11 +638,8 @@ def start_review(
if not self.shell_options.no_shell:
shell_config = ShellConfig(
cache_directory=path,
local_system=self.build_config.local_system,
build_graph=self.shell_options.build_graph,
nix_path=self.builddir.nix_path,
nixpkgs_config=self.build_config.nixpkgs_config,
nixpkgs_overlay=self.builddir.overlay.path,
run=self.shell_options.run,
sandbox=self.shell_options.sandbox,
)
Expand Down Expand Up @@ -1037,7 +1034,6 @@ def build_config_from_args(
return BuildConfig(
allow=allow,
nix_path=nix_path,
local_system=current_system(),
nixpkgs_config=nixpkgs_config,
num_eval_workers=args.num_eval_workers,
max_memory_size=args.max_memory_size,
Expand Down
13 changes: 11 additions & 2 deletions nixpkgs_review/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,30 @@ def sh( # noqa: PLR0913
*,
cwd: Path | str | None = None,
env: dict[str, str] | None = None,
stdin: str | None = None,
stdin: str | int | IO[Any] | None = None,
stdout: int | None = None,
stderr: int | None = None,
quiet: bool = False,
) -> subprocess.CompletedProcess[str]:
if not quiet:
info("$ " + shlex.join(command))
env = os.environ | env if env else None

input_: str | None = None
stdin_: int | IO[Any] | None = None
if isinstance(stdin, str):
input_ = stdin
else:
stdin_ = stdin

return subprocess.run(
command,
cwd=cwd,
text=True,
check=False,
env=env,
input=stdin,
input=input_,
stdin=stdin_,
stdout=stdout,
stderr=stderr,
)
Expand Down