Isolate build history per output directory, redux

mkosi/config.py

@@ -5445,7 +5445,12 @@ def want_default_initrd(config: Config) -> bool:
     return Path("default") in config.initrds
 
 
-def finalize_historydir(args: Args) -> Path:
+def finalize_historydir(args: Args, output_dir: Optional[Path] = None) -> Path:
+    # When an output directory is given, the build history is also stored there so that builds into
+    # distinct output directories don't read each other's history. Otherwise it lives in the config dir.
+    if output_dir is not None:
+        return output_dir / ".mkosi-private/history"
+
     configdir = finalize_configdir(args.directory)
     return (configdir or Path.cwd()) / ".mkosi-private/history"
 
@@ -5502,7 +5507,13 @@ def parse_config(
         return args, None, ()
 
     configdir = finalize_configdir(args.directory)
-    historydir = finalize_historydir(args)
+    config_historydir = finalize_historydir(args)
+    # Prefer the history stored in the output directory (when --output-directory is given on the CLI) so a
+    # consumer reads back exactly the build that wrote into that directory. Fall back to the config dir for
+    # consumers that don't pass --output-directory and for history written by older mkosi versions.
+    historydir = finalize_historydir(args, context.cli.get("output_dir"))
+    if not (historydir / "latest.json").exists():
+        historydir = config_historydir
 
     if have_history(args, historydir):
         history = Config.from_partial_json((historydir / "latest.json").read_text())
@@ -5555,8 +5566,15 @@ def parse_config(
     maincontext = copy.deepcopy(context)
 
     if config["history"] and want_new_history(args):
-        historydir.mkdir(parents=True, exist_ok=True)
-        (historydir / "latest.json").write_text(dump_json(Config.to_partial_dict(cli)))
+        # Store the history in the config dir (so consumers that don't pass --output-directory find it) and,
+        # when an output directory is configured, in the output directory too (so builds into distinct
+        # output directories stay isolated). These coincide when no output directory is set. This keys on
+        # the finalized output dir (config or CLI), unlike the read above which can only use the CLI value
+        # (the configuration isn't parsed yet there), so don't collapse the two into one variable.
+        latest_json = dump_json(Config.to_partial_dict(cli))
+        for hd in [config_historydir, finalize_historydir(args, config.get("output_dir"))]:
+            hd.mkdir(parents=True, exist_ok=True)
+            (hd / "latest.json").write_text(latest_json)
 
     tools = None
     if config.get("tools_tree") in (Path("default"), Path("yes")):

tests/__init__.py

@@ -126,12 +126,13 @@ def boot(self, options: Sequence[str] = (), args: Sequence[str] = ()) -> Complet
                 "--register=no",
                 "--machine",
                 self.machine,
+                "--output-directory", self.output_dir,
                 *options,
             ],
             args,
             stdin=sys.stdin if sys.stdin.isatty() else None,
             check=False,
-        )
+        )  # fmt: skip
 
         if result.returncode != 123:
             raise subprocess.CalledProcessError(result.returncode, result.args, result.stdout, result.stderr)
@@ -158,12 +159,13 @@ def vm(
                 "--register=no",
                 "--machine",
                 self.machine,
+                "--output-directory", self.output_dir,
                 *options,
             ],
             args,
             stdin=sys.stdin if sys.stdin.isatty() else None,
             check=False,
-        )
+        )  # fmt: skip
 
         if result.returncode != 123:
             raise subprocess.CalledProcessError(result.returncode, result.args, result.stdout, result.stderr)

tests/test_config.py

@@ -1992,3 +1992,68 @@ def test_history_empty_list(tmp_path: Path) -> None:
         _, _, [main] = parse_config(["summary"])
 
     assert main.package_directories == []
+
+
+def test_history_found_via_configured_output_directory(tmp_path: Path) -> None:
+    d = tmp_path
+
+    out = d / "out"
+
+    # Mirror what systemd does: it both declares OutputDirectory= in config *and* passes the same path via
+    # -O on the build command line (from its meson build), while consumers rely on the configured value
+    # alone.
+    (d / "mkosi.conf").write_text(
+        f"""\
+        [Distribution]
+        Distribution=fedora
+
+        [Output]
+        OutputDirectory={out}
+
+        [Build]
+        History=yes
+        """
+    )
+
+    with chdir(d):
+        # The build passes -O explicitly, pointing at the same directory the config already configures.
+        parse_config(["--output-directory", os.fspath(out), "--image-id", "img-x", "build"])
+
+        # A consumer relies on the configured OutputDirectory and passes no -O. It must still read back
+        # the build's history.
+        _, _, [config] = parse_config(["summary"])
+
+    assert config.image_id == "img-x"
+
+
+def test_history_isolated_per_output_directory(tmp_path: Path) -> None:
+    d = tmp_path
+
+    (d / "mkosi.conf").write_text(
+        """\
+        [Distribution]
+        Distribution=fedora
+
+        [Build]
+        History=yes
+        """
+    )
+
+    out_a = d / "out-a"
+    out_b = d / "out-b"
+
+    # Two builds into different output directories, each recording its own build history, distinguished
+    # by ImageId so we can tell which build a later consumer reads back.
+    with chdir(d):
+        parse_config(["--output-directory", os.fspath(out_a), "--image-id", "img-a", "build"])
+        parse_config(["--output-directory", os.fspath(out_b), "--image-id", "img-b", "build"])
+
+        # A verb that consumes a previous build, pointed at output directory A, must read back the
+        # configuration of the build that wrote into A -- not whichever build happened to run last.
+        _, _, [config_last] = parse_config(["summary"])
+        _, _, [config_a] = parse_config(["--output-directory", os.fspath(out_a), "summary"])
+        _, _, [config_b] = parse_config(["--output-directory", os.fspath(out_b), "summary"])
+
+    assert config_last.image_id == "img-b"
+    assert config_a.image_id == "img-a"
+    assert config_b.image_id == "img-b"