From 82dec56029e3e1c39401ff12bf807d68a44f4164 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Tue, 27 Oct 2026 10:36:46 -0700 Subject: [PATCH 1/6] scripts: draw the module diagrams of the nvx crate The nvx crate holds more than 200 modules across the host, the guest, and the code that they share, and nothing showed how they fit together. module_diagrams.py draws them with cargo-modules and Graphviz. It exports the modules of the crate's library, with its cli, testing, and guest features, for x86_64-unknown-linux-musl, the guests' target, and for x86_64-pc-windows-msvc, and merges the two exports into one set of diagrams: the top-level modules, with what each holds and how many modules it contains; the dependencies between top-level modules, counted by the pairs of their modules in which one uses the other; and the hierarchy of each top-level module that contains modules. A module that only one platform builds, such as the guest module or the host's sys module on Linux, names that platform, while the color of a box shows the part of the crate that holds the module and its border the module's visibility. It writes the diagrams, as SVG images, with the Markdown page that shows them, to build/modules, or to the directory that --output names. It replaces the PowerShell driver, scripts/module-diagrams.ps1, and the interactive atlas, scripts/module_atlas.py, of the wip/module-diagrams branch. A Markdown page of SVG images shows on GitHub, where the atlas's HTML page shows only as source, so the documentation can publish it; the page keeps the atlas's diagrams, colors, and descriptions. The script reads its targets from the build constants, which own the target triples, and refuses an export that holds anything but modules, their containment, and their uses, so that a change in cargo-modules' output cannot draw wrong diagrams silently. It replaces an output directory only when the directory holds nothing but the page and the diagrams of an earlier run, whose first line marks the page, and never writes through a symbolic link. When cargo-modules is missing, it names the release to install, 0.27.0, which builds with the Rust release that rust-toolchain.toml pins. Unit tests cover how it reads exports, written here in cargo-modules' DOT form, how it merges the platforms' modules, the graphs and the page that it draws, how it replaces an earlier run's output, and the commands that a drawing runs, with a stand-in for cargo-modules and Graphviz. validate-nvx runs them on Linux and Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/actions/validate-nvx/action.yml | 4 +- doc/project-structure.md | 1 + scripts/module_diagrams.py | 733 ++++++++++++++++++++++++ scripts/test_module_diagrams.py | 669 +++++++++++++++++++++ 4 files changed, 1405 insertions(+), 2 deletions(-) create mode 100644 scripts/module_diagrams.py create mode 100644 scripts/test_module_diagrams.py diff --git a/.github/actions/validate-nvx/action.yml b/.github/actions/validate-nvx/action.yml index 89bf2e043..e48ab5d28 100644 --- a/.github/actions/validate-nvx/action.yml +++ b/.github/actions/validate-nvx/action.yml @@ -11,7 +11,7 @@ runs: set -euo pipefail python3 -m pip install --requirement scripts/setup/azure/requirements.txt python3 -m compileall -q scripts - python3 -m unittest scripts/test_performance.py scripts/test_nvx_tools.py scripts/test_guest_binaries.py scripts/test_microvm_tests.py scripts/test_nightly_ci.py scripts/test_development_release.py scripts/test_crate_release.py scripts/test_host_telemetry.py scripts/test_time_abi.py scripts/test_tool_versions.py scripts/test_markdown_links.py scripts/test_nvx_rs_mutants.py scripts/test_setup_azure.py -v + python3 -m unittest scripts/test_performance.py scripts/test_nvx_tools.py scripts/test_guest_binaries.py scripts/test_microvm_tests.py scripts/test_nightly_ci.py scripts/test_development_release.py scripts/test_crate_release.py scripts/test_host_telemetry.py scripts/test_time_abi.py scripts/test_tool_versions.py scripts/test_markdown_links.py scripts/test_module_diagrams.py scripts/test_nvx_rs_mutants.py scripts/test_setup_azure.py -v python3 scripts/test_adversarial.py -v python3 .github/skills/nvx-host-connect/scripts/test_hosts.py -v python3 scripts/nvx.py --help @@ -31,7 +31,7 @@ runs: if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } python -m compileall -q scripts if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - python -m unittest scripts/test_performance.py scripts/test_nvx_tools.py scripts/test_guest_binaries.py scripts/test_microvm_tests.py scripts/test_nightly_ci.py scripts/test_development_release.py scripts/test_crate_release.py scripts/test_host_telemetry.py scripts/test_time_abi.py scripts/test_tool_versions.py scripts/test_markdown_links.py scripts/test_nvx_rs_mutants.py scripts/test_setup_azure.py -v + python -m unittest scripts/test_performance.py scripts/test_nvx_tools.py scripts/test_guest_binaries.py scripts/test_microvm_tests.py scripts/test_nightly_ci.py scripts/test_development_release.py scripts/test_crate_release.py scripts/test_host_telemetry.py scripts/test_time_abi.py scripts/test_tool_versions.py scripts/test_markdown_links.py scripts/test_module_diagrams.py scripts/test_nvx_rs_mutants.py scripts/test_setup_azure.py -v if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } python scripts\test_adversarial.py -v if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } diff --git a/doc/project-structure.md b/doc/project-structure.md index b78b013e8..7bc5842af 100644 --- a/doc/project-structure.md +++ b/doc/project-structure.md @@ -150,6 +150,7 @@ nvx/ | | |-- setup-windows-whp.ps1 Windows/WHP development-host and Actions runner bootstrap | | `-- tool-versions.conf Shared tool release and checksum pins | |-- check_markdown_links.py Markdown link, anchor, and doc/ path check run by CI +| |-- module_diagrams.py Module diagram generator of the nvx crate | |-- nvx-hosts.example.json Template for the ignored .nvx-hosts.json | |-- nvx.py Supported command-line entry point | |-- nvx_adversarial_executor.py Restricted adversarial executor entry point diff --git a/scripts/module_diagrams.py b/scripts/module_diagrams.py new file mode 100644 index 000000000..64c8e5c0f --- /dev/null +++ b/scripts/module_diagrams.py @@ -0,0 +1,733 @@ +#!/usr/bin/env python3 +"""Draws the module diagrams of the nvx crate. + +cargo-modules exports the modules of the crate's library, with its cli, testing, +and guest features, as each of the crate's platforms, Linux and Windows, builds +it: the module that contains each module, and the modules that each uses. This +script merges the exports of the platforms, draws with Graphviz the crate's +top-level modules, the dependencies between them, and the modules that each +contains, and writes the page that shows the diagrams. +""" + +from __future__ import annotations + +import argparse +import os +import re +import shlex +import shutil +import subprocess +import sys +import textwrap +import xml.etree.ElementTree as ET +from collections import Counter +from collections.abc import Callable, Iterable, Mapping, Sequence +from dataclasses import dataclass +from itertools import pairwise +from pathlib import Path + +from nvx_tools.build_constants import ( + BuildConstants, + GuestRustBuildConstants, + OpenVMMBuildConstants, +) + +REPOSITORY = BuildConstants.REPO_ROOT +MANIFEST = GuestRustBuildConstants.DIRECTORY / "Cargo.toml" +# A run draws beside the build products unless --output names another directory. +DEFAULT_OUTPUT = BuildConstants.BUILD_DIR / "modules" +PAGE = "README.md" +# The first line of the page, by which a run knows the output of an earlier one. +MARKER = "" +CRATE = "nvx" +FEATURES = ("cli", "testing", "guest") +# The platforms that the diagrams cover, with the target that each builds: on +# Linux, the guests' target, for which the guest module builds. +TARGETS = { + "Linux": GuestRustBuildConstants.TARGET, + "Windows": OpenVMMBuildConstants.WINDOWS_RUST_TARGET, +} +# The cargo-modules release to install. It builds with the Rust release that +# rust-toolchain.toml pins. +CARGO_MODULES_VERSION = "0.27.0" + +# The fill and border colors of the crate root and of each part of the crate. +COLORS = { + CRATE: ("#14283f", "#14283f"), + "core": ("#e0f2fe", "#0284c7"), + "host": ("#ede9fe", "#8b5cf6"), + "guest": ("#ccfbf1", "#0d9488"), + "testing": ("#fef3c7", "#d97706"), +} +# What each top-level module holds, in the overview. +DESCRIPTIONS = { + "activation": "Layered root activation", + "contract": "Control compatibility", + "error": "Error types", + "exec": "Execution stdio and credit", + "gpt": "GPT partition reader", + "guest": "Linux guest programs", + "host": "Launch and supervision", + "hypervisor": "Backend selection", + "net": "IP networks", + "protocol": "NVXS / NVXC codecs", + "testing": "Fake OpenVMM", + "uuid": "UUID encoding", +} +# Graphviz measures the labels with the first of these fonts that is installed, +# and SVG viewers show them with the first that they have. With Liberation Sans +# installed, the layouts do not depend on a host's other fonts, and Arial and +# Helvetica have the same metrics. +FONT = "Liberation Sans,Arial,Helvetica,sans-serif" +HEADER = ( + 'graph [rankdir=LR, bgcolor="white", pad=0.25, nodesep=0.22, ranksep=0.65,' + f' splines=true, outputorder=edgesfirst, fontname="{FONT}"];', + f'node [shape=box, style="rounded,filled", fontname="{FONT}", fontsize=13,' + ' margin="0.16,0.11", penwidth=1.2];', + f'edge [color="#a8b8ca", arrowsize=0.65, fontname="{FONT}", fontsize=10,' + ' fontcolor="#52677d"];', +) +# The top-level modules in each row of the overview. +ROW = 4 +# The visibilities that a box shows with its border alone: solid if public, +# dashed if private. A box names any other visibility, such as pub(crate). +PUBLIC = ("crate", "pub") +PRIVATE = "pub(self)" +# The width of the page's lines. +WIDTH = 79 + +# cargo-modules writes one statement per line, and ends each with a comment that +# names the kind of node or edge that it draws. +STATEMENT = re.compile(r'// "[^"]*" (?:node|edge)$', re.M) +NODE = re.compile( + r'^ *"(?P[^"]+)" \[label="(?P[^|"]*)\|[^"]*"[^\n]*' + r'// "(?Pcrate|mod)" node$', + re.M, +) +EDGE = re.compile( + r'^ *"(?P[^"]+)" -> "(?P[^"]+)" [^\n]*' + r'// "(?Powns|uses)" edge$', + re.M, +) + + +class DiagramError(Exception): + """A failure to export, merge, or draw the modules.""" + + +@dataclass(frozen=True) +class Output: + """What a command wrote.""" + + stdout: str + stderr: str + + +# Runs a command, with the text for its standard input, if any, and returns its +# output; it fails with a DiagramError if the command fails. +Runner = Callable[[Sequence[str], str | None], Output] + + +@dataclass(frozen=True) +class Export: + """The modules of the crate that one platform builds, as cargo-modules + exports them: the visibility of each, by path, the module that contains each + module but the crate root, and the pairs of modules in which the first uses + the second.""" + + visibility: dict[str, str] + owner: dict[str, str] + uses: frozenset[tuple[str, str]] + + +@dataclass(frozen=True) +class Module: + """A module that any platform builds: its visibility, and the platforms that + build it.""" + + visibility: str + platforms: frozenset[str] + + +@dataclass(frozen=True) +class Crate: + """The modules that any platform builds, by path, the module that contains + each module but the crate root, and the pairs of modules in which the first + uses the second, with the platforms that build each pair.""" + + platforms: tuple[str, ...] + modules: dict[str, Module] + owner: dict[str, str] + uses: dict[tuple[str, str], frozenset[str]] + + def roots(self) -> list[str]: + """The top-level modules.""" + return sorted(path for path, owner in self.owner.items() if owner == CRATE) + + def subtree(self, root: str) -> list[str]: + """The module and every module that it contains.""" + return sorted( + path + for path in self.modules + if path == root or path.startswith(root + "::") + ) + + def only(self, path: str) -> str | None: + """The platforms that alone build a module that every platform would + otherwise build, as its owner is common to all: the outermost module of + those that only these platforms build. None for every other module.""" + platforms = self.modules[path].platforms + owner = self.owner.get(path) + if len(platforms) == len(self.platforms) or ( + owner is not None + and len(self.modules[owner].platforms) != len(self.platforms) + ): + return None + return " and ".join(name for name in self.platforms if name in platforms) + + +@dataclass(frozen=True) +class Diagram: + """A diagram of the page: the file that holds it, its title, what it shows, + and its Graphviz source. A hierarchy names its top-level module, and the + page says what it shows.""" + + name: str + title: str + text: str + source: str + root: str | None = None + + +def run(command: Sequence[str], stdin: str | None = None) -> Output: + """Runs a command in the repository, without terminal colors.""" + try: + result = subprocess.run( + list(command), + cwd=REPOSITORY, + input=stdin, + capture_output=True, + encoding="utf-8", + env={**os.environ, "NO_COLOR": "1"}, + check=False, + ) + except OSError as error: + raise DiagramError(f"{shlex.join(command)} failed: {error}") from error + if result.returncode != 0: + raise DiagramError( + f"{shlex.join(command)} failed with exit status {result.returncode}:" + f" {result.stderr.strip()}" + ) + return Output(result.stdout, result.stderr) + + +def warn(output: Output) -> str: + """Repeats the warnings that a command wrote and returns its standard + output.""" + if output.stderr.strip(): + print(output.stderr.strip(), file=sys.stderr) + return output.stdout + + +def export_command(target: str) -> list[str]: + """The cargo-modules command that exports the modules of one target, without + the functions, types, and traits that they hold or other crates.""" + return [ + "cargo", + "modules", + "dependencies", + "--lib", + "--manifest-path", + str(MANIFEST), + "--features", + ",".join(FEATURES), + "--target", + target, + "--no-fns", + "--no-traits", + "--no-types", + "--no-externs", + "--no-sysroot", + ] + + +def parse(text: str, target: str) -> Export: + """Reads and checks the cargo-modules export of one target.""" + + def fail(reason: str) -> DiagramError: + return DiagramError(f"unexpected cargo-modules export of {target}: {reason}") + + nodes = list(NODE.finditer(text)) + edges = list(EDGE.finditer(text)) + if len(nodes) + len(edges) != len(STATEMENT.findall(text)): + raise fail("it holds nodes or edges other than modules and their uses") + visibility: dict[str, str] = {} + for match in nodes: + path = match["path"] + if path in visibility: + raise fail(f"{path} appears twice") + if (match["type"] == "crate") != (path == CRATE): + raise fail(f"{path} is a {match['type']}") + visibility[path] = match["kind"].removesuffix(" mod") + if CRATE not in visibility: + raise fail(f"the crate {CRATE} is missing") + owner: dict[str, str] = {} + uses: set[tuple[str, str]] = set() + for match in edges: + user, used = match["user"], match["used"] + for path in (user, used): + if path not in visibility: + raise fail(f"{path} has an edge but is no module") + if match["type"] == "uses": + uses.add((user, used)) + elif used in owner or used.rpartition("::")[0] != user: + raise fail(f"{user} cannot contain {used}") + else: + owner[used] = user + if owner.keys() != visibility.keys() - {CRATE}: + raise fail("a module has no owner") + return Export(visibility, owner, frozenset(uses)) + + +def merge(exports: Mapping[str, Export]) -> Crate: + """Merges the exports of the platforms, in their order. A module whose + visibility differs between them names the visibility of each.""" + visibilities: dict[str, dict[str, str]] = {} + owner: dict[str, str] = {} + uses: dict[tuple[str, str], set[str]] = {} + for platform, export in exports.items(): + for path, visibility in export.visibility.items(): + visibilities.setdefault(path, {})[platform] = visibility + owner.update(export.owner) + for pair in export.uses: + uses.setdefault(pair, set()).add(platform) + modules: dict[str, Module] = {} + for path, by_platform in sorted(visibilities.items()): + distinct = sorted(set(by_platform.values())) + visibility = ( + distinct[0] + if len(distinct) == 1 + else ", ".join(f"{value} on {name}" for name, value in by_platform.items()) + ) + modules[path] = Module(visibility, frozenset(by_platform)) + return Crate( + tuple(exports), + modules, + dict(sorted(owner.items())), + {pair: frozenset(names) for pair, names in sorted(uses.items())}, + ) + + +def quote(value: object) -> str: + """A quoted DOT ID, whose newlines break the lines of a label.""" + text = str(value).replace("\\", "\\\\").replace('"', '\\"') + return '"' + text.replace("\n", "\\n") + '"' + + +def group(path: str) -> str: + """The part of the crate that holds a module, which colors it: its top-level + module, the core that the host and the guest share, or the crate root.""" + parts = path.split("::") + if len(parts) == 1: + return CRATE + return parts[1] if parts[1] in COLORS else "core" + + +def top(path: str) -> str: + """The top-level module that holds a module, or the crate root itself.""" + return "::".join(path.split("::")[:2]) + + +def plural(count: int, noun: str) -> str: + return f"{count} {noun}" + ("" if count == 1 else "s") + + +def node(crate: Crate, path: str, lines: Iterable[str]) -> str: + """A node statement, colored by the part of the crate that holds the module, + solid if the module is public and dashed if not, naming its visibility + unless it is public or private, and the platforms that alone build it.""" + fill, border = COLORS[group(path)] + module = crate.modules[path] + label = [line for line in lines if line] + if module.visibility not in (*PUBLIC, PRIVATE): + label.append(module.visibility) + only = crate.only(path) + if only is not None: + label.append(f"{only} only") + attributes = { + "label": "\n".join(label), + "tooltip": f"{path} ({module.visibility})", + "fillcolor": fill, + "color": border, + "fontcolor": "#ffffff" if path == CRATE else "#14283f", + "style": "rounded,filled" + if module.visibility in PUBLIC + else "rounded,filled,dashed", + } + listed = ", ".join(f"{key}={quote(value)}" for key, value in attributes.items()) + return f"{quote(path)} [{listed}];" + + +def graph(name: str, statements: Iterable[str]) -> str: + return "\n".join((f"digraph {quote(name)} {{", *HEADER, *statements, "}")) + "\n" + + +def overview(crate: Crate) -> str: + """The top-level modules in a grid, in the box of the crate that contains + them.""" + roots = crate.roots() + fill, border = COLORS[CRATE] + statements = [ + "graph [rankdir=TB, nodesep=0.25, ranksep=0.3];", + f"subgraph {quote('cluster_' + CRATE)} {{", + f'graph [label={quote(CRATE)}, labeljust=l, style="rounded,filled",' + f' fillcolor="#f8fafc", color={quote(fill)}, fontcolor={quote(border)},' + " fontsize=16, penwidth=1.5, margin=14];", + ] + for root in roots: + name = root.removeprefix(f"{CRATE}::") + nested = len(crate.subtree(root)) - 1 + lines = ( + name, + DESCRIPTIONS.get(name, ""), + plural(nested, "nested module") if nested else "", + ) + statements.append(node(crate, root, lines)) + rows = [roots[start : start + ROW] for start in range(0, len(roots), ROW)] + # Invisible edges order each row, and stack each column, of the grid. + statements.extend( + "{rank=same; " + + " -> ".join(map(quote, row)) + + (" [style=invis]" if len(row) > 1 else "") + + ";}" + for row in rows + ) + statements.extend( + f"{quote(upper)} -> {quote(lower)} [style=invis];" + for above, below in pairwise(rows) + # The last row may be shorter. + for upper, lower in zip(above, below, strict=False) + ) + statements.append("}") + return graph(CRATE, statements) + + +def dependencies(crate: Crate) -> str: + """The uses between top-level modules, counted by the pairs of their + modules, and the crate root's re-exports.""" + roots = crate.roots() + statements = [node(crate, CRATE, (CRATE, "crate re-exports"))] + statements.extend( + node(crate, root, (root.removeprefix(f"{CRATE}::"),)) for root in roots + ) + counts = Counter( + (top(user), top(used)) for user, used in crate.uses if top(user) != top(used) + ) + for (user, used), count in sorted(counts.items()): + tooltip = f"{user} uses {used}: {plural(count, 'pair')} of modules" + statements.append( + f"{quote(user)} -> {quote(used)} [label={quote(count)}," + f" tooltip={quote(tooltip)}, color={quote(COLORS[group(user)][1])}];" + ) + return graph(f"{CRATE} dependencies", statements) + + +def hierarchy(crate: Crate, root: str) -> str: + """A top-level module, the modules that it contains, and their + containment.""" + selected = crate.subtree(root) + statements = [ + "graph [nodesep=0.12, ranksep=0.5, ordering=out];", + 'node [height=0.3, margin="0.12,0.04"];', + ] + statements.extend( + node(crate, path, (path if path == root else path.rpartition("::")[2],)) + for path in selected + ) + statements.extend( + f"{quote(crate.owner[path])} -> {quote(path)};" + for path in selected + if path != root + ) + return graph(root, statements) + + +def diagrams(crate: Crate) -> list[Diagram]: + """The diagrams of the page, in its order: the overview, the dependencies, + and the hierarchy of each top-level module that contains modules.""" + result = [ + Diagram( + "overview.svg", + "Overview", + "The crate's top-level modules, with what each holds and how many" + " modules it contains.", + overview(crate), + ), + Diagram( + "dependencies.svg", + "Dependencies", + "The top-level modules that use one another. An arrow points from a" + " module to a module that it uses, and counts the pairs of their" + " modules in which the first uses the second, as cargo-modules finds" + " them: a module uses another whose items it imports, re-exports" + " included, or whose items the definitions of its own items name," + " which counts the implementations of a type's traits toward the" + " type's module, wherever they are. The crate's own arrows are its" + " re-exports. The uses within a top-level module are left out.", + dependencies(crate), + ), + ] + result.extend( + Diagram( + f"{root.replace('::', '-')}.svg", + f"`{root}`", + f"The modules that {root} contains", + hierarchy(crate, root), + root, + ) + for root in crate.roots() + if len(crate.subtree(root)) > 1 + ) + return result + + +def source(root: str) -> Path | None: + """The file of a top-level module, unless the module is inline.""" + directory = MANIFEST.parent / "src" + name = root.removeprefix(f"{CRATE}::") + for path in (directory / name / "mod.rs", directory / f"{name}.rs"): + if path.is_file(): + return path + return None + + +def paragraph(text: str) -> list[str]: + """The lines of a paragraph of the page.""" + return [ + *textwrap.wrap( + text, width=WIDTH, break_long_words=False, break_on_hyphens=False + ), + "", + ] + + +def page(crate: Crate, drawn: Sequence[Diagram], output: Path, tools: str) -> str: + """The page that shows the diagrams, in the output directory, whose links + lead to the files that they name in the repository.""" + + def link(text: str, path: Path, anchor: str = "") -> str: + try: + target = Path(os.path.relpath(path, output)).as_posix() + except ValueError: + # Windows has no relative path to another drive. + return text + return f"[{text}]({target}{anchor})" + + features = ", ".join(f"`{feature}`" for feature in FEATURES[:-1]) + targets = " and ".join( + f"{platform} (`{target}`)" for platform, target in TARGETS.items() + ) + lines = [ + MARKER, + "", + "# Module diagrams", + "", + *paragraph( + f"The modules of the {link('`nvx` crate', MANIFEST.parent / 'README.md')}," + " the modules that each contains, and the dependencies between them, as" + f" {tools}. They cover the crate's library with its {features}, and" + f" `{FEATURES[-1]}` features, on {targets}." + ), + *paragraph( + "Each box is a module, colored by the part of the crate that holds it:" + " blue for the modules that the host and the guest share, purple for" + " the host, teal for the guest, and amber for the test doubles. A solid" + " box is a public module, and a dashed box a private one, unless it" + " names another visibility, such as `pub(crate)`. A box that names" + " platforms, as in *Linux only*, is a module that only they build, as" + " are the modules that it contains. The diagrams leave out the crate's" + " tests and binaries, and other crates." + ), + ] + hierarchies = False + for diagram in drawn: + if diagram.root is None: + lines += [f"## {diagram.title}", "", *paragraph(diagram.text)] + alt = diagram.title + else: + if not hierarchies: + hierarchies = True + lines += [ + "## Hierarchies", + "", + *paragraph( + "The modules that each top-level module contains. An arrow" + " points from a module to a module that it contains." + ), + ] + count = plural(len(crate.subtree(diagram.root)) - 1, "module") + path = source(diagram.root) + where = ( + "" + if path is None + else ", in " + + link(f"`{path.relative_to(REPOSITORY).as_posix()}`", path) + + "," + ) + lines += [ + f"### {diagram.title}", + "", + *paragraph(f"`{diagram.root}`{where} contains {count}."), + ] + alt = diagram.text + lines += [f"![{alt}]({diagram.name})", ""] + return "\n".join(lines[:-1]) + "\n" + + +def render(dot: str, diagram: Diagram, execute: Runner) -> str: + """Lays out a diagram with Graphviz and returns the SVG document.""" + svg = warn(execute([dot, "-Tsvg"], diagram.source)) + try: + root = ET.fromstring(svg) + except ET.ParseError as error: + raise DiagramError(f"Graphviz wrote no SVG for {diagram.name}") from error + if root.tag != "{http://www.w3.org/2000/svg}svg": + raise DiagramError(f"Graphviz wrote no SVG for {diagram.name}") + return svg if svg.endswith("\n") else svg + "\n" + + +def version(execute: Runner, command: Sequence[str], pattern: str) -> str | None: + """The version that a tool reports, if it reports one.""" + try: + output = execute(command, None) + except DiagramError: + return None + match = re.search(pattern, output.stdout + output.stderr) + return match[1] if match else None + + +def replace(output: Path, files: Mapping[str, str]) -> None: + """Makes the output directory hold the files and nothing else, with LF line + endings. It refuses a directory that holds anything but the page and the + diagrams of an earlier run.""" + if output.exists(): + if not output.is_dir(): + raise DiagramError(f"{output} is not a directory") + entries = sorted(output.iterdir()) + # A symbolic link that holds a diagram's name would have the diagram + # written through it, outside the directory. + foreign = [ + entry.name + for entry in entries + if entry.is_symlink() + or not entry.is_file() + or (entry.name != PAGE and entry.suffix != ".svg") + ] + earlier = output / PAGE + if ( + entries + and not foreign + and not ( + earlier.is_file() + and earlier.read_text(encoding="utf-8").startswith(MARKER + "\n") + ) + ): + foreign = [PAGE if earlier.is_file() else "no page of an earlier run"] + if foreign: + raise DiagramError( + f"{output} holds more than the diagrams of an earlier run:" + f" {', '.join(foreign)}; remove it or choose another --output" + ) + output.mkdir(parents=True, exist_ok=True) + for entry in output.iterdir(): + if entry.name not in files: + entry.unlink() + for name, text in files.items(): + with open(output / name, "w", encoding="utf-8", newline="\n") as file: + file.write(text) + + +def find_dot() -> str: + """The Graphviz dot executable: the one on PATH, or on Windows the one where + the Graphviz installer puts it, which it does not always add to PATH.""" + found = shutil.which("dot") + if found is not None: + return found + if os.name == "nt": + programs = Path(os.environ.get("ProgramFiles", r"C:\Program Files")) + installed = programs / "Graphviz" / "bin" / "dot.exe" + if installed.is_file(): + return str(installed) + raise DiagramError( + "Graphviz is missing; install it, such as with apt-get install graphviz or" + " winget install --id Graphviz.Graphviz --exact" + ) + + +def draw(output: Path, dot: str, execute: Runner = run) -> list[str]: + """Exports the modules of every platform, draws the diagrams, and writes + them and their page to the output directory, which changes only once every + diagram is drawn. Returns the names of the files that it holds.""" + exports: dict[str, Export] = {} + for platform, target in TARGETS.items(): + print(f"Exporting the modules of {platform} ({target})", file=sys.stderr) + text = warn(execute(export_command(target), None)) + exports[platform] = parse(text, target) + crate = merge(exports) + drawn = diagrams(crate) + files: dict[str, str] = {} + for diagram in drawn: + print(f"Drawing {diagram.name}", file=sys.stderr) + files[diagram.name] = render(dot, diagram, execute) + modules = version(execute, ["cargo", "modules", "--version"], r"(\S+)\s*$") + graphviz = version(execute, [dot, "-V"], r"version (\S+)") + tools = ( + "[cargo-modules](https://github.com/regexident/cargo-modules)" + + (f" {modules}" if modules else "") + + " finds them and Graphviz" + + (f" {graphviz}" if graphviz else "") + + " draws them" + ) + files[PAGE] = page(crate, drawn, output, tools) + replace(output, files) + return sorted(files) + + +def main(argv: Sequence[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description=( + "Draws the module diagrams of the nvx crate, and the page that shows" + " them, with cargo-modules and Graphviz." + ) + ) + parser.add_argument( + "--output", + type=Path, + default=DEFAULT_OUTPUT, + help=( + "the directory to write, which holds nothing but the output of an" + " earlier run (default: build/modules)" + ), + ) + parser.add_argument( + "--dot", help="the Graphviz dot executable (default: dot on PATH)" + ) + arguments = parser.parse_args(argv) + output: Path = arguments.output.resolve() + try: + if shutil.which("cargo-modules") is None: + raise DiagramError( + "cargo-modules is missing; install it with: cargo install --locked" + f" cargo-modules --version {CARGO_MODULES_VERSION}" + ) + dot: str = arguments.dot or find_dot() + for name in draw(output, dot): + print(output / name) + except DiagramError as error: + print(f"error: {error}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test_module_diagrams.py b/scripts/test_module_diagrams.py new file mode 100644 index 000000000..da49218ee --- /dev/null +++ b/scripts/test_module_diagrams.py @@ -0,0 +1,669 @@ +#!/usr/bin/env python3 +"""Tests for module_diagrams.py, which draws the module diagrams of the nvx +crate. + +They check how the script reads and checks cargo-modules' exports, which are +written here in its DOT form, how it merges the platforms' modules, the graphs +that it draws, the page that shows them, how it replaces an earlier run's +output, and what it runs, with a stand-in for cargo-modules and Graphviz. +""" + +from __future__ import annotations + +import contextlib +import io +import re +import sys +import tempfile +import unittest +import xml.etree.ElementTree as ET +from collections.abc import Iterable, Mapping, Sequence +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).parent)) +import module_diagrams as diagrams # noqa: E402 + +REPO_ROOT = Path(__file__).resolve().parents[1] +LINUX, WINDOWS = diagrams.TARGETS + + +def owns(owner: str, path: str) -> str: + """The statement of a cargo-modules export in which a module contains + another.""" + return ( + f'"{owner}" -> "{path}" [label="owns", color="#000000", style="solid"]' + ' [constraint=true]; // "owns" edge' + ) + + +def export( + modules: Mapping[str, str], + uses: Iterable[tuple[str, str]] = (), + *, + extra: Iterable[str] = (), +) -> str: + """A cargo-modules export in its DOT form: the modules with their + visibility, the crate first, the containment that their paths imply, the + uses, and extra statements.""" + lines = [ + "digraph {", + "", + " graph [", + ' label="nvx",', + " ];", + "", + ] + for path, visibility in modules.items(): + if path == diagrams.CRATE: + lines.append( + f' "{path}" [label="crate|{path}", fillcolor="#5397c8"];' + ' // "crate" node' + ) + else: + name = path.removeprefix(f"{diagrams.CRATE}::") + lines.append( + f' "{path}" [label="{visibility} mod|{name}",' + ' fillcolor="#81c169"]; // "mod" node' + ) + lines.extend( + f" {owns(path.rpartition('::')[0], path)}" + for path in modules + if path != diagrams.CRATE + ) + for user, used in uses: + lines.append( + f' "{user}" -> "{used}" [label="uses", color="#7f7f7f",' + ' style="dashed"] [constraint=false]; // "uses" edge' + ) + lines.extend(f" {statement}" for statement in extra) + lines += ["", "}", ""] + return "\n".join(lines) + + +# The modules of a small crate on each platform: host::sys and the guest are +# Linux's alone, and host::windows is Windows'. +LINUX_MODULES = { + "nvx": "crate", + "nvx::error": "pub(crate)", + "nvx::guest": "pub", + "nvx::guest::time": "pub", + "nvx::host": "pub", + "nvx::host::cli": "pub", + "nvx::host::sys": "pub(self)", + "nvx::host::sys::socket": "pub(self)", + "nvx::protocol": "pub", + "nvx::protocol::nvxs": "pub(self)", +} +WINDOWS_MODULES = { + "nvx": "crate", + "nvx::error": "pub(crate)", + "nvx::host": "pub", + "nvx::host::cli": "pub", + "nvx::host::windows": "pub(self)", + "nvx::protocol": "pub", + "nvx::protocol::nvxs": "pub(self)", +} +LINUX_USES = ( + ("nvx", "nvx::error"), + ("nvx::host", "nvx::host::cli"), + ("nvx::host::cli", "nvx::protocol"), + ("nvx::host::cli", "nvx::protocol::nvxs"), + ("nvx::host::sys::socket", "nvx::error"), + ("nvx::guest::time", "nvx::protocol::nvxs"), +) +WINDOWS_USES = ( + ("nvx", "nvx::error"), + ("nvx::host::cli", "nvx::protocol::nvxs"), + ("nvx::host::windows", "nvx::error"), +) +EXPORTS = { + LINUX: export(LINUX_MODULES, LINUX_USES), + WINDOWS: export(WINDOWS_MODULES, WINDOWS_USES), +} + + +def crate() -> diagrams.Crate: + return diagrams.merge( + { + platform: diagrams.parse(EXPORTS[platform], target) + for platform, target in diagrams.TARGETS.items() + } + ) + + +_STATEMENT = re.compile( + r'^"(?P[^"]+)"(?: -> "(?P[^"]+)")?(?: \[(?P.*)\])?;$' +) +_ATTRIBUTE = re.compile(r'(\w+)=(?:"((?:[^"\\]|\\.)*)"|([^,\s\]]+))') + + +def statements(source: str) -> list[tuple[str, str | None, dict[str, str]]]: + """The node and edge statements of a graph that the script drew, one per + line, with their attributes, unquoted.""" + result: list[tuple[str, str | None, dict[str, str]]] = [] + for line in source.splitlines(): + match = _STATEMENT.match(line) + if match is None: + continue + attributes = { + key: (quoted.replace('\\"', '"').replace("\\\\", "\\") if quoted else bare) + for key, quoted, bare in _ATTRIBUTE.findall(match["attributes"] or "") + } + result.append((match["tail"], match["head"], attributes)) + return result + + +def nodes(source: str) -> dict[str, dict[str, str]]: + return { + tail: attributes for tail, head, attributes in statements(source) if not head + } + + +def edges(source: str) -> dict[tuple[str, str], dict[str, str]]: + return { + (tail, head): attributes + for tail, head, attributes in statements(source) + if head is not None and attributes.get("style") != "invis" + } + + +def label(attributes: Mapping[str, str]) -> list[str]: + return attributes["label"].split("\\n") + + +class ParseTests(unittest.TestCase): + def test_exports_give_each_module_its_visibility_owner_and_uses(self): + parsed = diagrams.parse(EXPORTS[LINUX], "linux") + self.assertEqual(parsed.visibility, LINUX_MODULES) + self.assertEqual( + parsed.owner, + { + path: path.rpartition("::")[0] + for path in LINUX_MODULES + if path != diagrams.CRATE + }, + ) + self.assertEqual(parsed.uses, frozenset(LINUX_USES)) + + def test_exports_of_anything_but_modules_and_their_uses_fail(self): + modules = {"nvx": "crate", "nvx::host": "pub"} + cases = { + "a type": export( + modules, + extra=['"nvx::host::Spec" [label="pub struct|Spec"]; // "type" node'], + ), + "an edge of another kind": export( + modules, + extra=['"nvx" -> "nvx::host" [label="x"]; // "calls" edge'], + ), + "a module twice": export( + modules, + extra=['"nvx::host" [label="pub mod|host"]; // "mod" node'], + ), + "no crate": export({"nvx::host": "pub"}), + "another crate": export( + modules, extra=['"other" [label="crate|other"]; // "crate" node'] + ), + "a module named as the crate": export(modules).replace( + '"nvx" [label="crate|nvx", fillcolor="#5397c8"]; // "crate" node', + '"nvx" [label="pub mod|nvx"]; // "mod" node', + ), + "an edge to no module": export(modules, [("nvx", "nvx::missing")]), + "a module without an owner": export(modules).replace( + owns("nvx", "nvx::host"), "" + ), + "a module that contains another's": export( + {"nvx": "crate", "nvx::a": "pub", "nvx::a::b": "pub"} + ).replace(owns("nvx::a", "nvx::a::b"), owns("nvx", "nvx::a::b")), + "a module with two owners": export( + modules, extra=[owns("nvx", "nvx::host")] + ), + } + for case, text in cases.items(): + with self.subTest(case=case): + self.assertNotEqual(text, export(modules)) + with self.assertRaisesRegex( + diagrams.DiagramError, "unexpected cargo-modules export of linux" + ): + diagrams.parse(text, "linux") + + +class MergeTests(unittest.TestCase): + def test_the_platforms_share_their_modules_and_uses(self): + merged = crate() + self.assertEqual(merged.platforms, ("Linux", "Windows")) + self.assertEqual( + sorted(merged.modules), sorted({**LINUX_MODULES, **WINDOWS_MODULES}) + ) + both = frozenset({"Linux", "Windows"}) + self.assertEqual(merged.modules["nvx::host"].platforms, both) + self.assertEqual( + merged.modules["nvx::host::sys"].platforms, frozenset({"Linux"}) + ) + self.assertEqual( + merged.modules["nvx::host::windows"].platforms, frozenset({"Windows"}) + ) + self.assertEqual(merged.uses[("nvx", "nvx::error")], both) + self.assertEqual( + merged.uses[("nvx::host::cli", "nvx::protocol")], frozenset({"Linux"}) + ) + self.assertEqual(merged.owner["nvx::host::windows"], "nvx::host") + self.assertEqual( + merged.roots(), + ["nvx::error", "nvx::guest", "nvx::host", "nvx::protocol"], + ) + self.assertEqual( + merged.subtree("nvx::host"), + [ + "nvx::host", + "nvx::host::cli", + "nvx::host::sys", + "nvx::host::sys::socket", + "nvx::host::windows", + ], + ) + + def test_only_the_outermost_module_of_a_platform_names_it(self): + merged = crate() + expected = { + "nvx": None, + "nvx::guest": "Linux", + "nvx::guest::time": None, + "nvx::host": None, + "nvx::host::sys": "Linux", + "nvx::host::sys::socket": None, + "nvx::host::windows": "Windows", + } + for path, platforms in expected.items(): + with self.subTest(path=path): + self.assertEqual(merged.only(path), platforms) + + def test_a_visibility_that_differs_names_each_platform(self): + merged = diagrams.merge( + { + "Linux": diagrams.parse( + export({"nvx": "crate", "nvx::a": "pub"}), "linux" + ), + "Windows": diagrams.parse( + export({"nvx": "crate", "nvx::a": "pub(crate)"}), "windows" + ), + } + ) + self.assertEqual( + merged.modules["nvx::a"].visibility, "pub on Linux, pub(crate) on Windows" + ) + drawn = nodes(diagrams.dependencies(merged)) + self.assertIn("dashed", drawn["nvx::a"]["style"]) + self.assertEqual( + label(drawn["nvx::a"]), ["a", "pub on Linux, pub(crate) on Windows"] + ) + + def test_labels_are_quoted_for_graphviz(self): + self.assertEqual(diagrams.quote('a"b\\c\nd'), '"a\\"b\\\\c\\nd"') + + +class GraphTests(unittest.TestCase): + def test_boxes_show_their_visibility_and_platforms(self): + source = diagrams.hierarchy(crate(), "nvx::host") + drawn = nodes(source) + # Public modules are solid, private ones dashed and unnamed. + self.assertEqual(label(drawn["nvx::host"]), ["nvx::host"]) + self.assertNotIn("dashed", drawn["nvx::host::cli"]["style"]) + self.assertEqual(label(drawn["nvx::host::sys"]), ["sys", "Linux only"]) + self.assertIn("dashed", drawn["nvx::host::sys"]["style"]) + self.assertEqual(label(drawn["nvx::host::sys::socket"]), ["socket"]) + self.assertEqual( + label(drawn["nvx::host::windows"]), ["windows", "Windows only"] + ) + # Any other visibility is named. + error = nodes(diagrams.dependencies(crate()))["nvx::error"] + self.assertEqual(label(error), ["error", "pub(crate)"]) + self.assertIn("dashed", error["style"]) + for path, attributes in drawn.items(): + with self.subTest(path=path): + self.assertIn(path, attributes["tooltip"]) + self.assertEqual( + (attributes["fillcolor"], attributes["color"]), + diagrams.COLORS["host"], + ) + + def test_hierarchies_draw_the_containment_in_a_top_level_module(self): + merged = crate() + source = diagrams.hierarchy(merged, "nvx::host") + self.assertEqual(set(nodes(source)), set(merged.subtree("nvx::host"))) + self.assertEqual( + set(edges(source)), + { + (merged.owner[path], path) + for path in merged.subtree("nvx::host") + if path != "nvx::host" + }, + ) + # Children keep the order of their paths. + self.assertIn("ordering=out", source) + + def test_the_overview_shows_every_top_level_module_in_the_crate(self): + merged = crate() + source = diagrams.overview(merged) + drawn = nodes(source) + self.assertEqual(set(drawn), set(merged.roots())) + self.assertEqual( + label(drawn["nvx::host"]), + ["host", diagrams.DESCRIPTIONS["host"], "4 nested modules"], + ) + self.assertEqual( + label(drawn["nvx::guest"]), + ["guest", diagrams.DESCRIPTIONS["guest"], "1 nested module", "Linux only"], + ) + self.assertEqual( + label(drawn["nvx::error"]), + ["error", diagrams.DESCRIPTIONS["error"], "pub(crate)"], + ) + self.assertIn('subgraph "cluster_nvx"', source) + self.assertEqual(edges(source), {}) + + def test_the_dependencies_count_the_uses_between_top_level_modules(self): + source = diagrams.dependencies(crate()) + counts = { + pair: attributes["label"] for pair, attributes in edges(source).items() + } + self.assertEqual( + counts, + { + ("nvx", "nvx::error"): "1", + ("nvx::guest", "nvx::protocol"): "1", + # host::sys on Linux and host::windows on Windows. + ("nvx::host", "nvx::error"): "2", + ("nvx::host", "nvx::protocol"): "2", + }, + ) + self.assertEqual( + edges(source)[("nvx::guest", "nvx::protocol")]["color"], + diagrams.COLORS["guest"][1], + ) + self.assertEqual(set(nodes(source)), {"nvx", *crate().roots()}) + + def test_the_page_shows_the_overview_the_dependencies_and_each_hierarchy(self): + drawn = diagrams.diagrams(crate()) + # error contains no module, so it has no hierarchy. + self.assertEqual( + [diagram.name for diagram in drawn], + [ + "overview.svg", + "dependencies.svg", + "nvx-guest.svg", + "nvx-host.svg", + "nvx-protocol.svg", + ], + ) + self.assertEqual( + [diagram.root for diagram in drawn], + [None, None, "nvx::guest", "nvx::host", "nvx::protocol"], + ) + + +class PageTests(unittest.TestCase): + def page( + self, output: Path = diagrams.DEFAULT_OUTPUT, tools: str = "the tools" + ) -> str: + merged = crate() + return diagrams.page(merged, diagrams.diagrams(merged), output, tools) + + def links(self, output: Path) -> dict[str, str]: + """The links of the page in the output directory, by their text.""" + text = " ".join(self.page(output).split()) + return dict(re.findall(r"(? str: + return f'{title}\n' + + +class FakeTools: + """cargo-modules and Graphviz: each export command prints the export of its + target, and dot draws an SVG titled with its graph's name.""" + + def __init__(self, exports: Mapping[str, str] = EXPORTS) -> None: + self.exports = { + tuple(diagrams.export_command(diagrams.TARGETS[platform])): text + for platform, text in exports.items() + } + self.commands: list[list[str]] = [] + self.sources: list[str] = [] + self.versions = True + self.svg = True + + def __call__(self, command: Sequence[str], stdin: str | None) -> diagrams.Output: + self.commands.append(list(command)) + if tuple(command) in self.exports: + return diagrams.Output(self.exports[tuple(command)], "") + if command[1:] == ["-Tsvg"] and stdin is not None: + self.sources.append(stdin) + name = re.match(r'digraph "([^"]+)"', stdin) + assert name is not None + return diagrams.Output( + _svg(name[1]) if self.svg else "", "warning: none\n" + ) + if not self.versions: + raise diagrams.DiagramError("no version") + if command == ["cargo", "modules", "--version"]: + return diagrams.Output("cargo-modules 9.9.9\n", "") + if command[1:] == ["-V"]: + return diagrams.Output("", "dot - graphviz version 1.2.3 (0)\n") + raise AssertionError(f"unexpected command {command}") + + +class DrawTests(unittest.TestCase): + def draw(self, output: Path, tools: FakeTools) -> list[str]: + with contextlib.redirect_stderr(io.StringIO()): + return diagrams.draw(output, "dot", tools) + + def test_draw_exports_each_platform_and_writes_the_page_and_diagrams(self): + tools = FakeTools() + with tempfile.TemporaryDirectory() as directory: + output = Path(directory) / "modules" + names = self.draw(output, tools) + drawn = diagrams.diagrams(crate()) + self.assertEqual( + names, sorted([diagrams.PAGE, *(diagram.name for diagram in drawn)]) + ) + self.assertEqual(sorted(path.name for path in output.iterdir()), names) + for diagram in drawn: + with self.subTest(diagram=diagram.name): + root = ET.parse(output / diagram.name).getroot() + self.assertEqual(root.tag, "{http://www.w3.org/2000/svg}svg") + page = (output / diagrams.PAGE).read_text(encoding="utf-8") + self.assertEqual( + tools.commands[: len(diagrams.TARGETS)], + [diagrams.export_command(target) for target in diagrams.TARGETS.values()], + ) + self.assertEqual(tools.sources, [diagram.source for diagram in drawn]) + flat = " ".join(page.split()) + self.assertIn("cargo-modules) 9.9.9 finds them and Graphviz 1.2.3 draws", flat) + + def test_the_page_names_no_version_that_a_tool_does_not_report(self): + tools = FakeTools() + tools.versions = False + with tempfile.TemporaryDirectory() as directory: + self.draw(Path(directory), tools) + page = (Path(directory) / diagrams.PAGE).read_text(encoding="utf-8") + self.assertIn("finds them and Graphviz draws them", " ".join(page.split())) + + def test_a_failed_drawing_leaves_the_output_unchanged(self): + tools = FakeTools() + with tempfile.TemporaryDirectory() as directory: + output = Path(directory) + self.draw(output, tools) + before = {path.name: path.read_bytes() for path in output.iterdir()} + failing = FakeTools( + {LINUX: export({"nvx": "crate"}), WINDOWS: EXPORTS[WINDOWS]} + ) + failing.svg = False + with self.assertRaisesRegex(diagrams.DiagramError, "no SVG"): + self.draw(output, failing) + broken = FakeTools({LINUX: "digraph {}", WINDOWS: EXPORTS[WINDOWS]}) + with self.assertRaisesRegex(diagrams.DiagramError, "export"): + self.draw(output, broken) + after = {path.name: path.read_bytes() for path in output.iterdir()} + self.assertEqual(after, before) + + def test_exports_cover_the_library_with_the_features_of_the_page(self): + for target in diagrams.TARGETS.values(): + command = diagrams.export_command(target) + with self.subTest(target=target): + self.assertEqual( + command[:4], ["cargo", "modules", "dependencies", "--lib"] + ) + options = dict(zip(command[4:], command[5:], strict=False)) + self.assertEqual(options["--manifest-path"], str(diagrams.MANIFEST)) + self.assertEqual(options["--features"], ",".join(diagrams.FEATURES)) + self.assertEqual(options["--target"], target) + for excluded in ("fns", "traits", "types", "externs", "sysroot"): + self.assertIn(f"--no-{excluded}", command) + self.assertNotIn("--no-uses", command) + self.assertNotIn("--no-owns", command) + + def test_a_missing_cargo_modules_names_the_release_to_install(self): + stderr = io.StringIO() + with ( + patch.object(diagrams.shutil, "which", return_value=None), + contextlib.redirect_stderr(stderr), + ): + status = diagrams.main(["--output", str(REPO_ROOT / "build" / "unused")]) + self.assertEqual(status, 1) + self.assertIn( + f"cargo-modules --version {diagrams.CARGO_MODULES_VERSION}", + stderr.getvalue(), + ) + + +class ReplaceTests(unittest.TestCase): + def setUp(self) -> None: + directory = tempfile.TemporaryDirectory() + self.addCleanup(directory.cleanup) + self.root = Path(directory.name) + self.files = {diagrams.PAGE: diagrams.MARKER + "\npage\n", "a.svg": "\n"} + + def contents(self, output: Path) -> dict[str, bytes]: + return {path.name: path.read_bytes() for path in output.iterdir()} + + def test_an_earlier_run_is_replaced_with_lf_line_endings(self): + output = self.root / "new" / "modules" + diagrams.replace(output, self.files) + (output / "stale.svg").write_text("\n", encoding="utf-8") + diagrams.replace(output, self.files | {"b.svg": "\n\n"}) + self.assertEqual( + self.contents(output), + { + diagrams.PAGE: (diagrams.MARKER + "\npage\n").encode(), + "a.svg": b"\n", + "b.svg": b"\n\n", + }, + ) + empty = self.root / "empty" + empty.mkdir() + diagrams.replace(empty, self.files) + self.assertEqual(sorted(self.contents(empty)), ["README.md", "a.svg"]) + + def test_directories_that_hold_more_than_an_earlier_run_are_refused(self): + cases: dict[str, dict[str, str]] = { + "another file": {diagrams.PAGE: diagrams.MARKER + "\n", "notes.txt": ""}, + "another page": {diagrams.PAGE: "# Notes\n", "a.svg": ""}, + "no page": {"a.svg": "\n"}, + } + for case, files in cases.items(): + with self.subTest(case=case): + output = self.root / case + output.mkdir() + for name, text in files.items(): + (output / name).write_text(text, encoding="utf-8") + before = self.contents(output) + with self.assertRaisesRegex(diagrams.DiagramError, "--output"): + diagrams.replace(output, self.files) + self.assertEqual(self.contents(output), before) + nested = self.root / "nested" + (nested / "sub").mkdir(parents=True) + with self.assertRaisesRegex(diagrams.DiagramError, "sub"): + diagrams.replace(nested, self.files) + regular = self.root / "file" + regular.write_text("", encoding="utf-8") + with self.assertRaisesRegex(diagrams.DiagramError, "not a directory"): + diagrams.replace(regular, self.files) + + def test_a_symbolic_link_is_refused_rather_than_written_through(self): + output = self.root / "linked" + diagrams.replace(output, self.files) + target = self.root / "outside.txt" + target.write_text("outside\n", encoding="utf-8") + try: + (output / "b.svg").symlink_to(target) + except OSError: + self.skipTest("this host cannot create symbolic links") + with self.assertRaisesRegex(diagrams.DiagramError, "b.svg"): + diagrams.replace(output, self.files | {"b.svg": "\n"}) + self.assertEqual(target.read_text(encoding="utf-8"), "outside\n") + + +if __name__ == "__main__": + unittest.main() From 568cd1a4e405ca52f9f9c8845a7b8b5b948deb3d Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Tue, 27 Oct 2026 10:40:29 -0700 Subject: [PATCH 2/6] doc: publish the module diagrams of the nvx crate doc/modules publishes the module diagrams of the nvx crate that scripts/module_diagrams.py draws: the overview of its top-level modules, the dependencies between them, and the hierarchies of the eight top-level modules that contain modules, as SVG images on a Markdown page that GitHub renders. The project README lists the page beside the project structure, and the crate's README and the project structure link to it. They were drawn on Ubuntu 24.04 with cargo-modules 0.27.0, built with the Rust release that rust-toolchain.toml pins, and Graphviz 2.43.0, which measures their labels with Liberation Sans, the first font that they name. Drawing them again there writes the same bytes, while other Graphviz releases and fonts lay them out differently. A test checks that doc/modules holds the page, whose first line marks it as the script's, and exactly the diagrams that the page shows, each an SVG document, so that no diagram that the page stopped showing lingers and none that it shows is missing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 + doc/modules/README.md | 101 +++ doc/modules/dependencies.svg | 354 ++++++++ doc/modules/nvx-activation.svg | 52 ++ doc/modules/nvx-exec.svg | 112 +++ doc/modules/nvx-gpt.svg | 97 +++ doc/modules/nvx-guest.svg | 1347 +++++++++++++++++++++++++++++++ doc/modules/nvx-host.svg | 1216 ++++++++++++++++++++++++++++ doc/modules/nvx-net.svg | 37 + doc/modules/nvx-protocol.svg | 232 ++++++ doc/modules/nvx-testing.svg | 37 + doc/modules/overview.svg | 166 ++++ doc/project-structure.md | 3 +- nvx-rs/README.md | 4 + scripts/test_module_diagrams.py | 22 +- 15 files changed, 3779 insertions(+), 3 deletions(-) create mode 100644 doc/modules/README.md create mode 100644 doc/modules/dependencies.svg create mode 100644 doc/modules/nvx-activation.svg create mode 100644 doc/modules/nvx-exec.svg create mode 100644 doc/modules/nvx-gpt.svg create mode 100644 doc/modules/nvx-guest.svg create mode 100644 doc/modules/nvx-host.svg create mode 100644 doc/modules/nvx-net.svg create mode 100644 doc/modules/nvx-protocol.svg create mode 100644 doc/modules/nvx-testing.svg create mode 100644 doc/modules/overview.svg diff --git a/README.md b/README.md index 3253c3505..3fd45f85c 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,8 @@ nvx-rs\target\release\nvx-rs.exe run - [Azure Linux guest support](doc/guests/azurelinux-guest.md) - How NVX runs Azure Linux userland on its own kernel. - [Project structure](doc/project-structure.md) - Overview of the NVX repository layout. +- [Module diagrams](doc/modules/README.md) - Diagrams of the modules of the `nvx` crate and the + dependencies between them. - [Continuous integration](doc/ci.md) - Instructions for running and maintaining NVX CI. - [Copilot-driven adversarial testing](doc/adversarial-testing.md) - Bounded adaptive stress and containment campaigns. diff --git a/doc/modules/README.md b/doc/modules/README.md new file mode 100644 index 000000000..b7a49eb7f --- /dev/null +++ b/doc/modules/README.md @@ -0,0 +1,101 @@ + + +# Module diagrams + +The modules of the [`nvx` crate](../../nvx-rs/README.md), the modules that each +contains, and the dependencies between them, as +[cargo-modules](https://github.com/regexident/cargo-modules) 0.27.0 finds them +and Graphviz 2.43.0 draws them. They cover the crate's library with its `cli`, +`testing`, and `guest` features, on Linux (`x86_64-unknown-linux-musl`) and +Windows (`x86_64-pc-windows-msvc`). + +Each box is a module, colored by the part of the crate that holds it: blue for +the modules that the host and the guest share, purple for the host, teal for +the guest, and amber for the test doubles. A solid box is a public module, and +a dashed box a private one, unless it names another visibility, such as +`pub(crate)`. A box that names platforms, as in *Linux only*, is a module that +only they build, as are the modules that it contains. The diagrams leave out +the crate's tests and binaries, and other crates. + +## Overview + +The crate's top-level modules, with what each holds and how many modules it +contains. + +![Overview](overview.svg) + +## Dependencies + +The top-level modules that use one another. An arrow points from a module to a +module that it uses, and counts the pairs of their modules in which the first +uses the second, as cargo-modules finds them: a module uses another whose items +it imports, re-exports included, or whose items the definitions of its own +items name, which counts the implementations of a type's traits toward the +type's module, wherever they are. The crate's own arrows are its re-exports. +The uses within a top-level module are left out. + +![Dependencies](dependencies.svg) + +## Hierarchies + +The modules that each top-level module contains. An arrow points from a module +to a module that it contains. + +### `nvx::activation` + +`nvx::activation`, in +[`nvx-rs/src/activation/mod.rs`](../../nvx-rs/src/activation/mod.rs), contains +2 modules. + +![The modules that nvx::activation contains](nvx-activation.svg) + +### `nvx::exec` + +`nvx::exec`, in [`nvx-rs/src/exec/mod.rs`](../../nvx-rs/src/exec/mod.rs), +contains 6 modules. + +![The modules that nvx::exec contains](nvx-exec.svg) + +### `nvx::gpt` + +`nvx::gpt`, in [`nvx-rs/src/gpt/mod.rs`](../../nvx-rs/src/gpt/mod.rs), contains +5 modules. + +![The modules that nvx::gpt contains](nvx-gpt.svg) + +### `nvx::guest` + +`nvx::guest`, in [`nvx-rs/src/guest/mod.rs`](../../nvx-rs/src/guest/mod.rs), +contains 88 modules. + +![The modules that nvx::guest contains](nvx-guest.svg) + +### `nvx::host` + +`nvx::host`, in [`nvx-rs/src/host/mod.rs`](../../nvx-rs/src/host/mod.rs), +contains 79 modules. + +![The modules that nvx::host contains](nvx-host.svg) + +### `nvx::net` + +`nvx::net`, in [`nvx-rs/src/net/mod.rs`](../../nvx-rs/src/net/mod.rs), contains +1 module. + +![The modules that nvx::net contains](nvx-net.svg) + +### `nvx::protocol` + +`nvx::protocol`, in +[`nvx-rs/src/protocol/mod.rs`](../../nvx-rs/src/protocol/mod.rs), contains 14 +modules. + +![The modules that nvx::protocol contains](nvx-protocol.svg) + +### `nvx::testing` + +`nvx::testing`, in +[`nvx-rs/src/testing/mod.rs`](../../nvx-rs/src/testing/mod.rs), contains 1 +module. + +![The modules that nvx::testing contains](nvx-testing.svg) diff --git a/doc/modules/dependencies.svg b/doc/modules/dependencies.svg new file mode 100644 index 000000000..cef80b707 --- /dev/null +++ b/doc/modules/dependencies.svg @@ -0,0 +1,354 @@ + + + + + + +nvx dependencies + + + +nvx->nvx::error + + + + + +1 + + + +nvx->nvx::host + + + + + +2 + + + +nvx->nvx::hypervisor + + + + + +1 + + + +nvx::activation->nvx::host + + + + + +1 + + + +nvx::activation->nvx::protocol + + + + + +2 + + + +nvx::activation->nvx::uuid + + + + + +2 + + + +nvx::error->nvx::host + + + + + +1 + + + +nvx::gpt->nvx::uuid + + + + + +2 + + + +nvx::guest->nvx::exec + + + + + +2 + + + +nvx::guest->nvx::gpt + + + + + +6 + + + +nvx::guest->nvx::protocol + + + + + +17 + + + +nvx::guest->nvx::uuid + + + + + +2 + + + +nvx::host->nvx::activation + + + + + +1 + + + +nvx::host->nvx::contract + + + + + +4 + + + +nvx::host->nvx::error + + + + + +52 + + + +nvx::host->nvx::hypervisor + + + + + +6 + + + +nvx::host->nvx::net + + + + + +5 + + + +nvx::host->nvx::protocol + + + + + +25 + + + +nvx::host->nvx::uuid + + + + + +4 + + + +nvx::hypervisor->nvx::error + + + + + +1 + + + +nvx::protocol->nvx::uuid + + + + + +1 + + + +nvx::testing->nvx::protocol + + + + + +6 + + + +nvx + + +nvx +crate re-exports + + + + + +nvx::activation + + +activation + + + + + +nvx::contract + + +contract + + + + + +nvx::error + + +error +pub(crate) + + + + + +nvx::exec + + +exec + + + + + +nvx::gpt + + +gpt + + + + + +nvx::guest + + +guest +Linux only + + + + + +nvx::host + + +host + + + + + +nvx::hypervisor + + +hypervisor +pub(crate) + + + + + +nvx::net + + +net + + + + + +nvx::protocol + + +protocol + + + + + +nvx::testing + + +testing + + + + + +nvx::uuid + + +uuid + + + + + diff --git a/doc/modules/nvx-activation.svg b/doc/modules/nvx-activation.svg new file mode 100644 index 000000000..c017d591e --- /dev/null +++ b/doc/modules/nvx-activation.svg @@ -0,0 +1,52 @@ + + + + + + +nvx::activation + + + +nvx::activation->nvx::activation::codec + + + + + +nvx::activation->nvx::activation::ready + + + + + +nvx::activation + + +nvx::activation + + + + + +nvx::activation::codec + + +codec + + + + + +nvx::activation::ready + + +ready + + + + + diff --git a/doc/modules/nvx-exec.svg b/doc/modules/nvx-exec.svg new file mode 100644 index 000000000..6bc850016 --- /dev/null +++ b/doc/modules/nvx-exec.svg @@ -0,0 +1,112 @@ + + + + + + +nvx::exec + + + +nvx::exec->nvx::exec::credit + + + + + +nvx::exec->nvx::exec::peer + + + + + +nvx::exec->nvx::exec::runner + + + + + +nvx::exec::credit->nvx::exec::credit::inbox + + + + + +nvx::exec::credit->nvx::exec::credit::receiver + + + + + +nvx::exec::credit->nvx::exec::credit::sender + + + + + +nvx::exec + + +nvx::exec + + + + + +nvx::exec::credit + + +credit + + + + + +nvx::exec::credit::inbox + + +inbox + + + + + +nvx::exec::credit::receiver + + +receiver + + + + + +nvx::exec::credit::sender + + +sender + + + + + +nvx::exec::peer + + +peer + + + + + +nvx::exec::runner + + +runner + + + + + diff --git a/doc/modules/nvx-gpt.svg b/doc/modules/nvx-gpt.svg new file mode 100644 index 000000000..fc886005f --- /dev/null +++ b/doc/modules/nvx-gpt.svg @@ -0,0 +1,97 @@ + + + + + + +nvx::gpt + + + +nvx::gpt->nvx::gpt::crc + + + + + +nvx::gpt->nvx::gpt::entry + + + + + +nvx::gpt->nvx::gpt::error + + + + + +nvx::gpt->nvx::gpt::header + + + + + +nvx::gpt->nvx::gpt::source + + + + + +nvx::gpt + + +nvx::gpt + + + + + +nvx::gpt::crc + + +crc + + + + + +nvx::gpt::entry + + +entry + + + + + +nvx::gpt::error + + +error + + + + + +nvx::gpt::header + + +header + + + + + +nvx::gpt::source + + +source + + + + + diff --git a/doc/modules/nvx-guest.svg b/doc/modules/nvx-guest.svg new file mode 100644 index 000000000..1ef8357ce --- /dev/null +++ b/doc/modules/nvx-guest.svg @@ -0,0 +1,1347 @@ + + + + + + +nvx::guest + + + +nvx::guest->nvx::guest::cgroup + + + + + +nvx::guest->nvx::guest::console_pending + + + + + +nvx::guest->nvx::guest::device_io + + + + + +nvx::guest->nvx::guest::entry + + + + + +nvx::guest->nvx::guest::exec + + + + + +nvx::guest->nvx::guest::exit + + + + + +nvx::guest->nvx::guest::generation + + + + + +nvx::guest->nvx::guest::gpt + + + + + +nvx::guest->nvx::guest::ingress + + + + + +nvx::guest->nvx::guest::kernel_text + + + + + +nvx::guest->nvx::guest::managed_agent + + + + + +nvx::guest->nvx::guest::mappings + + + + + +nvx::guest->nvx::guest::mmio_write + + + + + +nvx::guest->nvx::guest::nvxs + + + + + +nvx::guest->nvx::guest::parse + + + + + +nvx::guest->nvx::guest::process + + + + + +nvx::guest->nvx::guest::reseed + + + + + +nvx::guest->nvx::guest::root + + + + + +nvx::guest->nvx::guest::spawn + + + + + +nvx::guest->nvx::guest::status + + + + + +nvx::guest->nvx::guest::sys + + + + + +nvx::guest->nvx::guest::terminal + + + + + +nvx::guest->nvx::guest::time + + + + + +nvx::guest->nvx::guest::time_probe + + + + + +nvx::guest::exec->nvx::guest::exec::binding + + + + + +nvx::guest::ingress->nvx::guest::ingress::address + + + + + +nvx::guest::ingress->nvx::guest::ingress::collect + + + + + +nvx::guest::ingress->nvx::guest::ingress::diag + + + + + +nvx::guest::ingress->nvx::guest::ingress::encode + + + + + +nvx::guest::ingress->nvx::guest::ingress::sender + + + + + +nvx::guest::ingress->nvx::guest::ingress::table + + + + + +nvx::guest::ingress->nvx::guest::ingress::test_mode + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::cgroup + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::diagnostics + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::dispatch + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::environment + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::exec + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::exec_config + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::helper + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::launch + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::mappings + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::seam + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::session + + + + + +nvx::guest::managed_agent->nvx::guest::managed_agent::tty + + + + + +nvx::guest::mappings->nvx::guest::mappings::kernel + + + + + +nvx::guest::root->nvx::guest::root::block + + + + + +nvx::guest::root->nvx::guest::root::bootstrap + + + + + +nvx::guest::root->nvx::guest::root::kernel + + + + + +nvx::guest::root->nvx::guest::root::share + + + + + +nvx::guest::sys->nvx::guest::sys::clock + + + + + +nvx::guest::sys->nvx::guest::sys::discipline + + + + + +nvx::guest::sys->nvx::guest::sys::entry + + + + + +nvx::guest::sys->nvx::guest::sys::fd + + + + + +nvx::guest::sys->nvx::guest::sys::filesystem + + + + + +nvx::guest::sys->nvx::guest::sys::ioctl + + + + + +nvx::guest::sys->nvx::guest::sys::kernel + + + + + +nvx::guest::sys->nvx::guest::sys::mapping + + + + + +nvx::guest::sys->nvx::guest::sys::msr + + + + + +nvx::guest::sys->nvx::guest::sys::poll + + + + + +nvx::guest::sys->nvx::guest::sys::port + + + + + +nvx::guest::sys->nvx::guest::sys::process + + + + + +nvx::guest::sys->nvx::guest::sys::rooted + + + + + +nvx::guest::sys->nvx::guest::sys::scheduler + + + + + +nvx::guest::sys->nvx::guest::sys::signal + + + + + +nvx::guest::sys->nvx::guest::sys::sock_diag + + + + + +nvx::guest::sys->nvx::guest::sys::socket + + + + + +nvx::guest::sys->nvx::guest::sys::spawn + + + + + +nvx::guest::sys->nvx::guest::sys::terminal + + + + + +nvx::guest::sys->nvx::guest::sys::timer + + + + + +nvx::guest::sys->nvx::guest::sys::x86 + + + + + +nvx::guest::sys::port->nvx::guest::sys::port::access + + + + + +nvx::guest::sys::x86->nvx::guest::sys::x86::cpuid + + + + + +nvx::guest::sys::x86->nvx::guest::sys::x86::msr + + + + + +nvx::guest::time->nvx::guest::time::api + + + + + +nvx::guest::time->nvx::guest::time::checks + + + + + +nvx::guest::time->nvx::guest::time::commands + + + + + +nvx::guest::time->nvx::guest::time::cpu_mask + + + + + +nvx::guest::time->nvx::guest::time::daemon + + + + + +nvx::guest::time->nvx::guest::time::decoders + + + + + +nvx::guest::time->nvx::guest::time::events + + + + + +nvx::guest::time->nvx::guest::time::state + + + + + +nvx::guest::time->nvx::guest::time::system + + + + + +nvx::guest::time->nvx::guest::time::test_mode + + + + + +nvx::guest::time_probe->nvx::guest::time_probe::clock + + + + + +nvx::guest::time_probe->nvx::guest::time_probe::format + + + + + +nvx::guest::time_probe->nvx::guest::time_probe::latency + + + + + +nvx::guest::time_probe->nvx::guest::time_probe::support + + + + + +nvx::guest::time_probe->nvx::guest::time_probe::warp + + + + + +nvx::guest + + +nvx::guest +Linux only + + + + + +nvx::guest::cgroup + + +cgroup + + + + + +nvx::guest::console_pending + + +console_pending + + + + + +nvx::guest::device_io + + +device_io + + + + + +nvx::guest::entry + + +entry + + + + + +nvx::guest::exec + + +exec + + + + + +nvx::guest::exec::binding + + +binding + + + + + +nvx::guest::exit + + +exit + + + + + +nvx::guest::generation + + +generation + + + + + +nvx::guest::gpt + + +gpt + + + + + +nvx::guest::ingress + + +ingress + + + + + +nvx::guest::ingress::address + + +address + + + + + +nvx::guest::ingress::collect + + +collect + + + + + +nvx::guest::ingress::diag + + +diag + + + + + +nvx::guest::ingress::encode + + +encode + + + + + +nvx::guest::ingress::sender + + +sender + + + + + +nvx::guest::ingress::table + + +table + + + + + +nvx::guest::ingress::test_mode + + +test_mode + + + + + +nvx::guest::kernel_text + + +kernel_text + + + + + +nvx::guest::managed_agent + + +managed_agent + + + + + +nvx::guest::managed_agent::cgroup + + +cgroup + + + + + +nvx::guest::managed_agent::diagnostics + + +diagnostics + + + + + +nvx::guest::managed_agent::dispatch + + +dispatch + + + + + +nvx::guest::managed_agent::environment + + +environment + + + + + +nvx::guest::managed_agent::exec + + +exec + + + + + +nvx::guest::managed_agent::exec_config + + +exec_config + + + + + +nvx::guest::managed_agent::helper + + +helper + + + + + +nvx::guest::managed_agent::launch + + +launch + + + + + +nvx::guest::managed_agent::mappings + + +mappings + + + + + +nvx::guest::managed_agent::seam + + +seam + + + + + +nvx::guest::managed_agent::session + + +session + + + + + +nvx::guest::managed_agent::tty + + +tty + + + + + +nvx::guest::mappings + + +mappings + + + + + +nvx::guest::mappings::kernel + + +kernel + + + + + +nvx::guest::mmio_write + + +mmio_write + + + + + +nvx::guest::nvxs + + +nvxs + + + + + +nvx::guest::parse + + +parse + + + + + +nvx::guest::process + + +process + + + + + +nvx::guest::reseed + + +reseed + + + + + +nvx::guest::root + + +root + + + + + +nvx::guest::root::block + + +block + + + + + +nvx::guest::root::bootstrap + + +bootstrap + + + + + +nvx::guest::root::kernel + + +kernel + + + + + +nvx::guest::root::share + + +share + + + + + +nvx::guest::spawn + + +spawn + + + + + +nvx::guest::status + + +status + + + + + +nvx::guest::sys + + +sys + + + + + +nvx::guest::sys::clock + + +clock + + + + + +nvx::guest::sys::discipline + + +discipline + + + + + +nvx::guest::sys::entry + + +entry + + + + + +nvx::guest::sys::fd + + +fd + + + + + +nvx::guest::sys::filesystem + + +filesystem + + + + + +nvx::guest::sys::ioctl + + +ioctl + + + + + +nvx::guest::sys::kernel + + +kernel +pub(crate) + + + + + +nvx::guest::sys::mapping + + +mapping + + + + + +nvx::guest::sys::msr + + +msr + + + + + +nvx::guest::sys::poll + + +poll + + + + + +nvx::guest::sys::port + + +port + + + + + +nvx::guest::sys::port::access + + +access + + + + + +nvx::guest::sys::process + + +process + + + + + +nvx::guest::sys::rooted + + +rooted + + + + + +nvx::guest::sys::scheduler + + +scheduler + + + + + +nvx::guest::sys::signal + + +signal + + + + + +nvx::guest::sys::sock_diag + + +sock_diag + + + + + +nvx::guest::sys::socket + + +socket + + + + + +nvx::guest::sys::spawn + + +spawn + + + + + +nvx::guest::sys::terminal + + +terminal + + + + + +nvx::guest::sys::timer + + +timer + + + + + +nvx::guest::sys::x86 + + +x86 +pub(crate) + + + + + +nvx::guest::sys::x86::cpuid + + +cpuid +pub(crate) + + + + + +nvx::guest::sys::x86::msr + + +msr +pub(crate) + + + + + +nvx::guest::terminal + + +terminal + + + + + +nvx::guest::time + + +time + + + + + +nvx::guest::time::api + + +api + + + + + +nvx::guest::time::checks + + +checks + + + + + +nvx::guest::time::commands + + +commands + + + + + +nvx::guest::time::cpu_mask + + +cpu_mask + + + + + +nvx::guest::time::daemon + + +daemon + + + + + +nvx::guest::time::decoders + + +decoders + + + + + +nvx::guest::time::events + + +events + + + + + +nvx::guest::time::state + + +state + + + + + +nvx::guest::time::system + + +system + + + + + +nvx::guest::time::test_mode + + +test_mode + + + + + +nvx::guest::time_probe + + +time_probe + + + + + +nvx::guest::time_probe::clock + + +clock + + + + + +nvx::guest::time_probe::format + + +format + + + + + +nvx::guest::time_probe::latency + + +latency + + + + + +nvx::guest::time_probe::support + + +support + + + + + +nvx::guest::time_probe::warp + + +warp + + + + + diff --git a/doc/modules/nvx-host.svg b/doc/modules/nvx-host.svg new file mode 100644 index 000000000..36a806ff6 --- /dev/null +++ b/doc/modules/nvx-host.svg @@ -0,0 +1,1216 @@ + + + + + + +nvx::host + + + +nvx::host->nvx::host::artifacts + + + + + +nvx::host->nvx::host::cli + + + + + +nvx::host->nvx::host::filesystem + + + + + +nvx::host->nvx::host::instance + + + + + +nvx::host->nvx::host::launch + + + + + +nvx::host->nvx::host::network + + + + + +nvx::host->nvx::host::nvxs + + + + + +nvx::host->nvx::host::paths + + + + + +nvx::host->nvx::host::platform + + + + + +nvx::host->nvx::host::process + + + + + +nvx::host->nvx::host::report + + + + + +nvx::host->nvx::host::sandbox + + + + + +nvx::host->nvx::host::seal + + + + + +nvx::host->nvx::host::session + + + + + +nvx::host->nvx::host::spawn + + + + + +nvx::host->nvx::host::store + + + + + +nvx::host->nvx::host::supervisor + + + + + +nvx::host->nvx::host::sys + + + + + +nvx::host->nvx::host::template + + + + + +nvx::host::artifacts->nvx::host::artifacts::dir + + + + + +nvx::host::artifacts->nvx::host::artifacts::guest + + + + + +nvx::host::artifacts->nvx::host::artifacts::identity + + + + + +nvx::host::artifacts->nvx::host::artifacts::require + + + + + +nvx::host::cli->nvx::host::cli::args + + + + + +nvx::host::cli->nvx::host::cli::artifacts + + + + + +nvx::host::cli->nvx::host::cli::cpu_profile + + + + + +nvx::host::cli->nvx::host::cli::deadline + + + + + +nvx::host::cli->nvx::host::cli::egress + + + + + +nvx::host::cli->nvx::host::cli::json + + + + + +nvx::host::cli->nvx::host::cli::network + + + + + +nvx::host::cli->nvx::host::cli::output + + + + + +nvx::host::cli->nvx::host::cli::run + + + + + +nvx::host::cli->nvx::host::cli::sandbox + + + + + +nvx::host::cli->nvx::host::cli::session + + + + + +nvx::host::cli->nvx::host::cli::shell + + + + + +nvx::host::cli::sandbox->nvx::host::cli::sandbox::lifecycle + + + + + +nvx::host::cli::sandbox->nvx::host::cli::sandbox::lock + + + + + +nvx::host::cli::sandbox->nvx::host::cli::sandbox::one_shot + + + + + +nvx::host::cli::sandbox->nvx::host::cli::sandbox::options + + + + + +nvx::host::cli::sandbox->nvx::host::cli::sandbox::state + + + + + +nvx::host::network->nvx::host::network::address + + + + + +nvx::host::network->nvx::host::network::arguments + + + + + +nvx::host::network->nvx::host::network::loopback + + + + + +nvx::host::network->nvx::host::network::lowering + + + + + +nvx::host::network->nvx::host::network::policy + + + + + +nvx::host::network->nvx::host::network::proxy + + + + + +nvx::host::network->nvx::host::network::rule + + + + + +nvx::host::network::policy->nvx::host::network::policy::json + + + + + +nvx::host::platform->nvx::host::platform::inherit + + + + + +nvx::host::platform->nvx::host::platform::linux + + + + + +nvx::host::platform->nvx::host::platform::windows + + + + + +nvx::host::sandbox->nvx::host::sandbox::distro + + + + + +nvx::host::sandbox->nvx::host::sandbox::layer + + + + + +nvx::host::sandbox->nvx::host::sandbox::mount + + + + + +nvx::host::sandbox->nvx::host::sandbox::options + + + + + +nvx::host::sandbox->nvx::host::sandbox::plan + + + + + +nvx::host::sandbox->nvx::host::sandbox::prepared + + + + + +nvx::host::sandbox->nvx::host::sandbox::scratch + + + + + +nvx::host::sandbox::mount->nvx::host::sandbox::mount::aggregate + + + + + +nvx::host::sandbox::mount->nvx::host::sandbox::mount::boot + + + + + +nvx::host::sandbox::mount->nvx::host::sandbox::mount::host_paths + + + + + +nvx::host::sandbox::mount->nvx::host::sandbox::mount::request + + + + + +nvx::host::sandbox::plan->nvx::host::sandbox::plan::form + + + + + +nvx::host::sandbox::scratch->nvx::host::sandbox::scratch::imp + + + + + +nvx::host::seal->nvx::host::seal::imp + + + + + +nvx::host::seal->nvx::host::seal::windows + + + + + +nvx::host::store->nvx::host::store::caller + + + + + +nvx::host::store->nvx::host::store::files + + + + + +nvx::host::store->nvx::host::store::identity + + + + + +nvx::host::store->nvx::host::store::lifecycle + + + + + +nvx::host::store->nvx::host::store::record + + + + + +nvx::host::store->nvx::host::store::vmm + + + + + +nvx::host::sys->nvx::host::sys::interrupt + + + + + +nvx::host::sys->nvx::host::sys::process + + + + + +nvx::host::sys->nvx::host::sys::socket + + + + + +nvx::host::template->nvx::host::template::canonical + + + + + +nvx::host::template->nvx::host::template::key + + + + + +nvx::host::template->nvx::host::template::manifest + + + + + +nvx::host::template->nvx::host::template::wire + + + + + +nvx::host + + +nvx::host + + + + + +nvx::host::artifacts + + +artifacts + + + + + +nvx::host::artifacts::dir + + +dir + + + + + +nvx::host::artifacts::guest + + +guest + + + + + +nvx::host::artifacts::identity + + +identity + + + + + +nvx::host::artifacts::require + + +require + + + + + +nvx::host::cli + + +cli + + + + + +nvx::host::cli::args + + +args + + + + + +nvx::host::cli::artifacts + + +artifacts + + + + + +nvx::host::cli::cpu_profile + + +cpu_profile + + + + + +nvx::host::cli::deadline + + +deadline + + + + + +nvx::host::cli::egress + + +egress + + + + + +nvx::host::cli::json + + +json + + + + + +nvx::host::cli::network + + +network + + + + + +nvx::host::cli::output + + +output + + + + + +nvx::host::cli::run + + +run + + + + + +nvx::host::cli::sandbox + + +sandbox + + + + + +nvx::host::cli::sandbox::lifecycle + + +lifecycle + + + + + +nvx::host::cli::sandbox::lock + + +lock + + + + + +nvx::host::cli::sandbox::one_shot + + +one_shot + + + + + +nvx::host::cli::sandbox::options + + +options + + + + + +nvx::host::cli::sandbox::state + + +state + + + + + +nvx::host::cli::session + + +session + + + + + +nvx::host::cli::shell + + +shell + + + + + +nvx::host::filesystem + + +filesystem + + + + + +nvx::host::instance + + +instance + + + + + +nvx::host::launch + + +launch + + + + + +nvx::host::network + + +network + + + + + +nvx::host::network::address + + +address + + + + + +nvx::host::network::arguments + + +arguments + + + + + +nvx::host::network::loopback + + +loopback + + + + + +nvx::host::network::lowering + + +lowering + + + + + +nvx::host::network::policy + + +policy + + + + + +nvx::host::network::policy::json + + +json +pub(crate) + + + + + +nvx::host::network::proxy + + +proxy + + + + + +nvx::host::network::rule + + +rule + + + + + +nvx::host::nvxs + + +nvxs + + + + + +nvx::host::paths + + +paths +pub(crate) + + + + + +nvx::host::platform + + +platform + + + + + +nvx::host::platform::inherit + + +inherit +pub(crate) + + + + + +nvx::host::platform::linux + + +linux +Linux only + + + + + +nvx::host::platform::windows + + +windows +Windows only + + + + + +nvx::host::process + + +process + + + + + +nvx::host::report + + +report + + + + + +nvx::host::sandbox + + +sandbox + + + + + +nvx::host::sandbox::distro + + +distro + + + + + +nvx::host::sandbox::layer + + +layer + + + + + +nvx::host::sandbox::mount + + +mount + + + + + +nvx::host::sandbox::mount::aggregate + + +aggregate + + + + + +nvx::host::sandbox::mount::boot + + +boot + + + + + +nvx::host::sandbox::mount::host_paths + + +host_paths + + + + + +nvx::host::sandbox::mount::request + + +request + + + + + +nvx::host::sandbox::options + + +options + + + + + +nvx::host::sandbox::plan + + +plan + + + + + +nvx::host::sandbox::plan::form + + +form + + + + + +nvx::host::sandbox::prepared + + +prepared + + + + + +nvx::host::sandbox::scratch + + +scratch + + + + + +nvx::host::sandbox::scratch::imp + + +imp +Windows only + + + + + +nvx::host::seal + + +seal + + + + + +nvx::host::seal::imp + + +imp +Linux only + + + + + +nvx::host::seal::windows + + +windows +Windows only + + + + + +nvx::host::session + + +session + + + + + +nvx::host::spawn + + +spawn + + + + + +nvx::host::store + + +store + + + + + +nvx::host::store::caller + + +caller + + + + + +nvx::host::store::files + + +files + + + + + +nvx::host::store::identity + + +identity + + + + + +nvx::host::store::lifecycle + + +lifecycle + + + + + +nvx::host::store::record + + +record + + + + + +nvx::host::store::vmm + + +vmm + + + + + +nvx::host::supervisor + + +supervisor + + + + + +nvx::host::sys + + +sys +Linux only + + + + + +nvx::host::sys::interrupt + + +interrupt + + + + + +nvx::host::sys::process + + +process + + + + + +nvx::host::sys::socket + + +socket + + + + + +nvx::host::template + + +template + + + + + +nvx::host::template::canonical + + +canonical + + + + + +nvx::host::template::key + + +key + + + + + +nvx::host::template::manifest + + +manifest + + + + + +nvx::host::template::wire + + +wire + + + + + diff --git a/doc/modules/nvx-net.svg b/doc/modules/nvx-net.svg new file mode 100644 index 000000000..8b334d3c7 --- /dev/null +++ b/doc/modules/nvx-net.svg @@ -0,0 +1,37 @@ + + + + + + +nvx::net + + + +nvx::net->nvx::net::cidr + + + + + +nvx::net + + +nvx::net + + + + + +nvx::net::cidr + + +cidr + + + + + diff --git a/doc/modules/nvx-protocol.svg b/doc/modules/nvx-protocol.svg new file mode 100644 index 000000000..993c3699c --- /dev/null +++ b/doc/modules/nvx-protocol.svg @@ -0,0 +1,232 @@ + + + + + + +nvx::protocol + + + +nvx::protocol->nvx::protocol::app + + + + + +nvx::protocol->nvx::protocol::boot + + + + + +nvx::protocol->nvx::protocol::cmdline + + + + + +nvx::protocol->nvx::protocol::exec + + + + + +nvx::protocol->nvx::protocol::export + + + + + +nvx::protocol->nvx::protocol::features + + + + + +nvx::protocol->nvx::protocol::maps + + + + + +nvx::protocol->nvx::protocol::nvxs + + + + + +nvx::protocol->nvx::protocol::outer + + + + + +nvx::protocol->nvx::protocol::root + + + + + +nvx::protocol->nvx::protocol::share + + + + + +nvx::protocol::nvxs->nvx::protocol::nvxs::guest + + + + + +nvx::protocol::nvxs->nvx::protocol::nvxs::host + + + + + +nvx::protocol::nvxs->nvx::protocol::nvxs::record + + + + + +nvx::protocol + + +nvx::protocol + + + + + +nvx::protocol::app + + +app + + + + + +nvx::protocol::boot + + +boot + + + + + +nvx::protocol::cmdline + + +cmdline + + + + + +nvx::protocol::exec + + +exec + + + + + +nvx::protocol::export + + +export + + + + + +nvx::protocol::features + + +features + + + + + +nvx::protocol::maps + + +maps + + + + + +nvx::protocol::nvxs + + +nvxs + + + + + +nvx::protocol::nvxs::guest + + +guest + + + + + +nvx::protocol::nvxs::host + + +host + + + + + +nvx::protocol::nvxs::record + + +record + + + + + +nvx::protocol::outer + + +outer + + + + + +nvx::protocol::root + + +root + + + + + +nvx::protocol::share + + +share + + + + + diff --git a/doc/modules/nvx-testing.svg b/doc/modules/nvx-testing.svg new file mode 100644 index 000000000..b4db434ff --- /dev/null +++ b/doc/modules/nvx-testing.svg @@ -0,0 +1,37 @@ + + + + + + +nvx::testing + + + +nvx::testing->nvx::testing::fake_openvmm + + + + + +nvx::testing + + +nvx::testing + + + + + +nvx::testing::fake_openvmm + + +fake_openvmm + + + + + diff --git a/doc/modules/overview.svg b/doc/modules/overview.svg new file mode 100644 index 000000000..d7af12145 --- /dev/null +++ b/doc/modules/overview.svg @@ -0,0 +1,166 @@ + + + + + + +nvx + + +cluster_nvx + +nvx + + + + + + + + + + + + + + + + + + + + +nvx::activation + + +activation +Layered root activation +2 nested modules + + + + + +nvx::contract + + +contract +Control compatibility + + + + + +nvx::error + + +error +Error types +pub(crate) + + + + + +nvx::exec + + +exec +Execution stdio and credit +6 nested modules + + + + + +nvx::gpt + + +gpt +GPT partition reader +5 nested modules + + + + + +nvx::guest + + +guest +Linux guest programs +88 nested modules +Linux only + + + + + +nvx::host + + +host +Launch and supervision +79 nested modules + + + + + +nvx::hypervisor + + +hypervisor +Backend selection +pub(crate) + + + + + +nvx::net + + +net +IP networks +1 nested module + + + + + +nvx::protocol + + +protocol +NVXS / NVXC codecs +14 nested modules + + + + + +nvx::testing + + +testing +Fake OpenVMM +1 nested module + + + + + +nvx::uuid + + +uuid +UUID encoding + + + + + diff --git a/doc/project-structure.md b/doc/project-structure.md index 7bc5842af..b5834f33d 100644 --- a/doc/project-structure.md +++ b/doc/project-structure.md @@ -79,6 +79,7 @@ nvx/ | |-- design/ Architecture of the microVM and its guest runtime | |-- distribution.md Packaging and source delivery | |-- guests/ Guest userland chapters (Ubuntu and Azure Linux) +| |-- modules/ Module diagrams of the nvx crate | |-- openvmm-upstream-roadmap.md Plan for upstreaming the OpenVMM fork's microVM commits | |-- project-structure.md This guide | |-- run.md Guest launch and host mapping @@ -273,7 +274,7 @@ and a fake OpenVMM for tests. Its `guest` feature builds the nine static executables that every guest image installs at `/sbin/nvx-*`. It is a standalone crate with its own `Cargo.lock`, and builds independently of the OpenVMM submodule. See its -[README](../nvx-rs/README.md). +[README](../nvx-rs/README.md) and its [module diagrams](modules/README.md). ### `openvmm/` diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 7e63dc63c..fecfdb1de 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -71,6 +71,10 @@ nvx-rs/ └── artifacts.json the NVX release that the `bundled` feature stages ``` +The [module diagrams](../doc/modules/README.md) show the crate's modules, the +modules that each contains, and the dependencies between its top-level modules, +on Linux and Windows. + | Module | Contents | | --- | --- | | `nvx::protocol` | The codec of the control console: `NVXS` outer records, `NVXC` application frames, the `EXEC` payload that the host encodes and the guest agent decodes, the `MAPS` payloads of the host mapping table, `GuestFeatures`, and the `nvx_maps=` and `nvx_workload_account=create` tokens. The [conformance vectors](tests/vectors/README.md) of version 1, `tests/vectors/nvxc-v1.txt`, and OpenVMM's vectors and cases of the `NVXS` session pin every record, frame, and payload independently of the codec, which the host, the guest agent, and the fake OpenVMM share | diff --git a/scripts/test_module_diagrams.py b/scripts/test_module_diagrams.py index da49218ee..2dab0e1a0 100644 --- a/scripts/test_module_diagrams.py +++ b/scripts/test_module_diagrams.py @@ -1,11 +1,12 @@ #!/usr/bin/env python3 """Tests for module_diagrams.py, which draws the module diagrams of the nvx -crate. +crate that doc/modules publishes. They check how the script reads and checks cargo-modules' exports, which are written here in its DOT form, how it merges the platforms' modules, the graphs that it draws, the page that shows them, how it replaces an earlier run's -output, and what it runs, with a stand-in for cargo-modules and Graphviz. +output, and what it runs, with a stand-in for cargo-modules and Graphviz. They +also check that doc/modules holds a page and the diagrams that it shows. """ from __future__ import annotations @@ -25,6 +26,7 @@ import module_diagrams as diagrams # noqa: E402 REPO_ROOT = Path(__file__).resolve().parents[1] +PUBLISHED = REPO_ROOT / "doc" / "modules" LINUX, WINDOWS = diagrams.TARGETS @@ -665,5 +667,21 @@ def test_a_symbolic_link_is_refused_rather_than_written_through(self): self.assertEqual(target.read_text(encoding="utf-8"), "outside\n") +class PublishedTests(unittest.TestCase): + def test_doc_modules_holds_the_page_and_the_diagrams_that_it_shows(self): + page = (PUBLISHED / diagrams.PAGE).read_text(encoding="utf-8") + self.assertTrue(page.startswith(diagrams.MARKER + "\n")) + images = re.findall(r"!\[[^\]]*\]\(([^)]+)\)", page) + self.assertEqual(images[:2], ["overview.svg", "dependencies.svg"]) + self.assertEqual( + sorted(path.name for path in PUBLISHED.iterdir()), + sorted([diagrams.PAGE, *images]), + ) + for image in images: + with self.subTest(image=image): + root = ET.parse(PUBLISHED / image).getroot() + self.assertEqual(root.tag, "{http://www.w3.org/2000/svg}svg") + + if __name__ == "__main__": unittest.main() From 90745e49785596d40761d3413bbeef6953e8d934 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Tue, 27 Oct 2026 10:43:13 -0700 Subject: [PATCH 3/6] ci: draw the module diagrams again after every merge to dev The module diagrams that doc/modules publishes go stale as soon as the nvx crate changes. The module diagrams workflow draws them again after every push to dev that changes their inputs, nvx-rs/, rust-toolchain.toml, the script, the build constants that name the crate's targets, or the workflow, as the merge of such a pull request does, and commits them to dev when they change. Its draw job runs on ubuntu-24.04 and installs Graphviz and Liberation Sans, the first font that the diagrams name, so that the Graphviz release and the font that lay them out stay those that drew doc/modules: other releases and fonts lay them out differently. It builds cargo-modules 0.27.0 with the Rust release that rust-toolchain.toml pins, into a cache that only dev pushes save. It checks out without credentials the newest commit of the branch that a push or a dispatch names, rather than the commit that started the run, draws the diagrams into doc/modules, summarizes the files that they change, uploads them as an artifact, and names the commit that it drew in its outputs. The publish job runs only for dev. It checks out the newest dev with the deploy key with which CI persists the performance history, since deploy keys may push to dev past its rules, replaces doc/modules with the artifact, and, when that changes anything, commits it as github-actions[bot] with a message that names the drawn commit and skips CI, and pushes it, retrying on the newest dev when dev moved. It builds and runs nothing of the commit, refuses an artifact that holds anything but the page and SVG files, or a malformed drawn commit, and commits nothing outside doc/modules. Runs on dev wait for one another, and a newer push replaces the run that waits; since each run draws the newest dev, whichever run commits last commits the diagrams of the newest dev. A ready pull request that changes the inputs runs the draw job alone, so that a change that breaks the drawing fails before it merges, and its summary and artifact show the diagrams that its merge will commit. The workflow is not part of the required status check. The page now says that CI keeps the diagrams current, and links to the new Module diagrams section of doc/ci.md, which describes the workflow and how to draw the diagrams locally. Tests check the workflow's triggers, credentials, concurrency, and pins, and the handoff of the drawn commit and its diagrams to the publish job, and, on POSIX hosts, run the publish step against a local repository: it commits changed diagrams, commits nothing for unchanged ones, retries a rejected push on the newest dev, and refuses a malformed artifact or drawn commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/module-diagrams.yml | 233 +++++++++++++++++++ doc/ci.md | 53 +++++ doc/modules/README.md | 4 +- doc/project-structure.md | 16 +- scripts/module_diagrams.py | 22 +- scripts/test_module_diagrams.py | 320 +++++++++++++++++++++++++- 6 files changed, 631 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/module-diagrams.yml diff --git a/.github/workflows/module-diagrams.yml b/.github/workflows/module-diagrams.yml new file mode 100644 index 000000000..ea0690b7e --- /dev/null +++ b/.github/workflows/module-diagrams.yml @@ -0,0 +1,233 @@ +name: Module diagrams + +# Draws the module diagrams of the nvx crate, which doc/modules publishes, with +# scripts/module_diagrams.py (doc/ci.md, "Module diagrams"). A push to dev that +# changes their inputs, such as a merge, commits them to dev when they change. A +# pull request that changes their inputs draws them without committing them, so +# that a change that breaks the drawing fails before it merges, and uploads them +# for review. +on: + push: + branches: + - dev + paths: + - nvx-rs/** + - rust-toolchain.toml + - scripts/module_diagrams.py + - scripts/nvx_tools/build_constants.py + - .github/workflows/module-diagrams.yml + pull_request: + branches: + - dev + types: + - opened + - synchronize + - reopened + - ready_for_review + paths: + - nvx-rs/** + - rust-toolchain.toml + - scripts/module_diagrams.py + - scripts/nvx_tools/build_constants.py + - .github/workflows/module-diagrams.yml + workflow_dispatch: + +permissions: + contents: read + +# Runs on dev wait for one another, and a newer push replaces the run that +# waits. Each run draws the newest dev, so the run that commits last commits +# the diagrams of the newest dev. A newer push to a pull request cancels its +# older run. +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +env: + CARGO_MODULES_VERSION: 0.27.0 + +jobs: + # The diagrams depend on the Graphviz release and the fonts that lay them + # out, so this job runs on a fixed Ubuntu release, and builds and runs the + # commit's code without credentials. + draw: + name: Draw module diagrams + if: github.event_name != 'pull_request' || github.event.pull_request.draft == false + runs-on: ubuntu-24.04 + timeout-minutes: 30 + outputs: + sha: ${{ steps.draw.outputs.sha }} + steps: + # A push or a dispatch draws the newest commit of its branch, rather than + # the commit that started it, so that whichever run commits last commits + # the diagrams of the newest dev. + - name: Checkout + uses: actions/checkout@v5 + with: + ref: ${{ github.event_name != 'pull_request' && github.ref || '' }} + persist-credentials: false + + # Graphviz measures the labels with Liberation Sans, which the diagrams + # name first, so that their layouts do not depend on the image's other + # fonts. + - name: Install Graphviz + shell: bash + run: | + set -euo pipefail + sudo apt-get update + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ + graphviz fonts-liberation + dot -V + + - name: Install the pinned Rust release + shell: bash + run: | + set -euo pipefail + toolchain=$(sed -n 's/^channel = "\([^"]*\)".*/\1/p' rust-toolchain.toml) + test -n "${toolchain}" + rustup toolchain install "${toolchain}" --profile minimal --no-self-update + echo "RUST_TOOLCHAIN=${toolchain}" >> "${GITHUB_ENV}" + echo "${RUNNER_TEMP}/cargo-modules/bin" >> "${GITHUB_PATH}" + + - name: Restore cargo-modules + id: restore-cargo-modules + uses: actions/cache/restore@v5 + with: + path: ${{ runner.temp }}/cargo-modules + key: cargo-modules-${{ env.CARGO_MODULES_VERSION }}-rust-${{ env.RUST_TOOLCHAIN }}-${{ runner.os }}-${{ runner.arch }} + + - name: Install cargo-modules + if: steps.restore-cargo-modules.outputs.cache-hit != 'true' + shell: bash + run: >- + cargo "+${RUST_TOOLCHAIN}" install --locked cargo-modules + --version "${CARGO_MODULES_VERSION}" --root "${RUNNER_TEMP}/cargo-modules" + + # Only dev pushes save the build, as they save the other caches. + - name: Save cargo-modules + if: >- + github.event_name == 'push' && + github.ref == 'refs/heads/dev' && + steps.restore-cargo-modules.outputs.cache-hit != 'true' + uses: actions/cache/save@v5 + with: + path: ${{ runner.temp }}/cargo-modules + key: cargo-modules-${{ env.CARGO_MODULES_VERSION }}-rust-${{ env.RUST_TOOLCHAIN }}-${{ runner.os }}-${{ runner.arch }} + + - name: Draw the diagrams + id: draw + shell: bash + run: | + set -euo pipefail + version=$(cargo modules --version) + if [[ "${version}" != "cargo-modules ${CARGO_MODULES_VERSION}" ]]; then + echo "::error::${version} is not cargo-modules ${CARGO_MODULES_VERSION}" + exit 1 + fi + python3 scripts/module_diagrams.py --output doc/modules + echo "sha=$(git rev-parse HEAD)" >> "${GITHUB_OUTPUT}" + + - name: Summarize the changes + shell: bash + run: | + set -euo pipefail + sha=$(git rev-parse HEAD) + changes=$(git status --porcelain --untracked-files=all -- doc/modules) + { + echo "## Module diagrams" + echo + if [[ -z "${changes}" ]]; then + echo "The diagrams of \`${sha}\` match doc/modules." + else + echo "The diagrams of \`${sha}\` change doc/modules:" + echo + echo '```text' + echo "${changes}" + echo '```' + fi + } >> "${GITHUB_STEP_SUMMARY}" + + - name: Upload the diagrams + uses: actions/upload-artifact@v7 + with: + name: module-diagrams-${{ github.run_id }} + path: doc/modules + if-no-files-found: error + overwrite: true + retention-days: 7 + + # Commits the diagrams that the draw job drew to dev, with the deploy key that + # may push to dev past its rules, the one of the performance history. It + # builds and runs no code of the commit, and commits nothing but doc/modules. + publish: + name: Publish module diagrams + needs: draw + if: github.event_name != 'pull_request' && github.ref == 'refs/heads/dev' + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout dev + uses: actions/checkout@v5 + with: + ref: dev + fetch-depth: 0 + ssh-key: ${{ secrets.PERFORMANCE_PERSIST_DEPLOY_KEY }} + + - name: Download the diagrams + uses: actions/download-artifact@v8 + with: + name: module-diagrams-${{ github.run_id }} + path: ${{ runner.temp }}/module-diagrams + + - name: Commit the diagrams to dev + shell: bash + env: + DIAGRAMS: ${{ runner.temp }}/module-diagrams + DRAWN: ${{ needs.draw.outputs.sha }} + run: | + set -euo pipefail + if [[ ! "${DRAWN}" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::The draw job named no commit: '${DRAWN}'" + exit 1 + fi + unexpected=$(find "${DIAGRAMS}" -mindepth 1 \ + ! \( -type f \( -name README.md -o -name '*.svg' \) \) -print) + if [[ -n "${unexpected}" || ! -f "${DIAGRAMS}/README.md" ]]; then + echo "::error::The diagrams hold more than a page and SVG files, or no page:" + echo "${unexpected}" + exit 1 + fi + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + + for attempt in 1 2 3; do + git fetch origin dev + git checkout -B module-diagrams origin/dev + rm -rf doc/modules + mkdir -p doc/modules + cp "${DIAGRAMS}"/* doc/modules/ + git add --all -- doc/modules + + unexpected=$(git diff --cached --no-renames --name-only | + grep -v '^doc/modules/' || true) + if [[ -n "${unexpected}" ]]; then + echo "::error::Refusing to commit changes outside doc/modules:" + echo "${unexpected}" + exit 1 + fi + + if git diff --cached --quiet; then + echo "The module diagrams on dev are up to date" + exit 0 + fi + + git commit --no-verify \ + -m "doc: update module diagrams (${DRAWN}) [skip ci]" + if git push origin HEAD:dev; then + exit 0 + fi + if [ "${attempt}" -eq 3 ]; then + echo "::error::Failed to publish the module diagrams after ${attempt} attempts" + exit 1 + fi + done diff --git a/doc/ci.md b/doc/ci.md index b58abb28d..97101cfd3 100644 --- a/doc/ci.md +++ b/doc/ci.md @@ -1804,6 +1804,59 @@ comm -23 replaced.txt replacement.txt # The mutants that only the replaced test A push that changes the workflow checks it on `src/gpt/crc.rs` with the library's `gpt::crc::` tests, in two jobs. +## Module diagrams + +[`doc/modules`](modules/README.md) publishes diagrams of the modules of the +[`nvx` crate](../nvx-rs/README.md): its top-level modules, the dependencies +between them, and the modules that each contains, on Linux and Windows. +[`module_diagrams.py`](../scripts/module_diagrams.py) draws them. It exports +the modules of the crate's library, with its `cli`, `testing`, and `guest` +features, for each platform's target with +[cargo-modules](https://github.com/regexident/cargo-modules), merges the +platforms' modules, lays out the diagrams with Graphviz, and writes them with +the page that shows them. Nobody edits them by hand: +[`module-diagrams.yml`](../.github/workflows/module-diagrams.yml) draws them +again after every push to `dev` that changes their inputs, `nvx-rs/`, the Rust +pin, the script, the build constants that name the crate's targets, or the +workflow, as the merge of such a pull request does, and commits them to `dev` +when they change. + +Its `draw` job builds cargo-modules, at the release that the workflow pins, +with the Rust release that `rust-toolchain.toml` pins, and restores the build +from a cache that only `dev` pushes save. It runs on a fixed Ubuntu release, +whose Graphviz release lays out the diagrams, and installs Liberation Sans, the +font that they name first, with which Graphviz measures their labels, so that +their layouts depend neither on the runner image's other fonts nor on its +updates. It checks out, without credentials, the newest commit of the branch +that a push or a dispatch names, rather than the commit that started the run, +or a pull request's merge commit, draws the diagrams into `doc/modules`, +summarizes the files that they change, and uploads them as +`module-diagrams-`. For a push to `dev`, or a manual dispatch on it, +the `publish` job then checks out the newest `dev` with +`PERFORMANCE_PERSIST_DEPLOY_KEY`, the deploy key with which the +[performance history](#development-releases-and-performance-history) is +persisted, since deploy keys may push to `dev` past its rules. It replaces +`doc/modules` with the uploaded diagrams and, if that changes anything, +commits it as `doc: update module diagrams () [skip ci]`, where `` +is the commit that the `draw` job drew, and pushes it, retrying up to three +attempts in all on the newest `dev` when a push fails because `dev` moved. The +job builds and runs nothing of the commit, refuses an upload that holds +anything but the page and SVG files, and commits nothing outside +`doc/modules`. `[skip ci]` keeps its commit, which changes only documentation, +from starting the workflows of a push. + +Runs on `dev` wait for one another, and a newer push replaces the run that +waits. Each run draws the newest `dev`, so whichever run commits last commits +the diagrams of the newest `dev`, in whatever order the runs start. A ready +pull request that changes the inputs runs the `draw` job alone, so that a +change that breaks the drawing fails before it merges, and its summary and +artifact show the diagrams that its merge will commit. The workflow is not +part of `Required status check`. To draw the diagrams locally, install +cargo-modules and Graphviz, and run `python3 scripts/module_diagrams.py`, which +writes them to `build/modules`, or with `--output` to another directory that +holds nothing but an earlier run's diagrams. Leave `doc/modules` to CI: other +Graphviz releases and fonts lay the diagrams out differently. + ## Adversarial campaigns The separate diff --git a/doc/modules/README.md b/doc/modules/README.md index b7a49eb7f..7a1d6ec17 100644 --- a/doc/modules/README.md +++ b/doc/modules/README.md @@ -7,7 +7,9 @@ contains, and the dependencies between them, as [cargo-modules](https://github.com/regexident/cargo-modules) 0.27.0 finds them and Graphviz 2.43.0 draws them. They cover the crate's library with its `cli`, `testing`, and `guest` features, on Linux (`x86_64-unknown-linux-musl`) and -Windows (`x86_64-pc-windows-msvc`). +Windows (`x86_64-pc-windows-msvc`). CI draws them again after every merge to +`dev` that changes the crate, and commits them when they change; see [Module +diagrams](../ci.md#module-diagrams). Each box is a module, colored by the part of the crate that holds it: blue for the modules that the host and the guest share, purple for the host, teal for diff --git a/doc/project-structure.md b/doc/project-structure.md index b5834f33d..64e80ce5c 100644 --- a/doc/project-structure.md +++ b/doc/project-structure.md @@ -8,7 +8,7 @@ are build products or caches and are not part of the tracked source tree. The | Path | Purpose | | --- | --- | -| `.github/workflows` | CI, nightly failure report, release verification, adversarial campaign, and Copilot environment workflows | +| `.github/workflows` | CI, nightly failure report, module diagram, release verification, adversarial campaign, and Copilot environment workflows | | `.github/actions` | Reusable local CI actions | | `.github/skills` | Copilot agent skills for common development workflows | | `.github/agents` | Bounded Copilot strategist definitions | @@ -46,6 +46,7 @@ nvx/ | | |-- ci.yml Main build, test, benchmark, and release workflow | | |-- copilot-code-review.yml Copilot code review environment | | |-- copilot-setup-steps.yml Copilot cloud agent environment +| | |-- module-diagrams.yml Module diagrams of the nvx crate, drawn after merges | | |-- nightly-ci-report.yml Report of failed nightly CI runs | | |-- nvx-rs-mutants.yml On-demand mutation testing of nvx-rs | | |-- run-nvx-microvm-tests.yml Reusable NVX microVM test job @@ -190,9 +191,10 @@ reusable workflows: `build-openvmm-binary.yml` builds OpenVMM for one target, `run-nvx-microvm-tests.yml` runs the NVX microVM tests on one backend, and `run-platform.yml` benchmarks one platform and, on `dev` pushes, packages its release. The full runs of `ci.yml` on `dev` also publish the `nvx` crate to -crates.io. The other workflows report failed nightly CI runs, run adversarial -campaigns, run the `nvx` crate's mutation tests, verify published releases -with Specula, and prepare the +crates.io. The other workflows report failed nightly CI runs, draw the module +diagrams of the `nvx` crate after merges, run adversarial campaigns, run the +`nvx` crate's mutation tests, verify published releases with Specula, and +prepare the Copilot cloud agent and code review environments. See [Continuous integration](ci.md) for the jobs and gates. @@ -316,9 +318,11 @@ using the [tool manifest](../scripts/setup/tool-versions.conf); see [Automated environment bootstrap](setup.md#automated-environment-bootstrap). `nvx-hosts.example.json` is the template for the [remote agent host](setup.md#remote-agent-hosts) inventory, and CI runs -`publish_development_release.py` to publish development releases and +`publish_development_release.py` to publish development releases, `publish_nvx_crate.py` to publish the `nvx` crate to crates.io (see -[crates.io releases](ci.md#cratesio-releases)). +[crates.io releases](ci.md#cratesio-releases)), and `module_diagrams.py` to +draw the module diagrams that `doc/modules` publishes (see +[Module diagrams](ci.md#module-diagrams)). ### `tests/` diff --git a/scripts/module_diagrams.py b/scripts/module_diagrams.py index 64c8e5c0f..be103ffd4 100644 --- a/scripts/module_diagrams.py +++ b/scripts/module_diagrams.py @@ -1,12 +1,14 @@ #!/usr/bin/env python3 -"""Draws the module diagrams of the nvx crate. +"""Draws the module diagrams of the nvx crate, which doc/modules publishes. cargo-modules exports the modules of the crate's library, with its cli, testing, and guest features, as each of the crate's platforms, Linux and Windows, builds it: the module that contains each module, and the modules that each uses. This script merges the exports of the platforms, draws with Graphviz the crate's top-level modules, the dependencies between them, and the modules that each -contains, and writes the page that shows the diagrams. +contains, and writes the page that shows the diagrams. CI draws them after every +merge to dev that changes their inputs, and commits them when they change +(doc/ci.md, "Module diagrams"). """ from __future__ import annotations @@ -34,7 +36,8 @@ REPOSITORY = BuildConstants.REPO_ROOT MANIFEST = GuestRustBuildConstants.DIRECTORY / "Cargo.toml" -# A run draws beside the build products unless --output names another directory. +# CI draws into doc/modules, which it commits. A local run draws beside the build +# products by default, so that it leaves the published diagrams to CI. DEFAULT_OUTPUT = BuildConstants.BUILD_DIR / "modules" PAGE = "README.md" # The first line of the page, by which a run knows the output of an earlier one. @@ -75,9 +78,9 @@ "uuid": "UUID encoding", } # Graphviz measures the labels with the first of these fonts that is installed, -# and SVG viewers show them with the first that they have. With Liberation Sans -# installed, the layouts do not depend on a host's other fonts, and Arial and -# Helvetica have the same metrics. +# and SVG viewers show them with the first that they have. CI installs +# Liberation Sans, so that its layouts do not depend on the other fonts of its +# runner, and Arial and Helvetica have the same metrics. FONT = "Liberation Sans,Arial,Helvetica,sans-serif" HEADER = ( 'graph [rankdir=LR, bgcolor="white", pad=0.25, nodesep=0.22, ranksep=0.65,' @@ -527,6 +530,7 @@ def link(text: str, path: Path, anchor: str = "") -> str: targets = " and ".join( f"{platform} (`{target}`)" for platform, target in TARGETS.items() ) + ci = link("Module diagrams", REPOSITORY / "doc" / "ci.md", "#module-diagrams") lines = [ MARKER, "", @@ -536,7 +540,9 @@ def link(text: str, path: Path, anchor: str = "") -> str: f"The modules of the {link('`nvx` crate', MANIFEST.parent / 'README.md')}," " the modules that each contains, and the dependencies between them, as" f" {tools}. They cover the crate's library with its {features}, and" - f" `{FEATURES[-1]}` features, on {targets}." + f" `{FEATURES[-1]}` features, on {targets}. CI draws them again after" + " every merge to `dev` that changes the crate, and commits them when" + f" they change; see {ci}." ), *paragraph( "Each box is a module, colored by the part of the crate that holds it:" @@ -706,7 +712,7 @@ def main(argv: Sequence[str] | None = None) -> int: default=DEFAULT_OUTPUT, help=( "the directory to write, which holds nothing but the output of an" - " earlier run (default: build/modules)" + " earlier run (default: build/modules; CI writes doc/modules)" ), ) parser.add_argument( diff --git a/scripts/test_module_diagrams.py b/scripts/test_module_diagrams.py index 2dab0e1a0..080fee907 100644 --- a/scripts/test_module_diagrams.py +++ b/scripts/test_module_diagrams.py @@ -1,31 +1,41 @@ #!/usr/bin/env python3 """Tests for module_diagrams.py, which draws the module diagrams of the nvx -crate that doc/modules publishes. +crate that doc/modules publishes, and for the workflow that draws them after +every merge to dev and commits them. They check how the script reads and checks cargo-modules' exports, which are written here in its DOT form, how it merges the platforms' modules, the graphs that it draws, the page that shows them, how it replaces an earlier run's output, and what it runs, with a stand-in for cargo-modules and Graphviz. They -also check that doc/modules holds a page and the diagrams that it shows. +also check that doc/modules holds a page and the diagrams that it shows, the +workflow's triggers, credentials, and pins, and, on POSIX hosts, run its +publication step against a local repository. """ from __future__ import annotations import contextlib import io +import os import re +import shutil +import subprocess import sys import tempfile import unittest import xml.etree.ElementTree as ET from collections.abc import Iterable, Mapping, Sequence from pathlib import Path +from typing import Any, cast from unittest.mock import patch +import yaml + sys.path.insert(0, str(Path(__file__).parent)) import module_diagrams as diagrams # noqa: E402 REPO_ROOT = Path(__file__).resolve().parents[1] +WORKFLOW = REPO_ROOT / ".github" / "workflows" / "module-diagrams.yml" PUBLISHED = REPO_ROOT / "doc" / "modules" LINUX, WINDOWS = diagrams.TARGETS @@ -460,6 +470,16 @@ def test_the_page_links_lead_from_its_directory_to_the_files(self): path = (output / target.partition("#")[0]).resolve() self.assertTrue(path.is_file(), path) + def test_the_page_leads_to_how_ci_draws_the_diagrams(self): + text = " ".join(self.page().split()) + self.assertIn("CI draws them again after every merge to `dev`", text) + for output, target in ( + (diagrams.DEFAULT_OUTPUT, "../../doc/ci.md#module-diagrams"), + (PUBLISHED, "../ci.md#module-diagrams"), + ): + with self.subTest(output=output): + self.assertEqual(self.links(output)["Module diagrams"], target) + def test_an_inline_module_has_no_source_link(self): merged = diagrams.merge( { @@ -683,5 +703,301 @@ def test_doc_modules_holds_the_page_and_the_diagrams_that_it_shows(self): self.assertEqual(root.tag, "{http://www.w3.org/2000/svg}svg") +def _workflow() -> dict[Any, Any]: + return cast(dict[Any, Any], yaml.safe_load(WORKFLOW.read_text(encoding="utf-8"))) + + +def _job(name: str) -> dict[str, Any]: + return cast(dict[str, Any], _workflow()["jobs"][name]) + + +def _step(job: str, name: str) -> dict[str, Any]: + steps = cast(list[dict[str, Any]], _job(job)["steps"]) + [step] = [step for step in steps if step.get("name") == name] + return step + + +class WorkflowTests(unittest.TestCase): + def test_merges_and_pull_requests_that_change_the_inputs_draw(self): + workflow = _workflow() + # YAML 1.1 reads the key `on` as true. + triggers = cast(dict[str, Any], workflow.get("on", workflow.get(True))) + self.assertEqual(set(triggers), {"push", "pull_request", "workflow_dispatch"}) + constants = sys.modules[diagrams.GuestRustBuildConstants.__module__].__file__ + inputs = [ + f"{diagrams.MANIFEST.parent.relative_to(REPO_ROOT).as_posix()}/**", + "rust-toolchain.toml", + Path(diagrams.__file__).resolve().relative_to(REPO_ROOT).as_posix(), + # The script takes the crate and its targets from the build constants. + Path(cast(str, constants)).resolve().relative_to(REPO_ROOT).as_posix(), + WORKFLOW.relative_to(REPO_ROOT).as_posix(), + ] + self.assertEqual(triggers["push"], {"branches": ["dev"], "paths": inputs}) + self.assertEqual(triggers["pull_request"]["branches"], ["dev"]) + self.assertEqual(triggers["pull_request"]["paths"], inputs) + self.assertIn("ready_for_review", triggers["pull_request"]["types"]) + self.assertEqual( + _job("draw")["if"], + "github.event_name != 'pull_request' || " + "github.event.pull_request.draft == false", + ) + + def test_only_the_publication_of_dev_holds_a_credential(self): + workflow = _workflow() + self.assertEqual(workflow["permissions"], {"contents": "read"}) + for name, job in workflow["jobs"].items(): + with self.subTest(job=name): + self.assertNotIn("permissions", job) + text = WORKFLOW.read_text(encoding="utf-8") + self.assertEqual( + re.findall(r"secrets\.\w+", text), + ["secrets.PERFORMANCE_PERSIST_DEPLOY_KEY"], + ) + self.assertEqual( + _step("publish", "Checkout dev")["with"]["ssh-key"], + "${{ secrets.PERFORMANCE_PERSIST_DEPLOY_KEY }}", + ) + self.assertIs(_step("draw", "Checkout")["with"]["persist-credentials"], False) + publish = _job("publish") + self.assertEqual(publish["needs"], "draw") + self.assertEqual( + publish["if"], + "github.event_name != 'pull_request' && github.ref == 'refs/heads/dev'", + ) + # The publication runs nothing of the commit but Git. + for step in publish["steps"]: + with self.subTest(step=step["name"]): + self.assertFalse(step.get("uses", "").startswith("./")) + self.assertNotIn("${{", step.get("run", "")) + self.assertNotIn("python", step.get("run", "")) + + def test_the_publication_commits_what_the_draw_job_drew(self): + upload = _step("draw", "Upload the diagrams")["with"] + download = _step("publish", "Download the diagrams")["with"] + self.assertEqual(upload["path"], PUBLISHED.relative_to(REPO_ROOT).as_posix()) + self.assertEqual(upload["name"], download["name"]) + self.assertIn("github.run_id", upload["name"]) + self.assertEqual( + _step("publish", "Commit the diagrams to dev")["env"]["DIAGRAMS"], + download["path"], + ) + draw = _step("draw", "Draw the diagrams")["run"] + self.assertIn("python3 scripts/module_diagrams.py --output doc/modules", draw) + self.assertIn('"cargo-modules ${CARGO_MODULES_VERSION}"', draw) + + def test_dev_draws_its_newest_commit_and_names_it_in_the_commit(self): + # Whichever run commits last commits the diagrams of the newest dev, + # in whatever order the runs start. + self.assertEqual( + _step("draw", "Checkout")["with"]["ref"], + "${{ github.event_name != 'pull_request' && github.ref || '' }}", + ) + self.assertEqual( + _job("draw")["outputs"], {"sha": "${{ steps.draw.outputs.sha }}"} + ) + self.assertIn( + 'echo "sha=$(git rev-parse HEAD)" >> "${GITHUB_OUTPUT}"', + _step("draw", "Draw the diagrams")["run"], + ) + commit = _step("publish", "Commit the diagrams to dev") + self.assertEqual(commit["env"]["DRAWN"], "${{ needs.draw.outputs.sha }}") + self.assertIn( + '"doc: update module diagrams (${DRAWN}) [skip ci]"', commit["run"] + ) + + def test_the_workflow_pins_its_tools(self): + workflow = _workflow() + self.assertEqual( + workflow["env"]["CARGO_MODULES_VERSION"], diagrams.CARGO_MODULES_VERSION + ) + # A fixed Ubuntu release keeps the Graphviz release that lays out the + # diagrams, and Liberation Sans, the first font that they name. + self.assertRegex(_job("draw")["runs-on"], r"^ubuntu-\d\d\.\d\d$") + self.assertTrue(diagrams.FONT.startswith("Liberation Sans,")) + install = _step("draw", "Install Graphviz")["run"] + self.assertIn("graphviz fonts-liberation", install) + rust = _step("draw", "Install the pinned Rust release")["run"] + self.assertIn("rust-toolchain.toml", rust) + restore = _step("draw", "Restore cargo-modules") + save = _step("draw", "Save cargo-modules") + self.assertEqual(restore["with"], save["with"]) + self.assertIn("env.CARGO_MODULES_VERSION", restore["with"]["key"]) + self.assertIn("env.RUST_TOOLCHAIN", restore["with"]["key"]) + self.assertIn("github.ref == 'refs/heads/dev'", save["if"]) + + def test_runs_of_dev_wait_and_pull_requests_cancel(self): + concurrency = _workflow()["concurrency"] + self.assertEqual( + concurrency["cancel-in-progress"], + "${{ github.event_name == 'pull_request' }}", + ) + self.assertIn( + "github.event.pull_request.number || github.ref", concurrency["group"] + ) + + +BASH = shutil.which("bash") +GIT = shutil.which("git") +# The commit that the draw job drew, as its output names it. +DRAWN = "0123456789abcdef0123456789abcdef01234567" + + +@unittest.skipIf( + os.name == "nt" or BASH is None or GIT is None, + "runs the workflow's bash step with Git on POSIX hosts", +) +class PublishStepTests(unittest.TestCase): + """Runs the publication step in a clone of a local repository.""" + + def setUp(self) -> None: + directory = tempfile.TemporaryDirectory() + self.addCleanup(directory.cleanup) + self.root = Path(directory.name) + config = self.root / "gitconfig" + config.write_text("", encoding="utf-8") + # Git reads neither the host's configuration nor an identity from the + # environment, as on a runner, where the step sets its own. + self.environment = { + **{ + name: value + for name, value in os.environ.items() + if not name.startswith(("GIT_AUTHOR_", "GIT_COMMITTER_")) + }, + "GIT_CONFIG_GLOBAL": str(config), + "GIT_CONFIG_NOSYSTEM": "1", + } + self.origin = self.root / "origin.git" + self.git("init", "--bare", "--initial-branch=dev", str(self.origin)) + seed = self.root / "seed" + self.git("clone", str(self.origin), str(seed)) + for name, text in { + "doc/modules/README.md": "old page\n", + "doc/modules/overview.svg": "\n", + "doc/ci.md": "# CI\n", + }.items(): + (seed / name).parent.mkdir(parents=True, exist_ok=True) + (seed / name).write_text(text, encoding="utf-8") + self.git("-C", str(seed), "add", "--all") + self.git( + "-C", + str(seed), + "-c", + "user.name=Seed", + "-c", + "user.email=seed@example.com", + "commit", + "-m", + "seed", + ) + self.git("-C", str(seed), "push", "origin", "HEAD:dev") + self.checkout = self.root / "checkout" + self.git("clone", "--branch", "dev", str(self.origin), str(self.checkout)) + # The diagrams replace the page and remove overview.svg. + self.diagrams = self.root / "diagrams" + self.diagrams.mkdir() + (self.diagrams / "README.md").write_text("new page\n", encoding="utf-8") + (self.diagrams / "dependencies.svg").write_text("\n", "utf-8") + + def git(self, *arguments: str) -> str: + return subprocess.run( + [cast(str, GIT), *arguments], + env=self.environment, + capture_output=True, + text=True, + check=True, + ).stdout + + def publish(self, drawn: str = DRAWN) -> subprocess.CompletedProcess[str]: + script = _step("publish", "Commit the diagrams to dev")["run"] + return subprocess.run( + [cast(str, BASH), "--noprofile", "--norc", "-eo", "pipefail", "-c", script], + cwd=self.checkout, + env={ + **self.environment, + "DIAGRAMS": str(self.diagrams), + "DRAWN": drawn, + }, + capture_output=True, + text=True, + check=False, + ) + + def dev(self) -> tuple[str, list[str], list[str]]: + """The subject and author of dev's newest commit, and its files.""" + subject, author = self.git( + "-C", str(self.origin), "log", "-1", "--format=%s%n%an", "dev" + ).splitlines() + files = self.git( + "-C", str(self.origin), "ls-tree", "-r", "--name-only", "dev" + ).split() + return subject, [author], files + + def test_changed_diagrams_are_committed_to_dev_without_ci(self): + result = self.publish() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + subject, author, files = self.dev() + self.assertEqual(subject, f"doc: update module diagrams ({DRAWN}) [skip ci]") + self.assertEqual(author, ["github-actions[bot]"]) + self.assertEqual( + files, + ["doc/ci.md", "doc/modules/README.md", "doc/modules/dependencies.svg"], + ) + changed = self.git( + "-C", str(self.origin), "diff", "--name-only", "dev~1", "dev" + ).split() + self.assertTrue(all(path.startswith("doc/modules/") for path in changed)) + # The same diagrams again change nothing. + count = self.git("-C", str(self.origin), "rev-list", "--count", "dev") + result = self.publish() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("up to date", result.stdout) + self.assertEqual( + self.git("-C", str(self.origin), "rev-list", "--count", "dev"), count + ) + + def test_a_rejected_push_is_retried_on_the_newest_dev(self): + hook = self.origin / "hooks" / "pre-receive" + hook.write_text( + '#!/bin/sh\nmarker="$(dirname "$0")/rejected"\n' + '[ -e "$marker" ] && exit 0\n: > "$marker"\nexit 1\n', + encoding="utf-8", + ) + hook.chmod(0o755) + result = self.publish() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertTrue((self.origin / "hooks" / "rejected").exists()) + subject, _, files = self.dev() + self.assertIn("[skip ci]", subject) + self.assertIn("doc/modules/dependencies.svg", files) + + def test_diagrams_with_other_files_or_no_page_are_refused(self): + cases = { + "a script": lambda: (self.diagrams / "run.sh").write_text("", "utf-8"), + "a directory": lambda: (self.diagrams / "sub").mkdir(), + "no page": lambda: (self.diagrams / "README.md").unlink(), + } + head = self.git("-C", str(self.origin), "rev-parse", "dev") + for drawn in ("", "0123abcd", DRAWN.upper(), f"{DRAWN}\n{DRAWN}"): + with self.subTest(drawn=drawn): + result = self.publish(drawn) + self.assertEqual(result.returncode, 1) + self.assertIn("::error::", result.stdout) + self.assertEqual( + self.git("-C", str(self.origin), "rev-parse", "dev"), head + ) + for case, change in cases.items(): + with self.subTest(case=case): + change() + result = self.publish() + self.assertEqual(result.returncode, 1) + self.assertIn("::error::", result.stdout) + self.assertEqual( + self.git("-C", str(self.origin), "rev-parse", "dev"), head + ) + shutil.rmtree(self.diagrams) + self.diagrams.mkdir() + (self.diagrams / "README.md").write_text("new page\n", "utf-8") + + if __name__ == "__main__": unittest.main() From 6b951a259b7a187927df8ec9d093ff351e1bacf6 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Tue, 27 Oct 2026 11:24:47 -0700 Subject: [PATCH 4/6] scripts: restore the modules that cargo-modules mistakes for tests Unless it analyzes with cfg(test), cargo-modules drops as a test module every module whose cfg holds when test holds and every other condition fails: has_test_cfg, in its analyzer.rs, folds the whole condition with test true and every other condition false, so that cfg(not(windows)) reads as a test. The pinned 0.27.0, 0.28.0, and its main branch all do. The Linux export therefore lacked nvx::host::sandbox::scratch::imp, which builds under cfg(not(windows)), and the host diagram labeled the module of that path that Windows builds "Windows only", though both platforms build one. The script now also exports each target with cfg(test), and reads the cfg conditions of each module from cargo-modules' structure. A module whose conditions, or those of a module that contains it, include test builds only for tests. Every other module of the analysis with cfg(test) builds outside tests, so the script restores those that the plain analysis lacks, with their uses in the analysis with cfg(test). It stops at a condition that depends on test in another way, such as any(test, unix) or not(test), whose modules it cannot tell apart, at a module that only the plain analysis holds, and at a structure that it cannot read or that names other modules than the analysis. For the crate as it is, it restores scratch::imp alone, with its four uses, which are also the only uses that the analysis with cfg(test) adds between modules that build outside tests; Windows lacks nothing. Drawn again, the host diagram no longer labels scratch::imp, and the other diagrams and the page stay the same. Tests cover the reading of the structure, the restoration and its refusals, and a drawing in which each platform builds a module of the same path and cargo-modules drops Linux's. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/ci.md | 12 +- doc/modules/nvx-host.svg | 105 ++++++------ scripts/module_diagrams.py | 136 ++++++++++++++- scripts/test_module_diagrams.py | 282 +++++++++++++++++++++++++++++--- 4 files changed, 446 insertions(+), 89 deletions(-) diff --git a/doc/ci.md b/doc/ci.md index 97101cfd3..6b64f1c0a 100644 --- a/doc/ci.md +++ b/doc/ci.md @@ -1814,10 +1814,14 @@ the modules of the crate's library, with its `cli`, `testing`, and `guest` features, for each platform's target with [cargo-modules](https://github.com/regexident/cargo-modules), merges the platforms' modules, lays out the diagrams with Graphviz, and writes them with -the page that shows them. Nobody edits them by hand: -[`module-diagrams.yml`](../.github/workflows/module-diagrams.yml) draws them -again after every push to `dev` that changes their inputs, `nvx-rs/`, the Rust -pin, the script, the build constants that name the crate's targets, or the +the page that shows them. Unless it analyzes with `cfg(test)`, cargo-modules +drops as a test module every module whose cfg holds when `test` holds and every +other condition fails, such as one under `cfg(not(windows))`, so the script +also reads its analysis with `cfg(test)` and restores those modules, and stops +at a cfg that depends on `test` in another way. Nobody edits the diagrams by +hand: [`module-diagrams.yml`](../.github/workflows/module-diagrams.yml) draws +them again after every push to `dev` that changes their inputs, `nvx-rs/`, the +Rust pin, the script, the build constants that name the crate's targets, or the workflow, as the merge of such a pull request does, and commits them to `dev` when they change. diff --git a/doc/modules/nvx-host.svg b/doc/modules/nvx-host.svg index 36a806ff6..74c6e2b2f 100644 --- a/doc/modules/nvx-host.svg +++ b/doc/modules/nvx-host.svg @@ -4,11 +4,11 @@ - + nvx::host - + nvx::host->nvx::host::artifacts @@ -222,32 +222,32 @@ nvx::host::cli::sandbox->nvx::host::cli::sandbox::lifecycle - - + + nvx::host::cli::sandbox->nvx::host::cli::sandbox::lock - - + + nvx::host::cli::sandbox->nvx::host::cli::sandbox::one_shot - - + + nvx::host::cli::sandbox->nvx::host::cli::sandbox::options - - + + nvx::host::cli::sandbox->nvx::host::cli::sandbox::state - - + + @@ -294,8 +294,8 @@ nvx::host::network::policy->nvx::host::network::policy::json - - + + @@ -360,38 +360,38 @@ nvx::host::sandbox::mount->nvx::host::sandbox::mount::aggregate - - + + nvx::host::sandbox::mount->nvx::host::sandbox::mount::boot - - + + nvx::host::sandbox::mount->nvx::host::sandbox::mount::host_paths - - + + nvx::host::sandbox::mount->nvx::host::sandbox::mount::request - - + + nvx::host::sandbox::plan->nvx::host::sandbox::plan::form - - + + nvx::host::sandbox::scratch->nvx::host::sandbox::scratch::imp - - + + @@ -640,8 +640,8 @@ nvx::host::cli::sandbox::lifecycle - -lifecycle + +lifecycle @@ -649,8 +649,8 @@ nvx::host::cli::sandbox::lock - -lock + +lock @@ -658,8 +658,8 @@ nvx::host::cli::sandbox::one_shot - -one_shot + +one_shot @@ -667,8 +667,8 @@ nvx::host::cli::sandbox::options - -options + +options @@ -676,8 +676,8 @@ nvx::host::cli::sandbox::state - -state + +state @@ -784,9 +784,9 @@ nvx::host::network::policy::json - -json -pub(crate) + +json +pub(crate) @@ -924,8 +924,8 @@ nvx::host::sandbox::mount::aggregate - -aggregate + +aggregate @@ -933,8 +933,8 @@ nvx::host::sandbox::mount::boot - -boot + +boot @@ -942,8 +942,8 @@ nvx::host::sandbox::mount::host_paths - -host_paths + +host_paths @@ -951,8 +951,8 @@ nvx::host::sandbox::mount::request - -request + +request @@ -978,8 +978,8 @@ nvx::host::sandbox::plan::form - -form + +form @@ -1005,9 +1005,8 @@ nvx::host::sandbox::scratch::imp - -imp -Windows only + +imp diff --git a/scripts/module_diagrams.py b/scripts/module_diagrams.py index be103ffd4..b4abacbf7 100644 --- a/scripts/module_diagrams.py +++ b/scripts/module_diagrams.py @@ -3,8 +3,10 @@ cargo-modules exports the modules of the crate's library, with its cli, testing, and guest features, as each of the crate's platforms, Linux and Windows, builds -it: the module that contains each module, and the modules that each uses. This -script merges the exports of the platforms, draws with Graphviz the crate's +it: the module that contains each module, and the modules that each uses. It +mistakes modules under some cfg conditions, such as cfg(not(windows)), for test +modules, so this script also reads its analysis with cfg(test), and restores +them. It merges the exports of the platforms, draws with Graphviz the crate's top-level modules, the dependencies between them, and the modules that each contains, and writes the page that shows the diagrams. CI draws them after every merge to dev that changes their inputs, and commits them when they change @@ -112,6 +114,13 @@ r'// "(?Powns|uses)" edge$', re.M, ) +# cargo-modules' structure writes the crate, then one module a line, four columns +# deeper than the module that contains it, with its visibility and the cfg +# conditions that must all hold for it. +TREE = re.compile( + r"^(?P(?:│ | )*)[├└]── mod (?P[\w#]+): (?P.*)$" +) +CONDITION = re.compile(r"#\[cfg\((?P.+?)\)\](?=, #\[cfg\(|$)") class DiagramError(Exception): @@ -232,9 +241,10 @@ def warn(output: Output) -> str: return output.stdout -def export_command(target: str) -> list[str]: +def export_command(target: str, *, cfg_test: bool = False) -> list[str]: """The cargo-modules command that exports the modules of one target, without - the functions, types, and traits that they hold or other crates.""" + the functions, types, and traits that they hold or other crates, and with the + test modules if cfg_test.""" return [ "cargo", "modules", @@ -251,6 +261,28 @@ def export_command(target: str) -> list[str]: "--no-types", "--no-externs", "--no-sysroot", + *(["--cfg-test"] if cfg_test else []), + ] + + +def structure_command(target: str) -> list[str]: + """The cargo-modules command that writes the modules of one target, with the + test modules, and the cfg conditions of each.""" + return [ + "cargo", + "modules", + "structure", + "--lib", + "--manifest-path", + str(MANIFEST), + "--features", + ",".join(FEATURES), + "--target", + target, + "--no-fns", + "--no-traits", + "--no-types", + "--cfg-test", ] @@ -292,6 +324,94 @@ def fail(reason: str) -> DiagramError: return Export(visibility, owner, frozenset(uses)) +def conditions(text: str, target: str) -> dict[str, tuple[str, ...]]: + """Reads and checks cargo-modules' structure of one target: the cfg + conditions of each module, all of which must hold for it.""" + + def fail(reason: str) -> DiagramError: + return DiagramError(f"unexpected cargo-modules structure of {target}: {reason}") + + lines = [line for line in text.splitlines() if line.strip()] + if not lines or lines[0] != f"crate {CRATE}": + raise fail(f"it does not start with the crate {CRATE}") + result: dict[str, tuple[str, ...]] = {CRATE: ()} + stack = [CRATE] + for line in lines[1:]: + match = TREE.match(line) + if match is None: + raise fail(f"it holds {line!r}") + depth = len(match["indent"]) // 4 + 1 + if depth > len(stack): + raise fail(f"{match['name']} is deeper than a module that contains it") + stack = [*stack[:depth], match["name"]] + path = "::".join(stack) + _, separator, listed = match["rest"].partition(" #[cfg(") + listed = f"#[cfg({listed}" if separator else "" + found = tuple(CONDITION.findall(listed)) + if ", ".join(f"#[cfg({condition})]" for condition in found) != listed: + raise fail(f"the cfg conditions of {path} are unreadable") + if path in result: + raise fail(f"{path} appears twice") + result[path] = found + return result + + +def test_only(found: Mapping[str, Sequence[str]], path: str) -> bool: + """Whether a module builds only for tests: one of its cfg conditions, or of a + module that contains it, is test.""" + parts = path.split("::") + return any( + "test" in found["::".join(parts[:depth])] for depth in range(1, len(parts) + 1) + ) + + +def restore( + plain: Export, tested: Export, found: Mapping[str, Sequence[str]], target: str +) -> Export: + """The modules that one target builds, outside tests, and their uses. + + Unless it analyzes with cfg(test), cargo-modules drops as a test module every + module whose cfg holds when test holds and every other condition fails, such + as one under cfg(not(windows)): has_test_cfg, in its analyzer.rs, folds the + whole condition with test true and the others false. Its analysis with + cfg(test) holds those modules, and the test modules, which a cfg condition of + their own or of a module that contains them, test, marks. The modules that + build outside tests are those of the analysis with cfg(test) but the test + modules, so this restores the modules that the plain analysis lacks among + them, with their uses in the analysis with cfg(test). It refuses conditions + that depend on test otherwise, such as any(test, unix) or not(test), whose + modules it cannot tell apart.""" + + def fail(reason: str) -> DiagramError: + return DiagramError(f"cannot restore the modules of {target}: {reason}") + + if found.keys() != tested.visibility.keys(): + raise fail("its structure and its dependencies name different modules") + for path, listed in found.items(): + for condition in listed: + # A string, such as the name of a feature, holds no condition. + unquoted = re.sub(r'"[^"]*"', '""', condition) + if condition != "test" and re.search(r"\btest\b", unquoted): + raise fail(f"{path} builds under cfg({condition})") + built = {path for path in tested.visibility if not test_only(found, path)} + missing = sorted(plain.visibility.keys() - built) + if missing: + raise fail(f"{', '.join(missing)} build outside tests only") + restored = built - plain.visibility.keys() + return Export( + {path: tested.visibility[path] for path in sorted(built)}, + {path: tested.owner[path] for path in sorted(built - {CRATE})}, + plain.uses + | { + (user, used) + for user, used in tested.uses + if (user in restored or used in restored) + and user in built + and used in built + }, + ) + + def merge(exports: Mapping[str, Export]) -> Crate: """Merges the exports of the platforms, in their order. A module whose visibility differs between them names the visibility of each.""" @@ -677,8 +797,12 @@ def draw(output: Path, dot: str, execute: Runner = run) -> list[str]: exports: dict[str, Export] = {} for platform, target in TARGETS.items(): print(f"Exporting the modules of {platform} ({target})", file=sys.stderr) - text = warn(execute(export_command(target), None)) - exports[platform] = parse(text, target) + plain = parse(warn(execute(export_command(target), None)), target) + tested = parse( + warn(execute(export_command(target, cfg_test=True), None)), target + ) + found = conditions(warn(execute(structure_command(target), None)), target) + exports[platform] = restore(plain, tested, found, target) crate = merge(exports) drawn = diagrams(crate) files: dict[str, str] = {} diff --git a/scripts/test_module_diagrams.py b/scripts/test_module_diagrams.py index 080fee907..5cb6c127a 100644 --- a/scripts/test_module_diagrams.py +++ b/scripts/test_module_diagrams.py @@ -93,6 +93,30 @@ def export( return "\n".join(lines) +def structure(found: Mapping[str, Sequence[str]]) -> str: + """cargo-modules' structure of the modules of found, with their cfg + conditions: the crate, then each module below the module that contains it, + in the order of their names.""" + children: dict[str, list[str]] = {} + for path in sorted(found): + if path != diagrams.CRATE: + children.setdefault(path.rpartition("::")[0], []).append(path) + lines = ["", f"crate {diagrams.CRATE}"] + + def walk(path: str, prefix: str) -> None: + below = children.get(path, []) + for index, child in enumerate(below): + last = index == len(below) - 1 + listed = ", ".join(f"#[cfg({condition})]" for condition in found[child]) + name = child.rpartition("::")[2] + line = f"{prefix}{'└── ' if last else '├── '}mod {name}: pub" + lines.append(f"{line} {listed}" if listed else line) + walk(child, prefix + (" " if last else "│ ")) + + walk(diagrams.CRATE, "") + return "\n".join(lines) + "\n" + + # The modules of a small crate on each platform: host::sys and the guest are # Linux's alone, and host::windows is Windows'. LINUX_MODULES = { @@ -241,6 +265,143 @@ def test_exports_of_anything_but_modules_and_their_uses_fail(self): diagrams.parse(text, "linux") +STRUCTURE = """ +crate nvx +├── mod guest: pub #[cfg(feature = "guest")], #[cfg(target_os = "linux")] +│ └── mod tests: pub(self) #[cfg(test)], #[cfg(any(target_os = "linux", windows))] +├── mod host: pub +│ ├── mod imp: pub(self) #[cfg(not(windows))] +│ └── mod spawn: pub(in crate::host) +└── mod uuid: pub + └── mod tests: pub(self) #[cfg(test)] +""" + + +class ConditionsTests(unittest.TestCase): + def test_the_structure_gives_each_module_its_cfg_conditions(self): + expected = { + "nvx": (), + "nvx::guest": ('feature = "guest"', 'target_os = "linux"'), + "nvx::guest::tests": ("test", 'any(target_os = "linux", windows)'), + "nvx::host": (), + "nvx::host::imp": ("not(windows)",), + "nvx::host::spawn": (), + "nvx::uuid": (), + "nvx::uuid::tests": ("test",), + } + self.assertEqual(diagrams.conditions(STRUCTURE, "linux"), expected) + self.assertEqual(diagrams.conditions(structure(expected), "linux"), expected) + + def test_structures_that_cannot_be_read_fail(self): + cases = { + "nothing": "", + "no crate": "├── mod host: pub\n", + "another crate": "crate other\n", + "a function": "crate nvx\n└── fn main: pub\n", + "a skipped level": "crate nvx\n│ └── mod deep: pub\n", + "a module twice": "crate nvx\n├── mod a: pub\n└── mod a: pub\n", + "an unreadable condition": "crate nvx\n└── mod a: pub #[cfg(test)] x\n", + } + for case, text in cases.items(): + with self.subTest(case=case): + with self.assertRaisesRegex( + diagrams.DiagramError, "unexpected cargo-modules structure" + ): + diagrams.conditions(text, "linux") + + +# A plain analysis that lacks nvx::host::imp, which builds under cfg(not(windows)), +# and an analysis with cfg(test) that holds it and the test modules. +PLAIN = {"nvx": "crate", "nvx::error": "pub(crate)", "nvx::host": "pub"} +TESTED = { + **PLAIN, + "nvx::host::imp": "pub(self)", + "nvx::host::tests": "pub(self)", + "nvx::host::tests::cases": "pub(self)", +} +TESTED_USES = ( + ("nvx::host", "nvx::error"), + ("nvx::host", "nvx::host::imp"), + ("nvx::host::imp", "nvx::error"), + ("nvx::host::tests", "nvx::host::imp"), + ("nvx::host::tests::cases", "nvx::error"), +) + + +def found(**changes: tuple[str, ...]) -> dict[str, tuple[str, ...]]: + """The cfg conditions of TESTED's modules, with the changes of the modules + named by their last part.""" + result: dict[str, tuple[str, ...]] = {path: () for path in TESTED} + result["nvx::host::imp"] = ("not(windows)",) + result["nvx::host::tests"] = ("test",) + for name, listed in changes.items(): + [path] = [path for path in result if path.rpartition("::")[2] == name] + result[path] = listed + return result + + +class RestoreTests(unittest.TestCase): + def restore( + self, + conditions: Mapping[str, Sequence[str]], + plain: Mapping[str, str] = PLAIN, + ) -> diagrams.Export: + return diagrams.restore( + diagrams.parse(export(plain, (("nvx::host", "nvx::error"),)), "linux"), + diagrams.parse(export(TESTED, TESTED_USES), "linux"), + conditions, + "linux", + ) + + def test_modules_that_cargo_modules_drops_as_tests_return_with_their_uses(self): + restored = self.restore(found()) + self.assertEqual(restored.visibility, {**PLAIN, "nvx::host::imp": "pub(self)"}) + self.assertEqual( + restored.owner, + { + "nvx::error": "nvx", + "nvx::host": "nvx", + "nvx::host::imp": "nvx::host", + }, + ) + # The uses of the test modules stay out with them. + self.assertEqual( + restored.uses, + { + ("nvx::host", "nvx::error"), + ("nvx::host", "nvx::host::imp"), + ("nvx::host::imp", "nvx::error"), + }, + ) + + def test_a_test_in_a_string_is_no_condition(self): + restored = self.restore(found(imp=('feature = "test"',))) + self.assertIn("nvx::host::imp", restored.visibility) + + def test_conditions_that_depend_on_test_otherwise_fail(self): + for condition in ("any(test, unix)", "not(test)", "any(all(test, unix), x)"): + with self.subTest(condition=condition): + with self.assertRaisesRegex( + diagrams.DiagramError, + re.escape(f"nvx::host::imp builds under cfg({condition})"), + ): + self.restore(found(imp=(condition,))) + + def test_modules_that_only_the_plain_analysis_holds_fail(self): + # Such as one under cfg(not(test)). + plain = {**PLAIN, "nvx::real": "pub"} + with self.assertRaisesRegex( + diagrams.DiagramError, "nvx::real build outside tests only" + ): + self.restore(found(), plain) + + def test_the_structure_and_the_dependencies_name_the_same_modules(self): + conditions = found() + del conditions["nvx::host::tests::cases"] + with self.assertRaisesRegex(diagrams.DiagramError, "different modules"): + self.restore(conditions) + + class MergeTests(unittest.TestCase): def test_the_platforms_share_their_modules_and_uses(self): merged = crate() @@ -503,14 +664,30 @@ def _svg(title: str) -> str: class FakeTools: - """cargo-modules and Graphviz: each export command prints the export of its - target, and dot draws an SVG titled with its graph's name.""" - - def __init__(self, exports: Mapping[str, str] = EXPORTS) -> None: - self.exports = { - tuple(diagrams.export_command(diagrams.TARGETS[platform])): text - for platform, text in exports.items() - } + """cargo-modules and Graphviz. Each export command prints the export of its + target, plain or with cfg(test), which defaults to the plain one; each + structure command prints the modules of the export with cfg(test), with the + cfg conditions that found names; and dot draws an SVG titled with its graph's + name.""" + + def __init__( + self, + exports: Mapping[str, str] = EXPORTS, + tested: Mapping[str, str] | None = None, + found: Mapping[str, Mapping[str, Sequence[str]]] | None = None, + ) -> None: + self.outputs: dict[tuple[str, ...], str] = {} + for platform, target in diagrams.TARGETS.items(): + with_tests = (tested or exports)[platform] + listed = (found or {}).get(platform, {}) + modules = { + match["path"]: listed.get(match["path"], ()) + for match in diagrams.NODE.finditer(with_tests) + } + self.outputs[tuple(diagrams.export_command(target))] = exports[platform] + command = diagrams.export_command(target, cfg_test=True) + self.outputs[tuple(command)] = with_tests + self.outputs[tuple(diagrams.structure_command(target))] = structure(modules) self.commands: list[list[str]] = [] self.sources: list[str] = [] self.versions = True @@ -518,8 +695,8 @@ def __init__(self, exports: Mapping[str, str] = EXPORTS) -> None: def __call__(self, command: Sequence[str], stdin: str | None) -> diagrams.Output: self.commands.append(list(command)) - if tuple(command) in self.exports: - return diagrams.Output(self.exports[tuple(command)], "") + if tuple(command) in self.outputs: + return diagrams.Output(self.outputs[tuple(command)], "") if command[1:] == ["-Tsvg"] and stdin is not None: self.sources.append(stdin) name = re.match(r'digraph "([^"]+)"', stdin) @@ -557,13 +734,55 @@ def test_draw_exports_each_platform_and_writes_the_page_and_diagrams(self): self.assertEqual(root.tag, "{http://www.w3.org/2000/svg}svg") page = (output / diagrams.PAGE).read_text(encoding="utf-8") self.assertEqual( - tools.commands[: len(diagrams.TARGETS)], - [diagrams.export_command(target) for target in diagrams.TARGETS.values()], + tools.commands[: 3 * len(diagrams.TARGETS)], + [ + command + for target in diagrams.TARGETS.values() + for command in ( + diagrams.export_command(target), + diagrams.export_command(target, cfg_test=True), + diagrams.structure_command(target), + ) + ], ) self.assertEqual(tools.sources, [diagram.source for diagram in drawn]) flat = " ".join(page.split()) self.assertIn("cargo-modules) 9.9.9 finds them and Graphviz 1.2.3 draws", flat) + def test_draw_restores_the_modules_that_cargo_modules_drops_as_tests(self): + # As nvx::host::sandbox::scratch::imp: each platform builds a module of + # the same path under its own condition, but cargo-modules drops Linux's, + # under cfg(not(windows)), unless it analyzes with cfg(test). + linux = { + **LINUX_MODULES, + "nvx::host::imp": "pub(self)", + "nvx::host::tests": "pub(self)", + } + windows = {**WINDOWS_MODULES, "nvx::host::imp": "pub(self)"} + imports = (*LINUX_USES, ("nvx::host::imp", "nvx::error")) + tools = FakeTools( + {LINUX: EXPORTS[LINUX], WINDOWS: export(windows, WINDOWS_USES)}, + {LINUX: export(linux, imports), WINDOWS: export(windows, WINDOWS_USES)}, + { + LINUX: { + "nvx::host::imp": ("not(windows)",), + "nvx::host::tests": ("test",), + }, + WINDOWS: {"nvx::host::imp": ("windows",)}, + }, + ) + with tempfile.TemporaryDirectory() as directory: + self.draw(Path(directory), tools) + [host] = [ + source + for source in tools.sources + if source.startswith('digraph "nvx::host"') + ] + drawn = nodes(host) + # Both platforms build it, so it names neither. + self.assertEqual(label(drawn["nvx::host::imp"]), ["imp"]) + self.assertNotIn("nvx::host::tests", drawn) + def test_the_page_names_no_version_that_a_tool_does_not_report(self): tools = FakeTools() tools.versions = False @@ -590,21 +809,32 @@ def test_a_failed_drawing_leaves_the_output_unchanged(self): after = {path.name: path.read_bytes() for path in output.iterdir()} self.assertEqual(after, before) - def test_exports_cover_the_library_with_the_features_of_the_page(self): + def test_the_commands_cover_the_library_with_the_features_of_the_page(self): for target in diagrams.TARGETS.values(): - command = diagrams.export_command(target) - with self.subTest(target=target): - self.assertEqual( - command[:4], ["cargo", "modules", "dependencies", "--lib"] - ) - options = dict(zip(command[4:], command[5:], strict=False)) - self.assertEqual(options["--manifest-path"], str(diagrams.MANIFEST)) - self.assertEqual(options["--features"], ",".join(diagrams.FEATURES)) - self.assertEqual(options["--target"], target) - for excluded in ("fns", "traits", "types", "externs", "sysroot"): - self.assertIn(f"--no-{excluded}", command) - self.assertNotIn("--no-uses", command) - self.assertNotIn("--no-owns", command) + commands = { + "plain": diagrams.export_command(target), + "tested": diagrams.export_command(target, cfg_test=True), + "structure": diagrams.structure_command(target), + } + for kind, command in commands.items(): + with self.subTest(target=target, kind=kind): + subcommand = "structure" if kind == "structure" else "dependencies" + self.assertEqual( + command[:4], ["cargo", "modules", subcommand, "--lib"] + ) + options = dict(zip(command[4:], command[5:], strict=False)) + self.assertEqual(options["--manifest-path"], str(diagrams.MANIFEST)) + self.assertEqual(options["--features"], ",".join(diagrams.FEATURES)) + self.assertEqual(options["--target"], target) + for excluded in ("fns", "traits", "types"): + self.assertIn(f"--no-{excluded}", command) + self.assertEqual("--cfg-test" in command, kind != "plain") + for kind in ("plain", "tested"): + with self.subTest(target=target, kind=kind): + for excluded in ("externs", "sysroot"): + self.assertIn(f"--no-{excluded}", commands[kind]) + self.assertNotIn("--no-uses", commands[kind]) + self.assertNotIn("--no-owns", commands[kind]) def test_a_missing_cargo_modules_names_the_release_to_install(self): stderr = io.StringIO() From b50a8a538ba2619622f0e1a58f2e359ff784a50a Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Tue, 27 Oct 2026 11:30:36 -0700 Subject: [PATCH 5/6] ci: publish no drawing of inputs that dev has since changed Rerunning a run's failed jobs reruns its publish job alone, which reuses the run's drawing and the commit that it drew. When a newer run had published the diagrams of inputs that a newer commit changed, such a rerun put the older diagrams back on dev, in a commit whose message skips CI, so that nothing drew the newer ones again until the inputs changed once more. On every attempt, once it fetches dev, the publish job now compares the inputs at the drawn commit and on the newest dev, as the paths that start the workflow name them, which a test keeps equal to the job's. When dev changed them, it publishes nothing and says why, since the push that changed them started a run that publishes their diagrams; when dev changed other files only, it publishes the drawing as before. A drawn commit that the repository lacks fails the comparison, and the job. Tests run the publish step against a local repository: a rerun after a newer commit changed an input leaves dev untouched, the drawing still lands after a newer commit that changes no input, and an unknown drawn commit fails. doc/ci.md describes the comparison. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/module-diagrams.yml | 19 ++++ doc/ci.md | 11 +- scripts/test_module_diagrams.py | 143 +++++++++++++++++++------- 3 files changed, 131 insertions(+), 42 deletions(-) diff --git a/.github/workflows/module-diagrams.yml b/.github/workflows/module-diagrams.yml index ea0690b7e..797ba7bac 100644 --- a/.github/workflows/module-diagrams.yml +++ b/.github/workflows/module-diagrams.yml @@ -184,6 +184,11 @@ jobs: env: DIAGRAMS: ${{ runner.temp }}/module-diagrams DRAWN: ${{ needs.draw.outputs.sha }} + # The inputs of the diagrams: the paths that start this workflow. + INPUTS: >- + nvx-rs rust-toolchain.toml scripts/module_diagrams.py + scripts/nvx_tools/build_constants.py + .github/workflows/module-diagrams.yml run: | set -euo pipefail if [[ ! "${DRAWN}" =~ ^[0-9a-f]{40}$ ]]; then @@ -197,12 +202,26 @@ jobs: echo "${unexpected}" exit 1 fi + read -r -a inputs <<< "${INPUTS}" git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" for attempt in 1 2 3; do git fetch origin dev git checkout -B module-diagrams origin/dev + # A rerun of this job alone reuses the drawing of its run, which a + # newer run may have replaced. Publish it only while dev holds the + # inputs that it was drawn from: the push that changed them started + # a run that publishes their diagrams. + status=0 + git diff --quiet "${DRAWN}" HEAD -- "${inputs[@]}" || status=$? + if [[ "${status}" -eq 1 ]]; then + echo "::notice::dev changed the inputs of the diagrams since ${DRAWN}; the run of the newer commit publishes their diagrams" + exit 0 + elif [[ "${status}" -ne 0 ]]; then + echo "::error::Cannot compare the inputs of the diagrams at ${DRAWN} with dev" + exit 1 + fi rm -rf doc/modules mkdir -p doc/modules cp "${DIAGRAMS}"/* doc/modules/ diff --git a/doc/ci.md b/doc/ci.md index 6b64f1c0a..5e0c9ecae 100644 --- a/doc/ci.md +++ b/doc/ci.md @@ -1851,9 +1851,14 @@ from starting the workflows of a push. Runs on `dev` wait for one another, and a newer push replaces the run that waits. Each run draws the newest `dev`, so whichever run commits last commits -the diagrams of the newest `dev`, in whatever order the runs start. A ready -pull request that changes the inputs runs the `draw` job alone, so that a -change that breaks the drawing fails before it merges, and its summary and +the diagrams of the newest `dev`, in whatever order the runs start. A rerun of +the `publish` job alone, as of a run's failed jobs, reuses the drawing of its +run, which a newer run may have replaced. So, on each attempt, the job compares +the inputs, the paths that start the workflow, at the drawn commit and on the +newest `dev`, and publishes nothing once `dev` has changed them: the push that +changed them started a run that publishes their diagrams. +A ready pull request that changes the inputs runs the `draw` job alone, so that +a change that breaks the drawing fails before it merges, and its summary and artifact show the diagrams that its merge will commit. The workflow is not part of `Required status check`. To draw the diagrams locally, install cargo-modules and Graphviz, and run `python3 scripts/module_diagrams.py`, which diff --git a/scripts/test_module_diagrams.py b/scripts/test_module_diagrams.py index 5cb6c127a..d468bf276 100644 --- a/scripts/test_module_diagrams.py +++ b/scripts/test_module_diagrams.py @@ -1065,11 +1065,25 @@ def test_runs_of_dev_wait_and_pull_requests_cancel(self): "github.event.pull_request.number || github.ref", concurrency["group"] ) + def test_the_publication_compares_the_paths_that_start_the_workflow(self): + # A change of any other path starts no run that would draw it again. + workflow = _workflow() + triggers = cast(dict[str, Any], workflow.get("on", workflow.get(True))) + commit = _step("publish", "Commit the diagrams to dev") + inputs = cast(str, commit["env"]["INPUTS"]).split() + for event in ("push", "pull_request"): + with self.subTest(event=event): + paths = cast(list[str], triggers[event]["paths"]) + self.assertEqual(inputs, [path.removesuffix("/**") for path in paths]) + self.assertIn( + 'git diff --quiet "${DRAWN}" HEAD -- "${inputs[@]}"', commit["run"] + ) + BASH = shutil.which("bash") GIT = shutil.which("git") -# The commit that the draw job drew, as its output names it. -DRAWN = "0123456789abcdef0123456789abcdef01234567" +# A commit that the local repository does not hold. +UNKNOWN = "0123456789abcdef0123456789abcdef01234567" @unittest.skipIf( @@ -1098,28 +1112,17 @@ def setUp(self) -> None: } self.origin = self.root / "origin.git" self.git("init", "--bare", "--initial-branch=dev", str(self.origin)) - seed = self.root / "seed" - self.git("clone", str(self.origin), str(seed)) - for name, text in { - "doc/modules/README.md": "old page\n", - "doc/modules/overview.svg": "\n", - "doc/ci.md": "# CI\n", - }.items(): - (seed / name).parent.mkdir(parents=True, exist_ok=True) - (seed / name).write_text(text, encoding="utf-8") - self.git("-C", str(seed), "add", "--all") - self.git( - "-C", - str(seed), - "-c", - "user.name=Seed", - "-c", - "user.email=seed@example.com", - "commit", - "-m", - "seed", + self.seed = self.root / "seed" + self.git("clone", str(self.origin), str(self.seed)) + # The drawn commit: the diagrams of an input, nvx-rs/src/lib.rs. + self.drawn = self.advance( + { + "doc/modules/README.md": "old page\n", + "doc/modules/overview.svg": "\n", + "doc/ci.md": "# CI\n", + "nvx-rs/src/lib.rs": "// The crate.\n", + } ) - self.git("-C", str(seed), "push", "origin", "HEAD:dev") self.checkout = self.root / "checkout" self.git("clone", "--branch", "dev", str(self.origin), str(self.checkout)) # The diagrams replace the page and remove overview.svg. @@ -1137,21 +1140,49 @@ def git(self, *arguments: str) -> str: check=True, ).stdout - def publish(self, drawn: str = DRAWN) -> subprocess.CompletedProcess[str]: - script = _step("publish", "Commit the diagrams to dev")["run"] + def advance(self, files: Mapping[str, str]) -> str: + """Pushes a commit of the files to dev, after dev's newest commit, and + returns it.""" + if self.git("-C", str(self.origin), "branch", "--list", "dev").strip(): + self.git("-C", str(self.seed), "pull", "--ff-only", "origin", "dev") + for name, text in files.items(): + (self.seed / name).parent.mkdir(parents=True, exist_ok=True) + (self.seed / name).write_text(text, encoding="utf-8") + self.git("-C", str(self.seed), "add", "--all") + self.git( + "-C", + str(self.seed), + "-c", + "user.name=Seed", + "-c", + "user.email=seed@example.com", + "commit", + "-m", + "advance", + ) + self.git("-C", str(self.seed), "push", "origin", "HEAD:dev") + return self.git("-C", str(self.seed), "rev-parse", "HEAD").strip() + + def publish(self, drawn: str | None = None) -> subprocess.CompletedProcess[str]: + step = _step("publish", "Commit the diagrams to dev") return subprocess.run( - [cast(str, BASH), "--noprofile", "--norc", "-eo", "pipefail", "-c", script], + [cast(str, BASH), "--noprofile", "--norc", "-eo", "pipefail", "-c"] + + [step["run"]], cwd=self.checkout, env={ **self.environment, "DIAGRAMS": str(self.diagrams), - "DRAWN": drawn, + "DRAWN": self.drawn if drawn is None else drawn, + "INPUTS": step["env"]["INPUTS"], }, capture_output=True, text=True, check=False, ) + def head(self) -> str: + return self.git("-C", str(self.origin), "rev-parse", "dev").strip() + def dev(self) -> tuple[str, list[str], list[str]]: """The subject and author of dev's newest commit, and its files.""" subject, author = self.git( @@ -1166,24 +1197,54 @@ def test_changed_diagrams_are_committed_to_dev_without_ci(self): result = self.publish() self.assertEqual(result.returncode, 0, result.stdout + result.stderr) subject, author, files = self.dev() - self.assertEqual(subject, f"doc: update module diagrams ({DRAWN}) [skip ci]") + self.assertEqual( + subject, f"doc: update module diagrams ({self.drawn}) [skip ci]" + ) self.assertEqual(author, ["github-actions[bot]"]) self.assertEqual( files, - ["doc/ci.md", "doc/modules/README.md", "doc/modules/dependencies.svg"], + [ + "doc/ci.md", + "doc/modules/README.md", + "doc/modules/dependencies.svg", + "nvx-rs/src/lib.rs", + ], ) changed = self.git( "-C", str(self.origin), "diff", "--name-only", "dev~1", "dev" ).split() self.assertTrue(all(path.startswith("doc/modules/") for path in changed)) # The same diagrams again change nothing. - count = self.git("-C", str(self.origin), "rev-list", "--count", "dev") + head = self.head() result = self.publish() self.assertEqual(result.returncode, 0, result.stdout + result.stderr) self.assertIn("up to date", result.stdout) + self.assertEqual(self.head(), head) + + def test_a_rerun_after_dev_changed_the_inputs_publishes_nothing(self): + # A newer run drew and published the diagrams of newer inputs. A rerun of + # the older publication alone reuses the older drawing, and must not + # replace the newer diagrams with it. + newer = self.advance( + {"nvx-rs/src/lib.rs": "// A newer crate.\n", "doc/modules/README.md": "x\n"} + ) + result = self.publish() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("::notice::", result.stdout) + self.assertEqual(self.head(), newer) + + def test_newer_commits_that_leave_the_inputs_alone_hold_nothing_back(self): + newer = self.advance({"doc/ci.md": "# CI, revised\n"}) + result = self.publish() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + subject, _, files = self.dev() self.assertEqual( - self.git("-C", str(self.origin), "rev-list", "--count", "dev"), count + subject, f"doc: update module diagrams ({self.drawn}) [skip ci]" ) + self.assertEqual( + self.git("-C", str(self.origin), "rev-parse", "dev~1").strip(), newer + ) + self.assertIn("doc/modules/dependencies.svg", files) def test_a_rejected_push_is_retried_on_the_newest_dev(self): hook = self.origin / "hooks" / "pre-receive" @@ -1206,24 +1267,28 @@ def test_diagrams_with_other_files_or_no_page_are_refused(self): "a directory": lambda: (self.diagrams / "sub").mkdir(), "no page": lambda: (self.diagrams / "README.md").unlink(), } - head = self.git("-C", str(self.origin), "rev-parse", "dev") - for drawn in ("", "0123abcd", DRAWN.upper(), f"{DRAWN}\n{DRAWN}"): + head = self.head() + # A drawn commit that the repository lacks fails the comparison of the + # inputs. + for drawn in ( + "", + "0123abcd", + UNKNOWN.upper(), + f"{UNKNOWN}\n{UNKNOWN}", + UNKNOWN, + ): with self.subTest(drawn=drawn): result = self.publish(drawn) self.assertEqual(result.returncode, 1) self.assertIn("::error::", result.stdout) - self.assertEqual( - self.git("-C", str(self.origin), "rev-parse", "dev"), head - ) + self.assertEqual(self.head(), head) for case, change in cases.items(): with self.subTest(case=case): change() result = self.publish() self.assertEqual(result.returncode, 1) self.assertIn("::error::", result.stdout) - self.assertEqual( - self.git("-C", str(self.origin), "rev-parse", "dev"), head - ) + self.assertEqual(self.head(), head) shutil.rmtree(self.diagrams) self.diagrams.mkdir() (self.diagrams / "README.md").write_text("new page\n", "utf-8") From 4667ebc92abecd43c0d15d1fad896ad0e73bde0d Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Tue, 27 Oct 2026 11:53:23 -0700 Subject: [PATCH 6/6] scripts: count each use of a restored module once cargo-modules drops every module whose cfg holds when test holds and every other condition fails, such as one under cfg(not(windows)), unless it analyzes with cfg(test), so module_diagrams.py restores those modules from its analysis with cfg(test). But cargo-modules' filter (filter.rs) does not drop the uses of a module that it drops: it moves them, and the uses of the modules that use it, to the nearest module that contains it and that the analysis keeps, and drops those that this turns into a module's use of itself. restore() added the restored modules' own uses to all the plain uses, so a use that only a restored module made counted twice: once as that module's, and once as the use that cargo-modules had moved to the module that contains it. The dependencies diagram, which counts the pairs of modules in which one uses the other across top-level modules, would show 2 where 1 is right. restore() now projects each use of the analysis with cfg(test) between modules that build outside tests: it maps each end to itself, or to the nearest module that contains it and that the plain analysis keeps. It keeps the use if its projection is a plain use, or a module's use of itself, which filter.rs drops, and marks that plain use as moved. The uses are those, with the plain uses that no projection lands on, such as those of an item under cfg(not(test)). A moved plain use stays only if the analysis with cfg(test) holds it too, as when the module that contains a restored module makes the same use itself. The uses that only tests make stay out, as before. Where a use that involves a restored module lands on a plain use, the two analyses cannot tell whether only tests make the first, or whether only an item under cfg(not(test)) makes the second; the docstring says so. The published diagrams do not change. The only restored module, nvx::host::sandbox::scratch::imp on Linux, uses the module that contains it, a use that filter.rs drops rather than moves, and nvx::error, nvx::host::seal and nvx::host::sandbox::mount::request, which that module uses itself. So the analysis with cfg(test) holds each moved use, and both versions of restore() give the same uses on Linux and on Windows, as a redraw of doc/modules confirms. A regression test has the plain analysis hold host's use of protocol, which only the restored module imp makes, and checks that the use counts once, as imp's, while the use of error, which host and imp both make, counts twice. It fails with the previous union. Two more tests pin that the plain uses that no projection explains stay, and that the uses that only tests make between modules that the plain analysis keeps stay out. doc/ci.md now says that the script restores those modules with their own uses in place of the ones that the plain analysis moves. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/ci.md | 12 +++++---- scripts/module_diagrams.py | 43 ++++++++++++++++++++++--------- scripts/test_module_diagrams.py | 45 ++++++++++++++++++++++++++++++--- 3 files changed, 80 insertions(+), 20 deletions(-) diff --git a/doc/ci.md b/doc/ci.md index 5e0c9ecae..051bfba97 100644 --- a/doc/ci.md +++ b/doc/ci.md @@ -1817,11 +1817,13 @@ platforms' modules, lays out the diagrams with Graphviz, and writes them with the page that shows them. Unless it analyzes with `cfg(test)`, cargo-modules drops as a test module every module whose cfg holds when `test` holds and every other condition fails, such as one under `cfg(not(windows))`, so the script -also reads its analysis with `cfg(test)` and restores those modules, and stops -at a cfg that depends on `test` in another way. Nobody edits the diagrams by -hand: [`module-diagrams.yml`](../.github/workflows/module-diagrams.yml) draws -them again after every push to `dev` that changes their inputs, `nvx-rs/`, the -Rust pin, the script, the build constants that name the crate's targets, or the +also reads its analysis with `cfg(test)` and restores those modules, with their +own uses in place of those that the plain analysis moves to the modules that +contain them, and stops at a cfg that depends on `test` in another way. Nobody +edits the diagrams by hand: +[`module-diagrams.yml`](../.github/workflows/module-diagrams.yml) draws them +again after every push to `dev` that changes their inputs, `nvx-rs/`, the Rust +pin, the script, the build constants that name the crate's targets, or the workflow, as the merge of such a pull request does, and commits them to `dev` when they change. diff --git a/scripts/module_diagrams.py b/scripts/module_diagrams.py index b4abacbf7..fa9e7e88d 100644 --- a/scripts/module_diagrams.py +++ b/scripts/module_diagrams.py @@ -378,9 +378,19 @@ def restore( their own or of a module that contains them, test, marks. The modules that build outside tests are those of the analysis with cfg(test) but the test modules, so this restores the modules that the plain analysis lacks among - them, with their uses in the analysis with cfg(test). It refuses conditions - that depend on test otherwise, such as any(test, unix) or not(test), whose - modules it cannot tell apart.""" + them. It refuses conditions that depend on test otherwise, such as + any(test, unix) or not(test), whose modules it cannot tell apart. + + The plain analysis moves the uses of a module that it drops, and those of the + modules that use it, to the nearest module that contains it and that it + keeps, and drops those that this turns into a module's use of itself + (filter.rs). So the uses are those of the analysis with cfg(test) whose move + lands on a plain use, or on a module itself, and the plain uses that no such + use lands on, such as those of an item under cfg(not(test)). This leaves out + the uses that only tests make, and the moved uses. Where a use that involves + a restored module lands on a plain use, neither analysis tells whether only + tests make the first, which stays, or whether only an item under + cfg(not(test)) makes the second, which goes.""" def fail(reason: str) -> DiagramError: return DiagramError(f"cannot restore the modules of {target}: {reason}") @@ -397,18 +407,27 @@ def fail(reason: str) -> DiagramError: missing = sorted(plain.visibility.keys() - built) if missing: raise fail(f"{', '.join(missing)} build outside tests only") - restored = built - plain.visibility.keys() + + def kept(path: str) -> str: + """The module, or the nearest module that contains it, that the plain + analysis keeps.""" + while path not in plain.visibility: + path = tested.owner[path] + return path + + uses: set[tuple[str, str]] = set() + moved: set[tuple[str, str]] = set() + for user, used in tested.uses: + if user not in built or used not in built: + continue + move = (kept(user), kept(used)) + if move in plain.uses or (move[0] == move[1] and move != (user, used)): + uses.add((user, used)) + moved.add(move) return Export( {path: tested.visibility[path] for path in sorted(built)}, {path: tested.owner[path] for path in sorted(built - {CRATE})}, - plain.uses - | { - (user, used) - for user, used in tested.uses - if (user in restored or used in restored) - and user in built - and used in built - }, + frozenset(uses | (plain.uses - moved)), ) diff --git a/scripts/test_module_diagrams.py b/scripts/test_module_diagrams.py index d468bf276..6eb0e3628 100644 --- a/scripts/test_module_diagrams.py +++ b/scripts/test_module_diagrams.py @@ -345,10 +345,13 @@ def restore( self, conditions: Mapping[str, Sequence[str]], plain: Mapping[str, str] = PLAIN, + plain_uses: Iterable[tuple[str, str]] = (("nvx::host", "nvx::error"),), + tested: Mapping[str, str] = TESTED, + tested_uses: Iterable[tuple[str, str]] = TESTED_USES, ) -> diagrams.Export: return diagrams.restore( - diagrams.parse(export(plain, (("nvx::host", "nvx::error"),)), "linux"), - diagrams.parse(export(TESTED, TESTED_USES), "linux"), + diagrams.parse(export(plain, plain_uses), "linux"), + diagrams.parse(export(tested, tested_uses), "linux"), conditions, "linux", ) @@ -378,6 +381,42 @@ def test_a_test_in_a_string_is_no_condition(self): restored = self.restore(found(imp=('feature = "test"',))) self.assertIn("nvx::host::imp", restored.visibility) + def test_a_use_that_only_a_restored_module_makes_counts_once(self): + # cargo-modules moves the uses of a module that it drops to the module + # that contains it, so the plain analysis has host use protocol, which + # only imp does. + protocol = {"nvx::protocol": "pub"} + restored = self.restore( + {**found(), "nvx::protocol": ()}, + plain={**PLAIN, **protocol}, + plain_uses=( + ("nvx::host", "nvx::error"), + ("nvx::host", "nvx::protocol"), + ), + tested={**TESTED, **protocol}, + tested_uses=(*TESTED_USES, ("nvx::host::imp", "nvx::protocol")), + ) + self.assertIn(("nvx::host::imp", "nvx::protocol"), restored.uses) + self.assertNotIn(("nvx::host", "nvx::protocol"), restored.uses) + dependencies = edges(diagrams.dependencies(diagrams.merge({"Linux": restored}))) + self.assertEqual(dependencies[("nvx::host", "nvx::protocol")]["label"], "1") + # host uses error itself, as imp does: both uses count. + self.assertEqual(dependencies[("nvx::host", "nvx::error")]["label"], "2") + + def test_uses_that_only_the_plain_analysis_holds_stay(self): + # Such as those of an item under cfg(not(test)). + restored = self.restore( + found(), + plain_uses=(("nvx::host", "nvx::error"), ("nvx::error", "nvx::host")), + ) + self.assertIn(("nvx::error", "nvx::host"), restored.uses) + + def test_uses_that_only_tests_make_stay_out(self): + restored = self.restore( + found(), tested_uses=(*TESTED_USES, ("nvx::error", "nvx::host")) + ) + self.assertNotIn(("nvx::error", "nvx::host"), restored.uses) + def test_conditions_that_depend_on_test_otherwise_fail(self): for condition in ("any(test, unix)", "not(test)", "any(all(test, unix), x)"): with self.subTest(condition=condition): @@ -393,7 +432,7 @@ def test_modules_that_only_the_plain_analysis_holds_fail(self): with self.assertRaisesRegex( diagrams.DiagramError, "nvx::real build outside tests only" ): - self.restore(found(), plain) + self.restore(found(), plain=plain) def test_the_structure_and_the_dependencies_name_the_same_modules(self): conditions = found()