From c60582988740e38de4b9029b36ccbeb9dcfbcc75 Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Fri, 21 Aug 2026 01:07:15 +0200 Subject: [PATCH 1/3] feat: add a configuration file for dvsim itself Flow configs describe a flow; there has been no way to configure the tool itself. Settings that belong to a workspace rather than to a flow had to be repeated on every command line. This commit adds `dvsim.config`, which finds and parses an hjson configuration file: an explicit path if given, else the nearest `dvsim.hjson` walking up from the working directory, else `$XDG_CONFIG_HOME/lowRISC/dvsim/config.hjson`. Walking upwards matters because dvsim is normally invoked from inside the project it builds, while a file describing a workspace sits alongside that project. Each feature owns a section of the file. Unknown keys within a section are rejected, so that a typo fails at the point it is made rather than silently doing nothing. No feature reads a section yet. Signed-off-by: Andreas Kurth --- src/dvsim/config.py | 112 ++++++++++++++++++++++++++++++++++++++++++ tests/test_config.py | 114 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100644 src/dvsim/config.py create mode 100644 tests/test_config.py diff --git a/src/dvsim/config.py b/src/dvsim/config.py new file mode 100644 index 00000000..93dd8454 --- /dev/null +++ b/src/dvsim/config.py @@ -0,0 +1,112 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""DVSim's own configuration file. + +This is distinct from the flow configs dvsim takes as its positional argument: +it configures the tool rather than describing a flow, and it is meant to be +checked in next to a project so that a workspace's settings do not have to be +repeated on every command line. + +The file is hjson, the same format as the flow configs. Each feature owns a +section of it; see e.g. :mod:`dvsim.fusesoc` for the ``fusesoc`` section. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +import hjson + +__all__ = ( + "CONFIG_BASENAME", + "XDG_SUBPATH", + "as_str_list", + "find_config_file", + "load_config_file", + "read_section", +) + +#: Name of the discoverable config file. Deliberately not hidden. +CONFIG_BASENAME = "dvsim.hjson" + +#: Location of the per-user config file, under $XDG_CONFIG_HOME. +XDG_SUBPATH = Path("lowRISC") / "dvsim" / CONFIG_BASENAME + + +def find_config_file(explicit: str | None, start_dir: Path | None = None) -> Path | None: + """Locate the config file. + + Search order: + + 1. ``explicit``, if given (an error if it does not exist), + 2. the nearest :data:`CONFIG_BASENAME` walking upwards from ``start_dir``, + 3. ``$XDG_CONFIG_HOME/lowRISC/dvsim/dvsim.hjson``. + + Walking upwards matters because dvsim is normally invoked from inside the + project it is building, while a config file describing a workspace + naturally lives alongside that project rather than inside it. + """ + if explicit is not None: + path = Path(explicit).expanduser() + if not path.is_file(): + msg = f"dvsim config file not found: {path}" + raise FileNotFoundError(msg) + return path + + start = (start_dir or Path.cwd()).resolve() + for directory in (start, *start.parents): + candidate = directory / CONFIG_BASENAME + if candidate.is_file(): + return candidate + + xdg = Path(os.environ.get("XDG_CONFIG_HOME") or Path.home() / ".config") + candidate = xdg.expanduser() / XDG_SUBPATH + return candidate if candidate.is_file() else None + + +def load_config_file(path: Path) -> dict: + """Parse a config file, returning its top-level dict.""" + try: + data = hjson.loads(path.read_text()) + except Exception as e: + msg = f"Failed to parse dvsim config file {path}: {e}" + raise RuntimeError(msg) from e + + if not isinstance(data, dict): + msg = f"dvsim config file {path} must contain a dict at the top level" + raise RuntimeError(msg) + + return data + + +def read_section(data: dict, name: str, known_keys: frozenset[str], path: Path) -> dict: + """Return one section of a config file, rejecting unknown keys. + + Rejecting rather than ignoring them turns a typo into an error at the point + it is made, instead of a setting that silently does nothing. + """ + section = data.get(name, {}) + if not isinstance(section, dict): + msg = f"dvsim config file {path}: '{name}' must be a dict" + raise RuntimeError(msg) + + unknown = set(section) - known_keys + if unknown: + msg = f"dvsim config file {path}: unknown key(s) in '{name}': {sorted(unknown)}" + raise RuntimeError(msg) + + return section + + +def as_str_list(section: dict, key: str, name: str, path: Path) -> list[str]: + """Read a section key that may be given as a string or a list of strings.""" + value = section.get(key, []) + if isinstance(value, str): + return [value] + if not isinstance(value, list) or not all(isinstance(v, str) for v in value): + msg = f"dvsim config file {path}: '{name}.{key}' must be a string or list of strings" + raise RuntimeError(msg) + return list(value) diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 00000000..3c3d2f6c --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,114 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for dvsim's own config file.""" + +from pathlib import Path + +import pytest + +from dvsim.config import ( + CONFIG_BASENAME, + XDG_SUBPATH, + find_config_file, + load_config_file, + read_section, +) + + +def write(path: Path, body: str) -> Path: + path.write_text(body) + return path + + +class TestDiscovery: + def test_walks_upwards(self, tmp_path): + """The config file sits beside the project dvsim is run from inside.""" + cfg = write(tmp_path / CONFIG_BASENAME, "{}") + nested = tmp_path / "opentitan" / "hw" / "ip" + nested.mkdir(parents=True) + + assert find_config_file(None, start_dir=nested) == cfg + + def test_nearest_wins(self, tmp_path): + write(tmp_path / CONFIG_BASENAME, "{}") + nested = tmp_path / "opentitan" + nested.mkdir() + nearer = write(nested / CONFIG_BASENAME, "{}") + + assert find_config_file(None, start_dir=nested) == nearer + + def test_explicit_path_wins(self, tmp_path): + write(tmp_path / CONFIG_BASENAME, "{}") + explicit = write(tmp_path / "other.hjson", "{}") + + assert find_config_file(str(explicit), start_dir=tmp_path) == explicit + + def test_missing_explicit_path_raises(self, tmp_path): + with pytest.raises(FileNotFoundError): + find_config_file(str(tmp_path / "absent.hjson")) + + +class TestLoading: + def test_parses_a_dict(self, tmp_path): + cfg = write(tmp_path / CONFIG_BASENAME, "{fusesoc: {mapping: []}}") + + assert load_config_file(cfg) == {"fusesoc": {"mapping": []}} + + def test_rejects_a_non_dict(self, tmp_path): + cfg = write(tmp_path / CONFIG_BASENAME, "[1, 2]") + + with pytest.raises(RuntimeError, match="must contain a dict"): + load_config_file(cfg) + + def test_rejects_malformed_hjson(self, tmp_path): + cfg = write(tmp_path / CONFIG_BASENAME, "{ this is not valid") + + with pytest.raises(RuntimeError, match="Failed to parse"): + load_config_file(cfg) + + +class TestReadSection: + KEYS = frozenset({"mapping"}) + + def test_missing_section_is_empty(self, tmp_path): + assert read_section({}, "fusesoc", self.KEYS, tmp_path) == {} + + def test_unknown_key_is_rejected(self, tmp_path): + """A typo should fail loudly rather than silently do nothing.""" + with pytest.raises(RuntimeError, match="unknown key"): + read_section({"fusesoc": {"mappings": []}}, "fusesoc", self.KEYS, tmp_path) + + def test_non_dict_section_is_rejected(self, tmp_path): + with pytest.raises(RuntimeError, match="must be a dict"): + read_section({"fusesoc": []}, "fusesoc", self.KEYS, tmp_path) + + +class TestXdgFallback: + """The per-user file is named like the in-tree one, for consistency.""" + + def test_used_when_nothing_is_found_by_walking_up(self, tmp_path, monkeypatch): + xdg = tmp_path / "xdg" + cfg = xdg / XDG_SUBPATH + cfg.parent.mkdir(parents=True) + write(cfg, "{}") + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + + empty = tmp_path / "elsewhere" + empty.mkdir() + + assert find_config_file(None, start_dir=empty) == cfg + assert cfg.name == CONFIG_BASENAME + + def test_a_nearer_file_wins_over_it(self, tmp_path, monkeypatch): + xdg = tmp_path / "xdg" + (xdg / XDG_SUBPATH).parent.mkdir(parents=True) + write(xdg / XDG_SUBPATH, "{}") + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + + workspace = tmp_path / "workspace" + workspace.mkdir() + nearer = write(workspace / CONFIG_BASENAME, "{}") + + assert find_config_file(None, start_dir=workspace) == nearer From 11da85535ae08ff094f628ea632608665402afe8 Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Fri, 21 Aug 2026 01:07:50 +0200 Subject: [PATCH 2/3] feat: select the FuseSoC mapping and cores-root from dvsim Flow configs hardcode the FuseSoC arguments they pass, including the `--mapping` that selects which technology library a design is built against, so building an existing config tree against a different library meant editing those configs. The hjson `overrides:` key cannot do it either: a primary config loads its children before processing its own overrides, so an override written in a wrapper config never reaches them. This commit adds `--fusesoc-mapping [OLD=]NEW` and `--fusesoc-extra-cores-root PATH`, both repeatable, together with the `fusesoc` section of the configuration file and the `--dvsim-config` option that selects the file. Command-line values are appended to those from the file. The rewrite runs at the end of `FlowCfg._expand()`. By then the option lists hold literal arguments rather than wildcards, and it still precedes the `_create_objects()` call that subclasses make from their own `_expand()`, whose objects copy those lists. Because every child of a primary config is constructed with the same arguments object, the rewrite reaches all of them. Only lists whose command is FuseSoC are touched. `--cores-root` is a global option and is inserted before the `run` subcommand; `--mapping` belongs to `run` and is inserted after it. Repeated mappings are dropped, because configs build these lists by appending and FuseSoC rejects the same mapping given twice. Every substitution is logged at INFO, naming the config whose arguments were changed. These options come from outside the project, so an unlogged one would be a change to the build that leaves no trace in the tree. Signed-off-by: Andreas Kurth --- src/dvsim/cli/run.py | 45 ++++++ src/dvsim/flow/base.py | 36 +++++ src/dvsim/fusesoc.py | 292 +++++++++++++++++++++++++++++++++++ tests/test_fusesoc.py | 339 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 712 insertions(+) create mode 100644 src/dvsim/fusesoc.py create mode 100644 tests/test_fusesoc.py diff --git a/src/dvsim/cli/run.py b/src/dvsim/cli/run.py index 73dee12c..f2b4d50c 100644 --- a/src/dvsim/cli/run.py +++ b/src/dvsim/cli/run.py @@ -32,6 +32,7 @@ from pathlib import Path from dvsim.flow.factory import make_cfg +from dvsim.fusesoc import resolve_options as resolve_fusesoc_options from dvsim.instrumentation.factory import InstrumentationFactory from dvsim.instrumentation.runtime import set_instrumentation from dvsim.job.deploy import RunTest @@ -527,6 +528,33 @@ def parse_args(argv: list[str] | None = None): help="Clean the scratch directory before running.", ) + fusesocg = parser.add_argument_group("FuseSoC integration") + + fusesocg.add_argument( + "--fusesoc-mapping", + action="append", + default=[], + metavar="[OLD=]NEW", + help=( + "Select a FuseSoC mapping. OLD=NEW replaces an existing " + "--mapping=OLD in the generated FuseSoC command line; a bare NEW " + "appends a mapping. Repeatable. Also settable from the config " + "file, whose values are applied first." + ), + ) + + fusesocg.add_argument( + "--fusesoc-extra-cores-root", + action="append", + default=[], + metavar="PATH", + help=( + "Add a --cores-root to the generated FuseSoC command line, so that " + "cores outside the project tree are discoverable. Repeatable. Also " + "settable from the config file." + ), + ) + buildg = parser.add_argument_group("Options for building") buildg.add_argument( @@ -828,6 +856,18 @@ def parse_args(argv: list[str] | None = None): dvg = parser.add_argument_group("Controlling DVSim itself") + dvg.add_argument( + "--dvsim-config", + metavar="FILE", + help=( + "Path to dvsim's own config file. If not specified, the nearest " + "dvsim.hjson found by walking up from the current directory is " + "used, else $XDG_CONFIG_HOME/lowRISC/dvsim/dvsim.hjson. This " + "configures the tool, and is distinct from the flow config file " + "passed as the positional argument." + ), + ) + dvg.add_argument( "--instrument", dest="instrumentation", @@ -980,6 +1020,11 @@ def main(argv: list[str] | None = None) -> None: Launcher.max_odirs = args.max_odirs RuntimeBackend.max_output_dirs = args.max_odirs + # Resolve the FuseSoC options once, from the config file and the command + # line, and stash them on args so that every FlowCfg -- including the + # children of a primary cfg -- sees the same set. + args.resolved_fusesoc_options = resolve_fusesoc_options(args) + # Configure the runtime backend. set_backend_type(is_local=args.local, fake=args.fake) diff --git a/src/dvsim/flow/base.py b/src/dvsim/flow/base.py index 497e8a42..c6badf9a 100644 --- a/src/dvsim/flow/base.py +++ b/src/dvsim/flow/base.py @@ -18,6 +18,7 @@ import dvsim.instrumentation.runtime as instrumentation 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 @@ -225,6 +226,13 @@ def _expand(self) -> None: ignore_error=partial, ) + # Now that the wildcards are substituted, the FuseSoC command lines hold + # literal arguments and can be rewritten. This has to happen at the end + # of _expand() rather than after it, because subclasses call + # _create_objects() from their own _expand(), and the objects they + # create take a copy of these option lists. + self._apply_fusesoc_opts() + def _post_init(self) -> None: # Run some post init checks if not self.is_primary_cfg: # noqa: SIM102 @@ -319,6 +327,34 @@ def _conv_inline_cfg_to_hjson(self, idict: Mapping) -> str | None: # Return the temp cfg file created return temp_cfg_file + def _apply_fusesoc_opts(self) -> None: + """Rewrite the FuseSoC command lines this cfg will run. + + Only option lists whose command is actually FuseSoC are touched, so the + --fusesoc-* arguments are inert for flows driven by something else. + """ + options = getattr(self.args, "resolved_fusesoc_options", None) + if not options: + return + + for cmd_attr, opts_attr in ( + ("build_cmd", "build_opts"), + ("sv_flist_gen_cmd", "sv_flist_gen_opts"), + ): + opts = getattr(self, opts_attr, None) + if not opts: + continue + + cmd = str(getattr(self, cmd_attr, "") or "") + # The command may carry a prefix, e.g. "{job_prefix} fusesoc". + if not any(Path(word).name == "fusesoc" for word in cmd.split()): + continue + + # Name the config these arguments belong to, so that the overrides + # logged from dvsim.fusesoc can be traced back to it. + context = f"{getattr(self, 'name', '?')} {opts_attr} [{self.flow_cfg_file}]" + setattr(self, opts_attr, rewrite_fusesoc_opts(opts, options, context)) + def _process_overrides(self) -> None: # Look through the dict and find available overrides. # If override is available, check if the type of the value for existing diff --git a/src/dvsim/fusesoc.py b/src/dvsim/fusesoc.py new file mode 100644 index 00000000..91b19d18 --- /dev/null +++ b/src/dvsim/fusesoc.py @@ -0,0 +1,292 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""FuseSoC integration: mapping and cores-root control. + +Flow configs frequently hardcode the FuseSoC arguments they pass, including +which ``--mapping`` selects the technology library to build against. That makes +it impossible to build an existing config tree against a different library +without editing the config files, which is exactly what an out-of-tree +(e.g. partner-supplied) library needs to do. + +The hjson ``overrides:`` key cannot solve this, because a primary config loads +its children before processing its own overrides, so an override written in a +wrapper config never reaches the children. Command-line arguments do reach +them, because every child is constructed with the same ``args`` object. + +This module provides the ``fusesoc`` section of the dvsim config file (see +:mod:`dvsim.config`), the parsing of the corresponding command-line arguments, +and the rewriting of an assembled FuseSoC argument list. + +Both ``--fusesoc-mapping`` and ``--fusesoc-extra-cores-root`` are repeatable, +and values from the config file are applied before values from the command line. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import TYPE_CHECKING, NamedTuple + +from dvsim.config import as_str_list, find_config_file, load_config_file, read_section +from dvsim.logging import log + +if TYPE_CHECKING: + from collections.abc import Iterable, Sequence + +__all__ = ( + "FuseSoCOptions", + "MappingSpec", + "options_from_config", + "resolve_options", + "rewrite_fusesoc_opts", +) + + +class MappingSpec(NamedTuple): + """A ``--fusesoc-mapping`` value. + + ``old`` is the VLNV of a mapping to replace; when it is ``None`` the mapping + is appended rather than replacing anything. + """ + + old: str | None + new: str + + @classmethod + def parse(cls, value: str) -> MappingSpec: + """Parse ``[OLD=]NEW``. + + A VLNV never contains ``=``, so splitting on the first one is safe. + """ + value = value.strip() + if not value: + msg = "--fusesoc-mapping: empty value" + raise ValueError(msg) + + old, sep, new = value.partition("=") + if not sep: + return cls(None, old) + + if not old or not new: + msg = f"--fusesoc-mapping: expected [OLD=]NEW, got {value!r}" + raise ValueError(msg) + + return cls(old, new) + + +class FuseSoCOptions(NamedTuple): + """The resolved FuseSoC options for a run.""" + + mappings: tuple[MappingSpec, ...] = () + extra_cores_roots: tuple[str, ...] = () + + def __bool__(self) -> bool: + return bool(self.mappings or self.extra_cores_roots) + + +#: Keys understood inside the config file's ``fusesoc`` section. +CONFIG_SECTION = "fusesoc" +CONFIG_KEYS = frozenset({"mapping", "extra_cores_root"}) + + +def options_from_config(path: Path) -> FuseSoCOptions: + """Read the ``fusesoc`` section of a dvsim config file. + + Relative ``extra_cores_root`` entries resolve against the config file's own + directory, not the working directory, so that a checked-in config file is + valid wherever dvsim is invoked from. + """ + section = read_section(load_config_file(path), CONFIG_SECTION, CONFIG_KEYS, path) + + mappings = tuple( + MappingSpec.parse(m) for m in as_str_list(section, "mapping", CONFIG_SECTION, path) + ) + + base = path.parent + roots = tuple( + str(base / root) if not Path(root).is_absolute() else root + for root in as_str_list(section, "extra_cores_root", CONFIG_SECTION, path) + ) + + return FuseSoCOptions(mappings, roots) + + +def resolve_options(args) -> FuseSoCOptions: # noqa: ANN001 + """Combine config-file and command-line options, config file first.""" + config_path = find_config_file(getattr(args, "dvsim_config", None)) + from_file = options_from_config(config_path) if config_path is not None else FuseSoCOptions() + if config_path is not None and from_file: + log.verbose("Read FuseSoC options from %s", config_path) + + cli_mappings = tuple(MappingSpec.parse(m) for m in getattr(args, "fusesoc_mapping", []) or []) + cli_roots = tuple(getattr(args, "fusesoc_extra_cores_root", []) or []) + + return FuseSoCOptions( + from_file.mappings + cli_mappings, + from_file.extra_cores_roots + cli_roots, + ) + + +def _split(opts: Iterable[str]) -> list[str]: + """Split an option list into individual tokens. + + Config files routinely put several tokens into one list entry, e.g. + ``"--cores-root {proj_root}/hw"``. Working on tokens keeps the rewriting + rules below simple; the result is re-joined by the caller of the command. + """ + return [token for entry in opts for token in str(entry).split()] + + +def rewrite_fusesoc_opts( + opts: Sequence[str], + options: FuseSoCOptions, + context: str = "", +) -> list[str]: + """Apply ``options`` to an assembled FuseSoC argument list. + + Placement follows FuseSoC's own argument grammar: + + * ``--cores-root`` is a global option, so it goes *before* the ``run`` + subcommand. + * ``--mapping`` is an option of ``run``, so appended mappings go *after* it. + + Both the ``--mapping=VLNV`` and ``--mapping VLNV`` spellings are recognised + when replacing. + + Every substitution is logged at INFO. These options come from outside the + project, so an unlogged one is a change to the build that leaves no trace in + the tree; ``context`` says which config the arguments belong to. + """ + if not options: + return list(opts) + + tokens = _split(opts) + + try: + run_index = tokens.index("run") + except ValueError: + log.warning( + "FuseSoC options were given, but no 'run' subcommand was found in the " + "command line %s -- leaving it unchanged.", + " ".join(tokens), + ) + return list(opts) + + tokens = _replace_mappings(tokens, options.mappings, context) + + # Recompute: _replace_mappings preserves length, but be explicit about it. + run_index = tokens.index("run") + + head, tail = tokens[:run_index], tokens[run_index:] + + for root in options.extra_cores_roots: + head += ["--cores-root", root] + log.info("FuseSoC --cores-root added in %s: %s", context, root) + + appended = [f"--mapping={m.new}" for m in options.mappings if m.old is None] + if appended: + # tail[0] is "run"; insert directly after it. + tail = [tail[0], *appended, *tail[1:]] + for m in options.mappings: + if m.old is None: + log.info("FuseSoC --mapping added in %s: %s", context, m.new) + + return _dedupe_mappings(head + tail) + + +def _dedupe_mappings(tokens: list[str]) -> list[str]: + """Drop repeated --mapping arguments, keeping the first of each. + + Config files often build these argument lists by appending, so the same + mapping can legitimately appear twice before rewriting, and replacing both + occurrences would then yield two identical mappings. FuseSoC rejects that + with "The following sources are in multiple mappings", even though the + duplicate asks for exactly what the original did. + """ + seen: set[str] = set() + result: list[str] = [] + pending_flag = False + + for arg in tokens: + if pending_flag: + pending_flag = False + if arg in seen: + result.pop() # also drop the "--mapping" that introduced it + continue + seen.add(arg) + result.append(arg) + continue + + if arg == "--mapping": + pending_flag = True + result.append(arg) + continue + + if arg.startswith("--mapping="): + vlnv = arg[len("--mapping=") :] + if vlnv in seen: + continue + seen.add(vlnv) + + result.append(arg) + + return result + + +def _log_override(context: str, old: str, new: str) -> None: + """Record a substitution of an in-tree value, at INFO so it cannot be missed.""" + log.info("FuseSoC --mapping override in %s: %s -> %s", context, old, new) + + +def _replace_mappings( + tokens: list[str], + mappings: Sequence[MappingSpec], + context: str = "", +) -> list[str]: + replacements = {m.old: m.new for m in mappings if m.old is not None} + if not replacements: + return tokens + + seen: set[str] = set() + result: list[str] = [] + skip_next_value_of: str | None = None + + for arg in tokens: + if skip_next_value_of is not None: + # The previous argument was a bare "--mapping"; this one is its value. + new = replacements.get(arg) + if new is not None: + seen.add(arg) + result.append(new) + _log_override(context, arg, new) + else: + result.append(arg) + skip_next_value_of = None + continue + + if arg == "--mapping": + skip_next_value_of = arg + result.append(arg) + continue + + if arg.startswith("--mapping="): + old = arg[len("--mapping=") :] + new = replacements.get(old) + if new is not None: + seen.add(old) + result.append(f"--mapping={new}") + _log_override(context, old, new) + continue + + result.append(arg) + + for old in replacements: + if old not in seen: + log.warning( + "--fusesoc-mapping: no '--mapping=%s' found to replace; " + "the mapping was left unchanged.", + old, + ) + + return result diff --git a/tests/test_fusesoc.py b/tests/test_fusesoc.py new file mode 100644 index 00000000..82fe977c --- /dev/null +++ b/tests/test_fusesoc.py @@ -0,0 +1,339 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for the FuseSoC integration.""" + +import logging +from argparse import Namespace +from pathlib import Path + +import pytest + +from dvsim.config import CONFIG_BASENAME +from dvsim.fusesoc import ( + FuseSoCOptions, + MappingSpec, + options_from_config, + resolve_options, + rewrite_fusesoc_opts, +) + +GENERIC = "lowrisc:prim_generic:all:0.1" +MY_TECH = "lowrisc:prim_my_tech:all:0.1" +TOP = "lowrisc:systems:top_earlgrey:0.1" + +# The lint flow's build_opts, as OpenTitan writes them: note that several list +# entries hold more than one token. +LINT_OPTS = [ + "--cores-root /proj/hw", + "run", + "--target=lint", + "--tool=veriblelint", + "--work-root=/scratch/fusesoc-work", + f"--mapping={GENERIC}", + f"--mapping={TOP}", + "lowrisc:ip:uart", +] + + +class TestMappingSpec: + def test_bare_value_appends(self): + assert MappingSpec.parse(MY_TECH) == MappingSpec(None, MY_TECH) + + def test_old_equals_new_replaces(self): + assert MappingSpec.parse(f"{GENERIC}={MY_TECH}") == MappingSpec(GENERIC, MY_TECH) + + @pytest.mark.parametrize("value", ["", " ", f"={MY_TECH}", f"{GENERIC}="]) + def test_rejects_malformed(self, value): + with pytest.raises(ValueError): + MappingSpec.parse(value) + + +class TestRewrite: + def test_no_options_is_identity(self): + assert rewrite_fusesoc_opts(LINT_OPTS, FuseSoCOptions()) == LINT_OPTS + + def test_replaces_only_the_named_mapping(self): + opts = FuseSoCOptions(mappings=(MappingSpec(GENERIC, MY_TECH),)) + result = rewrite_fusesoc_opts(LINT_OPTS, opts) + + assert f"--mapping={MY_TECH}" in result + assert f"--mapping={GENERIC}" not in result + # The top mapping selects the top's constants and must survive. + assert f"--mapping={TOP}" in result + + def test_replaces_two_token_spelling(self): + result = rewrite_fusesoc_opts( + ["run", "--mapping", GENERIC, "core"], + FuseSoCOptions(mappings=(MappingSpec(GENERIC, MY_TECH),)), + ) + assert result == ["run", "--mapping", MY_TECH, "core"] + + def test_bare_mapping_is_appended_after_run(self): + result = rewrite_fusesoc_opts( + LINT_OPTS, + FuseSoCOptions(mappings=(MappingSpec(None, MY_TECH),)), + ) + assert result[result.index("run") + 1] == f"--mapping={MY_TECH}" + # The pre-existing mappings are untouched. + assert f"--mapping={GENERIC}" in result + + def test_cores_root_goes_before_run(self): + result = rewrite_fusesoc_opts( + LINT_OPTS, + FuseSoCOptions(extra_cores_roots=("/elsewhere/prim_my_tech",)), + ) + run_index = result.index("run") + assert result[run_index - 2 : run_index] == ["--cores-root", "/elsewhere/prim_my_tech"] + # The project's own cores-root is kept. + assert "/proj/hw" in result + + def test_multi_token_entries_are_split(self): + result = rewrite_fusesoc_opts(LINT_OPTS, FuseSoCOptions(extra_cores_roots=("/x",))) + assert "--cores-root /proj/hw" not in result + assert result.count("--cores-root") == 2 + + def test_without_run_the_list_is_untouched(self): + opts = ["--version"] + result = rewrite_fusesoc_opts(opts, FuseSoCOptions(extra_cores_roots=("/x",))) + assert result == opts + + def test_unmatched_replacement_warns_and_keeps_list(self): + opts = FuseSoCOptions(mappings=(MappingSpec("lowrisc:nope:all:0.1", MY_TECH),)) + result = rewrite_fusesoc_opts(LINT_OPTS, opts) + assert f"--mapping={GENERIC}" in result + assert MY_TECH not in " ".join(result) + + +class TestFuseSoCSection: + def _write(self, path: Path, body: str) -> Path: + path.write_text(body) + return path + + def test_relative_cores_root_resolves_against_config_file(self, tmp_path): + cfg = self._write( + tmp_path / CONFIG_BASENAME, + '{fusesoc: {mapping: ["%s=%s"], extra_cores_root: ["prim_my_tech"]}}' + % (GENERIC, MY_TECH), + ) + options = options_from_config(cfg) + + assert options.mappings == (MappingSpec(GENERIC, MY_TECH),) + assert options.extra_cores_roots == (str(tmp_path / "prim_my_tech"),) + + def test_absolute_cores_root_is_left_alone(self, tmp_path): + cfg = self._write( + tmp_path / CONFIG_BASENAME, + '{fusesoc: {extra_cores_root: ["/abs/path"]}}', + ) + assert options_from_config(cfg).extra_cores_roots == ("/abs/path",) + + def test_string_value_is_accepted_as_a_singleton(self, tmp_path): + cfg = self._write(tmp_path / CONFIG_BASENAME, '{fusesoc: {mapping: "%s"}}' % MY_TECH) + assert options_from_config(cfg).mappings == (MappingSpec(None, MY_TECH),) + + def test_unknown_key_is_rejected(self, tmp_path): + cfg = self._write(tmp_path / CONFIG_BASENAME, "{fusesoc: {mappings: []}}") + with pytest.raises(RuntimeError, match="unknown key"): + options_from_config(cfg) + + def test_missing_section_is_empty(self, tmp_path): + cfg = self._write(tmp_path / CONFIG_BASENAME, "{}") + assert not options_from_config(cfg) + + +class TestResolveOptions: + def test_config_then_cli(self, tmp_path): + (tmp_path / CONFIG_BASENAME).write_text( + '{fusesoc: {mapping: ["%s=%s"], extra_cores_root: ["/from/file"]}}' + % (GENERIC, MY_TECH), + ) + args = Namespace( + dvsim_config=str(tmp_path / CONFIG_BASENAME), + fusesoc_mapping=["lowrisc:other:all:0.1"], + fusesoc_extra_cores_root=["/from/cli"], + ) + options = resolve_options(args) + + assert options.mappings == ( + MappingSpec(GENERIC, MY_TECH), + MappingSpec(None, "lowrisc:other:all:0.1"), + ) + assert options.extra_cores_roots == ("/from/file", "/from/cli") + + +class TestExpandIntegration: + """The rewrite must happen inside FlowCfg._expand(). + + OneShotCfg._expand() calls super()._expand() and then _create_objects(), + and the build modes created there take a copy of build_opts. Rewriting + after _expand() returns would therefore be silently ignored by the flow + that actually runs FuseSoC, even though the cfg attribute looked right. + """ + + def _make_cfg(self, **attrs: object): + from dvsim.flow.base import FlowCfg + + class _DummyCfg(FlowCfg): + def _purge(self) -> None: ... + def _print_list(self) -> None: ... + def _create_deploy_objects(self) -> None: ... + def gen_results(self, results) -> None: ... + + cfg = object.__new__(_DummyCfg) + cfg.__dict__.update( + args=Namespace(dump_script=None, resolved_fusesoc_options=None), + is_primary_cfg=False, + ignored_wildcards=[], + build_cmd="", + build_opts=[], + flow_cfg_file="/stub/cfg.hjson", + sv_flist_gen_cmd="", + sv_flist_gen_opts=[], + ) + cfg.__dict__.update(attrs) + return cfg + + def test_expand_rewrites_build_opts(self): + options = FuseSoCOptions( + mappings=(MappingSpec(GENERIC, MY_TECH),), + extra_cores_roots=("/elsewhere",), + ) + cfg = self._make_cfg( + args=Namespace(dump_script=None, resolved_fusesoc_options=options), + # As OpenTitan's common_lint_cfg.hjson spells it. + build_cmd=" fusesoc", + build_opts=list(LINT_OPTS), + ) + + cfg._expand() + + assert f"--mapping={MY_TECH}" in cfg.build_opts + assert f"--mapping={GENERIC}" not in cfg.build_opts + assert "/elsewhere" in cfg.build_opts + + def test_expand_rewrites_sv_flist_gen_opts(self): + options = FuseSoCOptions(mappings=(MappingSpec(GENERIC, MY_TECH),)) + cfg = self._make_cfg( + args=Namespace(dump_script=None, resolved_fusesoc_options=options), + sv_flist_gen_cmd="fusesoc", + sv_flist_gen_opts=["run", f"--mapping={GENERIC}", "--setup core"], + ) + + cfg._expand() + + assert f"--mapping={MY_TECH}" in cfg.sv_flist_gen_opts + + def test_non_fusesoc_commands_are_left_alone(self): + options = FuseSoCOptions(mappings=(MappingSpec(GENERIC, MY_TECH),)) + cfg = self._make_cfg( + args=Namespace(dump_script=None, resolved_fusesoc_options=options), + build_cmd="make", + build_opts=list(LINT_OPTS), + ) + + cfg._expand() + + assert cfg.build_opts == LINT_OPTS + + def test_without_options_nothing_changes(self): + cfg = self._make_cfg(build_cmd=" fusesoc", build_opts=list(LINT_OPTS)) + + cfg._expand() + + assert cfg.build_opts == LINT_OPTS + + +class TestDeduplication: + """Config files append to these lists, so duplicates arise naturally. + + hw/dv/tools/dvsim/common_sim_cfg.hjson supplies the prim mapping, and a + chip-level cfg importing it appends its own. Replacing every occurrence of + the old mapping would then emit the new one twice, and FuseSoC rejects that + with "The following sources are in multiple mappings". + """ + + def test_replacing_a_repeated_mapping_yields_one(self): + opts = [ + "run", + f"--mapping={GENERIC}", + f"--mapping={TOP}", + f"--mapping={GENERIC}", + "core", + ] + result = rewrite_fusesoc_opts( + opts, + FuseSoCOptions(mappings=(MappingSpec(GENERIC, MY_TECH),)), + ) + + assert result.count(f"--mapping={MY_TECH}") == 1 + assert result.count(f"--mapping={TOP}") == 1 + assert result == ["run", f"--mapping={MY_TECH}", f"--mapping={TOP}", "core"] + + def test_appending_a_mapping_that_is_already_present(self): + opts = ["run", f"--mapping={MY_TECH}", "core"] + result = rewrite_fusesoc_opts( + opts, + FuseSoCOptions(mappings=(MappingSpec(None, MY_TECH),)), + ) + + assert result.count(f"--mapping={MY_TECH}") == 1 + + def test_two_token_duplicates_are_dropped_whole(self): + opts = ["run", "--mapping", GENERIC, "--mapping", GENERIC, "core"] + result = rewrite_fusesoc_opts( + opts, + FuseSoCOptions(mappings=(MappingSpec(GENERIC, MY_TECH),)), + ) + + assert result == ["run", "--mapping", MY_TECH, "core"] + + +@pytest.fixture +def dvsim_log(caplog): + """Capture dvsim's own logger, which deliberately does not propagate.""" + logger = logging.getLogger("dvsim") + logger.addHandler(caplog.handler) + caplog.set_level(logging.INFO, logger="dvsim") + yield caplog + logger.removeHandler(caplog.handler) + + +class TestOverrideLogging: + """Every substitution is logged at INFO. + + These options come from outside the project, so an unlogged one is a change + to the build that leaves no trace in the tree. + """ + + CONTEXT = "uart_lint build_opts [/proj/hw/ip/uart/dv/uart_sim_cfg.hjson]" + + def test_replacement_is_logged_with_both_vlnvs(self, dvsim_log): + rewrite_fusesoc_opts( + LINT_OPTS, + FuseSoCOptions(mappings=(MappingSpec(GENERIC, MY_TECH),)), + self.CONTEXT, + ) + + assert [ + m for m in dvsim_log.messages if GENERIC in m and MY_TECH in m and self.CONTEXT in m + ] + + def test_added_mapping_and_cores_root_are_logged(self, dvsim_log): + rewrite_fusesoc_opts( + LINT_OPTS, + FuseSoCOptions( + mappings=(MappingSpec(None, MY_TECH),), + extra_cores_roots=("/elsewhere",), + ), + self.CONTEXT, + ) + + assert [m for m in dvsim_log.messages if "--mapping added" in m and MY_TECH in m] + assert [m for m in dvsim_log.messages if "--cores-root added" in m and "/elsewhere" in m] + + def test_nothing_is_logged_when_nothing_changes(self, dvsim_log): + rewrite_fusesoc_opts(LINT_OPTS, FuseSoCOptions(), self.CONTEXT) + + assert not [m for m in dvsim_log.messages if "FuseSoC" in m] From 4f2ec6b5ad8ba0c6924b5d4dc731012b1687a5f1 Mon Sep 17 00:00:00 2001 From: Andreas Kurth Date: Tue, 25 Aug 2026 12:43:22 +0200 Subject: [PATCH 3/3] feat: set proj_root from the dvsim config file Selecting a technology library from a config file above the repository still required running dvsim from inside that repository, because proj_root was only ever discovered from the working directory or given on the command line. This commit lets the config file set `proj_root`, so that dvsim can be invoked from the workspace that holds the config file rather than from the project it builds. A relative `--proj-root` is now resolved against the working directory, and a relative `proj_root` in the config file against the directory holding that file; both may be absolute. The discovery order for the config file itself is unchanged. Resolving these to absolute paths matters because the value is handed on to flows, and from there to tools such as FuseSoC, which run with a different working directory. The branch name is now read from the git repository at proj_root rather than from the working directory, which has none to read when dvsim is invoked from outside the project. The config file is loaded once, in main(), rather than separately by each feature that reads a section of it, and its path is logged alongside proj_root. Signed-off-by: Andreas Kurth --- src/dvsim/cli/run.py | 78 ++++++++++++++++++++++++++++++++++++++----- src/dvsim/config.py | 34 +++++++++++++++++++ src/dvsim/fusesoc.py | 28 ++++++---------- tests/test_config.py | 48 ++++++++++++++++++++++++++ tests/test_fusesoc.py | 18 +++++----- 5 files changed, 171 insertions(+), 35 deletions(-) diff --git a/src/dvsim/cli/run.py b/src/dvsim/cli/run.py index f2b4d50c..0defd37e 100644 --- a/src/dvsim/cli/run.py +++ b/src/dvsim/cli/run.py @@ -31,7 +31,14 @@ from importlib.metadata import version from pathlib import Path +from dvsim.config import ( + check_top_level_keys, + find_config_file, + load_config_file, + proj_root_from_config, +) 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 from dvsim.instrumentation.factory import InstrumentationFactory from dvsim.instrumentation.runtime import set_instrumentation @@ -175,12 +182,14 @@ def parse_resource(s: str) -> tuple[str, int | None]: raise argparse.ArgumentTypeError(msg) from e -def resolve_branch(branch): +def resolve_branch(branch, cwd=None): """Choose a branch name for output files. If the --branch argument was passed on the command line, the branch - argument is the branch name to use. Otherwise it is None and we use git to - find the name of the current branch in the working directory. + argument is the branch name to use. Otherwise it is None and we use git to + find the name of the current branch in `cwd`, which is the project root + rather than the working directory: dvsim may be invoked from outside the + project, where there is no repository to read. Note, as this name will be used to generate output files any forward slashes are replaced with single dashes to avoid being interpreted as directory hierarchy. @@ -191,6 +200,8 @@ def resolve_branch(branch): result = subprocess.run( ["git", "rev-parse", "--abbrev-ref", "HEAD"], stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + cwd=cwd, check=False, ) branch = result.stdout.decode("utf-8").strip().replace("/", "-") @@ -217,6 +228,33 @@ def get_proj_root(): return proj_root +def find_proj_root(args) -> str: + """Choose the project root. + + In order of precedence: the --proj-root argument, relative to the working + directory; then `proj_root` from dvsim's config file, relative to the + directory holding that file; then the git repository containing the working + directory. + + Both settings may equally be given as absolute paths. Resolving them here + is what allows dvsim to be invoked from outside the project, since the value + is handed on to flows (and from there to tools such as FuseSoC) that run + with a different working directory. + """ + if args.proj_root: + return str(Path(args.proj_root).expanduser().resolve()) + + from_config = ( + proj_root_from_config(args.dvsim_config_data, args.dvsim_config_path) + if args.dvsim_config_path is not None + else None + ) + if from_config is not None: + return str(from_config) + + return get_proj_root() + + def resolve_proj_root(args): """Update proj_root based on how DVSim is invoked. @@ -229,7 +267,8 @@ def resolve_proj_root(args): --remote switch is not set, the destination path is identical to the src path. Likewise, if --dry-run is set. """ - proj_root_src = args.proj_root or get_proj_root() + proj_root_src = find_proj_root(args) + args.branch = resolve_branch(args.branch, proj_root_src) # Check if jobs are dispatched to external compute machines. If yes, # then the repo needs to be copied over to the scratch area @@ -490,9 +529,11 @@ def parse_args(argv: list[str] | None = None): "-pr", metavar="PATH", help=( - "The root directory of the project. If not " - "specified, dvsim will search for a git " - "repository containing the current directory." + "The root directory of the project, relative to the current " + "directory unless absolute. If not specified, dvsim uses " + "`proj_root` from its config file, resolved relative to that " + "file; failing that, it searches for a git repository containing " + "the current directory." ), ) @@ -984,7 +1025,24 @@ def main(argv: list[str] | None = None) -> None: log.fatal("Path to config file %s appears to be invalid.", args.cfg) sys.exit(1) - args.branch = resolve_branch(args.branch) + # Load dvsim's own config file once, before anything reads it. The keys it + # may define are listed here because this is the only place that knows about + # every feature that owns a section. + args.dvsim_config_path = find_config_file(args.dvsim_config) + args.dvsim_config_data = ( + load_config_file(args.dvsim_config_path) if args.dvsim_config_path is not None else {} + ) + if args.dvsim_config_path is not None: + check_top_level_keys( + args.dvsim_config_data, + frozenset({"proj_root", FUSESOC_CONFIG_SECTION}), + args.dvsim_config_path, + ) + log.info("[dvsim_config]: %s", args.dvsim_config_path) + + # proj_root is resolved before the branch, because the branch is read from + # the git repository at proj_root rather than from the working directory: + # dvsim may be invoked from outside the project entirely. proj_root_src, proj_root = resolve_proj_root(args) args.scratch_root = resolve_scratch_root(args.scratch_root, proj_root) log.info("[proj_root]: %s", proj_root) @@ -1023,7 +1081,9 @@ def main(argv: list[str] | None = None) -> None: # Resolve the FuseSoC options once, from the config file and the command # line, and stash them on args so that every FlowCfg -- including the # children of a primary cfg -- sees the same set. - args.resolved_fusesoc_options = resolve_fusesoc_options(args) + args.resolved_fusesoc_options = resolve_fusesoc_options( + args, args.dvsim_config_data, args.dvsim_config_path + ) # Configure the runtime backend. set_backend_type(is_local=args.local, fake=args.fake) diff --git a/src/dvsim/config.py b/src/dvsim/config.py index 93dd8454..3db238e3 100644 --- a/src/dvsim/config.py +++ b/src/dvsim/config.py @@ -24,9 +24,12 @@ "CONFIG_BASENAME", "XDG_SUBPATH", "as_str_list", + "check_top_level_keys", "find_config_file", "load_config_file", + "proj_root_from_config", "read_section", + "resolve_path", ) #: Name of the discoverable config file. Deliberately not hidden. @@ -110,3 +113,34 @@ def as_str_list(section: dict, key: str, name: str, path: Path) -> list[str]: msg = f"dvsim config file {path}: '{name}.{key}' must be a string or list of strings" raise RuntimeError(msg) return list(value) + + +def resolve_path(value: str, base: Path) -> Path: + """Resolve a config-file path value against the config file's own directory. + + Absolute values are left alone. Relative ones are taken to be relative to + the directory holding the config file rather than to the working directory, + so that a checked-in file means the same thing wherever dvsim is invoked + from. + """ + path = Path(value).expanduser() + return path if path.is_absolute() else (base / path).resolve() + + +def check_top_level_keys(data: dict, known_keys: frozenset[str], path: Path) -> None: + """Reject unknown top-level keys, so that a typo fails rather than doing nothing.""" + unknown = set(data) - known_keys + if unknown: + msg = f"dvsim config file {path}: unknown top-level key(s): {sorted(unknown)}" + raise RuntimeError(msg) + + +def proj_root_from_config(data: dict, path: Path) -> Path | None: + """Read the top-level ``proj_root``, resolved against the config file's directory.""" + value = data.get("proj_root") + if value is None: + return None + if not isinstance(value, str): + msg = f"dvsim config file {path}: 'proj_root' must be a string" + raise RuntimeError(msg) + return resolve_path(value, path.parent) diff --git a/src/dvsim/fusesoc.py b/src/dvsim/fusesoc.py index 91b19d18..65013239 100644 --- a/src/dvsim/fusesoc.py +++ b/src/dvsim/fusesoc.py @@ -25,14 +25,14 @@ from __future__ import annotations -from pathlib import Path from typing import TYPE_CHECKING, NamedTuple -from dvsim.config import as_str_list, find_config_file, load_config_file, read_section +from dvsim.config import as_str_list, read_section, resolve_path from dvsim.logging import log if TYPE_CHECKING: from collections.abc import Iterable, Sequence + from pathlib import Path __all__ = ( "FuseSoCOptions", @@ -90,34 +90,26 @@ def __bool__(self) -> bool: CONFIG_KEYS = frozenset({"mapping", "extra_cores_root"}) -def options_from_config(path: Path) -> FuseSoCOptions: - """Read the ``fusesoc`` section of a dvsim config file. - - Relative ``extra_cores_root`` entries resolve against the config file's own - directory, not the working directory, so that a checked-in config file is - valid wherever dvsim is invoked from. - """ - section = read_section(load_config_file(path), CONFIG_SECTION, CONFIG_KEYS, path) +def options_from_config(data: dict, path: Path) -> FuseSoCOptions: + """Read the ``fusesoc`` section of an already-loaded dvsim config file.""" + section = read_section(data, CONFIG_SECTION, CONFIG_KEYS, path) mappings = tuple( MappingSpec.parse(m) for m in as_str_list(section, "mapping", CONFIG_SECTION, path) ) - - base = path.parent roots = tuple( - str(base / root) if not Path(root).is_absolute() else root + str(resolve_path(root, path.parent)) for root in as_str_list(section, "extra_cores_root", CONFIG_SECTION, path) ) return FuseSoCOptions(mappings, roots) -def resolve_options(args) -> FuseSoCOptions: # noqa: ANN001 +def resolve_options(args, data: dict, path: Path | None) -> FuseSoCOptions: # noqa: ANN001 """Combine config-file and command-line options, config file first.""" - config_path = find_config_file(getattr(args, "dvsim_config", None)) - from_file = options_from_config(config_path) if config_path is not None else FuseSoCOptions() - if config_path is not None and from_file: - log.verbose("Read FuseSoC options from %s", config_path) + from_file = options_from_config(data, path) if path is not None else FuseSoCOptions() + if from_file: + log.verbose("Read FuseSoC options from %s", path) cli_mappings = tuple(MappingSpec.parse(m) for m in getattr(args, "fusesoc_mapping", []) or []) cli_roots = tuple(getattr(args, "fusesoc_extra_cores_root", []) or []) diff --git a/tests/test_config.py b/tests/test_config.py index 3c3d2f6c..ebd9507a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -11,11 +11,16 @@ from dvsim.config import ( CONFIG_BASENAME, XDG_SUBPATH, + check_top_level_keys, find_config_file, load_config_file, + proj_root_from_config, read_section, + resolve_path, ) +KNOWN = frozenset({"proj_root", "fusesoc"}) + def write(path: Path, body: str) -> Path: path.write_text(body) @@ -85,6 +90,49 @@ def test_non_dict_section_is_rejected(self, tmp_path): read_section({"fusesoc": []}, "fusesoc", self.KEYS, tmp_path) +class TestResolvePath: + """Relative values are relative to the config file, not the caller.""" + + def test_relative_resolves_against_the_base(self, tmp_path): + assert resolve_path("opentitan", tmp_path) == tmp_path / "opentitan" + + def test_absolute_is_left_alone(self, tmp_path): + assert resolve_path("/elsewhere/opentitan", tmp_path) == Path("/elsewhere/opentitan") + + +class TestProjRoot: + def test_relative_resolves_against_the_config_file(self, tmp_path): + cfg = write(tmp_path / CONFIG_BASENAME, '{proj_root: "opentitan"}') + + assert proj_root_from_config(load_config_file(cfg), cfg) == tmp_path / "opentitan" + + def test_absolute_is_left_alone(self, tmp_path): + cfg = write(tmp_path / CONFIG_BASENAME, '{proj_root: "/elsewhere/ot"}') + + assert proj_root_from_config(load_config_file(cfg), cfg) == Path("/elsewhere/ot") + + def test_absent_is_none(self, tmp_path): + cfg = write(tmp_path / CONFIG_BASENAME, "{}") + + assert proj_root_from_config(load_config_file(cfg), cfg) is None + + def test_non_string_is_rejected(self, tmp_path): + cfg = write(tmp_path / CONFIG_BASENAME, "{proj_root: 3}") + + with pytest.raises(RuntimeError, match="must be a string"): + proj_root_from_config(load_config_file(cfg), cfg) + + +class TestTopLevelKeys: + def test_known_keys_accepted(self, tmp_path): + check_top_level_keys({"proj_root": "x", "fusesoc": {}}, KNOWN, tmp_path) + + def test_unknown_key_is_rejected(self, tmp_path): + """A typo should fail loudly rather than silently do nothing.""" + with pytest.raises(RuntimeError, match="unknown top-level key"): + check_top_level_keys({"proj_roots": "x"}, KNOWN, tmp_path) + + class TestXdgFallback: """The per-user file is named like the in-tree one, for consistency.""" diff --git a/tests/test_fusesoc.py b/tests/test_fusesoc.py index 82fe977c..7db24db6 100644 --- a/tests/test_fusesoc.py +++ b/tests/test_fusesoc.py @@ -10,7 +10,7 @@ import pytest -from dvsim.config import CONFIG_BASENAME +from dvsim.config import CONFIG_BASENAME, load_config_file from dvsim.fusesoc import ( FuseSoCOptions, MappingSpec, @@ -117,7 +117,7 @@ def test_relative_cores_root_resolves_against_config_file(self, tmp_path): '{fusesoc: {mapping: ["%s=%s"], extra_cores_root: ["prim_my_tech"]}}' % (GENERIC, MY_TECH), ) - options = options_from_config(cfg) + options = options_from_config(load_config_file(cfg), cfg) assert options.mappings == (MappingSpec(GENERIC, MY_TECH),) assert options.extra_cores_roots == (str(tmp_path / "prim_my_tech"),) @@ -127,20 +127,22 @@ def test_absolute_cores_root_is_left_alone(self, tmp_path): tmp_path / CONFIG_BASENAME, '{fusesoc: {extra_cores_root: ["/abs/path"]}}', ) - assert options_from_config(cfg).extra_cores_roots == ("/abs/path",) + assert options_from_config(load_config_file(cfg), cfg).extra_cores_roots == ("/abs/path",) def test_string_value_is_accepted_as_a_singleton(self, tmp_path): cfg = self._write(tmp_path / CONFIG_BASENAME, '{fusesoc: {mapping: "%s"}}' % MY_TECH) - assert options_from_config(cfg).mappings == (MappingSpec(None, MY_TECH),) + assert options_from_config(load_config_file(cfg), cfg).mappings == ( + MappingSpec(None, MY_TECH), + ) def test_unknown_key_is_rejected(self, tmp_path): cfg = self._write(tmp_path / CONFIG_BASENAME, "{fusesoc: {mappings: []}}") with pytest.raises(RuntimeError, match="unknown key"): - options_from_config(cfg) + options_from_config(load_config_file(cfg), cfg) def test_missing_section_is_empty(self, tmp_path): cfg = self._write(tmp_path / CONFIG_BASENAME, "{}") - assert not options_from_config(cfg) + assert not options_from_config(load_config_file(cfg), cfg) class TestResolveOptions: @@ -150,11 +152,11 @@ def test_config_then_cli(self, tmp_path): % (GENERIC, MY_TECH), ) args = Namespace( - dvsim_config=str(tmp_path / CONFIG_BASENAME), fusesoc_mapping=["lowrisc:other:all:0.1"], fusesoc_extra_cores_root=["/from/cli"], ) - options = resolve_options(args) + cfg = tmp_path / CONFIG_BASENAME + options = resolve_options(args, load_config_file(cfg), cfg) assert options.mappings == ( MappingSpec(GENERIC, MY_TECH),