feat: add --on-cfg-extend-error to skip a broken cfg extension

doc/design_doc.md

@@ -149,6 +149,31 @@ Within any of these files, `{file_dir}` names the directory that holds that file
 `{self_dir}`, by contrast, is the directory of the configuration file that dvsim loaded, at the top of the import tree.
 `{file_dir}` is replaced as each file is loaded, so it can only be used in Hjson files, not in options given on the command line.
 
+A configuration can also be extended from outside the project tree with `--cfg-extend NAME=PATH`, for example to add tests that need a licensed verification IP that the project itself can't hold:
+
+```hjson
+// ext.hjson, merged with: dvsim top_sim_cfgs.hjson --cfg-extend foo=/path/to/ext.hjson ...
+{
+  import_cfgs: ["{file_dir}/vip_tests.hjson"]
+  build_modes: [{name: "vip", build_opts: ["-f {file_dir}/vip.f"]}]
+  overrides: [{name: "foo_setting", value: "with_vip"}]
+}
+```
+
+The Hjson file at `PATH`, with its imports, is merged into the configuration whose `name` is `NAME` once that configuration has loaded all of its own files; this works whether that configuration is run on its own or is a child of a primary configuration.
+`NAME` is matched against the name that the configuration's own files give it, before any `overrides`.
+The usual merge rules apply: the extension's list entries follow the configuration's own, and a conflicting scalar is an error, so an extension changes a scalar with `overrides` (but not one that the configuration already overrides).
+In an extension, `{self_dir}` is still the directory of the configuration file that dvsim loaded, so the extension names its own files with `{file_dir}`.
+Several extensions apply in command-line order.
+An extension whose `NAME` matches no loaded configuration is an error, as is naming one extension twice.
+With `--on-cfg-extend-error skip`, an extension that can't be loaded or merged, or whose `overrides` don't fit the configuration, is left out with a warning instead: the configuration runs as if the extension had not been named, later extensions still apply, and its reports say which extension was skipped and why.
+An extension's `overrides` can only name keys that the configuration or an earlier extension define.
+In that mode, an extension whose `NAME` matches nothing only gets a warning, as no report belongs to it.
+Errors found after the merge are not covered: for example an unknown wildcard in one of the extension's values, a test that names a build mode that doesn't exist, or a failing build.
+The sim reports and JSON results list the extensions that each configuration matched, with the git revision of the checkout that holds each extension file: its `origin` (without credentials), its commit, and whether it was dirty, which includes the extension file itself not being tracked.
+Files that an extension imports from elsewhere are not recorded.
+The JSON results of a configuration that merged no extension have no `extensions` key, as before extensions were recorded.
+
 ### Parser stage
 
 The **parser stage** parses all Hjson files in the tree and constructs a **flow manager object**.

src/dvsim/cli/run.py

@@ -37,6 +37,7 @@
     load_config_file,
     proj_root_from_config,
 )
+from dvsim.flow.extension import CfgExtension
 from dvsim.flow.factory import make_cfg
 from dvsim.fusesoc import CONFIG_SECTION as FUSESOC_CONFIG_SECTION
 from dvsim.fusesoc import resolve_options as resolve_fusesoc_options
@@ -185,6 +186,14 @@ def parse_resource(s: str) -> tuple[str, int | None]:
         raise argparse.ArgumentTypeError(msg) from e
 
 
