From c65bb3d8c2440de37bcf0803df5c9a7d452f9a92 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Wed, 21 Oct 2026 11:17:12 -0300 Subject: [PATCH 1/3] build: check the guest build's dep-info against its source identity The guest image cache keys end with the source identity of the Rust guest binaries, a digest of the paths and contents of the files that GuestRustBuildConstants lists, which the package manifests also record. Nothing tied that list to what the build reads, though: a module that the build compiled from outside nvx-rs/src, an include_str! target there, or a file that the build script watches would change the executables without changing the keys, and CI would reuse stale guest images. Cargo writes a dep-info file beside each executable that lists every source file of the crate that rustc read, the targets of include_str! and include_bytes! included, and every path that the build script watches. build_guest_binaries now reads it for each of the nine executables after the build, and fails, naming the files, unless the source identity covers each of them. Each file of a directory that the build script watches must be covered. An empty directory never is, because a file added to it later could change the build without changing the identity, and neither is a path that does not exist or lies outside the checkout; a relative path is rejected. Files that the build generates in its fresh target directory, such as those in OUT_DIR, derive from the others and are exempt. The Docker guest stage, the nvx-rs job, native guest builds, and verify-guest-determinism all build through this routine, so each of them runs the check. The identity covers the build today: the nine dep-info files of a guest build of this tree list build.rs and 96 files under nvx-rs/src. Tests run the build against a stand-in Cargo whose dep-info lists covered files and a generated one. They check that a file, a directory, a missing path, a file outside the checkout, and a relative path that the identity omits each fail the build without installing an output, that a missing dep-info file fails it, that an empty watched directory fails the check until it holds a covered file, and that the parser reads escaped spaces, continuation lines, comments, and phony rules. doc/usage.md describes the check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/usage.md | 11 +- scripts/nvx_tools/guest_binaries.py | 78 ++++++++++++ scripts/test_guest_binaries.py | 185 ++++++++++++++++++++++++++-- 3 files changed, 265 insertions(+), 9 deletions(-) diff --git a/doc/usage.md b/doc/usage.md index 883a814e2..ad23dbc7f 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -234,8 +234,15 @@ The Linux-only build command checks the exact `rust-toolchain.toml` pin and builds the nine Rust guest executables for `x86_64-unknown-linux-musl` with one `cargo build` of the `guest` profile, a fresh target directory, Cargo's locked mode, incremental compilation disabled, inherited Cargo profile overrides -removed, remapped source paths, and no linker build IDs. It runs the static-ELF -check and writes `manifest.json` with the source and executable SHA-256 digests. +removed, remapped source paths, and no linker build IDs. It then fails unless +the source identity, which +[`guest-binary-source-identity`](#guest-binary-source-identity) prints and the +guest image cache keys end with, covers every file that Cargo's dep-info lists +for the executables: each source file that rustc read, including the targets of +`include_str!` and `include_bytes!`, and each path that the build script +watches. Files that the build generates in its own target directory are exempt. +It runs the static-ELF check and writes `manifest.json` with the source and +executable SHA-256 digests. It updates the output of an earlier build in place, adding `nvx-managed-agent`, `nvx-ingress`, `nvx-time`, and `nvx-time-probe` when that build's manifest does not record them and removing the retired `nvx-port-io` executable when that diff --git a/scripts/nvx_tools/guest_binaries.py b/scripts/nvx_tools/guest_binaries.py index 9dfeb53ff..f09042ca1 100644 --- a/scripts/nvx_tools/guest_binaries.py +++ b/scripts/nvx_tools/guest_binaries.py @@ -119,6 +119,83 @@ def _cargo_environment(target_directory: Path) -> dict[str, str]: return environment +def _dep_info_paths(dep_info: Path) -> tuple[str, ...]: + """Return the prerequisites that a Cargo dep-info file lists. + + Cargo writes one Makefile rule per artifact and escapes each space in a path + with a backslash. + """ + contents = dep_info.read_text(encoding="utf-8").replace("\\\n", " ") + paths: list[str] = [] + for line in contents.splitlines(): + tokens = [ + token.replace("\\ ", " ") for token in re.findall(r"(?:\\ |\S)+", line) + ] + if not tokens or tokens[0].startswith("#"): + continue + if not tokens[0].endswith(":"): + raise ScriptError(f"Cargo dep-info is malformed: {dep_info}") + paths.extend(tokens[1:]) + return tuple(paths) + + +def _check_guest_build_inputs(target_directory: Path) -> None: + """Fail unless the source identity covers every file that the build read. + + Cargo's dep-info for each executable lists every source file of the crate + that rustc read, the targets of ``include_str!`` and ``include_bytes!`` + included, and every path that the build script watches. A file under the + build's own target directory, such as one in ``OUT_DIR``, derives from those. + Every file of a watched directory must be covered, and an empty one fails, + because a file added to it later could change the build without changing + the identity. + """ + root = BuildConstants.REPO_ROOT.resolve() + target_directory = target_directory.resolve() + identity = {path.resolve() for path in guest_binary_source_files()} + outputs = ( + target_directory + / GuestRustBuildConstants.TARGET + / GuestRustBuildConstants.PROFILE + ) + omitted: set[str] = set() + for binary in GuestRustBuildConstants.BINARIES: + dep_info = require_file( + outputs / f"{binary}.d", f"Cargo dep-info of Rust guest binary {binary}" + ) + for dependency in _dep_info_paths(dep_info): + path = Path(dependency) + if not path.is_absolute(): + raise ScriptError( + f"Cargo dep-info of Rust guest binary {binary} lists a relative " + f"path: {dependency}" + ) + path = path.resolve() + if path.is_relative_to(target_directory): + continue + files = ( + [entry for entry in path.rglob("*") if entry.is_file()] + if path.is_dir() + else [] + ) + # The identity holds files only, so it never covers an empty + # directory or a missing path. + omitted.update( + file.relative_to(root).as_posix() + if file.is_relative_to(root) + else file.as_posix() + for file in files or [path] + if file not in identity + ) + if omitted: + raise ScriptError( + "Rust guest binary build depends on files that its source identity " + "omits, so the guest image cache keys would miss changes to them; " + "GuestRustBuildConstants in scripts/nvx_tools/build_constants.py must " + "cover: " + ", ".join(sorted(omitted)) + ) + + def _guest_cargo_arguments() -> list[str]: """Return the Cargo arguments that select the canonical guest build.""" return [ @@ -366,6 +443,7 @@ def build_guest_binaries(output: Path) -> GuestBinaryBuildResult: cwd=GuestRustBuildConstants.DIRECTORY, env=environment, ) + _check_guest_build_inputs(target) built_directory = ( target / GuestRustBuildConstants.TARGET / GuestRustBuildConstants.PROFILE diff --git a/scripts/test_guest_binaries.py b/scripts/test_guest_binaries.py index a93b1634c..c84d95651 100644 --- a/scripts/test_guest_binaries.py +++ b/scripts/test_guest_binaries.py @@ -5,10 +5,11 @@ import io import json import os +import re import sys import tempfile import unittest -from collections.abc import Generator +from collections.abc import Generator, Sequence from pathlib import Path from typing import cast from unittest.mock import MagicMock, patch @@ -44,9 +45,30 @@ def _contents(directory: Path) -> dict[str, bytes]: return {path.name: path.read_bytes() for path in directory.iterdir()} +def _write_dep_info(directory: Path, names: list[str], sources: Sequence[Path]) -> None: + """Write the dep-info rule that Cargo writes beside each executable.""" + + def escape(path: Path) -> str: + return os.fspath(path).replace(" ", "\\ ") + + prerequisites = "".join(f" {escape(path)}" for path in sources) + for name in names: + (directory / f"{name}.d").write_text( + f"{escape(directory / name)}:{prerequisites}\n", encoding="utf-8" + ) + + @contextlib.contextmanager -def _fake_guest_build() -> Generator[MagicMock]: - """Stand for Cargo, which writes each binary's name as its contents.""" +def _fake_guest_build( + sources: Sequence[Path] = ( + BuildConstants.REPO_ROOT / "nvx-rs" / "build.rs", + BuildConstants.REPO_ROOT / "nvx-rs" / "src" / "lib.rs", + ), +) -> Generator[MagicMock]: + """Stand for Cargo, which writes each binary's name as its contents. + + The dep-info beside each binary lists ``sources``. + """ def run_cargo( command: list[str | os.PathLike[str]], @@ -61,10 +83,14 @@ def run_cargo( / GuestRustBuildConstants.PROFILE ) destination.mkdir(parents=True, exist_ok=True) - for index, value in enumerate(rendered): - if value == "--bin": - name = rendered[index + 1] - (destination / name).write_bytes(name.encode()) + names = [ + rendered[index + 1] + for index, value in enumerate(rendered) + if value == "--bin" + ] + for name in names: + (destination / name).write_bytes(name.encode()) + _write_dep_info(destination, names, sources) cargo = MagicMock(side_effect=run_cargo) with ( @@ -245,6 +271,14 @@ def run_cargo( ] for name in names: (destination / name).write_bytes(name.encode()) + # A file that the build generates in its own target directory + # derives from the inputs that the dep-info lists. + generated = destination / "build" / "nvx-0" / "out" / "generated.rs" + _write_dep_info( + destination, + names, + (crate / "build.rs", source / "lib.rs", generated), + ) cargo_home = root / "cargo-home" with ( @@ -344,6 +378,143 @@ def run_cargo( self.assertTrue(stale.exists()) self.assertFalse(any(root.glob(".artifacts.work-*"))) + def test_build_refuses_inputs_that_the_source_identity_omits(self): + # The guest image cache keys end with the source identity, so a file that + # the build reads outside it could change the executables without + # changing the keys. + crate = BuildConstants.REPO_ROOT / "nvx-rs" + with tempfile.TemporaryDirectory() as outside_directory: + outside = Path(outside_directory).resolve() / "outside.rs" + outside.write_text("pub fn outside() {}\n", encoding="utf-8") + for case, dependency, message in ( + ( + "file in the checkout", + crate / "artifacts.json", + "source identity omits.*: nvx-rs/artifacts.json$", + ), + ( + "directory in the checkout", + crate / "tests" / "fixtures", + "source identity omits.*nvx-rs/tests/fixtures/cli.json", + ), + ( + "file outside the checkout", + outside, + f"source identity omits.*: {re.escape(outside.as_posix())}$", + ), + ( + "missing file", + crate / "src" / "missing.rs", + "source identity omits.*: nvx-rs/src/missing.rs$", + ), + ("relative path", Path("src") / "lib.rs", "lists a relative path"), + ): + with ( + self.subTest(case), + tempfile.TemporaryDirectory( + prefix=".test-guest-binary-omitted-", + dir=BuildConstants.REPO_ROOT, + ) as temporary, + ): + root = Path(temporary) + output = root / "output" + with ( + _fake_guest_build((crate / "src" / "lib.rs", dependency)), + self.assertRaisesRegex(guest_binaries.ScriptError, message), + ): + guest_binaries.build_guest_binaries(output) + self.assertFalse(output.exists()) + self.assertFalse(any(root.glob(".output.work-*"))) + + def test_build_requires_the_dep_info_of_every_executable(self): + with tempfile.TemporaryDirectory( + prefix=".test-guest-binary-dep-info-", + dir=BuildConstants.REPO_ROOT, + ) as temporary: + output = Path(temporary) / "output" + with _fake_guest_build() as cargo: + built = cargo.side_effect + + def run_cargo( + command: list[str | os.PathLike[str]], + *, + cwd: Path, + env: dict[str, str], + ) -> None: + built(command, cwd=cwd, env=env) + ( + Path(env["CARGO_TARGET_DIR"]) + / GuestRustBuildConstants.TARGET + / GuestRustBuildConstants.PROFILE + / "nvx-time.d" + ).unlink() + + cargo.side_effect = run_cargo + with self.assertRaisesRegex( + guest_binaries.ScriptError, + "Cargo dep-info of Rust guest binary nvx-time not found", + ): + guest_binaries.build_guest_binaries(output) + self.assertFalse(output.exists()) + + def test_build_refuses_an_empty_watched_directory(self): + # Cargo reruns the build script when a file appears in a directory that + # it watches, so an empty one could later hold an input that leaves the + # source identity, and the guest image cache keys, as they are. + with tempfile.TemporaryDirectory( + prefix=".test-guest-binary-empty-", + dir=BuildConstants.REPO_ROOT, + ) as temporary: + root = Path(temporary) + for path in ( + *GuestRustBuildConstants.SOURCE_FILES, + Path("nvx-rs") / "src" / "lib.rs", + ): + (root / path).parent.mkdir(parents=True, exist_ok=True) + (root / path).write_text(f"{path.name}\n", encoding="utf-8") + watched = root / "nvx-rs" / "src" / "guest" / "watched" + watched.mkdir(parents=True) + target = root / "target" + outputs = ( + target + / GuestRustBuildConstants.TARGET + / GuestRustBuildConstants.PROFILE + ) + outputs.mkdir(parents=True) + _write_dep_info( + outputs, + list(GuestRustBuildConstants.BINARIES), + (root / "nvx-rs" / "src" / "lib.rs", watched), + ) + with patch.object(BuildConstants, "REPO_ROOT", root): + with self.assertRaisesRegex( + guest_binaries.ScriptError, + "source identity omits.*: nvx-rs/src/guest/watched$", + ): + guest_binaries._check_guest_build_inputs(target) + # Once the directory holds a file that the identity covers, it + # passes. + (watched / "module.rs").write_text("pub fn f() {}\n", encoding="utf-8") + guest_binaries._check_guest_build_inputs(target) + + def test_dep_info_lists_every_escaped_prerequisite(self): + with tempfile.TemporaryDirectory() as temporary: + dep_info = Path(temporary) / "nvx-exit.d" + dep_info.write_text( + "/out/nvx\\ exit: /repo/a\\ b.rs /repo/c.rs \\\n /repo/d.rs\n" + "\n" + "# env-dep:CARGO_PKG_NAME=nvx\n" + "/repo/c.rs:\n", + encoding="utf-8", + ) + self.assertEqual( + guest_binaries._dep_info_paths(dep_info), + ("/repo/a b.rs", "/repo/c.rs", "/repo/d.rs"), + ) + dep_info.write_text("/out/nvx-exit /repo/a.rs\n", encoding="utf-8") + with self.assertRaisesRegex(guest_binaries.ScriptError, "malformed"): + guest_binaries._dep_info_paths(dep_info) + def test_output_must_not_overlap_sources_or_their_ancestors(self): with tempfile.TemporaryDirectory( prefix=".test-guest-binary-source-", From 930e3fbfbb2ab3788cb06cae0b5d7f1c2a4c1d2a Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Wed, 21 Oct 2026 11:36:44 -0300 Subject: [PATCH 2/3] build: limit the guest binaries' source identity to their build inputs The source identity of the Rust guest binaries, which the guest image cache keys end with and their package manifests and provenance record, covered every file under nvx-rs/src. The guest build compiles only part of it, though. Of the 168 tracked files there, the nine executables of a guest build of this tree read 96, as their Cargo dep-info shows. The host and testing modules, the nvx-rs and nvx-fake-openvmm binaries, the test modules of protocol, and the managed agent's loopback harness, which only host-featured test builds compile, make up the other 72. A change to any of them changed the identity without changing a guest binary. GuestRustBuildConstants.SOURCE_PATTERN now selects the inputs: the manifest, lockfile, and build script, the toolchain pin, and the two Python modules that define the build, as before; the crate's Cargo configuration under nvx-rs/.cargo, which a build in the crate reads and the identity didn't cover; and every file under nvx-rs/src but the host side and the test modules. The host side is the host and testing modules and the binaries other than the guest ones; the test modules are the tests.rs files, the tests directories, and the modules that TEST_MODULES lists, today the loopback harness. A new module elsewhere counts, so a new shared or guest module needs no change here, and the dep-info check that every guest build runs fails the build if an excluded file becomes an input. For this tree, the identity is the 97 files that the dep-info lists and the five that it can't: Cargo.toml, Cargo.lock, rust-toolchain.toml, and the two Python modules. A directory named target under nvx-rs/src no longer drops out of it; nothing builds there. SOURCE_DIGEST_FORMAT moves to 2, because the identity's definition changed, so every guest image cache key changes once. The initramfs provenance, the package manifests, the Ubuntu converter digest, and the Azure Linux input digest take the same files. The guest image keys still hash all of nvx-rs, so CI rebuilds the images as before; a following change narrows them. Tests check that the identity holds every source file but the host side and the test modules, against an independent statement of that rule over the tree; that changes to the host side, the tests, the README, or artifacts.json leave it as it is, while a change to any input, a new guest or top-level module, or Cargo configuration changes it; and that a guest build whose dep-info lists a host or test module fails. The release tests' fixture checkout now also copies the sources of the nvx-rs CLI, which packaging checks its provenance against and which the fixture took from the guest identity before. doc/usage.md describes the inputs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/usage.md | 20 ++-- scripts/nvx_tools/build_constants.py | 29 +++++- scripts/nvx_tools/guest_binaries.py | 16 +++- scripts/test_guest_binaries.py | 137 +++++++++++++++++++++++++-- scripts/test_nvx_tools.py | 3 + 5 files changed, 186 insertions(+), 19 deletions(-) diff --git a/doc/usage.md b/doc/usage.md index ad23dbc7f..54502f213 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -147,12 +147,20 @@ python3 scripts/nvx.py guest-binary-source-identity [--root PATH] ``` Prints the source digest that guest package manifests record for the Rust guest -binaries: a digest of the relative path and content of every file under -`nvx-rs/src/`, the crate's manifest, lockfile, and build script, -`rust-toolchain.toml`, and the `build_constants.py` and `guest_binaries.py` -build modules. The guest build appends it to the Alpine, Ubuntu, and Azure -Linux cache keys, because `hashFiles` hashes contents only: moving a module -into its own `mod.rs` changes the key even when no bytes change. +binaries: a digest of the relative path and content of every file that their +build reads. Those are the crate's manifest, lockfile, and build script, its +Cargo configuration under `nvx-rs/.cargo/`, every file under `nvx-rs/src/` +except the host side and the test modules, `rust-toolchain.toml`, and the +`build_constants.py` and `guest_binaries.py` build modules. The host side is +the `host` and `testing` modules and the `nvx-rs` and `nvx-fake-openvmm` +binaries; the test modules are the `tests.rs` files, the `tests` directories, +and the managed agent's `loopback` harness. `GuestRustBuildConstants` in +`scripts/nvx_tools/build_constants.py` defines the set, and +[`build-guest-binaries`](#build-guest-binaries) fails when Cargo's dep-info +lists a file outside it. The guest build appends the digest to the Alpine, +Ubuntu, and Azure Linux cache keys, because `hashFiles` hashes contents only: +moving a module into its own `mod.rs` changes the key even when no bytes +change. ### `classify-ci-changes` diff --git a/scripts/nvx_tools/build_constants.py b/scripts/nvx_tools/build_constants.py index 7fcaf660a..985d8ff82 100644 --- a/scripts/nvx_tools/build_constants.py +++ b/scripts/nvx_tools/build_constants.py @@ -2,6 +2,7 @@ from __future__ import annotations +import re from pathlib import Path from typing import Final @@ -158,7 +159,7 @@ class GuestRustBuildConstants: DIRECTORY: Final = BuildConstants.REPO_ROOT / "nvx-rs" # The inputs of every guest executable, relative to a checkout's root: these - # files and every file under SOURCE_DIRECTORY outside a `target` tree. + # files, and every file under SOURCE_DIRECTORIES that SOURCE_PATTERN matches. SOURCE_FILES: Final = ( Path("nvx-rs") / "Cargo.toml", Path("nvx-rs") / "Cargo.lock", @@ -168,13 +169,20 @@ class GuestRustBuildConstants: Path("scripts") / "nvx_tools" / "guest_binaries.py", ) SOURCE_DIRECTORY: Final = Path("nvx-rs") / "src" + # The Cargo configuration that a build in the crate reads, and the sources. + SOURCE_DIRECTORIES: Final = (Path("nvx-rs") / ".cargo", SOURCE_DIRECTORY) + # Test-only modules that no guest build compiles, besides `tests.rs` files and + # `tests` directories. + TEST_MODULES: Final = ( + SOURCE_DIRECTORY / "guest" / "managed_agent" / "loopback.rs", + ) TOOLCHAIN_FILE: Final = BuildConstants.REPO_ROOT / "rust-toolchain.toml" TOOLCHAIN: Final = OpenVMMBuildConstants.RUST_TOOLCHAIN TARGET: Final = OpenVMMBuildConstants.MUSL_RUST_TARGET PROFILE: Final = "guest" FEATURE: Final = "guest" BINARIES_VARIABLE: Final = "NVX_GUEST_BINARIES" - SOURCE_DIGEST_FORMAT: Final = 1 + SOURCE_DIGEST_FORMAT: Final = 2 SOURCE_DIGEST_DOMAIN: Final = "nvx-rust-guest-binaries" BUILD_MANIFEST_NAME: Final = "manifest.json" BUILD_MANIFEST_FORMAT: Final = 1 @@ -203,6 +211,23 @@ class GuestRustBuildConstants: "nvx-time", "nvx-time-probe", ) + # Matches the path, relative to a checkout's root, of every input of the guest + # executables: SOURCE_FILES, the Cargo configuration, and every source file + # but the host side, that is, the `host` and `testing` modules and the + # binaries other than BINARIES, and the test modules. A guest build fails when + # Cargo's dep-info lists a file outside the set that it selects. + SOURCE_PATTERN: Final = re.compile( + "|".join(re.escape(path.as_posix()) for path in SOURCE_FILES) + + r"|nvx-rs/\.cargo/.+" + + r"|nvx-rs/src/bin/(?:" + + "|".join(re.escape(binary) for binary in BINARIES) + + r")\.rs" + + r"|(?!(?:" + + "|".join(re.escape(path.as_posix()) for path in TEST_MODULES) + + r")$)" + + r"nvx-rs/src/(?!bin/|(?:host|testing)(?:/|\.rs$))" + + r"(?!(?:[^/]+/)*tests(?:/|\.rs$)).+" + ) class AlpineBuildConstants: diff --git a/scripts/nvx_tools/guest_binaries.py b/scripts/nvx_tools/guest_binaries.py index f09042ca1..4d35b1715 100644 --- a/scripts/nvx_tools/guest_binaries.py +++ b/scripts/nvx_tools/guest_binaries.py @@ -40,11 +40,17 @@ def guest_binary_source_files(root: Path | None = None) -> tuple[Path, ...]: source = root / GuestRustBuildConstants.SOURCE_DIRECTORY if not source.is_dir(): raise ScriptError(f"nvx-rs source directory not found: {source}") - files.update( - path - for path in source.rglob("*") - if path.is_file() and "target" not in path.relative_to(source).parts - ) + for directory in GuestRustBuildConstants.SOURCE_DIRECTORIES: + if not (root / directory).is_dir(): + continue + files.update( + path + for path in (root / directory).rglob("*") + if path.is_file() + and GuestRustBuildConstants.SOURCE_PATTERN.fullmatch( + path.relative_to(root).as_posix() + ) + ) return tuple( sorted( (require_file(path, "Rust guest binary source input") for path in files), diff --git a/scripts/test_guest_binaries.py b/scripts/test_guest_binaries.py index c84d95651..fd7df2633 100644 --- a/scripts/test_guest_binaries.py +++ b/scripts/test_guest_binaries.py @@ -106,12 +106,13 @@ def run_cargo( class GuestBinaryBuildTests(unittest.TestCase): - def test_source_identity_covers_the_crate_and_the_toolchain(self): + def test_source_identity_covers_the_guest_build_inputs_only(self): + root = BuildConstants.REPO_ROOT relative = { - path.relative_to(BuildConstants.REPO_ROOT).as_posix() + path.relative_to(root).as_posix() for path in guest_binaries.guest_binary_source_files() } - # The build constants list the inputs that the inventory reads. + # The build constants list the inputs that Cargo's dep-info omits. self.assertLessEqual( {path.as_posix() for path in GuestRustBuildConstants.SOURCE_FILES}, relative, @@ -122,15 +123,129 @@ def test_source_identity_covers_the_crate_and_the_toolchain(self): self.assertIn("nvx-rs/build.rs", relative) self.assertIn("scripts/nvx_tools/build_constants.py", relative) self.assertIn("scripts/nvx_tools/guest_binaries.py", relative) - self.assertIn("nvx-rs/src/guest/exit.rs", relative) - self.assertIn("nvx-rs/src/guest/device_io.rs", relative) - self.assertFalse(any("/target/" in f"/{path}/" for path in relative)) + # The guest build compiles every source file of the crate but the host + # side, which the host and testing modules and the host binaries hold, and + # the test modules. + source = root / GuestRustBuildConstants.SOURCE_DIRECTORY + guest_roots = { + f"bin/{binary}.rs" for binary in GuestRustBuildConstants.BINARIES + } + test_modules = { + path.relative_to(GuestRustBuildConstants.SOURCE_DIRECTORY).as_posix() + for path in GuestRustBuildConstants.TEST_MODULES + } + compiled: set[str] = set() + for path in source.rglob("*"): + name = path.relative_to(source).as_posix() + parts = path.relative_to(source).parts + host = parts[0] in ("host", "host.rs", "testing", "testing.rs") or ( + parts[0] == "bin" and name not in guest_roots + ) + test = ( + "tests" in parts[:-1] or parts[-1] == "tests.rs" or name in test_modules + ) + if path.is_file() and not host and not test: + compiled.add(f"nvx-rs/src/{name}") + self.assertEqual( + {path for path in relative if path.startswith("nvx-rs/src/")}, compiled + ) + for path in ( + "nvx-rs/src/lib.rs", + "nvx-rs/src/contract.rs", + "nvx-rs/src/protocol/nvxs/host.rs", + "nvx-rs/src/guest/exit.rs", + "nvx-rs/src/guest/device_io.rs", + *(f"nvx-rs/src/{name}" for name in sorted(guest_roots)), + ): + with self.subTest(compiled=path): + self.assertIn(path, relative) + for path in ( + "nvx-rs/README.md", + "nvx-rs/artifacts.json", + "nvx-rs/tests/guest_tools.rs", + "nvx-rs/src/host/mod.rs", + "nvx-rs/src/testing/mod.rs", + "nvx-rs/src/bin/nvx-rs.rs", + "nvx-rs/src/bin/nvx-fake-openvmm.rs", + "nvx-rs/src/protocol/tests.rs", + "nvx-rs/src/protocol/nvxs/tests/guest.rs", + "nvx-rs/src/guest/managed_agent/loopback.rs", + ): + with self.subTest(omitted=path): + self.assertTrue((root / path).is_file()) + self.assertNotIn(path, relative) self.assertRegex(guest_binaries.guest_binary_source_sha256(), r"^[0-9a-f]{64}$") self.assertEqual( guest_binaries.pinned_guest_rust_toolchain(), GuestRustBuildConstants.TOOLCHAIN, ) + def test_source_identity_changes_only_with_the_guest_build_inputs(self): + # The guest image cache keys end with this identity, so a change to the + # host side or to a test must leave it as it is, and a change to a file + # that the guest build reads must change it. + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + inputs = ( + *(path.as_posix() for path in GuestRustBuildConstants.SOURCE_FILES), + "nvx-rs/src/lib.rs", + "nvx-rs/src/contract.rs", + "nvx-rs/src/net/cidr.rs", + "nvx-rs/src/protocol/mod.rs", + "nvx-rs/src/protocol/nvxs/host.rs", + "nvx-rs/src/guest/managed_agent/exec.rs", + "nvx-rs/src/bin/nvx-time.rs", + ) + others = ( + "nvx-rs/README.md", + "nvx-rs/artifacts.json", + "nvx-rs/tests/guest_tools.rs", + "nvx-rs/src/host/mod.rs", + "nvx-rs/src/host/cli/args.rs", + "nvx-rs/src/testing/fake_openvmm.rs", + "nvx-rs/src/bin/nvx-rs.rs", + "nvx-rs/src/bin/nvx-fake-openvmm.rs", + "nvx-rs/src/protocol/tests.rs", + "nvx-rs/src/protocol/nvxs/tests/host.rs", + "nvx-rs/src/guest/managed_agent/loopback.rs", + ) + for name in (*inputs, *others): + path = root / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(f"{name}\n", encoding="utf-8") + identity = guest_binaries.guest_binary_source_sha256(root) + + for name in (*others, "nvx-rs/src/host/added.rs", "nvx-rs/tests/added.rs"): + with self.subTest(unchanged=name): + (root / name).write_text("changed\n", encoding="utf-8") + self.assertEqual( + guest_binaries.guest_binary_source_sha256(root), identity + ) + for name in inputs: + with self.subTest(changed=name): + path = root / name + original = path.read_bytes() + path.write_text("changed\n", encoding="utf-8") + self.assertNotEqual( + guest_binaries.guest_binary_source_sha256(root), identity + ) + path.write_bytes(original) + # A new module counts unless it belongs to the host side or tests. + for name in ( + "nvx-rs/src/guest/added.rs", + "nvx-rs/src/added.rs", + "nvx-rs/.cargo/config.toml", + ): + with self.subTest(added=name): + path = root / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("added\n", encoding="utf-8") + self.assertNotEqual( + guest_binaries.guest_binary_source_sha256(root), identity + ) + path.unlink() + self.assertEqual(guest_binaries.guest_binary_source_sha256(root), identity) + def test_toolchain_pin_must_match_the_guest_build_constants(self): with tempfile.TemporaryDirectory() as temporary: root = Path(temporary) @@ -392,6 +507,16 @@ def test_build_refuses_inputs_that_the_source_identity_omits(self): crate / "artifacts.json", "source identity omits.*: nvx-rs/artifacts.json$", ), + ( + "host module", + crate / "src" / "host" / "mod.rs", + "source identity omits.*: nvx-rs/src/host/mod.rs$", + ), + ( + "test module", + crate / "src" / "protocol" / "tests.rs", + "source identity omits.*: nvx-rs/src/protocol/tests.rs$", + ), ( "directory in the checkout", crate / "tests" / "fixtures", diff --git a/scripts/test_nvx_tools.py b/scripts/test_nvx_tools.py index 10f1199fd..f31037b20 100644 --- a/scripts/test_nvx_tools.py +++ b/scripts/test_nvx_tools.py @@ -259,10 +259,13 @@ def _write_release_fixture( binary = openvmm_dir / "target" / "release" / binary_name revision = "0bc357bbcf3a654b63dfb51f1103c5751bf3d31f" guest_names = ReleaseBuildConstants.GUEST_ARTIFACT_NAMES + # The guest artifacts' inputs, and the sources of the nvx-rs executable that + # the release ships, whose host side no guest artifact reads. source_files = ( set(build._initramfs_source_files()) | set(ubuntu.customization_files()) | set(azurelinux.input_files()) + | set(nvx_rs.source_files()) ) for source in source_files: destination = root / source.relative_to(BuildConstants.REPO_ROOT) From 6d40e1e99d55becb83c496a5cde4a3ac247d36f1 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Wed, 21 Oct 2026 12:07:34 -0300 Subject: [PATCH 3/3] ci: key the guest images and VMM tests on the guest binaries' sources The Alpine, Ubuntu, Ubuntu distro, and Azure Linux cache keys hashed all of nvx-rs but its target directory, and the toolchain pin, and the OpenVMM VMM tests took every nvx-rs path as an input, because they boot the Alpine initramfs. The images carry only the crate's Rust guest binaries, though, so a change to its host side or tests rebuilt every guest image and ran the VMM tests, the longest WHP jobs. Of the 42 dev merges through #659, the VMM tests ran on 33, 26 of them only because nvx-rs changed, and each of the 27 that changed nvx-rs changed every guest image key. A cold guest-artifact job takes a median 5.3 minutes instead of 0.4, and every job that boots a guest waits for it. The keys already end with the guest binaries' source identity, which now covers exactly the files that their build reads, and which every guest build checks against Cargo's dep-info. Drop nvx-rs and rust-toolchain.toml from the hashFiles of the image keys, so that the identity alone covers them, and select the VMM tests by the same pattern, GuestRustBuildConstants.SOURCE_PATTERN, instead of by every nvx-rs path. run-tests still covers all of nvx-rs, because the NVX microVM tests run the nvx-rs CLI. Replaying the classifier over those 42 merges, 11 of the 26 no longer run the VMM tests, and 12 of the 27 keep every guest image key: they changed only the host side, protocol tests, integration tests, or the loopback harness. The other 15 changed files that the guest binaries compile, such as guest and protocol modules, lib.rs, Cargo.toml, and Cargo.lock, and still run both. The drift guard that derives the VMM tests' inputs from the guest cache keys now also takes every file of the identity, and checks that the Alpine key ends with it. A new test checks every file under nvx-rs: those in the identity select the VMM tests, the others neither select them nor match a hashFiles pattern of an image key, and all of them select the microVM tests. New modules count unless they belong to the host side or the tests. For changes to the host side, the CLI and fake binaries, protocol tests, the loopback harness, integration tests, and artifacts.json, it checks that pushes and pull requests run the microVM tests but skip the VMM tests, and that the identity, and with it every image key, stays as it is. For changes to the manifest, lockfile, build script, lib.rs, protocol, a guest module, and a guest binary, it checks that they run both and change the identity. doc/ci.md, doc/usage.md, and the nvx-rs README describe the inputs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../actions/build-guest-artifacts/action.yml | 10 +- doc/ci.md | 43 +++-- doc/usage.md | 9 +- nvx-rs/README.md | 15 ++ scripts/nvx_tools/ci.py | 9 +- scripts/test_nvx_tools.py | 174 +++++++++++++++++- 6 files changed, 234 insertions(+), 26 deletions(-) diff --git a/.github/actions/build-guest-artifacts/action.yml b/.github/actions/build-guest-artifacts/action.yml index e043f0b71..2e8b32912 100644 --- a/.github/actions/build-guest-artifacts/action.yml +++ b/.github/actions/build-guest-artifacts/action.yml @@ -79,14 +79,16 @@ runs: AZURELINUX_VERSION: ${{ steps.guest-versions.outputs.azurelinux-version }} KERNEL_INPUT_HASH: ${{ hashFiles('SOURCE-MANIFEST.json', 'kernel/config-microvm', 'kernel/patches/**', 'docker/Dockerfile', '.dockerignore', 'scripts/nvx_tools/build.py', 'scripts/nvx_tools/build_config.py', 'scripts/nvx_tools/build_constants.py') }} DEBUG_KERNEL_INPUT_HASH: ${{ hashFiles('SOURCE-MANIFEST.json', 'kernel/config-microvm', 'kernel/config-microvm-debug', 'kernel/patches/**', 'docker/Dockerfile', '.dockerignore', 'scripts/nvx_tools/build.py', 'scripts/nvx_tools/build_config.py', 'scripts/nvx_tools/build_constants.py') }} - ALPINE_INPUT_HASH: ${{ hashFiles('SOURCE-MANIFEST.json', 'guest/common/**', 'guest/alpine/**', 'nvx-rs/**', '!nvx-rs/target/**', 'rust-toolchain.toml', 'docker/Dockerfile', '.dockerignore', 'scripts/nvx_tools/build.py', 'scripts/nvx_tools/build_config.py', 'scripts/nvx_tools/build_constants.py', 'scripts/nvx_tools/common.py', 'scripts/nvx_tools/elf.py', 'scripts/nvx_tools/guest_binaries.py', 'scripts/nvx_tools/guests.py') }} - UBUNTU_INPUT_HASH: ${{ hashFiles('SOURCE-MANIFEST.json', 'guest/common/**', 'guest/ubuntu/**', 'ubuntu/**', 'nvx-rs/**', '!nvx-rs/target/**', 'rust-toolchain.toml', 'docker/Dockerfile', '.dockerignore', 'scripts/nvx_tools/build.py', 'scripts/nvx_tools/build_config.py', 'scripts/nvx_tools/build_constants.py', 'scripts/nvx_tools/common.py', 'scripts/nvx_tools/elf.py', 'scripts/nvx_tools/guest_binaries.py', 'scripts/nvx_tools/guests.py', 'scripts/nvx_tools/ubuntu.py') }} - AZURELINUX_INPUT_HASH: ${{ hashFiles('SOURCE-MANIFEST.json', 'guest/common/**', 'azurelinux/**', 'nvx-rs/**', '!nvx-rs/target/**', 'rust-toolchain.toml', 'docker/Dockerfile', '.dockerignore', 'scripts/nvx_tools/azurelinux.py', 'scripts/nvx_tools/build.py', 'scripts/nvx_tools/build_config.py', 'scripts/nvx_tools/build_constants.py', 'scripts/nvx_tools/common.py', 'scripts/nvx_tools/elf.py', 'scripts/nvx_tools/guest_binaries.py', 'scripts/nvx_tools/guests.py') }} + ALPINE_INPUT_HASH: ${{ hashFiles('SOURCE-MANIFEST.json', 'guest/common/**', 'guest/alpine/**', 'docker/Dockerfile', '.dockerignore', 'scripts/nvx_tools/build.py', 'scripts/nvx_tools/build_config.py', 'scripts/nvx_tools/build_constants.py', 'scripts/nvx_tools/common.py', 'scripts/nvx_tools/elf.py', 'scripts/nvx_tools/guest_binaries.py', 'scripts/nvx_tools/guests.py') }} + UBUNTU_INPUT_HASH: ${{ hashFiles('SOURCE-MANIFEST.json', 'guest/common/**', 'guest/ubuntu/**', 'ubuntu/**', 'docker/Dockerfile', '.dockerignore', 'scripts/nvx_tools/build.py', 'scripts/nvx_tools/build_config.py', 'scripts/nvx_tools/build_constants.py', 'scripts/nvx_tools/common.py', 'scripts/nvx_tools/elf.py', 'scripts/nvx_tools/guest_binaries.py', 'scripts/nvx_tools/guests.py', 'scripts/nvx_tools/ubuntu.py') }} + AZURELINUX_INPUT_HASH: ${{ hashFiles('SOURCE-MANIFEST.json', 'guest/common/**', 'azurelinux/**', 'docker/Dockerfile', '.dockerignore', 'scripts/nvx_tools/azurelinux.py', 'scripts/nvx_tools/build.py', 'scripts/nvx_tools/build_config.py', 'scripts/nvx_tools/build_constants.py', 'scripts/nvx_tools/common.py', 'scripts/nvx_tools/elf.py', 'scripts/nvx_tools/guest_binaries.py', 'scripts/nvx_tools/guests.py') }} run: | set -euo pipefail # hashFiles hashes contents only, so the kernel keys also name each patch # path and its order, and the guest image keys the path-aware digest of - # the Rust guest sources that their package manifests record. + # the Rust guest sources that their package manifests record. Those are + # the files that the guest binaries' build reads, the only nvx-rs files + # that the image keys cover. patch_identity=$(python3 scripts/nvx.py kernel-patch-identity) rust_source_identity=$(python3 scripts/nvx.py guest-binary-source-identity) { diff --git a/doc/ci.md b/doc/ci.md index e284a975e..931cd4d11 100644 --- a/doc/ci.md +++ b/doc/ci.md @@ -233,13 +233,25 @@ process exits, so delayed final output chunks cannot create a false failure. Shared guest artifacts are built with Docker on a GitHub-hosted Ubuntu runner. The kernel, Alpine initramfs, Ubuntu initramfs, and Ubuntu EROFS layer use -separate cache keys. Every guest key includes `nvx-rs/**` except its `target` -directory, `rust-toolchain.toml`, the shared binary-build modules, the -Dockerfile, and `.dockerignore`; the Rust builder image itself is pinned by -digest. Because `hashFiles` hashes contents only, every guest key also ends -with the path-aware Rust source digest that the guest package manifests -record, which `nvx.py guest-binary-source-identity` prints. Ubuntu keys also -include the Canonical archive pin, supplemental +separate cache keys. Every guest image key includes the shared binary-build +modules, the Dockerfile, and `.dockerignore`; the Rust builder image itself is +pinned by digest. Every guest image key also ends with the path-aware digest +of the Rust guest binaries' inputs that the guest package manifests record, +which `nvx.py guest-binary-source-identity` prints, because the images carry +only those binaries of `nvx-rs` and `hashFiles` hashes contents only. The +inputs are the files that the guest binaries' build reads: +`rust-toolchain.toml`; the crate's manifest, lockfile, build script, and Cargo +configuration; every file under `nvx-rs/src/` but the host side, which is the +`host` and `testing` modules and the `nvx-rs` and `nvx-fake-openvmm` binaries, +and the test modules, which are the `tests.rs` files, the `tests` directories, +and the managed agent's `loopback` harness; and the two Python modules that +define the build. `GuestRustBuildConstants` in +[`build_constants.py`](../scripts/nvx_tools/build_constants.py) selects them, +and every guest build fails when Cargo's dep-info for the nine executables +lists another file, such as a host module that guest code starts to compile. +A change to the host side or a test of `nvx-rs` therefore rebuilds no guest +image, and a change to any file that a guest binary compiles rebuilds them +all. Ubuntu keys also include the Canonical archive pin, supplemental package lock, common guest sources, shared download and guest descriptor modules, and converter implementation. Artifact upload retains the Alpine filenames and adds the distinct Ubuntu filenames. Each @@ -510,12 +522,16 @@ participate in the decision: runner setup scripts, `ci.yml`, the actions that their job uses, `nvx.py`, and the modules that `ci.py` imports. The VMM tests also boot the kernel and Alpine initramfs, so they add every input of those artifacts' cache keys and - the `build-guest-artifacts` action. + the `build-guest-artifacts` action. Of `nvx-rs`, those are only the inputs + of the guest binaries, which `GuestRustBuildConstants.SOURCE_PATTERN` + matches, so a change to its host side or tests runs the NVX microVM tests + but not the VMM tests. A push or pull request that changes neither OpenVMM suite's inputs, such as -one that changes only the microVM tests, the benchmarks, or the Ubuntu or -Azure Linux guests, therefore skips the two jobs that occupy the WHP runners -longest. The nightly schedule runs at 05:17 UTC, and a manual workflow +one that changes only the microVM tests, the benchmarks, the Ubuntu or Azure +Linux guests, or the host side of `nvx-rs`, therefore skips the two jobs that +occupy the WHP runners longest. The nightly schedule runs at 05:17 UTC, and a +manual workflow dispatch starts the same full run. Both select the full matrix before the detector examines a comparison base. Each full run has a separate concurrency key, so it neither cancels nor queues behind a `dev` push. Full runs build, @@ -537,7 +553,10 @@ missing output would skip its jobs. expects every scheduled job to succeed and every other job to be skipped. Tests in `scripts/test_nvx_tools.py` derive each suite's inputs from the actions that its jobs use, the modules that `ci.py` imports, and the guest -cache keys, and fail when a pattern misses one. +cache keys, including the files of the guest binaries' source identity, and +fail when a pattern misses one. They also check that no other file of +`nvx-rs` selects the VMM tests or changes a guest image key, while every one +selects the NVX microVM tests. ## Host qualification diff --git a/doc/usage.md b/doc/usage.md index 54502f213..af9252694 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -157,10 +157,11 @@ binaries; the test modules are the `tests.rs` files, the `tests` directories, and the managed agent's `loopback` harness. `GuestRustBuildConstants` in `scripts/nvx_tools/build_constants.py` defines the set, and [`build-guest-binaries`](#build-guest-binaries) fails when Cargo's dep-info -lists a file outside it. The guest build appends the digest to the Alpine, -Ubuntu, and Azure Linux cache keys, because `hashFiles` hashes contents only: -moving a module into its own `mod.rs` changes the key even when no bytes -change. +lists a file outside it. The Alpine, Ubuntu, and Azure Linux cache keys end +with the digest and hash no other file of `nvx-rs`, so a change to the +crate's host side or tests rebuilds no guest image. Because the digest covers +paths, and `hashFiles` hashes contents only, moving a module into its own +`mod.rs` changes the keys even when no bytes change. ### `classify-ci-changes` diff --git a/nvx-rs/README.md b/nvx-rs/README.md index b5c5945a6..7b7440741 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -526,6 +526,21 @@ Native guest builds, the digest-pinned Docker Rust stage, and CI all call this routine. Docker builds the set once and installs identical bytes in all three guest images; Ubuntu determinism checks rebuild from fresh targets. +Of this crate, the guest images depend only on the files that this build reads: +`Cargo.toml`, `Cargo.lock`, `build.rs`, any Cargo configuration in `.cargo/`, +and every file under `src/` but the host side and the test modules. The host +side is `src/host/`, `src/testing/`, and the `nvx-rs` and `nvx-fake-openvmm` +binaries; the test modules are the `tests.rs` files, the `tests` directories, +and the managed agent's `loopback` harness. CI keys the guest images on those +files and runs the OpenVMM VMM tests, which boot the Alpine image, only when +one of them changes, while every change to the crate runs the NVX microVM +tests; see [Continuous integration](../doc/ci.md#change-classification). Keep +host-only code under `src/host/` and test-only modules in `tests.rs` files or +`tests` directories. The build fails when Cargo's dep-info lists another file, +which `GuestRustBuildConstants` in +[`build_constants.py`](../scripts/nvx_tools/build_constants.py) then has to +select. + ## Testing ```bash diff --git a/scripts/nvx_tools/ci.py b/scripts/nvx_tools/ci.py index 1708ba4b7..ad4fa3f09 100644 --- a/scripts/nvx_tools/ci.py +++ b/scripts/nvx_tools/ci.py @@ -13,6 +13,7 @@ from .build_constants import ( AlpineBuildConstants, + GuestRustBuildConstants, KernelBuildConstants, OpenVMMBuildConstants, ZstdBuildConstants, @@ -132,8 +133,11 @@ ) # The OpenVMM VMM tests also boot the NVX kernel and Alpine initramfs, so they # add the inputs of those artifacts' cache keys and of the job that builds them. +# Of nvx-rs, the Alpine key covers only the files that the guest binaries' build +# reads, through their source identity, so a change to its host side or tests +# leaves the VMM tests out. OPENVMM_VMM_TEST_INPUT_PATHS = re.compile( - r"^(openvmm($|/)|nvx-rs/|rust-toolchain\.toml$|scripts/setup/|" + r"^(openvmm($|/)|rust-toolchain\.toml$|scripts/setup/|" r"\.github/workflows/ci\.yml$|" r"\.github/actions/(build-guest-artifacts|checkout-openvmm|sccache|" r"setup-curl|validate-runner)/|" @@ -141,7 +145,8 @@ r"scripts/nvx_tools/" r"(__init__|build|build_config|build_constants|ci|common|elf|" r"guest_binaries|guests)\.py$|" - r"SOURCE-MANIFEST\.json$|docker/Dockerfile$|\.dockerignore$|kernel/|guest/(alpine|common)/)" + r"SOURCE-MANIFEST\.json$|docker/Dockerfile$|\.dockerignore$|kernel/|guest/(alpine|common)/|" + rf"(?:{GuestRustBuildConstants.SOURCE_PATTERN.pattern})$)" ) diff --git a/scripts/test_nvx_tools.py b/scripts/test_nvx_tools.py index f31037b20..f72486656 100644 --- a/scripts/test_nvx_tools.py +++ b/scripts/test_nvx_tools.py @@ -4206,6 +4206,11 @@ def test_ci_change_classification_selects_suites_by_input(self): (("aci_edge_sandboxes/src/lib.rs",), (True, True, False, False)), (("nvx-rs/src/lib.rs",), (True, True, False, True)), (("nvx-rs/Cargo.lock",), (True, True, False, True)), + (("nvx-rs/src/guest/exit.rs",), (True, True, False, True)), + (("nvx-rs/src/bin/nvx-time.rs",), (True, True, False, True)), + (("nvx-rs/src/host/cli/args.rs",), (True, True, False, False)), + (("nvx-rs/src/bin/nvx-rs.rs",), (True, True, False, False)), + (("nvx-rs/tests/cli.rs",), (True, True, False, False)), (("tests/runtime-adapter/src/lib.rs",), (True, True, False, False)), (("scripts/nvx_tools/microvm_tests.py",), (True, True, False, False)), ( @@ -4391,7 +4396,19 @@ def imported_modules(module: str) -> set[str]: set(guest_inputs), {"KERNEL", "DEBUG_KERNEL", "ALPINE", "UBUNTU", "AZURELINUX"}, ) + # The Alpine key also ends with the source identity of the Rust guest + # binaries, whose every file is an input of the image too. + self.assertIn( + 'echo "alpine=alpine-image-v2-${ALPINE_VERSION}-${ALPINE_INPUT_HASH}' + '-${rust_source_identity}"', + guest_action, + ) + guest_binary_inputs = { + path.relative_to(BuildConstants.REPO_ROOT).as_posix() + for path in guest_binaries.guest_binary_source_files() + } vmm_inputs |= guest_inputs["KERNEL"] | guest_inputs["ALPINE"] + vmm_inputs |= guest_binary_inputs test_inputs = vmm_inputs.union(*guest_inputs.values()) | { ".github/workflows/run-nvx-microvm-tests.yml", ".github/workflows/build-openvmm-binary.yml", @@ -4419,6 +4436,149 @@ def imported_modules(module: str) -> set[str]: with self.subTest(pattern=pattern.pattern, path=path): self.assertIsNotNone(pattern.match(path)) + def test_only_guest_binary_inputs_rebuild_guests_and_run_vmm_tests(self): + # The guest images carry only the Rust guest binaries of nvx-rs, so a + # change to a file that their build reads rebuilds every guest image and + # runs the OpenVMM VMM tests, which boot the Alpine image, and no other + # nvx-rs change does either. The NVX microVM tests run the nvx-rs CLI, so + # every nvx-rs change runs them. + root = BuildConstants.REPO_ROOT + identity = { + path.relative_to(root).as_posix() + for path in guest_binaries.guest_binary_source_files() + } + crate: set[str] = set() + for directory, names, files in os.walk(root / "nvx-rs"): + if Path(directory) == root / "nvx-rs": + names[:] = [name for name in names if name != "target"] + crate.update( + (Path(directory) / name).relative_to(root).as_posix() for name in files + ) + self.assertLessEqual({"nvx-rs/src/host/mod.rs", "nvx-rs/src/lib.rs"}, crate) + for path in sorted(crate | identity): + with self.subTest(path=path): + self.assertIsNotNone(ci.CI_TEST_INPUT_PATHS.match(path)) + if path in identity: + self.assertIsNotNone(ci.OPENVMM_VMM_TEST_INPUT_PATHS.match(path)) + else: + self.assertIsNone(ci.OPENVMM_VMM_TEST_INPUT_PATHS.match(path)) + # A new module counts unless it belongs to the host side or the tests, + # and a file beside an input doesn't. + for path, selected in ( + ("nvx-rs/src/added.rs", True), + ("nvx-rs/src/guest/added/mod.rs", True), + ("nvx-rs/.cargo/config.toml", True), + ("nvx-rs/src/host/added.rs", False), + ("nvx-rs/src/protocol/added/tests.rs", False), + ("nvx-rs/Cargo.toml.orig", False), + ("nvx-rs/src/bin/nvx-time.rs.orig", False), + ): + with self.subTest(path=path): + self.assertIsNotNone(ci.CI_TEST_INPUT_PATHS.match(path)) + self.assertEqual( + ci.OPENVMM_VMM_TEST_INPUT_PATHS.match(path) is not None, selected + ) + + # The guest image keys hash no nvx-rs file and end with the identity. + action = ( + root / ".github" / "actions" / "build-guest-artifacts" / "action.yml" + ).read_text(encoding="utf-8") + image_patterns = [ + re.compile( + "".join( + ".*" + if part == "**" + else "[^/]*" + if part == "*" + else re.escape(part) + for part in re.split(r"(\*\*|\*)", pattern) + ) + ) + for match in re.finditer( + r"^\s+(ALPINE|UBUNTU|AZURELINUX)_INPUT_HASH: " + r"\$\{\{ hashFiles\((.+)\) \}\}$", + action, + re.MULTILINE, + ) + for pattern in re.findall(r"'([^']+)'", match.group(2)) + ] + self.assertTrue(image_patterns) + for path in sorted(crate): + with self.subTest(hashed=path): + self.assertFalse( + any(pattern.fullmatch(path) for pattern in image_patterns) + ) + for key in ("alpine", "ubuntu", "ubuntu-distro", "azurelinux"): + with self.subTest(key=key): + line = next( + line.strip() + for line in action.splitlines() + if line.strip().startswith(f'echo "{key}=') + ) + self.assertTrue(line.endswith('-${rust_source_identity}"')) + + host_side = ( + "nvx-rs/src/host/cli/args.rs", + "nvx-rs/src/host/mod.rs", + "nvx-rs/src/testing/fake_openvmm.rs", + "nvx-rs/src/bin/nvx-rs.rs", + "nvx-rs/src/bin/nvx-fake-openvmm.rs", + "nvx-rs/src/protocol/nvxs/tests/host.rs", + "nvx-rs/src/guest/managed_agent/loopback.rs", + "nvx-rs/tests/guest_tools.rs", + "nvx-rs/artifacts.json", + ) + guest_side = ( + "nvx-rs/Cargo.toml", + "nvx-rs/Cargo.lock", + "nvx-rs/build.rs", + "nvx-rs/src/lib.rs", + "nvx-rs/src/protocol/nvxs/host.rs", + "nvx-rs/src/guest/time/checks.rs", + "nvx-rs/src/bin/nvx-managed-agent.rs", + ) + with tempfile.TemporaryDirectory() as temporary: + # A checkout that holds only the identity's files and the changed + # ones has the identity of this one. + checkout = Path(temporary) + for path in (*identity, *host_side): + destination = checkout / path + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(root / path, destination) + original = guest_binaries.guest_binary_source_sha256(checkout) + self.assertEqual(original, guest_binaries.guest_binary_source_sha256()) + for paths, guest in ( + *(((path,), False) for path in host_side), + (host_side, False), + *(((path,), True) for path in guest_side), + (("nvx-rs/src/host/mod.rs", "nvx-rs/src/guest/mod.rs"), True), + ): + for event_name in ("pull_request", "push"): + with self.subTest(changed=paths, event_name=event_name): + changes = ci.classify_ci_changes(event_name, paths) + self.assertEqual( + changes, ci.CiChanges(True, True, False, guest) + ) + expected = ci.required_ci_expected_results( + event_name, same_repository=True, changes=changes + ) + for job in ci.REQUIRED_CI_MICROVM_TEST_JOBS: + self.assertEqual(expected[job], "success", job) + self.assertEqual( + expected["openvmm-vmm-tests"], + "success" if guest else "skipped", + ) + with self.subTest(changed=paths): + contents = {path: (checkout / path).read_bytes() for path in paths} + for path, data in contents.items(): + (checkout / path).write_bytes(data + b"\n// changed\n") + self.assertEqual( + guest_binaries.guest_binary_source_sha256(checkout) != original, + guest, + ) + for path, data in contents.items(): + (checkout / path).write_bytes(data) + def test_guest_cache_keys_hash_the_docker_build_context_filter(self): # Docker excludes files by .dockerignore before COPY, so a change to that # file changes the guest build even when no other input changes. @@ -10305,11 +10465,16 @@ def test_ci_azurelinux_cache_key_covers_build_input_digest(self): ) patterns = cache_input.split("'")[1::2] self.assertIn("scripts/nvx_tools/azurelinux.py", patterns) + # The key ends with the source identity of the Rust guest binaries, + # which covers the files that their build reads. + self.assertIn('-${AZURELINUX_INPUT_HASH}-${rust_source_identity}"', action) + identity = set(guest_binaries.guest_binary_source_files()) for path in azurelinux.input_files(): relative = path.relative_to(BuildConstants.REPO_ROOT).as_posix() with self.subTest(path=relative): self.assertTrue( - any( + path in identity + or any( relative == pattern or ( pattern.endswith("/**") @@ -10421,9 +10586,10 @@ def test_ci_guest_cache_keys_include_shared_build_modules(self): self.assertIn("'scripts/nvx_tools/guests.py'", assignment) self.assertIn("'scripts/nvx_tools/elf.py'", assignment) self.assertIn("'scripts/nvx_tools/guest_binaries.py'", assignment) - self.assertIn("'nvx-rs/**'", assignment) - self.assertIn("'!nvx-rs/target/**'", assignment) - self.assertIn("'rust-toolchain.toml'", assignment) + # The Rust guest source identity at the end of each key covers the + # files of nvx-rs that the guest build reads, and the toolchain pin. + self.assertNotIn("nvx-rs", assignment) + self.assertNotIn("rust-toolchain.toml", assignment) for prefix in ( "alpine-image", "ubuntu-initramfs",