From 0a13be78683efa0b137488d0d2ec42b5943a7f90 Mon Sep 17 00:00:00 2001 From: Jochen Hoenle <173445474+hoe-jo@users.noreply.github.com> Date: Tue, 15 Sep 2026 09:13:39 +0200 Subject: [PATCH 1/5] architectural_design: per-view navigation with disambiguated stems - puml_utils.bzl: rewrite make_puml_rst_navigation to emit one directory- structured index.rst tree per view (mirroring source layout) - architectural_design.bzl: loop over the four views (static/dynamic/ public_api/internal_api) generically Colocate every view file (not just puml) preserving relative directory structure. Redesign the returned SphinxSourcesInfo so srcs holds only each view's root index - providers.bzl: replace the four separate *_doc_files fields on ArchitecturalDesignInfo with a single view_indexes dict (view name -> navigation struct or None) reflecting the generic per-view design. --- .github/skills/score-architecture/SKILL.md | 32 +- .../rules_score/docs/integration_guide.rst | 20 + .../docs/user_guide/architectural_design.rst | 104 +++++ .../rules_score/examples/seooc/design/BUILD | 3 +- .../examples/seooc/design/index.md | 20 + .../{arch_design.rst => public_api.rst} | 29 +- .../private/architectural_design.bzl | 322 ++++++++++------ .../private/dependable_element.bzl | 247 ++++++++---- .../rules/rules_score/private/puml_utils.bzl | 363 ++++++++++++++++-- bazel/rules/rules_score/providers.bzl | 5 +- .../rules_score/templates/conf.template.py | 8 + bazel/rules/rules_score/test/BUILD | 103 ++++- .../test/check_authored_layout_content.sh | 91 +++++ .../rules_score/test/check_seooc_dep_links.sh | 23 +- .../test/fixtures/authored/index.rst | 19 + .../test/fixtures/authored/overview.puml | 16 + .../fixtures/authored_override/overview.puml | 16 + .../fixtures/authored_override/overview.rst | 19 + .../test/puml_layout_crash_repro_test.bzl | 50 +++ .../rules_score/test/puml_layout_test.bzl | 311 +++++++++++++++ .../test/template/conf.template.py | 8 + plantuml/parser/puml_cli/src/main.rs | 59 ++- 22 files changed, 1590 insertions(+), 278 deletions(-) create mode 100644 bazel/rules/rules_score/examples/seooc/design/index.md rename bazel/rules/rules_score/examples/seooc/design/{arch_design.rst => public_api.rst} (56%) create mode 100755 bazel/rules/rules_score/test/check_authored_layout_content.sh create mode 100644 bazel/rules/rules_score/test/fixtures/authored/index.rst create mode 100644 bazel/rules/rules_score/test/fixtures/authored/overview.puml create mode 100644 bazel/rules/rules_score/test/fixtures/authored_override/overview.puml create mode 100644 bazel/rules/rules_score/test/fixtures/authored_override/overview.rst create mode 100644 bazel/rules/rules_score/test/puml_layout_crash_repro_test.bzl create mode 100644 bazel/rules/rules_score/test/puml_layout_test.bzl diff --git a/.github/skills/score-architecture/SKILL.md b/.github/skills/score-architecture/SKILL.md index 5f91c8d3..721ac475 100644 --- a/.github/skills/score-architecture/SKILL.md +++ b/.github/skills/score-architecture/SKILL.md @@ -1,9 +1,3 @@ ---- -name: score-architecture -description: "Software architectural design for S-CORE SEooCs using the rules_score Bazel rules. USE FOR: writing PlantUML static/dynamic/public_api/internal_api diagrams, structuring dependable_element → component → unit hierarchies, wiring architectural_design / unit / unit_design / component / dependable_element targets, PlantUML stereotype and interface/port conventions, the declared-vs-implemented architecture consistency check, integrity levels, certified scope, and requirement allocation to architectural elements. Use when working on architecture, .puml files, component/unit structure, or the rules_score architecture rules." -argument-hint: "component/unit or diagram to model" ---- - +--- +name: score-architecture +description: "Software architectural design for S-CORE SEooCs using the rules_score Bazel rules. USE FOR: writing PlantUML static/dynamic/public_api/internal_api diagrams, structuring dependable_element → component → unit hierarchies, wiring architectural_design / unit / unit_design / component / dependable_element targets, PlantUML stereotype and interface/port conventions, the declared-vs-implemented architecture consistency check, integrity levels, certified scope, and requirement allocation to architectural elements. Use when working on architecture, .puml files, component/unit structure, or the rules_score architecture rules." +argument-hint: "component/unit or diagram to model" +--- + # S-CORE Architecture Skill Software architectural design for a **Safety Element out of Context (SEooC)** using the @@ -344,6 +344,26 @@ diagram with prose, add both the RST/Markdown wrapper *and* the referenced `.pum list (as `static_design.puml` + `arch_design.rst` above); the wrapper embeds the diagram with `.. uml:: file.puml`. +Each view builds a navigation tree mirroring the on-disk directory layout of its diagrams: +every `.puml` gets an auto-generated wrapper page, and every directory gets a generated +`index.rst` listing its diagrams and sub-directories. Authored pages slot into that tree +**by name**: + +- **`.rst`/`.md` next to `.puml` overrides** that diagram's generated + wrapper page — this is exactly the wrapper pattern above. The `.puml` is still staged + beside it so `.. uml:: .puml` resolves. +- **`index.rst`/`index.md` in a directory composes** with that directory's generated + navigation: the authored body renders first, the generated toctree follows. It never + replaces it, because a missing toctree entry means an orphaned page. **Give the authored + body a section title** — it becomes the page title, and without one Sphinx warns that a + toctree entry has no title. + +Build-time errors (each naming the offending files): a diagram named `index.puml`/ +`index.plantuml` (that stem is reserved for the navigation page); two files in one view +resolving to the same staged path; a stem having both `.rst` and `.md`, or both `.puml` and +`.plantuml`. For a page you want fully outside this scheme, leave the `.puml` out of the view +attribute and reference it via your own `.. uml::` elsewhere in the docs tree. + ### `unit_design` ```starlark diff --git a/bazel/rules/rules_score/docs/integration_guide.rst b/bazel/rules/rules_score/docs/integration_guide.rst index d4ef05de..b7d2604e 100644 --- a/bazel/rules/rules_score/docs/integration_guide.rst +++ b/bazel/rules/rules_score/docs/integration_guide.rst @@ -258,6 +258,26 @@ Design Rationale Reference implementation: `examples/seooc `_ in the score-tooling repository. +Staged Layout of ``architectural_design`` Output +-------------------------------------------------- + +A ``dependable_element`` stages each of its ``architectural_design`` dependencies +under that dependency's **target name**: + +.. code-block:: text + + architectural_design///... + +This shape does not depend on how many ``architectural_design`` labels are +attached, so published HTML URLs stay stable as the element grows. The same +``/`` prefix is applied to every other artifact-type attribute +(``assumptions_of_use``, ``dependability_analysis``, ``checklists``, +``glossary``, and the requirements attributes). + +If two ``architectural_design`` labels from different packages share a target +name, or if two files within one label resolve to the same relative path, the +build fails with an error. + --- .. _sphinx-hermetic-tool-setup: diff --git a/bazel/rules/rules_score/docs/user_guide/architectural_design.rst b/bazel/rules/rules_score/docs/user_guide/architectural_design.rst index 14e0b496..3861848e 100644 --- a/bazel/rules/rules_score/docs/user_guide/architectural_design.rst +++ b/bazel/rules/rules_score/docs/user_guide/architectural_design.rst @@ -147,6 +147,78 @@ Common anti-patterns - **Leaky public API** — exposing an interface publicly for convenience. It then drags in unnecessary failure modes and AoUs. +Rendering: Diagrams, Wrapper Pages, and Directory Navigation +--------------------------------------------------------------- + +Each view (``static``, ``dynamic``, ``public_api``, ``internal_api``) is just a +flat list of ``.puml``/``.plantuml`` files, but Sphinx needs an actual page to +put every diagram on, plus a place in the sidebar to reach it from. +``architectural_design`` builds that structure automatically: + +- Every diagram gets an auto-generated wrapper page — a ``.rst`` file + containing a single ``.. uml::`` directive — named after the diagram's own + file stem (``foo.puml`` → page ``foo``). +- Every directory that contains at least one diagram or authored page gets a + generated ``index.rst`` with a ``toctree`` listing that directory's pages + and its subdirectories, mirroring the on-disk layout of the files you passed + to ``static``/``dynamic``/``public_api``/``internal_api``. Nesting is + unlimited. +- Directory levels that hold nothing of their own and lead to a single + subdirectory are skipped, so a diagram at ``foo/bar/baz.puml`` is reached + through one ``foo/bar/index.rst`` rather than a chain of navigation pages + that each contain a single link. A view consisting of one page and nothing + else gets no generated index at all; that page becomes the view's root. +- The view's top-level (root) index is the single toctree entry surfaced on + the enclosing ``dependable_element`` page for that view. + +Authoring pages alongside diagrams +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A generated wrapper page is only a placeholder; real prose belongs alongside +your ``.puml`` files, passed in the same view attribute. What happens depends +on the file's stem relative to the diagrams already in that directory: + +.. list-table:: + :header-rows: 1 + + * - You add + - Effect + - When to use it + * - ``.rst``/``.md`` with no matching ``.puml`` in the same + directory + - **Standalone page** — an ordinary extra entry in that directory's + toctree. + - Prose that isn't about one specific diagram — design rationale, an + overview, background context. + * - ``.rst``/``.md`` next to a same-stem ``.puml``/ + ``.plantuml`` + - **Override** — replaces that diagram's generated wrapper page + outright. The ``.puml`` is still staged as a sibling, so your page can + embed it with its own ``.. uml:: .puml``. + - You want narrative directly around one specific diagram instead of it + rendering bare. + * - ``index.rst``/``index.md`` + - **Compose** — your content is rendered *above* the generated + directory-level toctree, which is otherwise left untouched (every + diagram in that directory keeps its navigation entry). Your title + becomes the index page's title. + - A directory-level introduction that must not hide any diagram from + the navigation. + +A ``.puml``/``.plantuml`` file whose own stem is literally ``index`` is +rejected at analysis time — that stem is reserved for the directory's +generated navigation page; name the diagram something else. + +Two files that would stage at the same relative path (for example both a +``.rst`` and a ``.md`` for the same stem) also fail the build, with a message +naming both conflicting sources, instead of surfacing a raw Bazel +action-conflict error. + +See ``examples/seooc/design`` for a working demonstration of all three modes: +``arch_design.rst`` is a standalone page in the static view, ``index.md`` +composes an introduction above the static view's root navigation, and +``public_api.rst`` overrides the generated wrapper for ``public_api.puml``. + Static Architecture -------------------- @@ -428,6 +500,38 @@ Include both the wrapper file *and* the referenced ``.puml`` file in the same Ba ], ) +Generated Navigation, and How Authored Pages Interact With It +---------------------------------------------------------------- + +Each view organises its diagrams into a navigation tree that mirrors their +on-disk directory layout: every diagram gets an auto-generated wrapper page, +and every directory gets a generated ``index.rst`` listing that directory's +diagrams and sub-directories. Authored pages you pass in the same view slot +into that tree by name: + +- **Same-stem override.** A ``.rst`` or ``.md`` next to a + same-named ``.puml`` *replaces* that diagram's generated wrapper page. + The generated wrapper is only a placeholder for prose that doesn't exist + yet, so your page always wins. The ``.puml`` is still staged beside it, so + your own ``.. uml:: .puml`` resolves — this is exactly the wrapper + pattern shown above. + +- **Directory index compose.** An ``index.rst`` or ``index.md`` in a directory + *composes* with that directory's generated navigation instead of replacing + it: your text is rendered first, and the generated toctree follows below it. + + Give the authored body a section title. It becomes the page's title, and + without one Sphinx warns that a toctree entry has no title. + +Two constraints are enforced at build time, with an error naming the offending +files: + +- A diagram may not be named ``index.puml``/``index.plantuml``; that stem is + reserved for the directory's own navigation page. +- No two files in one view may resolve to the same staged path, and a given + stem may not have both a ``.rst`` and a ``.md`` (or both a ``.puml`` and a + ``.plantuml``). + Rule Reference: ``architectural_design`` ------------------------------------------- diff --git a/bazel/rules/rules_score/examples/seooc/design/BUILD b/bazel/rules/rules_score/examples/seooc/design/BUILD index daf0e18e..de5076a6 100644 --- a/bazel/rules/rules_score/examples/seooc/design/BUILD +++ b/bazel/rules/rules_score/examples/seooc/design/BUILD @@ -26,10 +26,11 @@ architectural_design( ], public_api = [ "public_api.puml", + "public_api.rst", ], static = [ "static_design.puml", - "arch_design.rst", + "index.md", ], visibility = ["//visibility:public"], ) diff --git a/bazel/rules/rules_score/examples/seooc/design/index.md b/bazel/rules/rules_score/examples/seooc/design/index.md new file mode 100644 index 00000000..7413b607 --- /dev/null +++ b/bazel/rules/rules_score/examples/seooc/design/index.md @@ -0,0 +1,20 @@ + + +# Safety Software SEooC Example — Static Design + +This is a **compose** example (see `architectural_design.rst`'s "Authoring +Pages Alongside Diagrams"): this file is named `index.md`, so its content is +rendered above the generated navigation for this directory instead of +replacing it — `static_design` and `arch_design` below both keep their own +entries. diff --git a/bazel/rules/rules_score/examples/seooc/design/arch_design.rst b/bazel/rules/rules_score/examples/seooc/design/public_api.rst similarity index 56% rename from bazel/rules/rules_score/examples/seooc/design/arch_design.rst rename to bazel/rules/rules_score/examples/seooc/design/public_api.rst index 087f1d2a..4ddae8ed 100644 --- a/bazel/rules/rules_score/examples/seooc/design/arch_design.rst +++ b/bazel/rules/rules_score/examples/seooc/design/public_api.rst @@ -12,27 +12,18 @@ # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* -Architectural Design -==================== - -This is the architectural design of the Safety Software SEooC Example: - -The :term:`Architectural Design` describes how each :term:`Component` -contributes to fulfilling :term:`Feature Requirements` under the selected -:term:`Integrity Level`. - -Static Architecture -------------------- +Public API +========== -.. uml:: static_design.puml - :align: center - :alt: Static Component Architectural Design - :width: 100% +This is an **override** example (see ``architectural_design.rst``'s +"Authoring Pages Alongside Diagrams"): this file shares its stem with +``public_api.puml``, so it replaces that diagram's generated wrapper page +outright. The diagram is still staged as a sibling, so it can be embedded +here directly. -Public API ----------- +The SEooC exposes exactly one operation, ``GetNumber()``, on +``SampleLibraryAPI``. .. uml:: public_api.puml :align: center - :alt: Public API of the SEooC Example - :width: 100% + :alt: SEooC example public API diff --git a/bazel/rules/rules_score/private/architectural_design.bzl b/bazel/rules/rules_score/private/architectural_design.bzl index 6e95e686..211901f1 100644 --- a/bazel/rules/rules_score/private/architectural_design.bzl +++ b/bazel/rules/rules_score/private/architectural_design.bzl @@ -22,16 +22,59 @@ The rule automatically invokes the PlantUML parser on .puml/.plantuml files to produce FlatBuffers binary representations of the parsed diagrams. """ +load("@bazel_skylib//lib:paths.bzl", "paths") load("//bazel/rules/rules_score:providers.bzl", "ArchitecturalDesignInfo", "SphinxSourcesInfo") -load("//bazel/rules/rules_score/private:puml_utils.bzl", "make_puml_rst_wrappers") +load("//bazel/rules/rules_score/private:puml_utils.bzl", "emit_view_navigation", "plan_view_layout", "relative_source_path") load("//bazel/rules/rules_score/private:validation.bzl", "PROFILES", "VALIDATION_ATTRS", "run_validation") load("//bazel/rules/rules_score/private:verbosity.bzl", "VERBOSITY_ATTR", "get_log_level") +# Views recognized by architectural_design, mapped to their display name used +# as the title of that view's top-level navigation index page. +_VIEWS = { + "static": "Static Design", + "dynamic": "Dynamic Design", + "public_api": "Public API", + "internal_api": "Internal API", +} + # ============================================================================ # Private Rule Implementation # ============================================================================ -def _run_puml_parser(ctx, puml_file): +def _disambiguated_stems(ctx, files): + """Compute a unique output stem (no directory, no extension) for every + .puml/.plantuml file in `files`. + + All diagrams of one architectural_design target share a flat output + directory (keyed by ctx.label.name) for their fbs/lobster/idmap + artifacts, so two files with the same basename but different source + directories (e.g. two `for_impl_apis.puml` files under different + subpackages) would otherwise collide on the same generated output path. + When a basename is unique, the plain stem is kept unchanged (preserving + existing filenames/titles); only colliding basenames are disambiguated, + using the file's package-relative directory. + + Args: + ctx: Rule context. + files: Iterable of File objects (non-.puml/.plantuml entries ignored). + Returns: + Dict from File.path to a unique stem string. + """ + puml_files = [f for f in files if f.extension in ("puml", "plantuml")] + basename_counts = {} + for f in puml_files: + basename_counts[f.basename] = basename_counts.get(f.basename, 0) + 1 + + stems = {} + for f in puml_files: + stem = f.basename.rsplit(".", 1)[0] + if basename_counts[f.basename] > 1: + dir_part = paths.dirname(relative_source_path(f, ctx.label.package, ctx.label.workspace_name)) + stem = "{}__{}".format(dir_part.replace("/", "_"), stem) if dir_part else stem + stems[f.path] = stem + return stems + +def _run_puml_parser(ctx, puml_file, file_stem): """Run the PlantUML parser on a single .puml file to produce a FlatBuffers binary, a lobster traceability file, and an idmap sidecar. @@ -39,6 +82,12 @@ def _run_puml_parser(ctx, puml_file): FlatBuffers schema (each diagram type uses its own root_type). Lobster output is produced in-process for component diagrams. + When the input file basename is not unique across all diagrams being + parsed by this target (see _disambiguated_stems), a symlink with a + disambiguated name is created and passed to puml_cli. This ensures + puml_cli produces outputs with unique names even when two source + diagrams share the same basename but live in different directories. + ``--source-name`` is passed as ``puml_file.short_path`` so the ``source`` field embedded in the fbs/lobster/idmap outputs is a stable, workspace-relative path. This is required by the `clickable_plantuml` @@ -51,10 +100,10 @@ def _run_puml_parser(ctx, puml_file): Args: ctx: Rule context puml_file: The .puml File object to parse + file_stem: Unique output stem for this file (see _disambiguated_stems). Returns: Tuple of (fbs_output, lobster_output, idmap_output) declared output Files. """ - file_stem = puml_file.basename.rsplit(".", 1)[0] fbs_output = ctx.actions.declare_file( "{}/{}.fbs.bin".format(ctx.label.name, file_stem), ) @@ -65,13 +114,23 @@ def _run_puml_parser(ctx, puml_file): "{}/{}.idmap.json".format(ctx.label.name, file_stem), ) + # A symlink under this target's own _puml_inputs/ dir, named after the + # disambiguated stem, so puml_cli's output filenames (derived from input + # basename) match the declared output files, and two architectural_design + # targets in the same package sharing a diagram basename never collide + # on the same _puml_inputs/ path. + input_symlink = ctx.actions.declare_file( + "{}/_puml_inputs/{}.{}".format(ctx.label.name, file_stem, puml_file.extension), + ) + ctx.actions.symlink(output = input_symlink, target_file = puml_file) + ctx.actions.run( - inputs = [puml_file], + inputs = [input_symlink], outputs = [fbs_output, lobster_output, idmap_output], executable = ctx.executable._puml_parser, arguments = [ "--file", - puml_file.path, + input_symlink.path, "--fbs-output-dir", fbs_output.dirname, "--lobster-output-dir", @@ -88,12 +147,13 @@ def _run_puml_parser(ctx, puml_file): return fbs_output, lobster_output, idmap_output -def _parse_puml_diagrams(ctx, files): +def _parse_puml_diagrams(ctx, files, stems): """Run the PlantUML parser on all .puml/.plantuml files in a list. Args: ctx: Rule context files: List of File objects + stems: Dict from File.path to unique output stem (see _disambiguated_stems). Returns: Tuple of (fbs_outputs, lobster_outputs, idmap_outputs) lists of generated Files. """ @@ -102,59 +162,39 @@ def _parse_puml_diagrams(ctx, files): idmap_outputs = [] for f in files: if f.extension in ("puml", "plantuml"): - fbs, lobster, idmap = _run_puml_parser(ctx, f) + fbs, lobster, idmap = _run_puml_parser(ctx, f, stems[f.path]) fbs_outputs.append(fbs) lobster_outputs.append(lobster) idmap_outputs.append(idmap) return fbs_outputs, lobster_outputs, idmap_outputs -def _colocate_puml_with_wrapper(ctx, puml_files, output_dir): - """Symlink .puml/.plantuml sources next to their generated RST wrapper. - - make_puml_rst_wrappers() declares each wrapper at - "{output_dir}/{stem}.rst" (output_dir is this target's ctx.label.name) - and embeds the diagram via a same-directory sibling reference - (``.. uml:: {basename}``). The .puml source itself, however, usually - lives directly in this target's package -- one directory above - `output_dir` -- not nested under it. When dependable_element.bzl later - stages every SphinxSourcesInfo file for the HTML build, it flattens paths - based on the *shortest common directory* across all of a label's files - (see its `_find_common_directory`); mixing a file that sits directly in - the package with one nested one level deeper collapses the common - directory to the package itself, so the wrapper ends up staged one - level deeper than the raw .puml file and the `.. uml::` sibling - reference breaks (PlantUML file "x.puml" cannot be read). Symlinking a - same-named copy of every diagram alongside its wrapper keeps them - siblings under `output_dir` regardless of the diagram's original - on-disk location, so downstream flattening logic stages them together. +def _colocate_view_files(ctx, staged_files, view_output_dir): + """Symlink each (source File, staged relative path) pair from a view's + layout plan into `view_output_dir`. + + emit_view_navigation() declares wrappers and per-directory indexes under + this same `view_output_dir`, at the paths plan_view_layout() computed. + Any other file participating in that view's navigation -- a diagram's + own .puml source, a hand-written .rst/.md page, or an asset such as + .svg -- must be staged as a sibling under its identical relative path, + or the generated `.. uml::`/toctree/`.. include::` references (resolved + as same-directory siblings) will not resolve once dependable_element.bzl + stages SphinxSourcesInfo files for the HTML build. Args: ctx: Rule context. - puml_files: Iterable of File objects; non-.puml/.plantuml files are - passed through unchanged. - output_dir: String prefix matching the one passed to - make_puml_rst_wrappers() (typically ctx.label.name). + staged_files: List of (File, relative_path) tuples -- plan.staged + from plan_view_layout(). + view_output_dir: String prefix for declared output files, e.g. + "{ctx.label.name}/{view_name}". Returns: - List of File objects with .puml/.plantuml entries replaced by - same-directory symlinked copies. + List of symlinked File objects, one per (File, relative_path) pair. """ colocated = [] - pkg_prefix = ctx.label.package + "/" if ctx.label.package else "" - for f in puml_files: - if f.extension not in ("puml", "plantuml"): - colocated.append(f) - continue - - rel_dir = "" - if pkg_prefix and f.short_path.startswith(pkg_prefix): - rel_path = f.short_path[len(pkg_prefix):] - if "/" in rel_path: - rel_dir = rel_path.rsplit("/", 1)[0] - - out_path = "{}/{}/{}".format(output_dir, rel_dir, f.basename) if rel_dir else "{}/{}".format(output_dir, f.basename) - copy = ctx.actions.declare_file(out_path) - ctx.actions.symlink(output = copy, target_file = f) + for source_file, relative_path in staged_files: + copy = ctx.actions.declare_file("{}/{}".format(view_output_dir, relative_path)) + ctx.actions.symlink(output = copy, target_file = source_file) colocated.append(copy) return colocated @@ -205,93 +245,114 @@ def _architectural_design_impl(ctx): List of providers including DefaultInfo, ArchitecturalDesignInfo, SphinxSourcesInfo """ - # Parse each architectural view separately so each provider field carries - # the flatbuffers for its own category. - static_fbs_list, static_lobster_list, static_idmap_list = _parse_puml_diagrams(ctx, ctx.files.static) - dynamic_fbs_list, dynamic_lobster_list, dynamic_idmap_list = _parse_puml_diagrams(ctx, ctx.files.dynamic) - public_api_fbs_list, public_api_lobster_list, public_api_idmap_list = _parse_puml_diagrams(ctx, ctx.files.public_api) - internal_api_fbs_list, _internal_api_lobster_list, internal_api_idmap_list = _parse_puml_diagrams(ctx, ctx.files.internal_api) - - static_fbs = depset(static_fbs_list) - dynamic_fbs = depset(dynamic_fbs_list) - public_api_fbs = depset(public_api_fbs_list) - internal_api_fbs = depset(internal_api_fbs_list) - public_api_lobster = depset(public_api_lobster_list) - - # Source files for SphinxSourcesInfo (sphinx documentation pipeline). - # .puml/.plantuml sources are colocated (symlinked) next to their - # generated RST wrapper -- see _colocate_puml_with_wrapper for why this - # is required for the `.. uml::` sibling reference to resolve once - # dependable_element.bzl stages these files for the HTML build. - all_source_files = depset( - transitive = [ - depset(_colocate_puml_with_wrapper(ctx, ctx.files.static, ctx.label.name)), - depset(_colocate_puml_with_wrapper(ctx, ctx.files.dynamic, ctx.label.name)), - depset(_colocate_puml_with_wrapper(ctx, ctx.files.public_api, ctx.label.name)), - depset(_colocate_puml_with_wrapper(ctx, ctx.files.internal_api, ctx.label.name)), - ], + # All diagrams of this target share one flat fbs/lobster/idmap namespace + # (keyed by ctx.label.name), so stems must be disambiguated across all + # four views together, not per-view. + stems = _disambiguated_stems( + ctx, + ctx.files.static + ctx.files.dynamic + ctx.files.public_api + ctx.files.internal_api, ) + view_fbs = {} + view_fbs_files = {} + view_lobster = {} + view_idmap = {} + view_indexes = {} + view_source_files = [] + view_sphinx_srcs = [] + view_root_indexes = [] + view_aux_docs = [] + + for view_name, root_title in _VIEWS.items(): + view_files = getattr(ctx.files, view_name) + + fbs_list, lobster_list, idmap_list = _parse_puml_diagrams(ctx, view_files, stems) + view_fbs[view_name] = depset(fbs_list) + view_fbs_files[view_name] = fbs_list + view_lobster[view_name] = lobster_list + view_idmap[view_name] = idmap_list + + # Reconcile generated wrappers/indexes against any hand-authored + # rst/md pages before staging anything -- see plan_view_layout for + # the compose/override rules. + plan = plan_view_layout(view_files, ctx.label.package, ctx.label.workspace_name) + if plan.errors: + fail("architectural_design {} view '{}': {}".format(ctx.label, view_name, "; ".join(plan.errors))) + + # Colocate every source file of this view (diagrams, hand-written + # rst/md pages, assets) under its own "{name}/{view}/" tree, mirroring + # on-disk directory structure -- see _colocate_view_files for why + # this is required for `.. uml::`/toctree sibling references to + # resolve once dependable_element.bzl stages these files. + view_output_dir = "{}/{}".format(ctx.label.name, view_name) + colocated_files = _colocate_view_files(ctx, plan.staged, view_output_dir) + view_source_files.append(depset(colocated_files)) + + navigation = emit_view_navigation( + ctx, + plan, + view_output_dir, + ctx.file._puml_rst_template, + root_title, + colocated_files, + ) + view_indexes[view_name] = navigation if navigation.root_index else None + if navigation.root_index: + view_sphinx_srcs.append(depset(navigation.wrappers + navigation.indexes + [navigation.root_index])) + view_root_indexes.append(navigation.root_index) + + # Wrapper pages and non-root indexes must be staged (so the root + # index's nested toctrees resolve) but are not themselves + # top-level toctree entries -- only the view's root index is. + view_aux_docs.extend(navigation.wrappers + navigation.indexes) + + # Hand-written .rst/.md pages colocated as-is (not generated + # wrappers) are likewise reached only via the directory navigation's + # nested toctrees, never as direct top-level entries -- except the + # one that emit_view_navigation surfaced as the root index itself + # (a single-file view with no generated navigation), which must + # stay out of aux_docs or it would be staged as both a top-level + # entry and an aux doc. + view_aux_docs.extend([f for f in colocated_files if f.extension in ("rst", "md") and f != navigation.root_index]) + + static_fbs = view_fbs["static"] + dynamic_fbs = view_fbs["dynamic"] + public_api_fbs = view_fbs["public_api"] + internal_api_fbs = view_fbs["internal_api"] + public_api_lobster = depset(view_lobster["public_api"]) + + all_source_files = depset(transitive = view_source_files) + # All idmap sidecars (across static/dynamic/public_api/internal_api) are # staged into the sphinx sources so the `clickable_plantuml` extension can # discover them (it scans `srcdir` recursively for `*.idmap.json`) and # resolve cross-diagram links — including component diagrams linking to # the class diagrams that elaborate their public/internal API interfaces. all_idmap_files = depset( - static_idmap_list + dynamic_idmap_list + public_api_idmap_list + internal_api_idmap_list, + view_idmap["static"] + view_idmap["dynamic"] + view_idmap["public_api"] + view_idmap["internal_api"], ) sphinx_files = depset( transitive = [all_idmap_files, all_source_files], ) - # Generate a thin RST wrapper for every .puml diagram so it appears as a - # toctree entry in the dependable_element index. - static_wrappers = make_puml_rst_wrappers( - ctx, - ctx.files.static, - ctx.label.name, - ctx.file._puml_rst_template, - ) - dynamic_wrappers = make_puml_rst_wrappers( - ctx, - ctx.files.dynamic, - ctx.label.name, - ctx.file._puml_rst_template, - ) - public_api_wrappers = make_puml_rst_wrappers( - ctx, - ctx.files.public_api, - ctx.label.name, - ctx.file._puml_rst_template, - ) - internal_api_wrappers = make_puml_rst_wrappers( - ctx, - ctx.files.internal_api, - ctx.label.name, - ctx.file._puml_rst_template, - ) - - rst_wrappers = static_wrappers + dynamic_wrappers + public_api_wrappers + internal_api_wrappers - - def _get_doc_files(files, wrappers): - docs = [f for f in files if f.extension in ("rst", "md")] - return depset(docs + wrappers) - - static_doc_files = _get_doc_files(ctx.files.static, static_wrappers) - dynamic_doc_files = _get_doc_files(ctx.files.dynamic, dynamic_wrappers) - public_api_doc_files = _get_doc_files(ctx.files.public_api, public_api_wrappers) - internal_api_doc_files = _get_doc_files(ctx.files.internal_api, internal_api_wrappers) - validation_log = _run_validation( ctx, - static_fbs_list, - dynamic_fbs_list, - public_api_fbs_list, - internal_api_fbs_list, + view_fbs_files["static"], + view_fbs_files["dynamic"], + view_fbs_files["public_api"], + view_fbs_files["internal_api"], ) - sphinx_srcs = depset(rst_wrappers, transitive = [sphinx_files]) + # `deps` carries everything needed in the Sphinx tree for this rule + # (colocated sources, idmap sidecars, wrappers, and indexes at every + # level). `srcs` is only each view's top-level root index -- the single + # toctree entry dependable_element.bzl surfaces per view -- and + # `aux_srcs` are the wrapper/sub-index/hand-written pages that must be + # staged but reached only via that root index's own nested toctrees. + sphinx_deps = depset(transitive = [sphinx_files] + view_sphinx_srcs) + sphinx_own_srcs = depset(view_root_indexes) + sphinx_aux_srcs = depset(view_aux_docs) return [ DefaultInfo(files = depset([validation_log.file], transitive = [all_source_files])), @@ -300,19 +361,18 @@ def _architectural_design_impl(ctx): dynamic = dynamic_fbs, public_api = public_api_fbs, internal_api = internal_api_fbs, - static_doc_files = static_doc_files, - dynamic_doc_files = dynamic_doc_files, - public_api_doc_files = public_api_doc_files, - internal_api_doc_files = internal_api_doc_files, + view_indexes = view_indexes, name = ctx.label.name, public_api_lobster_files = public_api_lobster, validation_logs = [validation_log], ), - # Source diagram files + *.idmap.json sidecars for the sphinx documentation build + # Each view's root index is the only top-level toctree entry; + # everything else (wrappers, sub-indexes, idmap sidecars, colocated + # sources) is staged via aux_srcs/deps for the sphinx documentation build. SphinxSourcesInfo( - srcs = sphinx_srcs, - deps = sphinx_srcs, - aux_srcs = depset(), + srcs = sphinx_own_srcs, + deps = sphinx_deps, + aux_srcs = sphinx_aux_srcs, ), ] @@ -333,7 +393,7 @@ def _architectural_design_attrs(): doc = "Dynamic architecture diagrams (sequence diagrams, activity diagrams, etc.)", ), "public_api": attr.label_list( - allow_files = [".puml", ".plantuml"], + allow_files = [".puml", ".plantuml", ".svg", ".rst", ".md"], mandatory = False, doc = "Public API diagrams (parsed identically to static/dynamic). " + "Classified separately so their lobster items are exposed via " + @@ -341,7 +401,7 @@ def _architectural_design_attrs(): "traceability at the dependable element level.", ), "internal_api": attr.label_list( - allow_files = [".puml", ".plantuml"], + allow_files = [".puml", ".plantuml", ".svg", ".rst", ".md"], mandatory = False, doc = "Internal API diagrams (class diagrams). " + "Classified separately so their FlatBuffers outputs are exposed via " + @@ -394,6 +454,16 @@ def architectural_design( the structural organization (classes, components, modules), while dynamic views show the behavioral aspects (sequences, activities, states). + Each view's diagrams are auto-wrapped and organized into a directory- + matching navigation tree (one generated index.rst per source directory). + A hand-authored index.rst/index.md placed alongside diagrams composes + with (its text is included above) that directory's generated toctree, + rather than being replaced by it. A hand-authored .rst/.md + next to a same-named .puml suppresses that diagram's generated + wrapper page, so real authored prose is always used over the generated + placeholder. For full control over a diagram's page, omit the .puml from + the view attribute below and reference it with your own `.. uml::`. + Args: name: The name of the architectural design target. Used as the base name for all generated targets. diff --git a/bazel/rules/rules_score/private/dependable_element.bzl b/bazel/rules/rules_score/private/dependable_element.bzl index aee6960c..928ab818 100644 --- a/bazel/rules/rules_score/private/dependable_element.bzl +++ b/bazel/rules/rules_score/private/dependable_element.bzl @@ -129,6 +129,17 @@ _INTEGRITY_LEVEL_RANK = {level: rank for rank, level in enumerate(_INTEGRITY_LEV # Helper Functions for Documentation Generation # ============================================================================ +# View name -> display title, mirroring architectural_design.bzl's own +# (private) _VIEWS mapping; kept in this same static/dynamic/public_api/ +# internal_api order so software_arch.rst's subsections appear in a stable, +# predictable order regardless of dict iteration order elsewhere. +_ARCH_VIEW_TITLES = [ + ("static", "Static Design"), + ("dynamic", "Dynamic Design"), + ("public_api", "Public API"), + ("internal_api", "Internal API"), +] + def _make_toctree(caption, entries, maxdepth = 1): """Return a toctree RST block with a caption, or empty string if entries is empty.""" if not entries: @@ -267,6 +278,48 @@ def _is_document_file(file): """ return file.extension in ["rst", "md"] +def _check_staged_path(seen_paths, declared_relative_path, source_label, errors): + """Record one path this dependable_element is about to stage, appending a + readable message (naming both source labels) to `errors` if a different + label already claimed the same path. + + Bazel's own `declare_file()` collision ("conflicting actions") error + names only actions/outputs, not the two source labels a documentation + author would actually need to fix -- so callers `fail()` on `errors` + themselves once all staging for this rule has been planned, rather than + letting that raw error surface first. + + Args: + seen_paths: Dict from declared relative path (e.g. + "architectural_design/foo/index.rst") to the label (as a string) + that already staged it this rule instantiation; mutated in place. + declared_relative_path: The path this file is about to be staged at, + relative to `ctx.label.name`. + source_label: The `Label` this file came from. + errors: List of human-readable collision messages; mutated in place. + """ + existing = seen_paths.get(declared_relative_path) + if existing != None: + if existing == str(source_label): + # Reachable within one label: _compute_relative_path falls back to + # the bare basename for files outside the label's common_dir. + errors.append( + "'{}' would be staged twice from {}; two of its files resolve to the same relative path -- rename one".format( + declared_relative_path, + source_label, + ), + ) + else: + errors.append( + "'{}' would be staged by both {} and {}; rename one of the conflicting files, or give the colliding label its own subdirectory".format( + declared_relative_path, + existing, + source_label, + ), + ) + return + seen_paths[declared_relative_path] = str(source_label) + def _create_artifact_symlink(ctx, artifact_name, artifact_file, relative_path): """Create symlink for artifact file in output directory. @@ -290,13 +343,22 @@ def _create_artifact_symlink(ctx, artifact_name, artifact_file, relative_path): return output_file -def _process_artifact_files(ctx, artifact_name, label): +def _process_artifact_files(ctx, artifact_name, label, seen_paths, errors, path_prefix = ""): """Process all files from a single label for a given artifact type. Args: ctx: Rule context artifact_name: Name of artifact type label: Label to process + seen_paths: Dict accumulating declared_relative_path -> source label + across every artifact section this dependable_element stages, so + cross-label collisions are caught with a readable message (see + _check_staged_path); mutated in place. + errors: List collecting collision messages from `seen_paths`; + mutated in place. + path_prefix: Optional prefix inserted before each file's relative + path, to disambiguate output paths when 2+ labels attach to the + same artifact_name attribute. Returns: Tuple of (output_files, index_references) @@ -325,14 +387,14 @@ def _process_artifact_files(ctx, artifact_name, label): # Process regular deps files for artifact_file in doc_files: - # Compute paths - relative_path = _compute_relative_path(artifact_file, common_dir) - # Document files (rst/md) that are transitive deps but not owned by # this rule are already placed in their own artifact section. if _is_document_file(artifact_file) and artifact_file.path not in srcs_paths: continue + relative_path = path_prefix + _compute_relative_path(artifact_file, common_dir) + _check_staged_path(seen_paths, artifact_name + "/" + relative_path, label.label, errors) + # Create symlink output_file = _create_artifact_symlink( ctx, @@ -350,7 +412,8 @@ def _process_artifact_files(ctx, artifact_name, label): # Process aux_srcs: symlink without adding to outer toctree index. for artifact_file in aux_files: - relative_path = _compute_relative_path(artifact_file, common_dir) + relative_path = path_prefix + _compute_relative_path(artifact_file, common_dir) + _check_staged_path(seen_paths, artifact_name + "/" + relative_path, label.label, errors) output_file = _create_artifact_symlink( ctx, artifact_name, @@ -361,9 +424,23 @@ def _process_artifact_files(ctx, artifact_name, label): return (output_files, index_refs) -def _process_architectural_design_files(ctx, label): +def _process_architectural_design_files(ctx, label, seen_paths, errors, path_prefix = ""): """Process all files from an architectural_design label, returning output_files and classified refs. + Args: + ctx: Rule context + label: The architectural_design label to process. + seen_paths: Dict accumulating declared_relative_path -> source label + across every artifact section this dependable_element stages + (see _process_artifact_files); mutated in place. + errors: List collecting collision messages; mutated in place. + path_prefix: Prefix (the target name, e.g. "my_arch_design/") inserted + before each file's relative path. Applied unconditionally --not + only when 2+ architectural_design labels are attached-- so the + staged layout, and therefore the generated HTML URLs, is the same + whether a dependable_element has one architectural_design label or + several, rather than flat for one and nested for several. + Returns: Tuple of (output_files, static_refs, dynamic_refs, public_api_refs, internal_api_refs, unclassified_refs) """ @@ -387,28 +464,24 @@ def _process_architectural_design_files(ctx, label): srcs_paths = {f.path: True for f in label[SphinxSourcesInfo].srcs.to_list()} common_dir = _find_common_directory(doc_files + aux_files) - static_paths = {} - dynamic_paths = {} - public_api_paths = {} - internal_api_paths = {} - + # Each view's top-level root index (the only file of that view present + # in srcs_paths) is the single toctree entry surfaced for that view; map + # its path back to the view name to classify it below. + view_by_path = {} if ArchitecturalDesignInfo in label: info = label[ArchitecturalDesignInfo] - if hasattr(info, "static_doc_files") and info.static_doc_files: - static_paths = {f.path: True for f in info.static_doc_files.to_list()} - if hasattr(info, "dynamic_doc_files") and info.dynamic_doc_files: - dynamic_paths = {f.path: True for f in info.dynamic_doc_files.to_list()} - if hasattr(info, "public_api_doc_files") and info.public_api_doc_files: - public_api_paths = {f.path: True for f in info.public_api_doc_files.to_list()} - if hasattr(info, "internal_api_doc_files") and info.internal_api_doc_files: - internal_api_paths = {f.path: True for f in info.internal_api_doc_files.to_list()} + if hasattr(info, "view_indexes"): + for view_name, navigation in info.view_indexes.items(): + if navigation and navigation.root_index: + view_by_path[navigation.root_index.path] = view_name for artifact_file in doc_files: - relative_path = _compute_relative_path(artifact_file, common_dir) - if _is_document_file(artifact_file) and artifact_file.path not in srcs_paths: continue + relative_path = path_prefix + _compute_relative_path(artifact_file, common_dir) + _check_staged_path(seen_paths, "architectural_design/" + relative_path, label.label, errors) + output_file = _create_artifact_symlink( ctx, "architectural_design", @@ -420,19 +493,21 @@ def _process_architectural_design_files(ctx, label): if _is_document_file(artifact_file): doc_path = "architectural_design/" + relative_path doc_ref = doc_path.removesuffix(".rst").removesuffix(".md") - if artifact_file.path in static_paths: + view_name = view_by_path.get(artifact_file.path) + if view_name == "static": static_refs.append(doc_ref) - elif artifact_file.path in dynamic_paths: + elif view_name == "dynamic": dynamic_refs.append(doc_ref) - elif artifact_file.path in public_api_paths: + elif view_name == "public_api": public_api_refs.append(doc_ref) - elif artifact_file.path in internal_api_paths: + elif view_name == "internal_api": internal_api_refs.append(doc_ref) else: unclassified_refs.append(doc_ref) for artifact_file in aux_files: - relative_path = _compute_relative_path(artifact_file, common_dir) + relative_path = path_prefix + _compute_relative_path(artifact_file, common_dir) + _check_staged_path(seen_paths, "architectural_design/" + relative_path, label.label, errors) output_file = _create_artifact_symlink( ctx, "architectural_design", @@ -475,50 +550,32 @@ def _generate_software_arch_page( " " + "\n ".join(feature_req_refs), "", ]) - if static_refs: - lines.extend([ - "Static Design", - "~~~~~~~~~~~~~", - "", - ".. toctree::", - " :maxdepth: 1", - "", - " " + "\n ".join(static_refs), - "", - ]) - if dynamic_refs: - lines.extend([ - "Dynamic Design", - "~~~~~~~~~~~~~~", - "", - ".. toctree::", - " :maxdepth: 1", - "", - " " + "\n ".join(dynamic_refs), - "", - ]) - if public_api_refs: - lines.extend([ - "Public API", - "~~~~~~~~~~", - "", - ".. toctree::", - " :maxdepth: 1", - "", - " " + "\n ".join(public_api_refs), - "", - ]) - if internal_api_refs: + + # Mirrors architectural_design.bzl's _VIEWS mapping (view name -> + # display title); iterated in the same static/dynamic/public_api/ + # internal_api order so each view gets its own subsection when it + # has at least one ref (normally just its one root index entry). + refs_by_view = { + "static": static_refs, + "dynamic": dynamic_refs, + "public_api": public_api_refs, + "internal_api": internal_api_refs, + } + for view_name, view_title in _ARCH_VIEW_TITLES: + view_refs = refs_by_view[view_name] + if not view_refs: + continue lines.extend([ - "Internal API", - "~~~~~~~~~~~~", + view_title, + "~" * len(view_title), "", ".. toctree::", " :maxdepth: 1", "", - " " + "\n ".join(internal_api_refs), + " " + "\n ".join(view_refs), "", ]) + if unclassified_refs: lines.extend([ "Other Architectural Design", @@ -559,12 +616,16 @@ def _generate_software_arch_page( output_files.append(page) return "software_arch" -def _process_artifact_type(ctx, artifact_name): +def _process_artifact_type(ctx, artifact_name, seen_paths, errors): """Process all labels for a given artifact type. Args: ctx: Rule context artifact_name: Name of artifact type (e.g., "architectural_design") + seen_paths: Dict accumulating declared_relative_path -> source label + across every artifact section (see _process_artifact_files); + passed through unchanged. + errors: List collecting collision messages; mutated in place. Returns: Tuple of (output_files, index_references) @@ -576,12 +637,25 @@ def _process_artifact_type(ctx, artifact_name): if not attr_list: return (output_files, index_refs) - # Process each label + # Each label's files are staged relative to its own common_dir (see + # _find_common_directory), so 2+ labels can genuinely resolve the same + # relative path (e.g. two "checklists" labels each exporting a top-level + # "checklist.md") even though this artifact type has no per-directory + # index.rst of its own. Namespace under each label's own target name in + # that case; a single label keeps the flat (unprefixed) layout for + # readability. The _check_staged_path safety net below still catches the + # residual case of two labels sharing a target *name* from different + # packages. + use_label_subdirectories = len(attr_list) > 1 for label in attr_list: + path_prefix = "{}/".format(label.label.name) if use_label_subdirectories else "" label_outputs, label_refs = _process_artifact_files( ctx, artifact_name, label, + seen_paths, + errors, + path_prefix = path_prefix, ) output_files.extend(label_outputs) index_refs.extend(label_refs) @@ -981,6 +1055,13 @@ def _dependable_element_index_impl(ctx): index_rst = ctx.actions.declare_file(ctx.label.name + "/index.rst") output_files = [index_rst] + # Accumulates declared_relative_path -> source label across every + # artifact section below, so two different labels staging to the same + # relative path fail with a readable message naming both. See + # _check_staged_path. + seen_staged_paths = {} + staging_errors = [] + # Process each well-known artifact type into symlinked output files and # toctree references for the index template. artifact_types = [ @@ -992,7 +1073,7 @@ def _dependable_element_index_impl(ctx): artifacts_by_type = {} for artifact_name in artifact_types: - files, refs = _process_artifact_type(ctx, artifact_name) + files, refs = _process_artifact_type(ctx, artifact_name, seen_paths = seen_staged_paths, errors = staging_errors) output_files.extend(files) artifacts_by_type[artifact_name] = refs @@ -1003,8 +1084,21 @@ def _dependable_element_index_impl(ctx): arch_unclassified_refs = [] if ctx.attr.architectural_design: + # Namespace every architectural_design label's output under its own + # "/" subdirectory, so a dependable_element's staged + # layout -- and therefore its generated HTML URLs -- keeps the same + # shape however many labels are attached. The _check_staged_path + # safety net below still catches the residual case of two labels + # sharing a target *name* from different packages. for ad_target in ctx.attr.architectural_design: - ad_files, s_refs, d_refs, p_refs, i_refs, u_refs = _process_architectural_design_files(ctx, ad_target) + path_prefix = "{}/".format(ad_target.label.name) + ad_files, s_refs, d_refs, p_refs, i_refs, u_refs = _process_architectural_design_files( + ctx, + ad_target, + seen_staged_paths, + staging_errors, + path_prefix = path_prefix, + ) output_files.extend(ad_files) arch_static_refs.extend(s_refs) arch_dynamic_refs.extend(d_refs) @@ -1017,7 +1111,13 @@ def _dependable_element_index_impl(ctx): feature_req_refs = [] for req_target in ctx.attr.requirements: if FeatureRequirementsInfo in req_target: - label_files, label_refs = _process_artifact_files(ctx, "feature_requirements", req_target) + label_files, label_refs = _process_artifact_files( + ctx, + "feature_requirements", + req_target, + seen_paths = seen_staged_paths, + errors = staging_errors, + ) output_files.extend(label_files) feature_req_refs.extend(label_refs) @@ -1026,10 +1126,19 @@ def _dependable_element_index_impl(ctx): assumed_system_req_refs = [] for req_target in ctx.attr.requirements: if AssumedSystemRequirementsInfo in req_target: - label_files, label_refs = _process_artifact_files(ctx, "assumed_system_requirements", req_target) + label_files, label_refs = _process_artifact_files( + ctx, + "assumed_system_requirements", + req_target, + seen_paths = seen_staged_paths, + errors = staging_errors, + ) output_files.extend(label_files) assumed_system_req_refs.extend(label_refs) + if staging_errors: + fail("dependable_element {}: {}".format(ctx.label, "; ".join(staging_errors))) + # Collect all units recursively from components all_units = _collect_units_recursive(ctx.attr.components) diff --git a/bazel/rules/rules_score/private/puml_utils.bzl b/bazel/rules/rules_score/private/puml_utils.bzl index 5673f5ec..f59fb484 100644 --- a/bazel/rules/rules_score/private/puml_utils.bzl +++ b/bazel/rules/rules_score/private/puml_utils.bzl @@ -11,56 +11,353 @@ # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* -"""Shared helper for generating RST wrapper pages for PlantUML diagram files.""" +"""Shared helper for generating RST wrapper pages and per-directory navigation +indexes for one architectural design view (static/dynamic/public_api/internal_api).""" -def make_puml_rst_wrappers(ctx, puml_files, output_dir, template, strip_prefix = "", filename_prefix = ""): - """Generate a thin RST wrapper page for each PlantUML diagram file. +load("@bazel_skylib//lib:paths.bzl", "paths") - The wrapper embeds the diagram via ``.. uml::`` so it appears as a - proper toctree entry while keeping the source ``.puml`` file separate. +def relative_source_path(file, package, own_repo = None): + """Return `file`'s path relative to `package`, or its full workspace-relative + short_path when it doesn't live under `package` -- never just the bare + basename, which would silently collide two same-named files that live in + different directories outside the package. + + `file.short_path` carries a "..//" marker whenever `file` doesn't + live in the build's *main* (root) repository -- even when it lives in the + same repository as the rule consuming it, e.g. this architectural_design + target's own package, when that target is itself built as someone else's + dependency rather than as the build's root module. When `own_repo` (the + consuming rule's own `ctx.label.workspace_name`) matches `file`'s owning + label's repository, that marker is stripped first so staging paths are + keyed off the rule's own repository, not whichever repository happens to + be the build's root. + """ + short_path = file.short_path + owner = file.owner + if own_repo and owner != None and owner.workspace_name == own_repo: + own_repo_marker = "../" + own_repo + "/" + if short_path.startswith(own_repo_marker): + short_path = short_path[len(own_repo_marker):] + prefix = package + "/" if package else "" + if prefix and short_path.startswith(prefix): + return short_path[len(prefix):] + return short_path + +def _directory_title(directory, root_title): + """Human-readable title for a directory's index page. + + The root directory ("") uses the caller-supplied `root_title` (e.g. the + view's display name, "Static Design"); nested directories are titled + after their own last path segment. + """ + if not directory: + return root_title + return directory.split("/")[-1].replace("_", " ").title() + +def plan_view_layout(view_files, package, own_repo = None): + """Plan how one architectural design view's files are staged, which + diagram wrappers are generated, and which per-directory navigation + indexes are generated -- reconciling generated navigation against any + hand-authored ``.rst``/``.md`` pages so neither is silently dropped nor + collides with the other on a staged path. + + Two coexistence rules: + + * A hand-authored ``index.rst``/``index.md`` in a directory is staged + under a non-document name (``index.rst.inc``/``index.md.inc``) and + ``.. include::``d into that directory's generated ``index.rst``. + Navigation completeness is a correctness property (a missing entry is + an orphaned page), so the generated index always owns that path; the + author's title becomes the page's title and its own text renders + above the generated toctree. + * A hand-authored ``.rst``/``.md`` alongside a same-stem + ``.puml``/``.plantuml`` suppresses that diagram's + generated wrapper -- the wrapper is only a placeholder for prose that + doesn't exist yet, so real authored content always wins. The ``.puml`` + is still staged as a sibling so the author's own ``.. uml::`` + resolves. + + A diagram literally named ``index`` is rejected via `errors`: that stem + is reserved for the directory's own navigation page. A file from another + repository -- one whose owning label's repository isn't `own_repo` -- + is also rejected via `errors`, since a symlinked staged path can't cross + repository roots. So are two files that would occupy the same staged + path. Rejected files are left out of `staged` entirely, so the returned + plan stays free of collisions even though callers are expected to + `fail()` on `errors`. Args: - ctx: Rule context. - puml_files: Iterable of File objects whose extension is ``puml`` or - ``plantuml``. - output_dir: String prefix for declared output files - (e.g. ``ctx.label.name``). - template: The ``puml_diagram.template.rst`` File (from - ``ctx.file._puml_rst_template``). - strip_prefix: Optional filename stem prefix to strip before deriving - the human-readable title (e.g. ``"fta_"``). - filename_prefix: Optional prefix prepended to the output RST filename - stem (e.g. ``"detail_"``). + view_files: Iterable of File objects for one architectural design + view (e.g. ``ctx.files.static``). + package: ``ctx.label.package`` of the rule instantiating this view. + own_repo: ``ctx.label.workspace_name`` of the rule instantiating this + view -- see `relative_source_path`'s docstring for why this + (not the build's main repository) is the right reference + point for "does this file live in the same repository as + the target". Returns: - List of declared ``.rst`` output Files, one per input diagram. + Struct with: + staged: List of ``(File, staged_relative_path)`` to be symlinked + into the view's output tree, one per accepted input file. + wrappers: List of ``(puml_file, relative_directory, stem)`` for + every diagram that still needs a generated RST wrapper. + indexes: List of struct(directory, entries, body_relative_path) + describing one generated per-directory index.rst; + `entries` is a sorted list of toctree entry stems relative + to `directory`, `body_relative_path` is the staged path of + an authored index body to include, or None. Directories + that would only link on to a single child are skipped, so + the parent links straight to the first descendant with + content. Includes the root directory (``""``); empty + (``[]``) if the view has no navigable files at all. + errors: List of human-readable collision messages. Callers must + ``fail()`` on these rather than let a downstream + `declare_file()` collision surface as an opaque Bazel + "conflicting actions" error. """ + directories = {} + stem_entries = {} + entries_by_directory = {} + index_bodies = {} + staged = [] + staged_paths = {} + errors = [] + + def _register_directory(relative_directory): + directories[relative_directory] = True + parts = relative_directory.split("/") if relative_directory else [] + for part_count in range(1, len(parts) + 1): + directories["/".join(parts[:part_count])] = True + + for f in view_files: + relative_path = relative_source_path(f, package, own_repo) + if relative_path.startswith("../"): + errors.append( + "'{}' lives outside this repository; architectural_design view files must live in the same repository as the target".format(f.short_path), + ) + continue + relative_directory = paths.dirname(relative_path) + stem = paths.basename(relative_path)[:-(len(f.extension) + 1)] if f.extension else paths.basename(relative_path) + + if f.extension in ("rst", "md") and stem == "index": + # Renamed so it's never picked up by Sphinx (source_suffix is + # only .rst/.md) as a standalone document of its own. + staged_path = relative_path + ".inc" + else: + staged_path = relative_path + + # Checked for every file, not just navigable ones: assets collide too, + # and the ".inc" rename above can collide with a literally-named + # "index.rst.inc" source. + previous = staged_paths.get(staged_path) + if previous != None: + errors.append( + "two files would both stage as '{}': '{}' and '{}'".format(staged_path, previous, f.short_path), + ) + continue + staged_paths[staged_path] = f.short_path + staged.append((f, staged_path)) + + if f.extension not in ("puml", "plantuml", "rst", "md"): + continue + + _register_directory(relative_directory) + stem_entries.setdefault((relative_directory, stem), {})[f.extension] = f + wrappers = [] - pkg_prefix = ctx.label.package + "/" if ctx.label.package else "" - for f in puml_files: - if f.extension not in ("puml", "plantuml"): + for (relative_directory, stem), group in stem_entries.items(): + if "rst" in group and "md" in group: + errors.append( + "both '{stem}.rst' and '{stem}.md' exist for '{stem}' in directory '{dir}'; keep only one".format( + stem = stem, + dir = relative_directory or ".", + ), + ) continue - stem = f.basename[:-(len(f.extension) + 1)] - if strip_prefix and stem.startswith(strip_prefix): - stem = stem[len(strip_prefix):] - title = stem.replace("_", " ").title() - rel_dir = "" - if pkg_prefix and f.short_path.startswith(pkg_prefix): - rel_path = f.short_path[len(pkg_prefix):] - if "/" in rel_path: - rel_dir = rel_path.rsplit("/", 1)[0] + if "puml" in group and "plantuml" in group: + errors.append( + "both '{stem}.puml' and '{stem}.plantuml' exist for '{stem}' in directory '{dir}'; keep only one".format( + stem = stem, + dir = relative_directory or ".", + ), + ) + continue + + doc_ext = "rst" if "rst" in group else ("md" if "md" in group else None) + doc_file = group.get(doc_ext) if doc_ext else None + puml_file = group.get("puml") or group.get("plantuml") + + if stem == "index": + if puml_file: + errors.append( + "'{}' is named 'index', which is reserved for the generated directory navigation page; rename it".format(puml_file.short_path), + ) + continue + if doc_file: + index_bodies[relative_directory] = relative_source_path(doc_file, package, own_repo) + ".inc" + continue + + if puml_file and not doc_file: + wrappers.append((puml_file, relative_directory, stem)) + entries_by_directory.setdefault(relative_directory, []).append(stem) + + if not directories: + return struct(staged = staged, wrappers = [], indexes = [], errors = errors) - out_file_path = "{}/{}/{}{}.rst".format(output_dir, rel_dir, filename_prefix, stem) if rel_dir else "{}/{}{}.rst".format(output_dir, filename_prefix, stem) - wrapper = ctx.actions.declare_file(out_file_path) + # Seeded only once a navigable file exists, so an empty view generates + # no index at all. + directories[""] = True + + def _children_of(directory): + directory_prefix = directory + "/" if directory else "" + return [ + child + for child in sorted(directories.keys()) + if child != directory and child.startswith(directory_prefix) and "/" not in child[len(directory_prefix):] + ] + + # A directory with no pages, no authored body and exactly one child + # contributes a navigation page whose only link is the next one down. + # Drop it and let its parent link straight through to its only + # descendant that has something to show. The root is always kept: it is + # the view's single entry point. + collapsed = {} + for directory in directories.keys(): + if not directory or index_bodies.get(directory) or entries_by_directory.get(directory): + continue + if len(_children_of(directory)) == 1: + collapsed[directory] = True + + def _resolve_entry(directory): + for _ in range(len(directories)): + if not collapsed.get(directory): + break + directory = _children_of(directory)[0] + return directory + + indexes = [] + for directory in sorted(directories.keys()): + if collapsed.get(directory): + continue + entries = list(entries_by_directory.get(directory, [])) + directory_prefix = directory + "/" if directory else "" + for child in _children_of(directory): + resolved = _resolve_entry(child) + entries.append(resolved[len(directory_prefix):] + "/index") + + indexes.append(struct( + directory = directory, + entries = sorted(entries), + body_relative_path = index_bodies.get(directory), + )) + + return struct(staged = staged, wrappers = wrappers, indexes = indexes, errors = errors) + +def emit_view_navigation(ctx, plan, output_dir, template, root_title, colocated_by_relative_path): + """Declare the wrapper and per-directory index files described by a + `plan_view_layout()` plan. + + Args: + ctx: Rule context. + plan: Struct returned by `plan_view_layout()`. + output_dir: String prefix for declared output files + (e.g. ``ctx.label.name``). + template: The ``puml_diagram.template.rst`` File (from + ``ctx.file._puml_rst_template``). + root_title: Title for the view's top-level index page (e.g. + ``"Static Design"``), used when that page has no + authored body of its own. + colocated_by_relative_path: Dict from `plan.staged` relative path to + its colocated File (see `_colocate_view_files`) -- needed + to resolve a single hand-authored page (not a generated + wrapper) as the root index, since that page's real + on-disk location is the colocated copy, not the + original source File. + + Returns: + Struct with: + wrappers: List of declared ``.rst`` wrapper Files, excluding + `root_index` if it turned out to be a wrapper. + indexes: List of declared per-directory ``index.rst`` Files, + excluding the root index. + root_index: The view's single top-level toctree entry: the + generated ``index.rst``, or -- when the whole view is + just one file with no authored index body, making that + index a redundant pass-through -- that one page + directly. None if `plan` has no indexes at all. + """ + wrappers = [] + wrapper_by_stem = {} + for puml_file, relative_directory, stem in plan.wrappers: + title = stem.replace("_", " ").title() + wrapper_relative_path = paths.join(relative_directory, stem + ".rst") + wrapper = ctx.actions.declare_file( + "{}/{}".format(output_dir, wrapper_relative_path), + ) ctx.actions.expand_template( template = template, output = wrapper, substitutions = { "{title}": title, "{underline}": "=" * len(title), - "{basename}": f.basename, + "{basename}": puml_file.basename, }, ) wrappers.append(wrapper) - return wrappers + if relative_directory == "": + wrapper_by_stem[stem] = wrapper + + if not plan.indexes: + return struct(wrappers = wrappers, indexes = [], root_index = None) + + # A view with exactly one navigable file and no authored index body has + # nothing to navigate: a generated root index would be a "" + # page linking only to that one page, repeating the same title three + # times in the sidebar alongside the caller's own section heading for + # this view. Surface that one page as the root index instead. + root_plan = plan.indexes[0] + if len(plan.indexes) == 1 and not root_plan.body_relative_path and len(root_plan.entries) == 1: + stem = root_plan.entries[0] + sole_page = ( + wrapper_by_stem.get(stem) or + colocated_by_relative_path.get(stem + ".rst") or + colocated_by_relative_path.get(stem + ".md") + ) + if sole_page != None: + return struct( + wrappers = [w for w in wrappers if w != sole_page], + indexes = [], + root_index = sole_page, + ) + + root_index = None + indexes = [] + for index_plan in plan.indexes: + if index_plan.body_relative_path: + # Compose with the authored body instead of emitting a title -- + # see plan_view_layout's docstring. + body_basename = paths.basename(index_plan.body_relative_path) + parser_option = "\n :parser: myst_parser.sphinx_" if body_basename.endswith(".md.inc") else "" + preamble = ".. include:: {}{}\n\n".format(body_basename, parser_option) + else: + title = _directory_title(index_plan.directory, root_title) + preamble = "{}\n{}\n\n".format(title, "-" * len(title)) + + index = ctx.actions.declare_file( + "{}/{}".format(output_dir, paths.join(index_plan.directory, "index.rst")), + ) + ctx.actions.write( + output = index, + content = "{}.. toctree::\n :maxdepth: 1\n\n{}\n".format( + preamble, + "\n".join([" " + entry for entry in index_plan.entries]), + ), + ) + if index_plan.directory == "": + root_index = index + else: + indexes.append(index) + + return struct(wrappers = wrappers, indexes = indexes, root_index = root_index) diff --git a/bazel/rules/rules_score/providers.bzl b/bazel/rules/rules_score/providers.bzl index 94fa80ab..658b761c 100644 --- a/bazel/rules/rules_score/providers.bzl +++ b/bazel/rules/rules_score/providers.bzl @@ -204,10 +204,7 @@ ArchitecturalDesignInfo = provider( "dynamic": "Depset of FlatBuffers binaries for dynamic architecture diagrams (sequence diagrams, activity diagrams, etc.)", "public_api": "Depset of FlatBuffers binaries for public API diagrams (class diagrams, etc.)", "internal_api": "Depset of FlatBuffers binaries for internal API diagrams (class diagrams, etc.)", - "static_doc_files": "Depset of Sphinx doc File objects for static architecture views.", - "dynamic_doc_files": "Depset of Sphinx doc File objects for dynamic architecture views.", - "public_api_doc_files": "Depset of Sphinx doc File objects for public API views.", - "internal_api_doc_files": "Depset of Sphinx doc File objects for internal API views.", + "view_indexes": "Dict mapping view name ('static', 'dynamic', 'public_api', 'internal_api') to that view's navigation struct (wrappers, indexes, root_index — see emit_view_navigation), or None for views with no navigable files.", "name": "Name of the architectural design target", "public_api_lobster_files": "Depset of .lobster traceability files generated from public_api diagrams.", "validation_logs": "List of validation log entries produced by this architectural design target. Each entry has file and name fields.", diff --git a/bazel/rules/rules_score/templates/conf.template.py b/bazel/rules/rules_score/templates/conf.template.py index c0747b9e..303301af 100644 --- a/bazel/rules/rules_score/templates/conf.template.py +++ b/bazel/rules/rules_score/templates/conf.template.py @@ -136,6 +136,14 @@ # HTML theme html_theme = "sphinx_rtd_theme" +# architectural_design views nest diagram pages arbitrarily deep by directory; +# the theme's defaults (collapse_navigation=True, navigation_depth=4) hide +# deeper levels behind an extra click and truncate very deep trees. +html_theme_options = { + "collapse_navigation": False, + "navigation_depth": -1, +} + # Note: version_flyout.css and version_flyout.js are injected by the # deploy workflow via _shared/ paths so they load once across all versions. diff --git a/bazel/rules/rules_score/test/BUILD b/bazel/rules/rules_score/test/BUILD index c8084df0..ebbeeb3c 100644 --- a/bazel/rules/rules_score/test/BUILD +++ b/bazel/rules/rules_score/test/BUILD @@ -50,6 +50,14 @@ load( ":lobster_config_test.bzl", "lobster_config_test_suite", ) +load( + ":puml_layout_crash_repro_test.bzl", + "puml_layout_crash_repro_test", +) +load( + ":puml_layout_test.bzl", + "puml_layout_test_suite", +) load( ":requirements_multi_spec_test.bzl", "asr_multi_spec_provider_test", @@ -275,6 +283,62 @@ architectural_design( static = ["fixtures/test_dependable_element_nested.puml"], ) +# Regression fixtures: a generated navigation page and an authored page +# claiming the same output path must be reconciled by plan_view_layout() +# rather than colliding in declare_file(); see +# puml_layout_crash_repro_test.bzl. +architectural_design( + name = "arch_design_authored_index_compose_repro", + static = [ + "fixtures/authored/index.rst", + "fixtures/authored/overview.puml", + ], +) + +architectural_design( + name = "arch_design_authored_rst_override_repro", + static = [ + "fixtures/authored_override/overview.puml", + "fixtures/authored_override/overview.rst", + ], +) + +puml_layout_crash_repro_test( + name = "puml_layout_authored_index_compose_builds", + target_under_test = ":arch_design_authored_index_compose_repro", +) + +puml_layout_crash_repro_test( + name = "puml_layout_authored_rst_override_builds", + target_under_test = ":arch_design_authored_rst_override_repro", +) + +# Wraps both crash-repro architectural_design targets above in a real +# dependable_element so the *staged, dependable-element-relative* RST content +# can be asserted on (not just "it builds without crashing") -- see +# :authored_layout_compose_test / :authored_layout_override_test below. +# +# The two targets deliberately use separate fixture directories: a .puml shared +# by two architectural_design targets of one dependable_element yields two +# idmaps with the same --source-name, which clickable_plantuml rejects. +dependable_element( + name = "authored_layout_example_lib", + architectural_design = [ + ":arch_design_authored_index_compose_repro", + ":arch_design_authored_rst_override_repro", + ], + assumptions_of_use = [":aous"], + components = [], + dependability_analysis = [":dependability_analysis_target"], + integrity_level = "B", + # Purely illustrative fixture (not a real SEooC), same reasoning as + # clickable_example_lib above. + maturity = "development", + requirements = [":feat_req"], + tests = [], + deps = [], +) + # - Safety Analysis (DFA): wp__sw_component_dfa # - Safety Analysis (FMEA): wp__sw_component_fmea dependability_analysis( @@ -1005,6 +1069,35 @@ sh_test( tags = ["manual"], ) +# Regression test at the level of the actual staged RST that Sphinx will read: +# an authored index.rst *composes* with the generated toctree (its title/prose +# is included, the diagram wrapper is still listed) -- see plan_view_layout()'s +# docstring in puml_utils.bzl. Real HTML rendering isn't exercised here since +# the `plantuml` renderer binary isn't runnable in this sandbox (see +# check_clickable_example_link.sh above for the same caveat). +sh_test( + name = "authored_layout_compose_test", + srcs = ["check_authored_layout_content.sh"], + args = [ + "compose", + "$(rootpaths :authored_layout_example_lib_index)", + ], + data = [":authored_layout_example_lib_index"], +) + +# Regression test: an authored overview.rst *overrides* (suppresses) the +# generated ".. uml::" wrapper page for the same-named overview.puml, so the +# staged page is the hand-authored prose verbatim, not the placeholder. +sh_test( + name = "authored_layout_override_test", + srcs = ["check_authored_layout_content.sh"], + args = [ + "override", + "$(rootpaths :authored_layout_example_lib_index)", + ], + data = [":authored_layout_example_lib_index"], +) + # ============================================================================ # SEooC-Specific Tests # ============================================================================ @@ -1019,7 +1112,10 @@ seooc_artifacts_copied_test( sh_test( name = "seooc_tests_dep_links_use_doc_dir", srcs = ["check_seooc_dep_links.sh"], - args = ["$(rootpaths :seooc_test_lib_index)"], + args = [ + "seooc_test_lib_index/index.rst", + "$(rootpaths :seooc_test_lib_index)", + ], data = [":seooc_test_lib_index"], ) @@ -1513,6 +1609,11 @@ requirements_multi_spec_test_suite(name = "requirements_multi_spec_tests") # ============================================================================ lobster_config_test_suite(name = "lobster_config") +# ============================================================================ +# puml_utils.bzl unit tests (plan_view_layout) +# ============================================================================ +puml_layout_test_suite(name = "puml_layout") + # ============================================================================ # Image srcs Tests (defined in sub-package to avoid workspace-root edge case) # ============================================================================ diff --git a/bazel/rules/rules_score/test/check_authored_layout_content.sh b/bazel/rules/rules_score/test/check_authored_layout_content.sh new file mode 100755 index 00000000..4c2fddb4 --- /dev/null +++ b/bazel/rules/rules_score/test/check_authored_layout_content.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +set -euo pipefail + +# Regression test for the compose/override reconciliation, at the level of the +# actual *staged* RST content dependable_element produces for Sphinx to consume +# -- see plan_view_layout()'s docstring in puml_utils.bzl. +# +# $1 selects which scenario to check: "compose" or "override". +# Remaining args are the `$(rootpaths :authored_layout_example_lib_index)` +# runfiles paths. + +mode="$1" +shift + +find_file() { + local suffix="$1" + shift + for rel_path in "$@"; do + candidate="${TEST_SRCDIR}/${TEST_WORKSPACE}/${rel_path}" + if [[ -f "${candidate}" && "${candidate}" == *"${suffix}" ]]; then + echo "${candidate}" + return 0 + fi + done + echo "Error: could not locate '*${suffix}' among: $*" >&2 + return 1 +} + +case "${mode}" in + compose) + index_file=$(find_file "arch_design_authored_index_compose_repro/static/fixtures/authored/index.rst" "$@") + inc_file=$(find_file "arch_design_authored_index_compose_repro/static/fixtures/authored/index.rst.inc" "$@") + + # The generated index.rst must include the authored body instead of + # emitting its own title/marker. + if ! grep -Fq '.. include:: index.rst.inc' "${index_file}"; then + echo "Error: expected 'compose' index.rst to include the authored body:" >&2 + cat "${index_file}" >&2 + exit 1 + fi + + # The generated toctree must still list the auto-wrapped diagram. + if ! grep -q '^ overview$' "${index_file}"; then + echo "Error: expected 'compose' index.rst toctree to still list 'overview':" >&2 + cat "${index_file}" >&2 + exit 1 + fi + + # The staged .inc file must carry the authored title/prose verbatim. + if ! grep -Fq 'Authored Overview' "${inc_file}"; then + echo "Error: expected staged index.rst.inc to carry the authored title/prose:" >&2 + cat "${inc_file}" >&2 + exit 1 + fi + ;; + override) + overview_file=$(find_file "arch_design_authored_rst_override_repro/static/fixtures/authored_override/overview.rst" "$@") + + # The staged overview.rst must be the authored file verbatim... + if ! grep -Fq 'Hand-authored prose for the overview diagram' "${overview_file}"; then + echo "Error: expected staged overview.rst to be the authored file, not a generated wrapper:" >&2 + cat "${overview_file}" >&2 + exit 1 + fi + + # ...not the generated ".. uml::" wrapper placeholder it suppresses. + if grep -Fq '.. uml::' "${overview_file}"; then + echo "Error: staged overview.rst still contains the generated '.. uml::' wrapper directive; authored override should have suppressed it:" >&2 + cat "${overview_file}" >&2 + exit 1 + fi + ;; + *) + echo "Error: unknown mode '${mode}' (expected 'compose' or 'override')" >&2 + exit 1 + ;; +esac + +echo "ok" diff --git a/bazel/rules/rules_score/test/check_seooc_dep_links.sh b/bazel/rules/rules_score/test/check_seooc_dep_links.sh index 94a8f7b2..b266eba0 100755 --- a/bazel/rules/rules_score/test/check_seooc_dep_links.sh +++ b/bazel/rules/rules_score/test/check_seooc_dep_links.sh @@ -13,19 +13,18 @@ # ******************************************************************************* set -euo pipefail -index_file="" -for rel_path in "$@"; do - candidate="${TEST_SRCDIR}/${TEST_WORKSPACE}/${rel_path}" - if [[ -f "${candidate}" && "${candidate}" == */index.rst ]]; then - index_file="${candidate}" - break - fi -done +# $1 is the expected index.rst path (suffix match against runfiles paths), +# e.g. "seooc_test_lib_index/index.rst" -- required because dependable_element +# generates many index.rst files (one per architectural_design view/directory +# plus its own top-level index), so a bare "*/index.rst" suffix match is +# ambiguous. -if [[ -z "${index_file}" ]]; then - echo "Error: Could not locate index.rst in provided runfiles paths: $*" >&2 - exit 1 -fi +source "${TEST_SRCDIR}/${TEST_WORKSPACE}/lib/find_runfile.sh" + +expected_suffix="$1" +shift + +index_file=$(find_runfile "${expected_suffix}" "$@") if ! grep -Fq '* `Dep Seooc Lib `_' "${index_file}"; then echo "Error: expected submodule link to dep_seooc_lib_doc/index.html in ${index_file}" >&2 diff --git a/bazel/rules/rules_score/test/fixtures/authored/index.rst b/bazel/rules/rules_score/test/fixtures/authored/index.rst new file mode 100644 index 00000000..93149dd5 --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/authored/index.rst @@ -0,0 +1,19 @@ +.. + # ******************************************************************************* + # Copyright (c) 2026 Contributors to the Eclipse Foundation + # + # See the NOTICE file(s) distributed with this work for additional + # information regarding copyright ownership. + # + # This program and the accompanying materials are made available under the + # terms of the Apache License Version 2.0 which is available at + # https://www.apache.org/licenses/LICENSE-2.0 + # + # SPDX-License-Identifier: Apache-2.0 + # ******************************************************************************* + +Authored Overview +================= + +This directory's navigation index has hand-authored prose above the +auto-generated toctree (compose, not override). diff --git a/bazel/rules/rules_score/test/fixtures/authored/overview.puml b/bazel/rules/rules_score/test/fixtures/authored/overview.puml new file mode 100644 index 00000000..301ed612 --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/authored/overview.puml @@ -0,0 +1,16 @@ +' ******************************************************************************* +' Copyright (c) 2026 Contributors to the Eclipse Foundation +' +' See the NOTICE file(s) distributed with this work for additional +' information regarding copyright ownership. +' +' This program and the accompanying materials are made available under the +' terms of the Apache License Version 2.0 which is available at +' https://www.apache.org/licenses/LICENSE-2.0 +' +' SPDX-License-Identifier: Apache-2.0 +' ******************************************************************************* + +@startuml overview +component Overview +@enduml diff --git a/bazel/rules/rules_score/test/fixtures/authored_override/overview.puml b/bazel/rules/rules_score/test/fixtures/authored_override/overview.puml new file mode 100644 index 00000000..66675d24 --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/authored_override/overview.puml @@ -0,0 +1,16 @@ +' ******************************************************************************* +' Copyright (c) 2026 Contributors to the Eclipse Foundation +' +' See the NOTICE file(s) distributed with this work for additional +' information regarding copyright ownership. +' +' This program and the accompanying materials are made available under the +' terms of the Apache License Version 2.0 which is available at +' https://www.apache.org/licenses/LICENSE-2.0 +' +' SPDX-License-Identifier: Apache-2.0 +' ******************************************************************************* + +@startuml overview_override +component OverviewOverride +@enduml diff --git a/bazel/rules/rules_score/test/fixtures/authored_override/overview.rst b/bazel/rules/rules_score/test/fixtures/authored_override/overview.rst new file mode 100644 index 00000000..3a73b479 --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/authored_override/overview.rst @@ -0,0 +1,19 @@ +.. + # ******************************************************************************* + # Copyright (c) 2026 Contributors to the Eclipse Foundation + # + # See the NOTICE file(s) distributed with this work for additional + # information regarding copyright ownership. + # + # This program and the accompanying materials are made available under the + # terms of the Apache License Version 2.0 which is available at + # https://www.apache.org/licenses/LICENSE-2.0 + # + # SPDX-License-Identifier: Apache-2.0 + # ******************************************************************************* + +Overview +======== + +Hand-authored prose for the overview diagram; this suppresses the +generated wrapper page for ``overview.puml`` in this directory. diff --git a/bazel/rules/rules_score/test/puml_layout_crash_repro_test.bzl b/bazel/rules/rules_score/test/puml_layout_crash_repro_test.bzl new file mode 100644 index 00000000..2bc898ae --- /dev/null +++ b/bazel/rules/rules_score/test/puml_layout_crash_repro_test.bzl @@ -0,0 +1,50 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +""" +Regression tests proving two previously-crashing architectural_design configs +now build successfully, per plan_view_layout()'s compose/override reconciliation +(bazel/rules/rules_score/private/puml_utils.bzl): + +- An authored directory `index.rst` alongside a `.puml` diagram in the same + view (compose case): used to collide with the generated `index.rst` via a + raw `declare_file()` "conflicting actions" error. +- An authored same-stem `overview.rst` alongside `overview.puml` (override + case): used to collide with the generated wrapper page the same way. + +Each analysistest below simply asserts `ArchitecturalDesignInfo` is present on +the target under test; if `plan_view_layout`/`_architectural_design_impl` still +raised a raw declare_file collision (or a `plan.errors` fail()), the +`architectural_design` target's own analysis would fail and the wrapping +analysistest target would fail to build -- so a passing test here is itself +the regression proof, not just the assertion's literal content. +""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("@score_tooling//bazel/rules/rules_score:providers.bzl", "ArchitecturalDesignInfo") + +def _puml_layout_crash_repro_test_impl(ctx): + env = analysistest.begin(ctx) + target_under_test = analysistest.target_under_test(env) + + asserts.true( + env, + ArchitecturalDesignInfo in target_under_test, + "Expected architectural_design to provide ArchitecturalDesignInfo " + + "(i.e. to have built successfully at all)", + ) + + return analysistest.end(env) + +puml_layout_crash_repro_test = analysistest.make( + impl = _puml_layout_crash_repro_test_impl, +) diff --git a/bazel/rules/rules_score/test/puml_layout_test.bzl b/bazel/rules/rules_score/test/puml_layout_test.bzl new file mode 100644 index 00000000..003b9834 --- /dev/null +++ b/bazel/rules/rules_score/test/puml_layout_test.bzl @@ -0,0 +1,311 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +""" +Loading-phase unit tests for plan_view_layout() +(bazel/rules/rules_score/private/puml_utils.bzl). + +`plan_view_layout` is pure Starlark -- it only reads `.short_path` / `.extension` +/ `.owner.workspace_name` off whatever is passed as a "file" -- so plain +`struct(short_path = ..., extension = ..., owner = ...)` fakes stand in for +real File objects, and skylib's `loadingtest` (not `analysistest`) is used: no +target_under_test / analysis phase is needed. +""" + +load("@bazel_skylib//lib:unittest.bzl", "loadingtest") +load( + "@score_tooling//bazel/rules/rules_score/private:puml_utils.bzl", + "plan_view_layout", +) + +def _fake_file(short_path, owner_repo = None): + """`owner_repo` fakes `File.owner.workspace_name` -- omitted (None) for + plain same-build-main-repo fakes used by most cases below; set it to + model a file whose owning label lives in a specific repository, for the + `own_repo`-aware short_path-marker-stripping tests.""" + basename = short_path.split("/")[-1] + extension = basename.split(".")[-1] if "." in basename else "" + owner = struct(workspace_name = owner_repo) if owner_repo != None else None + return struct(short_path = short_path, extension = extension, path = short_path, owner = owner) + +def puml_layout_test_suite(name): + """Defines the loading-phase test suite for plan_view_layout(). + + Args: + name: Suite name; individual test targets and the aggregating + `_tests` test_suite are derived from it. + """ + env = loadingtest.make(name) + + # --- authored index.rst composes with the generated directory index ------- + + overview = _fake_file("static/overview.puml") + authored_index = _fake_file("static/index.rst") + plan = plan_view_layout([overview, authored_index], "") + + loadingtest.equals(env, "plan_index_composes_no_errors", [], plan.errors) + loadingtest.equals( + env, + "plan_index_composes_staged", + [(overview, "static/overview.puml"), (authored_index, "static/index.rst.inc")], + plan.staged, + ) + loadingtest.equals( + env, + "plan_index_composes_wrappers", + [(overview, "static", "overview")], + plan.wrappers, + ) + loadingtest.equals( + env, + "plan_index_composes_indexes", + [ + struct(directory = "", entries = ["static/index"], body_relative_path = None), + struct(directory = "static", entries = ["overview"], body_relative_path = "static/index.rst.inc"), + ], + plan.indexes, + ) + + # --- authored same-stem .rst suppresses the generated diagram wrapper ------ + # The wrapper is only a placeholder for prose that doesn't exist yet; real + # authored content always wins, but the .puml is still staged as a sibling + # so the author's own `.. uml::` directive resolves. + + rst_overview_puml = _fake_file("static/overview.puml") + rst_overview_rst = _fake_file("static/overview.rst") + rst_plan = plan_view_layout([rst_overview_puml, rst_overview_rst], "") + + loadingtest.equals(env, "plan_rst_override_no_errors", [], rst_plan.errors) + loadingtest.equals(env, "plan_rst_override_no_wrapper", [], rst_plan.wrappers) + loadingtest.equals( + env, + "plan_rst_override_staged", + [(rst_overview_puml, "static/overview.puml"), (rst_overview_rst, "static/overview.rst")], + rst_plan.staged, + ) + + # --- authored same-stem .md also suppresses the generated wrapper ---------- + + md_overview_puml = _fake_file("static/overview.puml") + md_overview_md = _fake_file("static/overview.md") + md_plan = plan_view_layout([md_overview_puml, md_overview_md], "") + + loadingtest.equals(env, "plan_md_override_no_errors", [], md_plan.errors) + loadingtest.equals(env, "plan_md_override_no_wrapper", [], md_plan.wrappers) + + # --- nested directories: index entries are generated at every level -------- + + nested_overview = _fake_file("static/overview.puml") + nested_detail = _fake_file("static/sub/detail.puml") + nested_plan = plan_view_layout([nested_overview, nested_detail], "") + + loadingtest.equals(env, "nested_directories_no_errors", [], nested_plan.errors) + loadingtest.equals( + env, + "nested_directories_indexes", + [ + struct(directory = "", entries = ["static/index"], body_relative_path = None), + struct(directory = "static", entries = ["overview", "sub/index"], body_relative_path = None), + struct(directory = "static/sub", entries = ["detail"], body_relative_path = None), + ], + nested_plan.indexes, + ) + + # --- pass-through directories collapse into their only descendant --------- + # "a" and "a/b" hold nothing of their own and each lead to a single child, + # so they contribute no navigation page; the root links straight to + # "a/b/c/index". + + collapse_leaf = _fake_file("a/b/c/leaf.puml") + collapse_plan = plan_view_layout([collapse_leaf], "") + + loadingtest.equals(env, "collapse_pass_through_no_errors", [], collapse_plan.errors) + loadingtest.equals( + env, + "collapse_pass_through_indexes", + [ + struct(directory = "", entries = ["a/b/c/index"], body_relative_path = None), + struct(directory = "a/b/c", entries = ["leaf"], body_relative_path = None), + ], + collapse_plan.indexes, + ) + + # --- an authored index body keeps its directory in the navigation --------- + + kept_body = _fake_file("a/b/index.rst") + kept_leaf = _fake_file("a/b/c/leaf.puml") + kept_plan = plan_view_layout([kept_body, kept_leaf], "") + + loadingtest.equals(env, "collapse_authored_body_no_errors", [], kept_plan.errors) + loadingtest.equals( + env, + "collapse_authored_body_indexes", + [ + struct(directory = "", entries = ["a/b/index"], body_relative_path = None), + struct(directory = "a/b", entries = ["c/index"], body_relative_path = "a/b/index.rst.inc"), + struct(directory = "a/b/c", entries = ["leaf"], body_relative_path = None), + ], + kept_plan.indexes, + ) + + # --- empty view: no navigable files at all means no navigation at all ------ + + empty_plan = plan_view_layout([], "") + + loadingtest.equals(env, "empty_view_no_errors", [], empty_plan.errors) + loadingtest.equals(env, "empty_view_no_staged", [], empty_plan.staged) + loadingtest.equals(env, "empty_view_no_wrappers", [], empty_plan.wrappers) + loadingtest.equals(env, "empty_view_no_indexes", [], empty_plan.indexes) + + # --- cross-package, same-basename files never collide on bare basename ----- + # relative_source_path() falls back to the full workspace-relative + # short_path for files outside `package`; two same-named files in + # different directories must stay distinct rather than silently colliding + # on "overview.puml". + + cross_a = _fake_file("pkg_a/overview.puml") + cross_b = _fake_file("pkg_b/overview.puml") + cross_plan = plan_view_layout([cross_a, cross_b], "mypkg") + + loadingtest.equals(env, "cross_package_same_basename_no_errors", [], cross_plan.errors) + loadingtest.equals( + env, + "cross_package_same_basename_staged", + [(cross_a, "pkg_a/overview.puml"), (cross_b, "pkg_b/overview.puml")], + cross_plan.staged, + ) + loadingtest.equals( + env, + "cross_package_same_basename_wrappers", + [(cross_a, "pkg_a", "overview"), (cross_b, "pkg_b", "overview")], + cross_plan.wrappers, + ) + + # --- duplicate-path error: two files that would stage at the same path ----- + + dup_a = _fake_file("static/overview.puml") + dup_b = _fake_file("static/overview.puml") + dup_plan = plan_view_layout([dup_a, dup_b], "") + + loadingtest.equals( + env, + "duplicate_path_error", + ["two files would both stage as 'static/overview.puml': 'static/overview.puml' and 'static/overview.puml'"], + dup_plan.errors, + ) + loadingtest.equals( + env, + "duplicate_path_staged_once", + [(dup_a, "static/overview.puml")], + dup_plan.staged, + ) + + # --- duplicate-path detection also covers non-navigable assets -------------- + # Assets get no wrapper/index of their own, but they are still staged, so + # an undetected collision would surface as a raw declare_file() conflict. + + asset_a = _fake_file("static/diagram.svg") + asset_b = _fake_file("static/diagram.svg") + asset_plan = plan_view_layout([asset_a, asset_b], "") + + loadingtest.equals( + env, + "duplicate_asset_path_error", + ["two files would both stage as 'static/diagram.svg': 'static/diagram.svg' and 'static/diagram.svg'"], + asset_plan.errors, + ) + + # --- an authored index.rst collides with a literal index.rst.inc ------------ + # The authored index is staged under the ".inc" name, which a source file + # may already occupy. + + inc_authored = _fake_file("static/index.rst") + inc_literal = _fake_file("static/index.rst.inc") + inc_plan = plan_view_layout([inc_authored, inc_literal], "") + + loadingtest.equals( + env, + "plan_index_collides_with_literal_inc", + ["two files would both stage as 'static/index.rst.inc': 'static/index.rst' and 'static/index.rst.inc'"], + inc_plan.errors, + ) + + # --- a diagram literally named "index" is rejected -------------------------- + # That stem is reserved for the directory's own generated navigation page. + + reserved_index_puml = _fake_file("static/index.puml") + reserved_plan = plan_view_layout([reserved_index_puml], "") + + loadingtest.equals( + env, + "diagram_named_index_rejected", + ["'static/index.puml' is named 'index', which is reserved for the generated directory navigation page; rename it"], + reserved_plan.errors, + ) + + # --- same-stem .puml and .plantuml collide ----------------------------------- + + same_stem_puml = _fake_file("static/overview.puml") + same_stem_plantuml = _fake_file("static/overview.plantuml") + same_stem_plan = plan_view_layout([same_stem_puml, same_stem_plantuml], "") + + loadingtest.equals( + env, + "puml_and_plantuml_same_stem_rejected", + ["both 'overview.puml' and 'overview.plantuml' exist for 'overview' in directory 'static'; keep only one"], + same_stem_plan.errors, + ) + + # --- a file from another repository is rejected ------------------------------ + # short_path for an external-repo file starts with "../"; there is no + # in-tree relative path to stage it at. + + external_repo_file = _fake_file("../other_repo+/pkg/overview.puml") + external_repo_plan = plan_view_layout([external_repo_file], "") + + loadingtest.equals( + env, + "external_repo_file_rejected", + ["'../other_repo+/pkg/overview.puml' lives outside this repository; architectural_design view files must live in the same repository as the target"], + external_repo_plan.errors, + ) + + # --- own-repo file is accepted even when it isn't the build's main repo ----- + # Bazel's short_path prefixes every file living outside the build's *main* + # repository with "..//", even when that repository is the SAME one + # the consuming architectural_design target itself lives in (e.g. this + # target is built as someone else's dependency, not as the build's root + # module). own_repo lets relative_source_path() tell that apart from a + # genuinely different repository -- see relative_source_path's docstring. + + own_repo_overview = _fake_file("../some_other_library+/overview.puml", owner_repo = "some_other_library+") + own_repo_plan = plan_view_layout([own_repo_overview], "", own_repo = "some_other_library+") + + loadingtest.equals(env, "own_repo_marker_stripped_no_errors", [], own_repo_plan.errors) + loadingtest.equals( + env, + "own_repo_marker_stripped_staged", + [(own_repo_overview, "overview.puml")], + own_repo_plan.staged, + ) + + # --- a genuinely different repository is still rejected with own_repo set -- + + other_repo_overview = _fake_file("../other_repo+/overview.puml", owner_repo = "other_repo+") + other_repo_plan = plan_view_layout([other_repo_overview], "", own_repo = "some_other_library+") + + loadingtest.equals( + env, + "different_repo_still_rejected_with_own_repo_set", + ["'../other_repo+/overview.puml' lives outside this repository; architectural_design view files must live in the same repository as the target"], + other_repo_plan.errors, + ) diff --git a/bazel/rules/rules_score/test/template/conf.template.py b/bazel/rules/rules_score/test/template/conf.template.py index 31d8ab19..05f311f9 100644 --- a/bazel/rules/rules_score/test/template/conf.template.py +++ b/bazel/rules/rules_score/test/template/conf.template.py @@ -91,6 +91,14 @@ # HTML theme html_theme = "sphinx_rtd_theme" +# Mirrors the shipped default template: architectural_design views nest diagram +# pages arbitrarily deep by directory, so the sidebar must stay expanded and +# unpruned for those pages to be reachable. +html_theme_options = { + "collapse_navigation": False, + "navigation_depth": -1, +} + # Load external needs and log configuration needs_external_needs = bazel_sphinx_needs.load_external_needs() bazel_sphinx_needs.log_config_info(project) diff --git a/plantuml/parser/puml_cli/src/main.rs b/plantuml/parser/puml_cli/src/main.rs index 36b18102..3883aefb 100644 --- a/plantuml/parser/puml_cli/src/main.rs +++ b/plantuml/parser/puml_cli/src/main.rs @@ -229,7 +229,25 @@ fn run() -> Result<(), Box> { debug!("Parsing started"); for (path, content) in &preprocessed_files { - let parsed_content = parse_puml_file(path, content, log_level, args.diagram_type) + // `source_file` is the stable, workspace-relative path callers want + // embedded in outputs (see `--source-name`'s docs); `path` is the + // actual file handed to us (e.g. a disambiguating `_puml_inputs/` + // symlink) and must keep driving output *filenames*, which are + // derived from its basename via `_disambiguated_stems` and may not + // match `source_file`'s basename. Parsers only ever read `path` to + // stamp per-element `SourceLocation`s (never to access the + // filesystem — `content` is already loaded), so substituting a + // synthetic path built from `source_file` here is sufficient to + // make every `SourceLocation.file` in the resolved model — and thus + // in the FlatBuffers output — carry the corrected value too, + // matching what already reaches the lobster/idmap outputs below. + let source_file = args + .source_name + .clone() + .unwrap_or_else(|| source_path_for_output(path)); + let source_path: Rc = Rc::new(PathBuf::from(&source_file)); + + let parsed_content = parse_puml_file(&source_path, content, log_level, args.diagram_type) .map_err(|e| std::io::Error::other(e.to_string()))?; if emit_debug_json { if let Some(ref dir) = fbs_output_dir { @@ -256,10 +274,6 @@ fn run() -> Result<(), Box> { } } - let source_file = args - .source_name - .clone() - .unwrap_or_else(|| source_path_for_output(path)); let fbs_buffer = serialize_resolved_diagram(&logic_result); if let Some(ref dir) = fbs_output_dir { write_fbs_to_file(&fbs_buffer, path, dir)?; @@ -1089,8 +1103,39 @@ mod idmap_wiring_tests { cleanup_dir(&dir); } - /// Sequence diagrams route through `idmap_model_for` to the - /// `IdMapModel::Sequence` dispatch arm and `write_idmap_to_file`. + /// `SourceLocation.file` (and thus the FlatBuffers output) must carry the + /// corrected `source_file` -- never the raw staging `path` handed to + /// `parse_puml_file` -- when `--source-name` (or its computed fallback) + /// differs from `path`'s own basename/directory, e.g. the + /// `_puml_inputs/.puml` disambiguating symlink Bazel stages this + /// CLI's input from. This mirrors the exact `source_file`/`source_path` + /// computation from the main parsing loop above rather than spawning the + /// compiled binary. + #[test] + fn source_location_uses_corrected_source_not_staging_path() { + let staging_path = Path::new("_puml_inputs/dir_part__foo.puml"); + let source_name = Some("real/pkg/foo.puml".to_string()); + let content = "@startuml\nclass Foo {\n}\n@enduml"; + + let source_file = source_name.unwrap_or_else(|| source_path_for_output(staging_path)); + let source_path: Rc = Rc::new(PathBuf::from(&source_file)); + + let parsed = parse_puml_file(&source_path, content, LogLevel::Info, DiagramType::Class) + .expect("class parse must succeed"); + let resolved = resolve_parsed_diagram(parsed).expect("class diagram must resolve"); + let fbs_bytes = serialize_resolved_diagram(&resolved); + let fbs_text = String::from_utf8_lossy(&fbs_bytes); + + assert!( + fbs_text.contains("real/pkg/foo.puml"), + "expected the corrected source_file to be embedded in the FBS output" + ); + assert!( + !fbs_text.contains("_puml_inputs"), + "the disambiguating staging path must never leak into the FBS output, got: {fbs_text}" + ); + } + #[test] fn sequence_diagram_routes_to_idmap_model_dispatch() { let content = From 397641298e3ea0ab160709c1cb1ddc1c988ee497 Mon Sep 17 00:00:00 2001 From: Jochen Hoenle <173445474+hoe-jo@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:05:25 +0200 Subject: [PATCH 2/5] puml_cli: add --output-stem to name outputs independent of input path Every write_*_to_file helper (FlatBuffers, lobster, idmap) derives its output filename from the passed-in input path's file stem alone. Add --output-stem so a caller that needs a specific, disambiguated output filename (e.g. a Bazel rule staging same-named diagrams from different packages) can request it directly, instead of having to stage the actual input file under a symlink named after the desired stem. --- plantuml/parser/README.md | 1 + plantuml/parser/puml_cli/src/main.rs | 235 ++++++++++++++++++++++----- 2 files changed, 191 insertions(+), 45 deletions(-) diff --git a/plantuml/parser/README.md b/plantuml/parser/README.md index 41104805..f72045e3 100644 --- a/plantuml/parser/README.md +++ b/plantuml/parser/README.md @@ -57,6 +57,7 @@ Options: | `--diagram-type ` | Diagram type hint | `none` | | `--fbs-output-dir ` | Output directory for `.fbs.bin` FlatBuffers files | none (no output) | | `--lobster-output-dir ` | Output directory for `.lobster` traceability files | none (no output) | +| `--output-stem ` | Override the file stem used to name every output file for this run, instead of deriving it from the input file's own basename. Requires exactly one input file. | none (uses the input file's basename) | At least one of `--file` or `--folders` is required. diff --git a/plantuml/parser/puml_cli/src/main.rs b/plantuml/parser/puml_cli/src/main.rs index 3883aefb..7575e09c 100644 --- a/plantuml/parser/puml_cli/src/main.rs +++ b/plantuml/parser/puml_cli/src/main.rs @@ -15,6 +15,7 @@ use clap::{ArgGroup, Parser, ValueEnum}; use env_logger::Builder; use log::debug; use serde::Serialize; +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::fs; use std::path::{Path, PathBuf}; @@ -124,6 +125,15 @@ struct Args { /// Must be a relative path (never a machine-specific absolute path). #[arg(long)] source_name: Option, + + /// Override the file stem used to name every output file (`.fbs.bin`, + /// `.lobster`, `.idmap.json`, ...) for this run, instead of deriving it + /// from the input file's own basename. Only meaningful for a single + /// resolved diagram: requires exactly one input file (a single `--file`, + /// no `--folders`) and cannot be combined with `--fta-output-dir`. Must + /// be a single path component (no `/`, no `.`/`..`), never a path. + #[arg(long)] + output_stem: Option, } #[derive(Copy, Clone, ValueEnum, Debug)] @@ -171,12 +181,18 @@ fn run() -> Result<(), Box> { if args.source_name.is_some() { return Err("--source-name cannot be combined with --fta-output-dir".into()); } + if args.output_stem.is_some() { + return Err("--output-stem cannot be combined with --fta-output-dir".into()); + } return run_fta(&args, dir, log_level); } if let Some(name) = &args.source_name { validate_source_name(name)?; } + if let Some(stem) = &args.output_stem { + validate_output_stem(stem)?; + } let emit_debug_json = log_level.to_level_filter() >= log::LevelFilter::Debug; @@ -213,14 +229,8 @@ fn run() -> Result<(), Box> { if file_list.is_empty() { return Err("No valid PUML files found.".into()); } - if args.source_name.is_some() && file_list.len() != 1 { - return Err(format!( - "--source-name requires exactly one input file (a single --file, no \ - --folders); got {} files", - file_list.len(), - ) - .into()); - } + require_single_input(args.source_name.is_some(), file_list.len(), "source-name")?; + require_single_input(args.output_stem.is_some(), file_list.len(), "output-stem")?; debug!("Collected {} puml files.", file_list.len()); debug!("Preprocessing: include expansion"); @@ -229,29 +239,23 @@ fn run() -> Result<(), Box> { debug!("Parsing started"); for (path, content) in &preprocessed_files { - // `source_file` is the stable, workspace-relative path callers want - // embedded in outputs (see `--source-name`'s docs); `path` is the - // actual file handed to us (e.g. a disambiguating `_puml_inputs/` - // symlink) and must keep driving output *filenames*, which are - // derived from its basename via `_disambiguated_stems` and may not - // match `source_file`'s basename. Parsers only ever read `path` to - // stamp per-element `SourceLocation`s (never to access the - // filesystem — `content` is already loaded), so substituting a - // synthetic path built from `source_file` here is sufficient to - // make every `SourceLocation.file` in the resolved model — and thus - // in the FlatBuffers output — carry the corrected value too, - // matching what already reaches the lobster/idmap outputs below. - let source_file = args - .source_name - .clone() - .unwrap_or_else(|| source_path_for_output(path)); - let source_path: Rc = Rc::new(PathBuf::from(&source_file)); + // `naming_path` drives every output *filename* below (via its file + // stem); it's `path` unless `--output-stem` overrides it. `source_file` + // is the stable, workspace-relative path embedded *inside* outputs + // (see `--source-name`'s docs). Parsers only ever read `source_path` + // to stamp per-element `SourceLocation`s (never to access the + // filesystem -- `content` is already loaded), so with `--source-name` + // a synthetic path built from `source_file` is enough to make every + // `SourceLocation.file` in the resolved model -- and thus in the + // FlatBuffers output -- carry the corrected value as well. + let naming_path = output_naming_path(args.output_stem.as_deref(), path); + let (source_file, source_path) = output_source_path(args.source_name.as_deref(), path); let parsed_content = parse_puml_file(&source_path, content, log_level, args.diagram_type) .map_err(|e| std::io::Error::other(e.to_string()))?; if emit_debug_json { if let Some(ref dir) = fbs_output_dir { - write_json_to_file(&parsed_content, path, dir, "raw.ast")?; + write_json_to_file(&parsed_content, &naming_path, dir, "raw.ast")?; } } @@ -270,13 +274,13 @@ fn run() -> Result<(), Box> { ); if emit_debug_json { if let Some(ref dir) = fbs_output_dir { - write_json_to_file(&logic_result, path, dir, "logic.ast")?; + write_json_to_file(&logic_result, &naming_path, dir, "logic.ast")?; } } let fbs_buffer = serialize_resolved_diagram(&logic_result); if let Some(ref dir) = fbs_output_dir { - write_fbs_to_file(&fbs_buffer, path, dir)?; + write_fbs_to_file(&fbs_buffer, &naming_path, dir)?; } if let Some(ldir) = &lobster_output_dir { @@ -286,11 +290,11 @@ fn run() -> Result<(), Box> { ResolvedDiagram::Activity(_) => LobsterModel::Empty, ResolvedDiagram::Sequence(_) => LobsterModel::Empty, }; - write_lobster_to_file(lobster_model, path, &source_file, ldir)?; + write_lobster_to_file(lobster_model, &naming_path, &source_file, ldir)?; } if let Some(idir) = &idmap_output_dir { - let output_path = puml_idmap::idmap_output_path(path, idir); + let output_path = puml_idmap::idmap_output_path(&naming_path, idir); if !seen_idmap_outputs.insert(output_path.clone()) { return Err(format!( "duplicate idmap output {}: multiple input files map to the \ @@ -306,14 +310,14 @@ fn run() -> Result<(), Box> { Some(idmap_model) => { write_idmap_to_file( idmap_model, - path, + &naming_path, Some(&source_file), diagram_name.as_deref(), idir, )?; } None => { - write_empty_idmap_to_file(path, Some(&source_file), idir)?; + write_empty_idmap_to_file(&naming_path, Some(&source_file), idir)?; } } } @@ -687,6 +691,24 @@ fn resolve_path(path: &Path) -> Result> { Ok(base_dir.join(path)) } +/// Reject a flag that only makes sense for a single resolved diagram (e.g. +/// `--source-name`, `--output-stem`) when more than one input file was +/// collected. +fn require_single_input( + is_set: bool, + file_count: usize, + flag_name: &str, +) -> Result<(), Box> { + if is_set && file_count != 1 { + return Err(format!( + "--{flag_name} requires exactly one input file (a single --file, no \ + --folders); got {file_count} files" + ) + .into()); + } + Ok(()) +} + /// Validate a user-supplied `--source-name` value. /// /// Rejects empty strings and absolute paths: an absolute path would leak the @@ -733,6 +755,61 @@ fn source_path_for_output(path: &Path) -> String { .unwrap_or_else(|| path.to_string_lossy().into_owned()) } +/// Validate a user-supplied `--output-stem` value. +/// +/// Unlike `--source-name` (a full relative path embedded *inside* outputs), +/// this becomes the output *filename* stem itself, so it must be a single +/// path component: rejects empty strings and anything containing a `/` or +/// resolving to `.`/`..`. +fn validate_output_stem(stem: &str) -> Result<(), Box> { + if stem.is_empty() { + return Err("--output-stem must not be empty".into()); + } + let mut components = Path::new(stem).components(); + let is_single_normal_component = + matches!(components.next(), Some(std::path::Component::Normal(_))) + && components.next().is_none(); + if !is_single_normal_component { + return Err(format!( + "--output-stem must be a single path component (no '/', '.', or '..'), got: {stem}" + ) + .into()); + } + Ok(()) +} + +/// Compute the corrected, stable `source_file` string (embedded *inside* +/// outputs) and the `Rc` that parsers use to stamp `SourceLocation`s, +/// for one input `path`. +/// +/// With `--source-name`, both derive from that name so every `SourceLocation` +/// carries the corrected path too. Without it, `source_file` is +/// `source_path_for_output(path)` and `SourceLocation`s keep using `path` +/// itself. +fn output_source_path(source_name: Option<&str>, path: &Rc) -> (String, Rc) { + match source_name { + Some(name) => { + let source_file = name.to_string(); + let source_path = Rc::new(PathBuf::from(&source_file)); + (source_file, source_path) + } + None => (source_path_for_output(path), Rc::clone(path)), + } +} + +/// Compute the path whose file stem drives every output *filename* for one +/// input `path` (the `write_*_to_file` helpers all derive their output name +/// from a passed-in path's file stem alone). +/// +/// `output_stem` (`--output-stem`) overrides the stem outright; otherwise +/// `path` itself is used unchanged. +fn output_naming_path<'a>(output_stem: Option<&str>, path: &'a Path) -> Cow<'a, Path> { + match output_stem { + Some(stem) => Cow::Owned(path.with_file_name(stem)), + None => Cow::Borrowed(path), + } +} + fn add_single_file( path: &Path, file_list: &mut HashSet>, @@ -1103,22 +1180,17 @@ mod idmap_wiring_tests { cleanup_dir(&dir); } - /// `SourceLocation.file` (and thus the FlatBuffers output) must carry the - /// corrected `source_file` -- never the raw staging `path` handed to - /// `parse_puml_file` -- when `--source-name` (or its computed fallback) - /// differs from `path`'s own basename/directory, e.g. the - /// `_puml_inputs/.puml` disambiguating symlink Bazel stages this - /// CLI's input from. This mirrors the exact `source_file`/`source_path` - /// computation from the main parsing loop above rather than spawning the - /// compiled binary. + /// With `--source-name`, `SourceLocation.file` (and thus the FlatBuffers + /// output) must carry the corrected `source_file`, never the staging + /// `path` handed to `parse_puml_file`. Mirrors the `output_source_path` + /// call from the main parsing loop rather than spawning the binary. #[test] fn source_location_uses_corrected_source_not_staging_path() { - let staging_path = Path::new("_puml_inputs/dir_part__foo.puml"); - let source_name = Some("real/pkg/foo.puml".to_string()); + let staging_path = Rc::new(PathBuf::from("_puml_inputs/dir_part__foo.puml")); let content = "@startuml\nclass Foo {\n}\n@enduml"; - let source_file = source_name.unwrap_or_else(|| source_path_for_output(staging_path)); - let source_path: Rc = Rc::new(PathBuf::from(&source_file)); + let (_source_file, source_path) = + output_source_path(Some("real/pkg/foo.puml"), &staging_path); let parsed = parse_puml_file(&source_path, content, LogLevel::Info, DiagramType::Class) .expect("class parse must succeed"); @@ -1132,10 +1204,83 @@ mod idmap_wiring_tests { ); assert!( !fbs_text.contains("_puml_inputs"), - "the disambiguating staging path must never leak into the FBS output, got: {fbs_text}" + "the staging path must never leak into the FBS output, got: {fbs_text}" ); } + /// Without `--source-name`, `source_file` is + /// `source_path_for_output`'s workspace-relative computation while + /// `SourceLocation`s keep using the input path itself. + #[test] + fn source_location_falls_back_to_relative_path_without_source_name() { + let path = Rc::new(PathBuf::from("relative/dir/foo.puml")); + + let (source_file, source_path) = output_source_path(None, &path); + + assert_eq!(source_file, source_path_for_output(&path)); + assert!(Rc::ptr_eq(&source_path, &path)); + } + + /// `--output-stem` must override every output file's name, independent of + /// the input path's own basename. + #[test] + fn output_stem_overrides_output_filenames() { + let content = "@startuml\nclass A {\n +a\n}\n@enduml"; + let path = Rc::new(PathBuf::from("cls/original_name.puml")); + + let parsed = parse_puml_file(&path, content, LogLevel::Info, DiagramType::Class) + .expect("class parse must succeed"); + let resolved = resolve_parsed_diagram(parsed).expect("class must resolve"); + let idmap_model = + idmap_model_for(&resolved).expect("class diagrams must dispatch to an IdMapModel"); + + let naming_path = output_naming_path(Some("overridden_stem"), &path); + assert_eq!( + naming_path.file_name().and_then(|n| n.to_str()), + Some("overridden_stem") + ); + + let dir = unique_dir("output_stem"); + let source_file = source_path_for_output(&path); + let output = write_idmap_to_file(idmap_model, &naming_path, Some(&source_file), None, &dir) + .expect("idmap must be written"); + + assert_eq!( + output.file_name().and_then(|n| n.to_str()), + Some("overridden_stem.idmap.json") + ); + // The embedded `source` field is unaffected by the naming override. + let json: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&output).unwrap()).unwrap(); + assert_eq!(json["source"], "cls/original_name.puml"); + + cleanup_dir(&dir); + } + + /// `--output-stem` requires exactly one input file. + #[test] + fn output_stem_rejected_with_multiple_inputs() { + let err = require_single_input(true, 2, "output-stem") + .expect_err("must reject --output-stem with more than one input file"); + assert!(err.to_string().contains("--output-stem")); + assert!(err.to_string().contains("got 2 files")); + + require_single_input(true, 1, "output-stem").expect("a single input file must be fine"); + require_single_input(false, 2, "output-stem") + .expect("the flag being unset must never trigger the check"); + } + + /// `--output-stem` must be a single path component: no separators, and + /// not `.`/`..`. + #[test] + fn output_stem_validation_rejects_paths_and_empty_values() { + validate_output_stem("valid_stem").expect("a plain identifier must be accepted"); + assert!(validate_output_stem("").is_err()); + assert!(validate_output_stem("has/slash").is_err()); + assert!(validate_output_stem(".").is_err()); + assert!(validate_output_stem("..").is_err()); + } + #[test] fn sequence_diagram_routes_to_idmap_model_dispatch() { let content = From 28cca96890016d541a7184ee9d5354900617b92c Mon Sep 17 00:00:00 2001 From: Jochen Hoenle <173445474+hoe-jo@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:25:42 +0200 Subject: [PATCH 3/5] architectural_design: share ARCH_VIEWS, narrow view_root_indexes, cleanup - Extract the static/dynamic/public_api/internal_api view list into a new shared bazel/rules/rules_score/private/views.bzl - Narrow ArchitecturalDesignInfo.view_indexes (Dict[str, NavigationStruct]) to view_root_indexes (Dict[str, File|None]) - _colocate_view_files now returns Dict[str, File] keyed by relative path instead of a positionally-matched List[File]; emit_view_navigation takes that dict directly instead of rebuilding it internally from a zipped list. - relative_source_path reads file.owner directly instead of getattr(file, "owner", None) -- test fakes always set it. - Collapse the redundant view_fbs/view_fbs_files dicts in architectural_design.bzl into one. - Trim the architectural_design() macro's docstring, pointing to docs/user_guide/architectural_design.rst instead of duplicating its authoring-mode prose. - Replace the bespoke puml_layout_crash_repro_test.bzl analysistest rule with skylib's build_test for the two "does it build" regression fixtures. - Add a diagram-free view regression test (authored_layout_diagram_free_test): a view with only a hand-authored index.md and no diagrams at all must still stage a valid root index (authored body composed above an empty toctree). - Add stem_collision_fails_test: an expect_failure analysistest covering _disambiguated_stems' residual-collision fail() path (two files whose basenames collide and whose directory-disambiguated stems collide too). --- .../private/architectural_design.bzl | 117 ++++++++---------- .../private/dependable_element.bzl | 28 ++--- bazel/rules/rules_score/private/views.bzl | 31 +++++ bazel/rules/rules_score/providers.bzl | 2 +- bazel/rules/rules_score/test/BUILD | 61 +++++++-- .../test/check_authored_layout_content.sh | 30 ++++- .../test/fixtures/diagram_free/index.md | 19 +++ .../test/fixtures/stem_collision/a/b/foo.puml | 16 +++ .../test/fixtures/stem_collision/a_b/foo.puml | 16 +++ .../test/puml_layout_crash_repro_test.bzl | 50 -------- .../test/puml_stem_collision_test.bzl | 38 ++++++ 11 files changed, 263 insertions(+), 145 deletions(-) create mode 100644 bazel/rules/rules_score/private/views.bzl create mode 100644 bazel/rules/rules_score/test/fixtures/diagram_free/index.md create mode 100644 bazel/rules/rules_score/test/fixtures/stem_collision/a/b/foo.puml create mode 100644 bazel/rules/rules_score/test/fixtures/stem_collision/a_b/foo.puml delete mode 100644 bazel/rules/rules_score/test/puml_layout_crash_repro_test.bzl create mode 100644 bazel/rules/rules_score/test/puml_stem_collision_test.bzl diff --git a/bazel/rules/rules_score/private/architectural_design.bzl b/bazel/rules/rules_score/private/architectural_design.bzl index 211901f1..807c9fec 100644 --- a/bazel/rules/rules_score/private/architectural_design.bzl +++ b/bazel/rules/rules_score/private/architectural_design.bzl @@ -27,15 +27,7 @@ load("//bazel/rules/rules_score:providers.bzl", "ArchitecturalDesignInfo", "Sphi load("//bazel/rules/rules_score/private:puml_utils.bzl", "emit_view_navigation", "plan_view_layout", "relative_source_path") load("//bazel/rules/rules_score/private:validation.bzl", "PROFILES", "VALIDATION_ATTRS", "run_validation") load("//bazel/rules/rules_score/private:verbosity.bzl", "VERBOSITY_ATTR", "get_log_level") - -# Views recognized by architectural_design, mapped to their display name used -# as the title of that view's top-level navigation index page. -_VIEWS = { - "static": "Static Design", - "dynamic": "Dynamic Design", - "public_api": "Public API", - "internal_api": "Internal API", -} +load("//bazel/rules/rules_score/private:views.bzl", "ARCH_VIEWS") # ============================================================================ # Private Rule Implementation @@ -54,6 +46,12 @@ def _disambiguated_stems(ctx, files): existing filenames/titles); only colliding basenames are disambiguated, using the file's package-relative directory. + Disambiguating by directory can itself collide -- e.g. `a/b/foo.puml` + and `a_b/foo.puml` both produce the stem `a_b__foo` once their `/` is + replaced with `_` -- so every stem actually produced is tracked and a + residual collision fails the build, naming both source files, rather + than silently overwriting one diagram's output with the other's. + Args: ctx: Rule context. files: Iterable of File objects (non-.puml/.plantuml entries ignored). @@ -66,11 +64,20 @@ def _disambiguated_stems(ctx, files): basename_counts[f.basename] = basename_counts.get(f.basename, 0) + 1 stems = {} + stem_sources = {} for f in puml_files: stem = f.basename.rsplit(".", 1)[0] if basename_counts[f.basename] > 1: dir_part = paths.dirname(relative_source_path(f, ctx.label.package, ctx.label.workspace_name)) stem = "{}__{}".format(dir_part.replace("/", "_"), stem) if dir_part else stem + + previous = stem_sources.get(stem) + if previous != None: + fail(( + "architectural_design {}: '{}' and '{}' both disambiguate to the " + + "output stem '{}'; rename one of these files so their stems differ" + ).format(ctx.label, previous, f.short_path, stem)) + stem_sources[stem] = f.short_path stems[f.path] = stem return stems @@ -82,11 +89,11 @@ def _run_puml_parser(ctx, puml_file, file_stem): FlatBuffers schema (each diagram type uses its own root_type). Lobster output is produced in-process for component diagrams. - When the input file basename is not unique across all diagrams being - parsed by this target (see _disambiguated_stems), a symlink with a - disambiguated name is created and passed to puml_cli. This ensures - puml_cli produces outputs with unique names even when two source - diagrams share the same basename but live in different directories. + ``--output-stem`` is passed as the disambiguated `file_stem` (see + `_disambiguated_stems`) so puml_cli's output filenames always match the + declared output files below, even when two source diagrams share the + same basename but live in different directories -- without needing to + stage the input file itself under a differently-named symlink first. ``--source-name`` is passed as ``puml_file.short_path`` so the ``source`` field embedded in the fbs/lobster/idmap outputs is a stable, @@ -114,23 +121,13 @@ def _run_puml_parser(ctx, puml_file, file_stem): "{}/{}.idmap.json".format(ctx.label.name, file_stem), ) - # A symlink under this target's own _puml_inputs/ dir, named after the - # disambiguated stem, so puml_cli's output filenames (derived from input - # basename) match the declared output files, and two architectural_design - # targets in the same package sharing a diagram basename never collide - # on the same _puml_inputs/ path. - input_symlink = ctx.actions.declare_file( - "{}/_puml_inputs/{}.{}".format(ctx.label.name, file_stem, puml_file.extension), - ) - ctx.actions.symlink(output = input_symlink, target_file = puml_file) - ctx.actions.run( - inputs = [input_symlink], + inputs = [puml_file], outputs = [fbs_output, lobster_output, idmap_output], executable = ctx.executable._puml_parser, arguments = [ "--file", - input_symlink.path, + puml_file.path, "--fbs-output-dir", fbs_output.dirname, "--lobster-output-dir", @@ -139,6 +136,8 @@ def _run_puml_parser(ctx, puml_file, file_stem): idmap_output.dirname, "--source-name", puml_file.short_path, + "--output-stem", + file_stem, "--log-level", get_log_level(ctx), ], @@ -189,13 +188,14 @@ def _colocate_view_files(ctx, staged_files, view_output_dir): "{ctx.label.name}/{view_name}". Returns: - List of symlinked File objects, one per (File, relative_path) pair. + Dict from relative_path to its symlinked File object, one entry per + (File, relative_path) pair in `staged_files`. """ - colocated = [] + colocated = {} for source_file, relative_path in staged_files: copy = ctx.actions.declare_file("{}/{}".format(view_output_dir, relative_path)) ctx.actions.symlink(output = copy, target_file = source_file) - colocated.append(copy) + colocated[relative_path] = copy return colocated def _run_validation(ctx, component_fbs_files, sequence_fbs_files, public_api_fbs_files, internal_api_fbs_files): @@ -254,21 +254,19 @@ def _architectural_design_impl(ctx): ) view_fbs = {} - view_fbs_files = {} view_lobster = {} view_idmap = {} - view_indexes = {} + view_root_indexes = {} view_source_files = [] view_sphinx_srcs = [] - view_root_indexes = [] + root_index_files = [] view_aux_docs = [] - for view_name, root_title in _VIEWS.items(): + for view_name, root_title in ARCH_VIEWS: view_files = getattr(ctx.files, view_name) fbs_list, lobster_list, idmap_list = _parse_puml_diagrams(ctx, view_files, stems) - view_fbs[view_name] = depset(fbs_list) - view_fbs_files[view_name] = fbs_list + view_fbs[view_name] = fbs_list view_lobster[view_name] = lobster_list view_idmap[view_name] = idmap_list @@ -285,7 +283,8 @@ def _architectural_design_impl(ctx): # this is required for `.. uml::`/toctree sibling references to # resolve once dependable_element.bzl stages these files. view_output_dir = "{}/{}".format(ctx.label.name, view_name) - colocated_files = _colocate_view_files(ctx, plan.staged, view_output_dir) + colocated_by_relative_path = _colocate_view_files(ctx, plan.staged, view_output_dir) + colocated_files = colocated_by_relative_path.values() view_source_files.append(depset(colocated_files)) navigation = emit_view_navigation( @@ -294,12 +293,12 @@ def _architectural_design_impl(ctx): view_output_dir, ctx.file._puml_rst_template, root_title, - colocated_files, + colocated_by_relative_path, ) - view_indexes[view_name] = navigation if navigation.root_index else None + view_root_indexes[view_name] = navigation.root_index if navigation.root_index: view_sphinx_srcs.append(depset(navigation.wrappers + navigation.indexes + [navigation.root_index])) - view_root_indexes.append(navigation.root_index) + root_index_files.append(navigation.root_index) # Wrapper pages and non-root indexes must be staged (so the root # index's nested toctrees resolve) but are not themselves @@ -315,10 +314,10 @@ def _architectural_design_impl(ctx): # entry and an aux doc. view_aux_docs.extend([f for f in colocated_files if f.extension in ("rst", "md") and f != navigation.root_index]) - static_fbs = view_fbs["static"] - dynamic_fbs = view_fbs["dynamic"] - public_api_fbs = view_fbs["public_api"] - internal_api_fbs = view_fbs["internal_api"] + static_fbs = depset(view_fbs["static"]) + dynamic_fbs = depset(view_fbs["dynamic"]) + public_api_fbs = depset(view_fbs["public_api"]) + internal_api_fbs = depset(view_fbs["internal_api"]) public_api_lobster = depset(view_lobster["public_api"]) all_source_files = depset(transitive = view_source_files) @@ -338,10 +337,10 @@ def _architectural_design_impl(ctx): validation_log = _run_validation( ctx, - view_fbs_files["static"], - view_fbs_files["dynamic"], - view_fbs_files["public_api"], - view_fbs_files["internal_api"], + view_fbs["static"], + view_fbs["dynamic"], + view_fbs["public_api"], + view_fbs["internal_api"], ) # `deps` carries everything needed in the Sphinx tree for this rule @@ -351,7 +350,7 @@ def _architectural_design_impl(ctx): # `aux_srcs` are the wrapper/sub-index/hand-written pages that must be # staged but reached only via that root index's own nested toctrees. sphinx_deps = depset(transitive = [sphinx_files] + view_sphinx_srcs) - sphinx_own_srcs = depset(view_root_indexes) + sphinx_own_srcs = depset(root_index_files) sphinx_aux_srcs = depset(view_aux_docs) return [ @@ -361,7 +360,7 @@ def _architectural_design_impl(ctx): dynamic = dynamic_fbs, public_api = public_api_fbs, internal_api = internal_api_fbs, - view_indexes = view_indexes, + view_root_indexes = view_root_indexes, name = ctx.label.name, public_api_lobster_files = public_api_lobster, validation_logs = [validation_log], @@ -450,19 +449,13 @@ def architectural_design( """Define architectural design following S-CORE process guidelines. Architectural design documents describe the software architecture of a - component, including both static and dynamic views. Static views show - the structural organization (classes, components, modules), while dynamic - views show the behavioral aspects (sequences, activities, states). - - Each view's diagrams are auto-wrapped and organized into a directory- - matching navigation tree (one generated index.rst per source directory). - A hand-authored index.rst/index.md placed alongside diagrams composes - with (its text is included above) that directory's generated toctree, - rather than being replaced by it. A hand-authored .rst/.md - next to a same-named .puml suppresses that diagram's generated - wrapper page, so real authored prose is always used over the generated - placeholder. For full control over a diagram's page, omit the .puml from - the view attribute below and reference it with your own `.. uml::`. + component: static views (class/component/package diagrams) and dynamic + views (sequence/activity/state diagrams), plus public/internal API + diagrams. Each view's diagrams are auto-wrapped into a generated, + directory-matching navigation tree; hand-authored index/`` pages + compose with or override that generated navigation -- see + docs/user_guide/architectural_design.rst for the full authoring-mode + reference. Args: name: The name of the architectural design target. Used as the base diff --git a/bazel/rules/rules_score/private/dependable_element.bzl b/bazel/rules/rules_score/private/dependable_element.bzl index 928ab818..1bd4642d 100644 --- a/bazel/rules/rules_score/private/dependable_element.bzl +++ b/bazel/rules/rules_score/private/dependable_element.bzl @@ -58,6 +58,7 @@ load( load("//bazel/rules/rules_score/private:sphinx_module.bzl", "sphinx_module") load("//bazel/rules/rules_score/private:validation.bzl", "PROFILES", "VALIDATION_ATTRS", "run_validation") load("//bazel/rules/rules_score/private:verbosity.bzl", "VERBOSITY_ATTR", "get_log_level") +load("//bazel/rules/rules_score/private:views.bzl", "ARCH_VIEWS") # ============================================================================ # Template Constants @@ -129,17 +130,6 @@ _INTEGRITY_LEVEL_RANK = {level: rank for rank, level in enumerate(_INTEGRITY_LEV # Helper Functions for Documentation Generation # ============================================================================ -# View name -> display title, mirroring architectural_design.bzl's own -# (private) _VIEWS mapping; kept in this same static/dynamic/public_api/ -# internal_api order so software_arch.rst's subsections appear in a stable, -# predictable order regardless of dict iteration order elsewhere. -_ARCH_VIEW_TITLES = [ - ("static", "Static Design"), - ("dynamic", "Dynamic Design"), - ("public_api", "Public API"), - ("internal_api", "Internal API"), -] - def _make_toctree(caption, entries, maxdepth = 1): """Return a toctree RST block with a caption, or empty string if entries is empty.""" if not entries: @@ -470,10 +460,9 @@ def _process_architectural_design_files(ctx, label, seen_paths, errors, path_pre view_by_path = {} if ArchitecturalDesignInfo in label: info = label[ArchitecturalDesignInfo] - if hasattr(info, "view_indexes"): - for view_name, navigation in info.view_indexes.items(): - if navigation and navigation.root_index: - view_by_path[navigation.root_index.path] = view_name + for view_name, root_index in info.view_root_indexes.items(): + if root_index: + view_by_path[root_index.path] = view_name for artifact_file in doc_files: if _is_document_file(artifact_file) and artifact_file.path not in srcs_paths: @@ -551,17 +540,16 @@ def _generate_software_arch_page( "", ]) - # Mirrors architectural_design.bzl's _VIEWS mapping (view name -> - # display title); iterated in the same static/dynamic/public_api/ - # internal_api order so each view gets its own subsection when it - # has at least one ref (normally just its one root index entry). + # Iterated in ARCH_VIEWS' static/dynamic/public_api/internal_api order + # so each view gets its own subsection when it has at least one ref + # (normally just its one root index entry). refs_by_view = { "static": static_refs, "dynamic": dynamic_refs, "public_api": public_api_refs, "internal_api": internal_api_refs, } - for view_name, view_title in _ARCH_VIEW_TITLES: + for view_name, view_title in ARCH_VIEWS: view_refs = refs_by_view[view_name] if not view_refs: continue diff --git a/bazel/rules/rules_score/private/views.bzl b/bazel/rules/rules_score/private/views.bzl new file mode 100644 index 00000000..2a305010 --- /dev/null +++ b/bazel/rules/rules_score/private/views.bzl @@ -0,0 +1,31 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* + +"""Shared architectural_design view definitions. + +Both architectural_design.bzl (the producer) and dependable_element.bzl (the +consumer, generating software_arch.rst's per-view subsections) need the same +static/dynamic/public_api/internal_api view names, in the same order, mapped +to the same display titles -- kept here once so the two can never drift. +""" + +# Views recognized by architectural_design, in display order, mapped to the +# title used both as that view's top-level navigation index page heading +# (architectural_design.bzl) and its software_arch.rst subsection heading +# (dependable_element.bzl). +ARCH_VIEWS = [ + ("static", "Static Design"), + ("dynamic", "Dynamic Design"), + ("public_api", "Public API"), + ("internal_api", "Internal API"), +] diff --git a/bazel/rules/rules_score/providers.bzl b/bazel/rules/rules_score/providers.bzl index 658b761c..7a612d1b 100644 --- a/bazel/rules/rules_score/providers.bzl +++ b/bazel/rules/rules_score/providers.bzl @@ -204,7 +204,7 @@ ArchitecturalDesignInfo = provider( "dynamic": "Depset of FlatBuffers binaries for dynamic architecture diagrams (sequence diagrams, activity diagrams, etc.)", "public_api": "Depset of FlatBuffers binaries for public API diagrams (class diagrams, etc.)", "internal_api": "Depset of FlatBuffers binaries for internal API diagrams (class diagrams, etc.)", - "view_indexes": "Dict mapping view name ('static', 'dynamic', 'public_api', 'internal_api') to that view's navigation struct (wrappers, indexes, root_index — see emit_view_navigation), or None for views with no navigable files.", + "view_root_indexes": "Dict mapping view name ('static', 'dynamic', 'public_api', 'internal_api') to that view's single top-level toctree-entry File (see emit_view_navigation's root_index), or None for views with no navigable files.", "name": "Name of the architectural design target", "public_api_lobster_files": "Depset of .lobster traceability files generated from public_api diagrams.", "validation_logs": "List of validation log entries produced by this architectural design target. Each entry has file and name fields.", diff --git a/bazel/rules/rules_score/test/BUILD b/bazel/rules/rules_score/test/BUILD index ebbeeb3c..9169ee54 100644 --- a/bazel/rules/rules_score/test/BUILD +++ b/bazel/rules/rules_score/test/BUILD @@ -11,6 +11,7 @@ # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* +load("@bazel_skylib//rules:build_test.bzl", "build_test") load("@pip_tooling_test//:requirements.bzl", "requirement") load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test") load("@rules_python//python:defs.bzl", "py_test") @@ -50,14 +51,14 @@ load( ":lobster_config_test.bzl", "lobster_config_test_suite", ) -load( - ":puml_layout_crash_repro_test.bzl", - "puml_layout_crash_repro_test", -) load( ":puml_layout_test.bzl", "puml_layout_test_suite", ) +load( + ":puml_stem_collision_test.bzl", + "stem_collision_fails_test", +) load( ":requirements_multi_spec_test.bzl", "asr_multi_spec_provider_test", @@ -285,8 +286,7 @@ architectural_design( # Regression fixtures: a generated navigation page and an authored page # claiming the same output path must be reconciled by plan_view_layout() -# rather than colliding in declare_file(); see -# puml_layout_crash_repro_test.bzl. +# rather than colliding in declare_file(). architectural_design( name = "arch_design_authored_index_compose_repro", static = [ @@ -303,14 +303,41 @@ architectural_design( ], ) -puml_layout_crash_repro_test( +# Regression fixture: a view with no diagrams at all -- only a hand-authored +# index.md -- must still produce a valid root index (authored body composed +# above an empty toctree), not crash or emit a bogus generated title. +architectural_design( + name = "arch_design_diagram_free_repro", + static = ["fixtures/diagram_free/index.md"], +) + +build_test( name = "puml_layout_authored_index_compose_builds", - target_under_test = ":arch_design_authored_index_compose_repro", + targets = [":arch_design_authored_index_compose_repro"], ) -puml_layout_crash_repro_test( +build_test( name = "puml_layout_authored_rst_override_builds", - target_under_test = ":arch_design_authored_rst_override_repro", + targets = [":arch_design_authored_rst_override_repro"], +) + +# Regression fixture: two diagrams whose basenames collide ("foo.puml" under +# both "a/b/" and "a_b/") need directory-based disambiguation, which itself +# collides once "/" is replaced with "_" in both -- must fail analysis +# rather than silently overwrite one diagram's output with the other's; see +# :stem_collision_fails_test below. +architectural_design( + name = "arch_design_stem_collision_repro", + static = [ + "fixtures/stem_collision/a/b/foo.puml", + "fixtures/stem_collision/a_b/foo.puml", + ], + tags = ["manual"], +) + +stem_collision_fails_test( + name = "stem_collision_fails_test", + target_under_test = ":arch_design_stem_collision_repro", ) # Wraps both crash-repro architectural_design targets above in a real @@ -326,6 +353,7 @@ dependable_element( architectural_design = [ ":arch_design_authored_index_compose_repro", ":arch_design_authored_rst_override_repro", + ":arch_design_diagram_free_repro", ], assumptions_of_use = [":aous"], components = [], @@ -1098,6 +1126,19 @@ sh_test( data = [":authored_layout_example_lib_index"], ) +# Regression test: a diagram-free view (only a hand-authored index.md, no +# .puml at all) must still stage a root index whose toctree is empty and +# whose body is the authored markdown, not a crash or a bogus generated page. +sh_test( + name = "authored_layout_diagram_free_test", + srcs = ["check_authored_layout_content.sh"], + args = [ + "diagram_free", + "$(rootpaths :authored_layout_example_lib_index)", + ], + data = [":authored_layout_example_lib_index"], +) + # ============================================================================ # SEooC-Specific Tests # ============================================================================ diff --git a/bazel/rules/rules_score/test/check_authored_layout_content.sh b/bazel/rules/rules_score/test/check_authored_layout_content.sh index 4c2fddb4..16255196 100755 --- a/bazel/rules/rules_score/test/check_authored_layout_content.sh +++ b/bazel/rules/rules_score/test/check_authored_layout_content.sh @@ -17,7 +17,7 @@ set -euo pipefail # actual *staged* RST content dependable_element produces for Sphinx to consume # -- see plan_view_layout()'s docstring in puml_utils.bzl. # -# $1 selects which scenario to check: "compose" or "override". +# $1 selects which scenario to check: "compose", "override", or "diagram_free". # Remaining args are the `$(rootpaths :authored_layout_example_lib_index)` # runfiles paths. @@ -82,8 +82,34 @@ case "${mode}" in exit 1 fi ;; + diagram_free) + index_file=$(find_file "arch_design_diagram_free_repro/fixtures/diagram_free/index.rst" "$@") + inc_file=$(find_file "arch_design_diagram_free_repro/fixtures/diagram_free/index.md.inc" "$@") + + # The generated index.rst must include the authored markdown body. + if ! grep -Fq '.. include:: index.md.inc' "${index_file}"; then + echo "Error: expected diagram-free index.rst to include the authored body:" >&2 + cat "${index_file}" >&2 + exit 1 + fi + + # There are no diagrams to auto-wrap, so the toctree must be empty + # (no entries at all after the "maxdepth" line). + if [[ $(grep -c '^ [^:[:space:]]' "${index_file}") -ne 0 ]]; then + echo "Error: expected diagram-free index.rst toctree to have no entries:" >&2 + cat "${index_file}" >&2 + exit 1 + fi + + # The staged .inc file must carry the authored prose verbatim. + if ! grep -Fq 'Diagram-Free Overview' "${inc_file}"; then + echo "Error: expected staged index.md.inc to carry the authored prose:" >&2 + cat "${inc_file}" >&2 + exit 1 + fi + ;; *) - echo "Error: unknown mode '${mode}' (expected 'compose' or 'override')" >&2 + echo "Error: unknown mode '${mode}' (expected 'compose', 'override', or 'diagram_free')" >&2 exit 1 ;; esac diff --git a/bazel/rules/rules_score/test/fixtures/diagram_free/index.md b/bazel/rules/rules_score/test/fixtures/diagram_free/index.md new file mode 100644 index 00000000..ae437356 --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/diagram_free/index.md @@ -0,0 +1,19 @@ + + +# Diagram-Free Overview + +This view has no diagrams at all -- just this hand-authored page -- covering +the case where `plan_view_layout()`/`emit_view_navigation()` must produce a +valid root index (authored body composed above an empty toctree) with nothing +to auto-wrap. diff --git a/bazel/rules/rules_score/test/fixtures/stem_collision/a/b/foo.puml b/bazel/rules/rules_score/test/fixtures/stem_collision/a/b/foo.puml new file mode 100644 index 00000000..7fbdb588 --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/stem_collision/a/b/foo.puml @@ -0,0 +1,16 @@ +' ******************************************************************************* +' Copyright (c) 2026 Contributors to the Eclipse Foundation +' +' See the NOTICE file(s) distributed with this work for additional +' information regarding copyright ownership. +' +' This program and the accompanying materials are made available under the +' terms of the Apache License Version 2.0 which is available at +' https://www.apache.org/licenses/LICENSE-2.0 +' +' SPDX-License-Identifier: Apache-2.0 +' ******************************************************************************* + +@startuml +class Foo +@enduml diff --git a/bazel/rules/rules_score/test/fixtures/stem_collision/a_b/foo.puml b/bazel/rules/rules_score/test/fixtures/stem_collision/a_b/foo.puml new file mode 100644 index 00000000..7fbdb588 --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/stem_collision/a_b/foo.puml @@ -0,0 +1,16 @@ +' ******************************************************************************* +' Copyright (c) 2026 Contributors to the Eclipse Foundation +' +' See the NOTICE file(s) distributed with this work for additional +' information regarding copyright ownership. +' +' This program and the accompanying materials are made available under the +' terms of the Apache License Version 2.0 which is available at +' https://www.apache.org/licenses/LICENSE-2.0 +' +' SPDX-License-Identifier: Apache-2.0 +' ******************************************************************************* + +@startuml +class Foo +@enduml diff --git a/bazel/rules/rules_score/test/puml_layout_crash_repro_test.bzl b/bazel/rules/rules_score/test/puml_layout_crash_repro_test.bzl deleted file mode 100644 index 2bc898ae..00000000 --- a/bazel/rules/rules_score/test/puml_layout_crash_repro_test.bzl +++ /dev/null @@ -1,50 +0,0 @@ -# ******************************************************************************* -# Copyright (c) 2026 Contributors to the Eclipse Foundation -# -# See the NOTICE file(s) distributed with this work for additional -# information regarding copyright ownership. -# -# This program and the accompanying materials are made available under the -# terms of the Apache License Version 2.0 which is available at -# https://www.apache.org/licenses/LICENSE-2.0 -# -# SPDX-License-Identifier: Apache-2.0 -# ******************************************************************************* -""" -Regression tests proving two previously-crashing architectural_design configs -now build successfully, per plan_view_layout()'s compose/override reconciliation -(bazel/rules/rules_score/private/puml_utils.bzl): - -- An authored directory `index.rst` alongside a `.puml` diagram in the same - view (compose case): used to collide with the generated `index.rst` via a - raw `declare_file()` "conflicting actions" error. -- An authored same-stem `overview.rst` alongside `overview.puml` (override - case): used to collide with the generated wrapper page the same way. - -Each analysistest below simply asserts `ArchitecturalDesignInfo` is present on -the target under test; if `plan_view_layout`/`_architectural_design_impl` still -raised a raw declare_file collision (or a `plan.errors` fail()), the -`architectural_design` target's own analysis would fail and the wrapping -analysistest target would fail to build -- so a passing test here is itself -the regression proof, not just the assertion's literal content. -""" - -load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") -load("@score_tooling//bazel/rules/rules_score:providers.bzl", "ArchitecturalDesignInfo") - -def _puml_layout_crash_repro_test_impl(ctx): - env = analysistest.begin(ctx) - target_under_test = analysistest.target_under_test(env) - - asserts.true( - env, - ArchitecturalDesignInfo in target_under_test, - "Expected architectural_design to provide ArchitecturalDesignInfo " + - "(i.e. to have built successfully at all)", - ) - - return analysistest.end(env) - -puml_layout_crash_repro_test = analysistest.make( - impl = _puml_layout_crash_repro_test_impl, -) diff --git a/bazel/rules/rules_score/test/puml_stem_collision_test.bzl b/bazel/rules/rules_score/test/puml_stem_collision_test.bzl new file mode 100644 index 00000000..d932b638 --- /dev/null +++ b/bazel/rules/rules_score/test/puml_stem_collision_test.bzl @@ -0,0 +1,38 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +"""Analysis test for architectural_design's `_disambiguated_stems()` residual +collision `fail()` path (bazel/rules/rules_score/private/architectural_design.bzl). + +Two same-basename diagrams already need directory-based disambiguation +(`a/b/foo.puml` and `a_b/foo.puml` both have basename `foo.puml`), and that +disambiguation itself collides once `/` is replaced with `_` in both +directory parts (`a/b` -> `a_b`, `a_b` -> `a_b`) -- both end up wanting the +same output stem `a_b__foo`. This must fail analysis with a message naming +both source files, rather than silently letting one diagram's output +overwrite the other's. +""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") + +def _stem_collision_fails_test_impl(ctx): + env = analysistest.begin(ctx) + asserts.expect_failure( + env, + "both disambiguate to the output stem 'fixtures_stem_collision_a_b__foo'", + ) + return analysistest.end(env) + +stem_collision_fails_test = analysistest.make( + _stem_collision_fails_test_impl, + expect_failure = True, +) From c32f961e7243f3c435dd9147361e9066a76aeeed Mon Sep 17 00:00:00 2001 From: Jochen Hoenle <173445474+hoe-jo@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:48:45 +0200 Subject: [PATCH 4/5] dependable_element: unconditional label prefixing, view refs as dict, staged-path collision tests - _process_artifact_type: removed the use_label_subdirectories = len(attr_list) > 1 conditional. Every label attached to an artifact-type attribute (assumptions_of_use, dependability_analysis, checklists, glossary) now gets an unconditional "/" staging prefix - Applied the same unconditional "/" prefix to the feature_requirements and assumed_system_requirements loops (previously staged with no prefix at all, regardless of how many requirements targets were attached). - _process_architectural_design_files: collapsed its 6-tuple return (output_files, static_refs, dynamic_refs, public_api_refs, internal_api_refs, unclassified_refs) to (output_files, refs_by_view, unclassified_refs), with refs_by_view keyed generically over ARCH_VIEWS. - Fixed a latent bug in _check_staged_path's callers: a detected collision was only recorded in `errors` for a later fail(), but the colliding file's declare_file()/symlink() actions were still unconditionally registered -- so two genuinely different source files staging to the same path tripped Bazel's own ActionConflictException before this rule's fail() was ever reached - Added 2 regression tests - Extracted test/lib/find_runfile.sh --- .../private/dependable_element.bzl | 144 ++++++++++-------- bazel/rules/rules_score/test/BUILD | 81 +++++++++- .../test/check_authored_layout_content.sh | 26 +--- ...ble_element_staged_path_collision_test.bzl | 59 +++++++ .../test/fixtures/staged_path_collision/BUILD | 23 +++ .../staged_path_collision/content.rst | 18 +++ .../dup_srcs_aux_fixture.bzl | 37 +++++ .../staged_path_collision/pkg_a/BUILD | 26 ++++ .../staged_path_collision/pkg_a/content.rst | 17 +++ .../staged_path_collision/pkg_b/BUILD | 23 +++ .../staged_path_collision/pkg_b/content.rst | 17 +++ .../rules_score/test/lib/find_runfile.sh | 40 +++++ 12 files changed, 426 insertions(+), 85 deletions(-) create mode 100644 bazel/rules/rules_score/test/dependable_element_staged_path_collision_test.bzl create mode 100644 bazel/rules/rules_score/test/fixtures/staged_path_collision/BUILD create mode 100644 bazel/rules/rules_score/test/fixtures/staged_path_collision/content.rst create mode 100644 bazel/rules/rules_score/test/fixtures/staged_path_collision/dup_srcs_aux_fixture.bzl create mode 100644 bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_a/BUILD create mode 100644 bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_a/content.rst create mode 100644 bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_b/BUILD create mode 100644 bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_b/content.rst create mode 100755 bazel/rules/rules_score/test/lib/find_runfile.sh diff --git a/bazel/rules/rules_score/private/dependable_element.bzl b/bazel/rules/rules_score/private/dependable_element.bzl index 1bd4642d..a366ab36 100644 --- a/bazel/rules/rules_score/private/dependable_element.bzl +++ b/bazel/rules/rules_score/private/dependable_element.bzl @@ -276,8 +276,11 @@ def _check_staged_path(seen_paths, declared_relative_path, source_label, errors) Bazel's own `declare_file()` collision ("conflicting actions") error names only actions/outputs, not the two source labels a documentation author would actually need to fix -- so callers `fail()` on `errors` - themselves once all staging for this rule has been planned, rather than - letting that raw error surface first. + themselves once all staging for this rule has been planned. That only + works if callers also skip staging (declare_file/symlink) a path this + function reports as colliding: registering the same output twice trips + Bazel's own action-conflict detection immediately, before this rule's + `fail()` is ever reached. Args: seen_paths: Dict from declared relative path (e.g. @@ -287,6 +290,11 @@ def _check_staged_path(seen_paths, declared_relative_path, source_label, errors) relative to `ctx.label.name`. source_label: The `Label` this file came from. errors: List of human-readable collision messages; mutated in place. + + Returns: + True if `declared_relative_path` collides with a path already seen + (an error was appended; the caller must not stage this file), False + if it was newly recorded and is safe to stage. """ existing = seen_paths.get(declared_relative_path) if existing != None: @@ -307,8 +315,9 @@ def _check_staged_path(seen_paths, declared_relative_path, source_label, errors) source_label, ), ) - return + return True seen_paths[declared_relative_path] = str(source_label) + return False def _create_artifact_symlink(ctx, artifact_name, artifact_file, relative_path): """Create symlink for artifact file in output directory. @@ -383,7 +392,10 @@ def _process_artifact_files(ctx, artifact_name, label, seen_paths, errors, path_ continue relative_path = path_prefix + _compute_relative_path(artifact_file, common_dir) - _check_staged_path(seen_paths, artifact_name + "/" + relative_path, label.label, errors) + if _check_staged_path(seen_paths, artifact_name + "/" + relative_path, label.label, errors): + # Staging a path already reported as colliding would trip Bazel's + # own action-conflict detection before this rule's fail(). + continue # Create symlink output_file = _create_artifact_symlink( @@ -403,7 +415,8 @@ def _process_artifact_files(ctx, artifact_name, label, seen_paths, errors, path_ # Process aux_srcs: symlink without adding to outer toctree index. for artifact_file in aux_files: relative_path = path_prefix + _compute_relative_path(artifact_file, common_dir) - _check_staged_path(seen_paths, artifact_name + "/" + relative_path, label.label, errors) + if _check_staged_path(seen_paths, artifact_name + "/" + relative_path, label.label, errors): + continue output_file = _create_artifact_symlink( ctx, artifact_name, @@ -414,6 +427,31 @@ def _process_artifact_files(ctx, artifact_name, label, seen_paths, errors, path_ return (output_files, index_refs) +def _architectural_design_root(files, target_name): + """Return the staging root for one architectural_design label's files. + + Every file an architectural_design puts into SphinxSourcesInfo is declared + under "//", so the staged layout is always + "//...". `_find_common_directory` alone can't guarantee + that: a view without diagrams has no idmap sidecars sitting directly in + "/", so the common directory sinks one level deeper and the + "/" segment is lost. Truncating at the target-name segment pins it. + + Args: + files: List of File objects from one architectural_design label. + target_name: That label's target name. + + Returns: + String staging root, or `_find_common_directory`'s result unchanged + when no path segment matches `target_name`. + """ + common_dir = _find_common_directory(files) + parts = common_dir.split("/") + for index in range(len(parts) - 1, -1, -1): + if parts[index] == target_name: + return "/".join(parts[:index + 1]) + return common_dir + def _process_architectural_design_files(ctx, label, seen_paths, errors, path_prefix = ""): """Process all files from an architectural_design label, returning output_files and classified refs. @@ -425,20 +463,18 @@ def _process_architectural_design_files(ctx, label, seen_paths, errors, path_pre (see _process_artifact_files); mutated in place. errors: List collecting collision messages; mutated in place. path_prefix: Prefix (the target name, e.g. "my_arch_design/") inserted - before each file's relative path. Applied unconditionally --not - only when 2+ architectural_design labels are attached-- so the - staged layout, and therefore the generated HTML URLs, is the same - whether a dependable_element has one architectural_design label or - several, rather than flat for one and nested for several. + before each file's relative path. Applied whether or not a second + architectural_design label is attached, so the staged layout -- + and therefore the generated HTML URLs -- keeps the same shape as + the dependable_element grows. Returns: - Tuple of (output_files, static_refs, dynamic_refs, public_api_refs, internal_api_refs, unclassified_refs) + Tuple of (output_files, refs_by_view, unclassified_refs), where + refs_by_view is a dict keyed by each ARCH_VIEWS view name ("static", + "dynamic", "public_api", "internal_api") mapping to that view's refs. """ output_files = [] - static_refs = [] - dynamic_refs = [] - public_api_refs = [] - internal_api_refs = [] + refs_by_view = {view_name: [] for view_name, _ in ARCH_VIEWS} unclassified_refs = [] all_files = _get_sphinx_files(label) @@ -449,10 +485,10 @@ def _process_architectural_design_files(ctx, label, seen_paths, errors, path_pre aux_files = label[SphinxSourcesInfo].aux_srcs.to_list() if not doc_files and not aux_files: - return (output_files, static_refs, dynamic_refs, public_api_refs, internal_api_refs, unclassified_refs) + return (output_files, refs_by_view, unclassified_refs) srcs_paths = {f.path: True for f in label[SphinxSourcesInfo].srcs.to_list()} - common_dir = _find_common_directory(doc_files + aux_files) + common_dir = _architectural_design_root(doc_files + aux_files, label.label.name) # Each view's top-level root index (the only file of that view present # in srcs_paths) is the single toctree entry surfaced for that view; map @@ -469,7 +505,10 @@ def _process_architectural_design_files(ctx, label, seen_paths, errors, path_pre continue relative_path = path_prefix + _compute_relative_path(artifact_file, common_dir) - _check_staged_path(seen_paths, "architectural_design/" + relative_path, label.label, errors) + if _check_staged_path(seen_paths, "architectural_design/" + relative_path, label.label, errors): + # Staging a path already reported as colliding would trip Bazel's + # own action-conflict detection before this rule's fail(). + continue output_file = _create_artifact_symlink( ctx, @@ -483,20 +522,15 @@ def _process_architectural_design_files(ctx, label, seen_paths, errors, path_pre doc_path = "architectural_design/" + relative_path doc_ref = doc_path.removesuffix(".rst").removesuffix(".md") view_name = view_by_path.get(artifact_file.path) - if view_name == "static": - static_refs.append(doc_ref) - elif view_name == "dynamic": - dynamic_refs.append(doc_ref) - elif view_name == "public_api": - public_api_refs.append(doc_ref) - elif view_name == "internal_api": - internal_api_refs.append(doc_ref) + if view_name in refs_by_view: + refs_by_view[view_name].append(doc_ref) else: unclassified_refs.append(doc_ref) for artifact_file in aux_files: relative_path = path_prefix + _compute_relative_path(artifact_file, common_dir) - _check_staged_path(seen_paths, "architectural_design/" + relative_path, label.label, errors) + if _check_staged_path(seen_paths, "architectural_design/" + relative_path, label.label, errors): + continue output_file = _create_artifact_symlink( ctx, "architectural_design", @@ -505,22 +539,18 @@ def _process_architectural_design_files(ctx, label, seen_paths, errors, path_pre ) output_files.append(output_file) - return (output_files, static_refs, dynamic_refs, public_api_refs, internal_api_refs, unclassified_refs) + return (output_files, refs_by_view, unclassified_refs) def _generate_software_arch_page( ctx, feature_req_refs, - static_refs, - dynamic_refs, - public_api_refs, - internal_api_refs, + refs_by_view, unclassified_refs, dependability_refs, output_files): """Generate software_arch.rst page with section subheadings when categorized entries exist.""" has_categories = bool( - feature_req_refs or static_refs or dynamic_refs or - public_api_refs or internal_api_refs, + feature_req_refs or any(refs_by_view.values()), ) if not has_categories and not unclassified_refs and not dependability_refs: @@ -543,12 +573,6 @@ def _generate_software_arch_page( # Iterated in ARCH_VIEWS' static/dynamic/public_api/internal_api order # so each view gets its own subsection when it has at least one ref # (normally just its one root index entry). - refs_by_view = { - "static": static_refs, - "dynamic": dynamic_refs, - "public_api": public_api_refs, - "internal_api": internal_api_refs, - } for view_name, view_title in ARCH_VIEWS: view_refs = refs_by_view[view_name] if not view_refs: @@ -629,14 +653,13 @@ def _process_artifact_type(ctx, artifact_name, seen_paths, errors): # _find_common_directory), so 2+ labels can genuinely resolve the same # relative path (e.g. two "checklists" labels each exporting a top-level # "checklist.md") even though this artifact type has no per-directory - # index.rst of its own. Namespace under each label's own target name in - # that case; a single label keeps the flat (unprefixed) layout for - # readability. The _check_staged_path safety net below still catches the - # residual case of two labels sharing a target *name* from different - # packages. - use_label_subdirectories = len(attr_list) > 1 + # index.rst of its own. Namespacing under each label's own target name + # keeps the staged layout -- and therefore the generated HTML URLs -- + # the same shape whether one label is attached or several. The + # _check_staged_path safety net below still catches the residual case of + # two labels sharing a target *name* from different packages. for label in attr_list: - path_prefix = "{}/".format(label.label.name) if use_label_subdirectories else "" + path_prefix = "{}/".format(label.label.name) label_outputs, label_refs = _process_artifact_files( ctx, artifact_name, @@ -1065,10 +1088,7 @@ def _dependable_element_index_impl(ctx): output_files.extend(files) artifacts_by_type[artifact_name] = refs - arch_static_refs = [] - arch_dynamic_refs = [] - arch_public_api_refs = [] - arch_internal_api_refs = [] + arch_refs_by_view = {view_name: [] for view_name, _ in ARCH_VIEWS} arch_unclassified_refs = [] if ctx.attr.architectural_design: @@ -1080,7 +1100,7 @@ def _dependable_element_index_impl(ctx): # sharing a target *name* from different packages. for ad_target in ctx.attr.architectural_design: path_prefix = "{}/".format(ad_target.label.name) - ad_files, s_refs, d_refs, p_refs, i_refs, u_refs = _process_architectural_design_files( + ad_files, view_refs, u_refs = _process_architectural_design_files( ctx, ad_target, seen_staged_paths, @@ -1088,14 +1108,14 @@ def _dependable_element_index_impl(ctx): path_prefix = path_prefix, ) output_files.extend(ad_files) - arch_static_refs.extend(s_refs) - arch_dynamic_refs.extend(d_refs) - arch_public_api_refs.extend(p_refs) - arch_internal_api_refs.extend(i_refs) + for view_name, refs in view_refs.items(): + arch_refs_by_view[view_name].extend(refs) arch_unclassified_refs.extend(u_refs) # Collect feature_requirements refs from requirements targets that - # carry FeatureRequirementsInfo. + # carry FeatureRequirementsInfo. Namespaced under each label's own target + # name, matching the staged layout of architectural_design and every + # other artifact type. feature_req_refs = [] for req_target in ctx.attr.requirements: if FeatureRequirementsInfo in req_target: @@ -1105,12 +1125,14 @@ def _dependable_element_index_impl(ctx): req_target, seen_paths = seen_staged_paths, errors = staging_errors, + path_prefix = "{}/".format(req_target.label.name), ) output_files.extend(label_files) feature_req_refs.extend(label_refs) # Collect assumed_system_requirements refs from requirements targets that - # carry AssumedSystemRequirementsInfo. + # carry AssumedSystemRequirementsInfo. Namespaced the same way as + # feature_requirements above. assumed_system_req_refs = [] for req_target in ctx.attr.requirements: if AssumedSystemRequirementsInfo in req_target: @@ -1120,6 +1142,7 @@ def _dependable_element_index_impl(ctx): req_target, seen_paths = seen_staged_paths, errors = staging_errors, + path_prefix = "{}/".format(req_target.label.name), ) output_files.extend(label_files) assumed_system_req_refs.extend(label_refs) @@ -1213,10 +1236,7 @@ def _dependable_element_index_impl(ctx): software_arch_ref = _generate_software_arch_page( ctx, feature_req_refs = feature_req_refs, - static_refs = arch_static_refs, - dynamic_refs = arch_dynamic_refs, - public_api_refs = arch_public_api_refs, - internal_api_refs = arch_internal_api_refs, + refs_by_view = arch_refs_by_view, unclassified_refs = arch_unclassified_refs, dependability_refs = artifacts_by_type["dependability_analysis"], output_files = output_files, diff --git a/bazel/rules/rules_score/test/BUILD b/bazel/rules/rules_score/test/BUILD index 9169ee54..5df54825 100644 --- a/bazel/rules/rules_score/test/BUILD +++ b/bazel/rules/rules_score/test/BUILD @@ -34,6 +34,11 @@ load( load("@score_tooling//bazel/rules/rules_score:sphinx_toolchain.bzl", "score_sphinx_toolchain") load("@score_tooling//cpp/libclang:libclang_toolchain.bzl", "libclang_toolchain") load("@trlc//:trlc.bzl", "trlc_requirements", "trlc_requirements_test", "trlc_specification") +load( + ":dependable_element_staged_path_collision_test.bzl", + "cross_label_collision_fails_test", + "same_label_collision_fails_test", +) load( ":html_generation_test.bzl", "auto_config_generation_test", @@ -340,6 +345,62 @@ stem_collision_fails_test( target_under_test = ":arch_design_stem_collision_repro", ) +# Regression fixture: a single label whose SphinxSourcesInfo lists the same +# file in both `deps` and `aux_srcs` stages the exact same relative path +# twice from the exact same source label (see +# :staged_path_dup_srcs_aux_fixture in fixtures/staged_path_collision) -- +# must fail analysis with _check_staged_path's same-label message rather +# than a raw Bazel "conflicting actions" error; see +# :staged_path_same_label_collision_fails_test below. +dependable_element( + name = "staged_path_same_label_collision_repro", + architectural_design = [], + assumptions_of_use = [":aous"], + checklists = ["//fixtures/staged_path_collision:dup_srcs_aux"], + components = [], + dependability_analysis = [":dependability_analysis_target"], + integrity_level = "B", + maturity = "development", + requirements = [":feat_req"], + tags = ["manual"], + tests = [], + deps = [], +) + +same_label_collision_fails_test( + name = "staged_path_same_label_collision_fails_test", + target_under_test = ":staged_path_same_label_collision_repro_index", +) + +# Regression fixture: two distinct labels from different packages that +# happen to share a target name ("dup") stage to the same relative path +# (each gets a "dup/" prefix -- see _process_artifact_type) from two +# different source labels -- must fail analysis with _check_staged_path's +# cross-label message; see +# :staged_path_cross_label_collision_fails_test below. +dependable_element( + name = "staged_path_cross_label_collision_repro", + architectural_design = [], + assumptions_of_use = [":aous"], + checklists = [ + "//fixtures/staged_path_collision/pkg_a:dup", + "//fixtures/staged_path_collision/pkg_b:dup", + ], + components = [], + dependability_analysis = [":dependability_analysis_target"], + integrity_level = "B", + maturity = "development", + requirements = [":feat_req"], + tags = ["manual"], + tests = [], + deps = [], +) + +cross_label_collision_fails_test( + name = "staged_path_cross_label_collision_fails_test", + target_under_test = ":staged_path_cross_label_collision_repro_index", +) + # Wraps both crash-repro architectural_design targets above in a real # dependable_element so the *staged, dependable-element-relative* RST content # can be asserted on (not just "it builds without crashing") -- see @@ -1110,7 +1171,10 @@ sh_test( "compose", "$(rootpaths :authored_layout_example_lib_index)", ], - data = [":authored_layout_example_lib_index"], + data = [ + "lib/find_runfile.sh", + ":authored_layout_example_lib_index", + ], ) # Regression test: an authored overview.rst *overrides* (suppresses) the @@ -1123,7 +1187,10 @@ sh_test( "override", "$(rootpaths :authored_layout_example_lib_index)", ], - data = [":authored_layout_example_lib_index"], + data = [ + "lib/find_runfile.sh", + ":authored_layout_example_lib_index", + ], ) # Regression test: a diagram-free view (only a hand-authored index.md, no @@ -1136,7 +1203,10 @@ sh_test( "diagram_free", "$(rootpaths :authored_layout_example_lib_index)", ], - data = [":authored_layout_example_lib_index"], + data = [ + "lib/find_runfile.sh", + ":authored_layout_example_lib_index", + ], ) # ============================================================================ @@ -1157,7 +1227,10 @@ sh_test( "seooc_test_lib_index/index.rst", "$(rootpaths :seooc_test_lib_index)", ], - data = [":seooc_test_lib_index"], + data = [ + "lib/find_runfile.sh", + ":seooc_test_lib_index", + ], ) # Regression test: clickable_plantuml must actually inject a working link diff --git a/bazel/rules/rules_score/test/check_authored_layout_content.sh b/bazel/rules/rules_score/test/check_authored_layout_content.sh index 16255196..2615895f 100755 --- a/bazel/rules/rules_score/test/check_authored_layout_content.sh +++ b/bazel/rules/rules_score/test/check_authored_layout_content.sh @@ -21,27 +21,15 @@ set -euo pipefail # Remaining args are the `$(rootpaths :authored_layout_example_lib_index)` # runfiles paths. +source "${TEST_SRCDIR}/${TEST_WORKSPACE}/lib/find_runfile.sh" + mode="$1" shift -find_file() { - local suffix="$1" - shift - for rel_path in "$@"; do - candidate="${TEST_SRCDIR}/${TEST_WORKSPACE}/${rel_path}" - if [[ -f "${candidate}" && "${candidate}" == *"${suffix}" ]]; then - echo "${candidate}" - return 0 - fi - done - echo "Error: could not locate '*${suffix}' among: $*" >&2 - return 1 -} - case "${mode}" in compose) - index_file=$(find_file "arch_design_authored_index_compose_repro/static/fixtures/authored/index.rst" "$@") - inc_file=$(find_file "arch_design_authored_index_compose_repro/static/fixtures/authored/index.rst.inc" "$@") + index_file=$(find_runfile "arch_design_authored_index_compose_repro/static/fixtures/authored/index.rst" "$@") + inc_file=$(find_runfile "arch_design_authored_index_compose_repro/static/fixtures/authored/index.rst.inc" "$@") # The generated index.rst must include the authored body instead of # emitting its own title/marker. @@ -66,7 +54,7 @@ case "${mode}" in fi ;; override) - overview_file=$(find_file "arch_design_authored_rst_override_repro/static/fixtures/authored_override/overview.rst" "$@") + overview_file=$(find_runfile "arch_design_authored_rst_override_repro/static/fixtures/authored_override/overview.rst" "$@") # The staged overview.rst must be the authored file verbatim... if ! grep -Fq 'Hand-authored prose for the overview diagram' "${overview_file}"; then @@ -83,8 +71,8 @@ case "${mode}" in fi ;; diagram_free) - index_file=$(find_file "arch_design_diagram_free_repro/fixtures/diagram_free/index.rst" "$@") - inc_file=$(find_file "arch_design_diagram_free_repro/fixtures/diagram_free/index.md.inc" "$@") + index_file=$(find_runfile "arch_design_diagram_free_repro/static/fixtures/diagram_free/index.rst" "$@") + inc_file=$(find_runfile "arch_design_diagram_free_repro/static/fixtures/diagram_free/index.md.inc" "$@") # The generated index.rst must include the authored markdown body. if ! grep -Fq '.. include:: index.md.inc' "${index_file}"; then diff --git a/bazel/rules/rules_score/test/dependable_element_staged_path_collision_test.bzl b/bazel/rules/rules_score/test/dependable_element_staged_path_collision_test.bzl new file mode 100644 index 00000000..a376a449 --- /dev/null +++ b/bazel/rules/rules_score/test/dependable_element_staged_path_collision_test.bzl @@ -0,0 +1,59 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +"""Analysis tests for dependable_element's `_check_staged_path()` residual +`fail()` paths (bazel/rules/rules_score/private/dependable_element.bzl). + +Both scenarios attach labels under the same artifact-type attribute +(`checklists`) so their outputs are staged into the same "checklists/" +directory, and both are only reachable because every label gets its own +"/" subdirectory prefix (see _process_artifact_type): + +- same-label: :staged_path_same_label_collision_repro (test/BUILD) attaches + a single label whose SphinxSourcesInfo lists the exact same file in both + `deps` and `aux_srcs` (see + fixtures/staged_path_collision/dup_srcs_aux_fixture.bzl), so + _process_artifact_files stages it twice -- once as a doc file, once as an + aux file -- both times from the same source label. +- cross-label: :staged_path_cross_label_collision_repro (test/BUILD) attaches + two distinct labels that happen to share a target *name* ("dup") from two + different packages, so both stage to the same relative path from two + different source labels. +""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") + +def _same_label_collision_fails_test_impl(ctx): + env = analysistest.begin(ctx) + asserts.expect_failure( + env, + "'checklists/dup_srcs_aux/content.rst' would be staged twice from", + ) + return analysistest.end(env) + +same_label_collision_fails_test = analysistest.make( + _same_label_collision_fails_test_impl, + expect_failure = True, +) + +def _cross_label_collision_fails_test_impl(ctx): + env = analysistest.begin(ctx) + asserts.expect_failure( + env, + "'checklists/dup/content.rst' would be staged by both", + ) + return analysistest.end(env) + +cross_label_collision_fails_test = analysistest.make( + _cross_label_collision_fails_test_impl, + expect_failure = True, +) diff --git a/bazel/rules/rules_score/test/fixtures/staged_path_collision/BUILD b/bazel/rules/rules_score/test/fixtures/staged_path_collision/BUILD new file mode 100644 index 00000000..f5179a6a --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/staged_path_collision/BUILD @@ -0,0 +1,23 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* + +load(":dup_srcs_aux_fixture.bzl", "dup_srcs_aux_fixture") + +package(default_visibility = ["//:__pkg__"]) + +# See dup_srcs_aux_fixture.bzl and :staged_path_same_label_collision_repro in +# test/BUILD. +dup_srcs_aux_fixture( + name = "dup_srcs_aux", + src = "content.rst", +) diff --git a/bazel/rules/rules_score/test/fixtures/staged_path_collision/content.rst b/bazel/rules/rules_score/test/fixtures/staged_path_collision/content.rst new file mode 100644 index 00000000..006870ea --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/staged_path_collision/content.rst @@ -0,0 +1,18 @@ +.. + # ******************************************************************************* + # Copyright (c) 2026 Contributors to the Eclipse Foundation + # + # See the NOTICE file(s) distributed with this work for additional + # information regarding copyright ownership. + # + # This program and the accompanying materials are made available under the + # terms of the Apache License Version 2.0 which is available at + # https://www.apache.org/licenses/LICENSE-2.0 + # + # SPDX-License-Identifier: Apache-2.0 + # ******************************************************************************* +Duplicated Content +================== + +Fixture content for :staged_path_same_label_collision_repro, staged twice +(once as a doc file, once as an aux file) by :dup_srcs_aux_fixture. diff --git a/bazel/rules/rules_score/test/fixtures/staged_path_collision/dup_srcs_aux_fixture.bzl b/bazel/rules/rules_score/test/fixtures/staged_path_collision/dup_srcs_aux_fixture.bzl new file mode 100644 index 00000000..b2689a96 --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/staged_path_collision/dup_srcs_aux_fixture.bzl @@ -0,0 +1,37 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +"""Minimal test-only rule that lists the exact same file in both +`SphinxSourcesInfo.deps` and `SphinxSourcesInfo.aux_srcs`, so +dependable_element's `_process_artifact_files` stages it twice for a single +label -- once while iterating doc files, once while iterating aux files -- +both times from the same source label. Used by +:staged_path_same_label_collision_repro in test/BUILD to exercise +`_check_staged_path`'s same-label collision message +(dependable_element_staged_path_collision_test.bzl). +""" + +load("@score_tooling//bazel/rules/rules_score:providers.bzl", "SphinxSourcesInfo") + +def _dup_srcs_aux_fixture_impl(ctx): + files = depset([ctx.file.src]) + return [ + DefaultInfo(files = files), + SphinxSourcesInfo(srcs = files, deps = files, aux_srcs = files), + ] + +dup_srcs_aux_fixture = rule( + implementation = _dup_srcs_aux_fixture_impl, + attrs = { + "src": attr.label(allow_single_file = [".rst"], mandatory = True), + }, +) diff --git a/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_a/BUILD b/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_a/BUILD new file mode 100644 index 00000000..0386e768 --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_a/BUILD @@ -0,0 +1,26 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* + +load("@score_tooling//bazel/rules/rules_score:rules_score.bzl", "glossary") + +package(default_visibility = ["//:__pkg__"]) + +# Deliberately named the same as //fixtures/staged_path_collision/pkg_b:dup +# -- see :staged_path_cross_label_collision_repro in test/BUILD, which attaches +# both under the same artifact-type attribute to exercise _check_staged_path's +# cross-label collision message (two different labels sharing a target name +# from different packages). +glossary( + name = "dup", + srcs = ["content.rst"], +) diff --git a/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_a/content.rst b/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_a/content.rst new file mode 100644 index 00000000..c45c0aeb --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_a/content.rst @@ -0,0 +1,17 @@ +.. + # ******************************************************************************* + # Copyright (c) 2026 Contributors to the Eclipse Foundation + # + # See the NOTICE file(s) distributed with this work for additional + # information regarding copyright ownership. + # + # This program and the accompanying materials are made available under the + # terms of the Apache License Version 2.0 which is available at + # https://www.apache.org/licenses/LICENSE-2.0 + # + # SPDX-License-Identifier: Apache-2.0 + # ******************************************************************************* +Content A +========= + +Fixture content for :staged_path_cross_label_collision_repro (package a). diff --git a/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_b/BUILD b/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_b/BUILD new file mode 100644 index 00000000..3504ce4a --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_b/BUILD @@ -0,0 +1,23 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* + +load("@score_tooling//bazel/rules/rules_score:rules_score.bzl", "glossary") + +package(default_visibility = ["//:__pkg__"]) + +# Deliberately named the same as //fixtures/staged_path_collision/pkg_a:dup +# -- see :staged_path_cross_label_collision_repro in test/BUILD. +glossary( + name = "dup", + srcs = ["content.rst"], +) diff --git a/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_b/content.rst b/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_b/content.rst new file mode 100644 index 00000000..fd94eddc --- /dev/null +++ b/bazel/rules/rules_score/test/fixtures/staged_path_collision/pkg_b/content.rst @@ -0,0 +1,17 @@ +.. + # ******************************************************************************* + # Copyright (c) 2026 Contributors to the Eclipse Foundation + # + # See the NOTICE file(s) distributed with this work for additional + # information regarding copyright ownership. + # + # This program and the accompanying materials are made available under the + # terms of the Apache License Version 2.0 which is available at + # https://www.apache.org/licenses/LICENSE-2.0 + # + # SPDX-License-Identifier: Apache-2.0 + # ******************************************************************************* +Content B +========= + +Fixture content for :staged_path_cross_label_collision_repro (package b). diff --git a/bazel/rules/rules_score/test/lib/find_runfile.sh b/bazel/rules/rules_score/test/lib/find_runfile.sh new file mode 100755 index 00000000..78314633 --- /dev/null +++ b/bazel/rules/rules_score/test/lib/find_runfile.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* + +# Shared sh_test helper: locate a runfile by a suffix match against its full +# path. dependable_element generates many same-named files (one index.rst per +# architectural_design view/directory plus its own top-level index), so a bare +# basename match is ambiguous. +# +# Usage: find_runfile SUFFIX PATH... +# SUFFIX: the trailing portion of the desired file's path to match against. +# PATH...: candidate `$(rootpaths ...)`-style runfiles-relative paths. +# +# Echoes the first matching candidate's absolute path (resolved via +# TEST_SRCDIR/TEST_WORKSPACE) and returns 0, or prints an error to stderr and +# returns 1 if none of the candidates match. +find_runfile() { + local suffix="$1" + shift + local rel_path candidate + for rel_path in "$@"; do + candidate="${TEST_SRCDIR}/${TEST_WORKSPACE}/${rel_path}" + if [[ -f "${candidate}" && "${candidate}" == *"${suffix}" ]]; then + echo "${candidate}" + return 0 + fi + done + echo "Error: could not locate '*${suffix}' among: $*" >&2 + return 1 +} From 9962a57e109835eee84499d3a2deab68f8459ac5 Mon Sep 17 00:00:00 2001 From: Jochen Hoenle <173445474+hoe-jo@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:58:30 +0200 Subject: [PATCH 5/5] docs: fix stale arch_design.rst references, dedupe wrapper/compose sections - examples/seooc/design/BUILD, index.md: the "standalone page" example file arch_design.rst no longer exists in this exampl - docs/user_guide/architectural_design.rst: fixed the "working demonstration of all three modes" --- .github/skills/score-architecture/SKILL.md | 22 ++++++------ .../docs/user_guide/architectural_design.rst | 35 +------------------ .../examples/seooc/design/index.md | 3 +- 3 files changed, 14 insertions(+), 46 deletions(-) diff --git a/.github/skills/score-architecture/SKILL.md b/.github/skills/score-architecture/SKILL.md index 721ac475..911cdd24 100644 --- a/.github/skills/score-architecture/SKILL.md +++ b/.github/skills/score-architecture/SKILL.md @@ -1,3 +1,9 @@ +--- +name: score-architecture +description: "Software architectural design for S-CORE SEooCs using the rules_score Bazel rules. USE FOR: writing PlantUML static/dynamic/public_api/internal_api diagrams, structuring dependable_element → component → unit hierarchies, wiring architectural_design / unit / unit_design / component / dependable_element targets, PlantUML stereotype and interface/port conventions, the declared-vs-implemented architecture consistency check, integrity levels, certified scope, and requirement allocation to architectural elements. Use when working on architecture, .puml files, component/unit structure, or the rules_score architecture rules." +argument-hint: "component/unit or diagram to model" +--- + ---- -name: score-architecture -description: "Software architectural design for S-CORE SEooCs using the rules_score Bazel rules. USE FOR: writing PlantUML static/dynamic/public_api/internal_api diagrams, structuring dependable_element → component → unit hierarchies, wiring architectural_design / unit / unit_design / component / dependable_element targets, PlantUML stereotype and interface/port conventions, the declared-vs-implemented architecture consistency check, integrity levels, certified scope, and requirement allocation to architectural elements. Use when working on architecture, .puml files, component/unit structure, or the rules_score architecture rules." -argument-hint: "component/unit or diagram to model" ---- - # S-CORE Architecture Skill Software architectural design for a **Safety Element out of Context (SEooC)** using the @@ -330,9 +330,9 @@ One target bundles every diagram kind (from [`examples/seooc/design/BUILD`](../. ```starlark architectural_design( name = "sample_seooc_design", - static = ["static_design.puml", "arch_design.rst"], + static = ["static_design.puml", "index.md"], dynamic = ["dynamic_design.puml"], - public_api = ["public_api.puml"], + public_api = ["public_api.puml", "public_api.rst"], internal_api = ["internal_api.puml"], visibility = ["//visibility:public"], # maturity = "development", # write validation findings without failing the build @@ -341,8 +341,10 @@ architectural_design( `static`/`dynamic` accept `.puml`, `.plantuml`, `.png`, `.svg`, `.rst`, `.md`. To combine a diagram with prose, add both the RST/Markdown wrapper *and* the referenced `.puml` to the same -list (as `static_design.puml` + `arch_design.rst` above); the wrapper embeds the diagram with -`.. uml:: file.puml`. +list (as `public_api.puml` + `public_api.rst` above, which overrides `public_api.puml`'s +generated wrapper page); the wrapper embeds the diagram with `.. uml:: file.puml`. `index.md` +above is a directory-level `index` page instead, so it *composes* with (rather than overrides) +the static view's generated navigation — see the next paragraph. Each view builds a navigation tree mirroring the on-disk directory layout of its diagrams: every `.puml` gets an auto-generated wrapper page, and every directory gets a generated diff --git a/bazel/rules/rules_score/docs/user_guide/architectural_design.rst b/bazel/rules/rules_score/docs/user_guide/architectural_design.rst index 3861848e..3729dc3c 100644 --- a/bazel/rules/rules_score/docs/user_guide/architectural_design.rst +++ b/bazel/rules/rules_score/docs/user_guide/architectural_design.rst @@ -214,8 +214,7 @@ Two files that would stage at the same relative path (for example both a naming both conflicting sources, instead of surfacing a raw Bazel action-conflict error. -See ``examples/seooc/design`` for a working demonstration of all three modes: -``arch_design.rst`` is a standalone page in the static view, ``index.md`` +See ``examples/seooc/design`` for a working demonstration: ``index.md`` composes an introduction above the static view's root navigation, and ``public_api.rst`` overrides the generated wrapper for ``public_api.puml``. @@ -500,38 +499,6 @@ Include both the wrapper file *and* the referenced ``.puml`` file in the same Ba ], ) -Generated Navigation, and How Authored Pages Interact With It ----------------------------------------------------------------- - -Each view organises its diagrams into a navigation tree that mirrors their -on-disk directory layout: every diagram gets an auto-generated wrapper page, -and every directory gets a generated ``index.rst`` listing that directory's -diagrams and sub-directories. Authored pages you pass in the same view slot -into that tree by name: - -- **Same-stem override.** A ``.rst`` or ``.md`` next to a - same-named ``.puml`` *replaces* that diagram's generated wrapper page. - The generated wrapper is only a placeholder for prose that doesn't exist - yet, so your page always wins. The ``.puml`` is still staged beside it, so - your own ``.. uml:: .puml`` resolves — this is exactly the wrapper - pattern shown above. - -- **Directory index compose.** An ``index.rst`` or ``index.md`` in a directory - *composes* with that directory's generated navigation instead of replacing - it: your text is rendered first, and the generated toctree follows below it. - - Give the authored body a section title. It becomes the page's title, and - without one Sphinx warns that a toctree entry has no title. - -Two constraints are enforced at build time, with an error naming the offending -files: - -- A diagram may not be named ``index.puml``/``index.plantuml``; that stem is - reserved for the directory's own navigation page. -- No two files in one view may resolve to the same staged path, and a given - stem may not have both a ``.rst`` and a ``.md`` (or both a ``.puml`` and a - ``.plantuml``). - Rule Reference: ``architectural_design`` ------------------------------------------- diff --git a/bazel/rules/rules_score/examples/seooc/design/index.md b/bazel/rules/rules_score/examples/seooc/design/index.md index 7413b607..19c549d5 100644 --- a/bazel/rules/rules_score/examples/seooc/design/index.md +++ b/bazel/rules/rules_score/examples/seooc/design/index.md @@ -16,5 +16,4 @@ This is a **compose** example (see `architectural_design.rst`'s "Authoring Pages Alongside Diagrams"): this file is named `index.md`, so its content is rendered above the generated navigation for this directory instead of -replacing it — `static_design` and `arch_design` below both keep their own -entries. +replacing it — `static_design` below keeps its own entry.