+def parse_cfg_extension(s: str) -> CfgExtension:
+    """Parse a cfg extension (NAME=PATH) from the argparse CLI."""
+    try:
+        return CfgExtension.parse(s)
+    except ValueError as e:
+        raise argparse.ArgumentTypeError(str(e)) from e
+
+
 def resolve_branch(branch, cwd=None):
     """Choose a branch name for output files.
 
@@ -453,6 +462,34 @@ def parse_args(argv: list[str] | None = None):
         ),
     )
 
+    whatg.add_argument(
+        "--cfg-extend",
+        action="append",
+        default=[],
+        type=parse_cfg_extension,
+        metavar="NAME=PATH",
+        help=(
+            "Merge the hjson file PATH (and its imports) into the cfg whose "
+            "'name' is NAME, after the cfg's own files. NAME may be a child of "
+            "a primary config. Lists are appended to; scalars are changed "
+            "with 'overrides'. PATH names its own files with {file_dir}. "
+            "Repeatable; applied in order. It is an error if NAME matches no cfg."
+        ),
+    )
+
+    whatg.add_argument(
+        "--on-cfg-extend-error",
+        type=str.lower,
+        choices=["error", "skip"],
+        default="error",
+        help=(
+            "What to do when a --cfg-extend file can't be loaded, merged or "
+            "overridden, or names no cfg: stop the run (error), or leave the "
+            "extension out, run the cfg without it and say so in its report "
+            "(skip). Defaults to %(default)s."
+        ),
+    )
+
     disg = parser.add_argument_group("Dispatch options")
 
     disg.add_argument(

src/dvsim/flow/base.py

@@ -17,11 +17,18 @@
 import hjson
 
 import dvsim.instrumentation.runtime as instrumentation
+from dvsim.flow.extension import (
+    CfgExtension,
+    check_cfg_extension_overrides,
+    load_cfg_extension,
+    override_names,
+)
 from dvsim.flow.hjson import set_target_attribute
 from dvsim.fusesoc import rewrite_fusesoc_opts
 from dvsim.job.data import CompletedJobStatus, JobSpec, WorkspaceConfig
 from dvsim.job.status import JobStatus
 from dvsim.logging import log
+from dvsim.report.data import CfgExtensionMeta
 from dvsim.scheduler.resources import UnknownResourcePolicy
 from dvsim.scheduler.runner import (
     build_default_scheduler_backend,
@@ -33,7 +40,7 @@
     rm_path,
     subst_wildcards,
 )
-from dvsim.utils.git import git_commit_hash, git_is_dirty
+from dvsim.utils.git import git_commit_hash, git_is_dirty, git_revision
 
 if TYPE_CHECKING:
     from dvsim.job.deploy import Deploy
@@ -147,7 +154,16 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config) -> None:
         # configuration".
         self.is_primary_cfg = "use_cfgs" in hjson_data
 
+        # The cfg extensions (--cfg-extend) that this cfg matched, in order, as
+        # the reports describe them (merged, or skipped with the reason), and
+        # the name that they were matched against.
+        self.cfg_extensions: dict[CfgExtension, CfgExtensionMeta] = {}
+        self.cfg_extend_name: str | None = None
+
         if not self.is_primary_cfg:
+            # Extensions merge after everything the cfg loads itself, and
+            # before overrides and wildcards are processed.
+            self._merge_cfg_extensions()
             self.cfgs.append(self)
         else:
             for entry in self.use_cfgs:
@@ -205,6 +221,63 @@ def _merge_hjson(self, hjson_data: Mapping) -> None:
         for key, value in hjson_data.items():
             set_target_attribute(self.flow_cfg_file, self.__dict__, key, value)
 
+    def _merge_cfg_extensions(self) -> None:
+        """Merge in the cfg extensions (--cfg-extend) that name this cfg.
+
+        They apply in command-line order. Their list values follow the cfg's
+        own, and their overrides join the cfg's own.
+        """
+        # The name as the cfg's own files give it. Overrides could still change
+        # it, but those are processed after the extensions merge.
+        name = subst_wildcards(self.name, self.__dict__, ignore_error=True)
+        self.cfg_extend_name = name
+
+        # What load_hjson needs to resolve the names of imported files. As in
+        # the cfg's own files, {self_dir} is the cfg's directory: an extension
+        # names its own files with {file_dir}.
+        initial_values = {"proj_root": self.proj_root, "self_dir": self.self_dir}
+        if self.tool:
+            initial_values["tool"] = self.tool
+        # Only a tool given on the command line can't be changed by the files.
+        arg_keys = {"tool"} if self.args.tool is not None else set()
+
+        for ext in self.args.cfg_extend:
+            if ext.cfg_name != name:
+                continue
+            log.info("[cfg_extend]: %s: %s", name, ext.path)
+            meta = CfgExtensionMeta(path=str(ext.path), revision=git_revision(ext.path))
+            self.cfg_extensions[ext] = meta
+            # The overrides set before this extension, by the cfg itself or an
+            # earlier extension.
+            taken = override_names(self.__dict__.get("overrides", []))
+            ext_data = {}
+            saved = {}
+            try:
+                ext_data = load_cfg_extension(ext, initial_values, arg_keys)
+                # Keep the values that the merge will change, to put them back
+                # if the extension is skipped. set_target_attribute extends lists
+                # in place, so the merge goes into copies of them.
+                saved = {k: self.__dict__[k] for k in ext_data if k in self.__dict__}
+                for key, value in saved.items():
+                    if isinstance(value, list):
+                        self.__dict__[key] = list(value)
+                for key, value in ext_data.items():
+                    set_target_attribute(str(ext.path), self.__dict__, key, value)
+                check_cfg_extension_overrides(ext_data.get("overrides", []), self, taken)
+            except RuntimeError as err:
+                # Most messages start with the extension's path, which these give already.
+                reason = str(err).removeprefix(f"{str(ext.path)!r}: ")
+                if self.args.on_cfg_extend_error != "skip":
+                    log.error("Cannot extend cfg %r with %s: %s", name, ext.path, reason)
+                    sys.exit(1)
+                log.warning("Skipping cfg extension %s for %r: %s", ext.path, name, reason)
+                for key in ext_data:
+                    if key in saved:
+                        self.__dict__[key] = saved[key]
+                    else:
+                        self.__dict__.pop(key, None)
+                self.cfg_extensions[ext] = meta.model_copy(update={"skipped": reason})
+
     def _expand(self) -> None:
         """Expand wildcards after merging hjson.
 

src/dvsim/flow/extension.py

@@ -0,0 +1,168 @@
+# Copyright lowRISC contributors (OpenTitan project).
+# Licensed under the Apache License, Version 2.0, see LICENSE for details.
+# SPDX-License-Identifier: Apache-2.0
+
+"""Cfg extensions: hjson files merged into a named flow cfg (--cfg-extend).
+
+See the "DUT configuration" section of doc/design_doc.md.
+"""
+
+import os
+from collections.abc import Iterable, Mapping
+from dataclasses import dataclass
+from pathlib import Path
+
+from dvsim.flow.hjson import load_hjson
+
+__all__ = (
+    "CfgExtension",
+    "check_cfg_extension_overrides",
+    "check_cfg_extensions_used",
+    "load_cfg_extension",
+    "override_names",
+)
+
+
+@dataclass(frozen=True)
+class CfgExtension:
+    """An extension file, and the name of the cfg it extends."""
+
+    cfg_name: str
+    path: Path
+
+    @classmethod
+    def parse(cls, value: str) -> "CfgExtension":
+        """Parse NAME=PATH, as given to --cfg-extend.
+
+        A relative PATH is taken from the current directory. Symlinks are
+        kept, as they are in {file_dir}.
+        """
+        name, sep, path = value.partition("=")
+        name, path = name.strip(), path.strip()
+        if not sep or not name or not path:
+            msg = f"Invalid cfg extension {value!r}: expected NAME=PATH."
+            raise ValueError(msg)
+        return cls(name, Path(os.path.normpath(Path(path).absolute())))
+
+
+def load_cfg_extension(
+    ext: CfgExtension, initial_values: Mapping[str, object], arg_keys: Iterable[str]
+) -> dict:
+    """Load an extension file and its imports.
+
+    initial_values holds what is needed to resolve the names of imported files,
+    and arg_keys those of its keys that came from the command line (as for
+    load_hjson). The returned data holds only what the extension sets.
+
+    Raises a RuntimeError if the file can't be loaded or isn't allowed.
+    """
+    path = str(ext.path)
+    if not ext.path.is_file():
+        msg = f"{path!r}: Not a file."
+        raise RuntimeError(msg)
+
+    try:
+        data = load_hjson(path, initial_values, set(arg_keys))
+    except SystemExit as err:
+        # parse_hjson logs why a file can't be parsed and exits. Raise instead,
+        # so that --on-cfg-extend-error skip can leave the extension out.
+        msg = f"{path!r}: It, or a file it imports, can't be parsed (see above)."
+        raise RuntimeError(msg) from err
+    except ValueError as err:
+        # An import_cfgs entry with a wildcard that can't be resolved.
+        msg = f"{path!r}: Can't resolve the name of an imported file: {err}"
+        raise RuntimeError(msg) from err
+
+    if "use_cfgs" in data:
+        msg = f"{path!r}: An extension can't set use_cfgs."
+        raise RuntimeError(msg)
+
+    # The initial values are the cfg's own. If the extension set one of them,
+    # it set it to the same value (or load_hjson reported a conflict), so
+    # merging it would change nothing.
+    for key in initial_values:
+        data.pop(key, None)
+
+    return data
+
+
+def check_cfg_extensions_used(
+    extensions: Iterable[CfgExtension],
+    used: Iterable[CfgExtension],
+    cfg_names: Iterable[str],
+    primary_name: str | None,
+) -> list[str]:
+    """Check that every extension was used by a loaded cfg.
+
+    used are the extensions that some cfg matched. cfg_names are the names that
+    the cfgs that can be extended had when they matched them (before any
+    overrides); a primary cfg can't be extended, so its name is passed
+    separately to give a clearer message. Returns a list of error messages
+    (empty if all is well).
+    """
+    used = set(used)
+    names = set(cfg_names)
+    errors = []
+    for ext in extensions:
+        if ext in used:
+            continue
+        if ext.cfg_name == primary_name:
+            errors.append(
+                f"Cfg extension {ext.path} names the primary cfg {ext.cfg_name!r}: "
+                "only a cfg that runs jobs (a child, or a cfg run on its own) can be extended."
+            )
+        else:
+            known = ", ".join(sorted(names)) or "none"
+            errors.append(
+                f"Cfg extension {ext.path} names {ext.cfg_name!r}, but no loaded cfg has "
+                f"that name (loaded: {known})."
+            )
+    return errors
+
+
+def override_names(overrides: object) -> set[str]:
+    """Return the names of the keys that an ``overrides`` list sets."""
+    if not isinstance(overrides, list):
+        return set()
+    return {o["name"] for o in overrides if isinstance(o, dict) and isinstance(o.get("name"), str)}
+
+
+def check_cfg_extension_overrides(overrides: object, cfg: object, taken: Iterable[str]) -> None:
+    """Check an extension's overrides as the cfg will apply them.
+
+    The cfg processes overrides only once every file is merged (see
+    FlowCfg._process_overrides, whose checks these mirror). Checking an
+    extension's own here, against the cfg it has just been merged into, lets
+    a broken one (most often, a key that the cfg no longer has) be reported
+    as that extension's fault. So an extension can only override keys that
+    the cfg or an earlier extension define. taken are the keys already
+    overridden before this extension.
+
+    Raises a RuntimeError for the first problem found (its message gives the reason only).
+    """
+    if not isinstance(overrides, list):
+        msg = "Its 'overrides' must be a list."
+        raise RuntimeError(msg)
+    taken = set(taken)
+    for item in overrides:
+        if not (isinstance(item, dict) and set(item) == {"name", "value"}):
+            msg = f"Each of its overrides needs a name and a value, not {item!r}."
+            raise RuntimeError(msg)
+        name, value = item["name"], item["value"]
+        if not isinstance(name, str):
+            msg = f"Each of its overrides needs a key name, not {name!r}."
+            raise RuntimeError(msg)
+        if name in taken:
+            msg = f"It overrides {name!r}, which is already overridden."
+            raise RuntimeError(msg)
+        if not hasattr(cfg, name):
+            msg = f"It overrides {name!r}, which the cfg doesn't have."
+            raise RuntimeError(msg)
+        old = getattr(cfg, name)
+        if not isinstance(value, type(old)):
+            msg = (
+                f"It overrides {name!r} with {value!r}, "
+                f"which doesn't match the type of its value {old!r}."
+            )
+            raise RuntimeError(msg)
+        taken.add(name)