Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions bazel/rules/rules_score/private/sphinx_module.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -366,13 +366,16 @@ def _score_html_impl(ctx):
]
merge_inputs = [sphinx_html_output]

# Add each dependency
for dep in ctx.attr.deps:
if SphinxModuleInfo in dep:
dep_html_dir = dep[SphinxModuleInfo].html_dir
dep_name = dep.label.name
merge_inputs.append(dep_html_dir)
merge_args.extend(["--dep", dep_name + ":" + dep_html_dir.path])
# Every module transitively required by this one, each contributing its
# own_html_dir (never another module's already-merged html_dir), so each
# lands exactly once, at depth 1, regardless of how many paths reach it
# in a diamond dependency graph (see SphinxModuleInfo.transitive_modules).
transitive_modules_from_deps = depset(
transitive = [dep[SphinxModuleInfo].transitive_modules for dep in ctx.attr.deps if SphinxModuleInfo in dep],
).to_list()
for module in transitive_modules_from_deps:
merge_inputs.append(module.own_html_dir)
merge_args.extend(["--dep", module.name + ":" + module.own_html_dir.path])

# Auto-detect static files from srcs: any file whose short_path contains
# '/_static/' is a static asset that Sphinx may not copy correctly in the
Expand Down Expand Up @@ -400,6 +403,13 @@ def _score_html_impl(ctx):
DefaultInfo(files = depset([html_output])),
SphinxModuleInfo(
html_dir = html_output,
own_html_dir = sphinx_html_output,
# Reuses the already-flattened transitive_modules_from_deps list
# collected above for the merge step, plus this module itself.
transitive_modules = depset(
[struct(name = ctx.label.name, own_html_dir = sphinx_html_output)] +
transitive_modules_from_deps,
),
),
OutputGroupInfo(
sphinx_sources = depset([config_file] + sphinx_source_files),
Expand Down
16 changes: 15 additions & 1 deletion bazel/rules/rules_score/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,21 @@ SphinxIndexFileInfo = provider(
SphinxModuleInfo = provider(
doc = "Provider for Sphinx HTML module documentation",
fields = {
"html_dir": "Directory containing HTML files",
"html_dir": "Directory containing this module's HTML, merged with its " +
"transitive dependencies (what sphinx_html_merge.py produces).",
"own_html_dir": "Directory containing only this module's own HTML, before " +
"any dependency is merged in. What the flat merge (see " +
"transitive_modules) copies from — each transitive module " +
"contributes its own_html_dir, never another module's " +
"already-merged html_dir, so a diamond dependency doesn't " +
"get copied once per path to it.",
"transitive_modules": "Depset of struct(name, own_html_dir), one entry per " +
"module transitively required by this one, including " +
"this module itself. Self-inclusive union: each dep's " +
"own transitive_modules already contains itself, so " +
"unioning deps' depsets yields the full flat closure " +
"without this module needing to add each dep " +
"individually on top.",
},
)

Expand Down
29 changes: 16 additions & 13 deletions bazel/rules/rules_score/src/sphinx_html_merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@
"""Merge multiple Sphinx HTML output directories.

This script merges Sphinx HTML documentation from multiple modules into a single
output directory. It copies the main module's HTML as-is, and then copies each
dependency module's HTML into a subdirectory, excluding nested module directories
to avoid duplication.
output directory. It copies the main module's HTML as-is, then copies every
transitively required module's *own* (unmerged) HTML into a subdirectory --
flat, one level deep, one copy per module regardless of how many dependency
paths reach it. sphinx_module.bzl is responsible for passing --dep as the
transitive closure (SphinxModuleInfo.transitive_modules), with each --dep
pointing at that module's own_html_dir rather than its (possibly
further-merged) html_dir -- this script has no way to tell the two apart and
would duplicate a diamond dependency if handed the latter.

Usage:
sphinx_html_merge.py --output OUTPUT_DIR --main MAIN_HTML_DIR [--dep NAME:PATH ...]
Expand Down Expand Up @@ -94,10 +99,12 @@ def copy_html_files(src_dir, dst_dir, is_dependency=False, sibling_modules=None)
dropped (the merged site uses one shared _static/ at the
root) and their internal links rewritten for the new
nesting depth.
sibling_modules: Set of sibling module directory names to skip (so nested
copies of other modules already merged elsewhere aren't
duplicated) and to rewrite intra-site links for. Only
meaningful when is_dependency is True.
sibling_modules: Set of other module directory names to rewrite intra-site
links for (e.g. href="other_module/..." needs a "../" prefix
added for the new nesting depth). Only meaningful when
is_dependency is True. src_dir is always a module's own,
unmerged HTML now (never another module's already-merged
tree), so there is nothing nested under it to skip.
"""
src_path = Path(src_dir)
dst_path = Path(dst_dir)
Expand Down Expand Up @@ -175,9 +182,6 @@ def copy_tree(src, dst, rel_path):
# Never publish Sphinx's own build cache.
if item.name in BUILD_ARTIFACT_DIRS:
continue
# Skip nested copies of sibling modules to avoid duplication.
if item.name in sibling_modules:
continue
# Dependencies use the merged site's shared _static/ instead
# of their own.
if is_dependency and item.name in (
Expand Down Expand Up @@ -217,15 +221,14 @@ def merge_html_dirs(output_dir, main_html_dir, dependencies, extra_static=None):
shutil.copy2(src_file, dst)
logging.info("Copied extra static %s → _static/%s", src_file, dest_subpath)

# Collect all dependency names for link fixing and exclusion
# Collect all dependency names for intra-site link rewriting.
dep_names = [name for name, _ in dependencies]

# Then copy each dependency into a subdirectory with link fixing
for dep_name, dep_html_dir in dependencies:
dep_output = output_path / dep_name
logging.info("Copying dependency %s from %s to %s", dep_name, dep_html_dir, dep_output)
# Exclude other module directories to avoid nested modules
# Remove current module from the list to get actual siblings to exclude
# Other modules in this merge, to rewrite intra-site links for.
sibling_modules = set(n for n in dep_names if n != dep_name)
copy_html_files(
dep_html_dir,
Expand Down
38 changes: 37 additions & 1 deletion bazel/rules/rules_score/test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ load(
"score_toolchain_override_test",
"sphinx_module_info_fields_test",
"sphinx_module_providers_test_suite",
"transitive_modules_dedup_test",
"two_phase_html_second_test",
"two_phase_needs_first_test",
)
Expand Down Expand Up @@ -149,17 +150,32 @@ toolchain(
# Test 1: Multi-Module Aggregation
# Dependency graph: module_a_lib -> module_b_lib -> module_c_lib
# module_a_lib -> module_c_lib (also direct)
# module_d_lib is a diamond dependency reachable only transitively, via
# module_b_lib AND module_c_lib (never directly from module_a_lib) -- a
# regression fixture for the HTML-merge flattening: module_d_lib's HTML must
# be merged into module_a_lib's published site exactly once, not once per
# path that reaches it.
sphinx_module(
name = "module_d_lib",
srcs = glob(["fixtures/module_d/*.rst"]),
index = "fixtures/module_d/index.rst",
)

sphinx_module(
name = "module_c_lib",
srcs = glob(["fixtures/module_c/*.rst"]),
index = "fixtures/module_c/index.rst",
deps = [":module_d_lib"],
)

sphinx_module(
name = "module_b_lib",
srcs = glob(["fixtures/module_b/*.rst"]),
index = "fixtures/module_b/index.rst",
deps = [":module_c_lib"],
deps = [
":module_c_lib",
":module_d_lib",
],
)

sphinx_module(
Expand Down Expand Up @@ -879,6 +895,17 @@ sh_test(
tags = ["manual"],
)

# End-to-end regression test: module_d_lib (a diamond dep of both
# module_b_lib and module_c_lib, never directly of module_a_lib) must be
# merged into module_a_lib's published site exactly once, flat, not once
# per dependency path that reaches it.
sh_test(
name = "module_d_dedup_test",
srcs = ["check_module_d_dedup.sh"],
data = [":module_a_lib"],
tags = ["manual"],
)

# ============================================================================
# SEooC-Specific Tests
# ============================================================================
Expand Down Expand Up @@ -1093,6 +1120,14 @@ sphinx_module_info_fields_test(
target_under_test = ":module_a_lib",
)

# Regression test for the diamond-dependency HTML-merge flattening:
# module_d_lib is a dep of both module_b_lib and module_c_lib (never
# directly of module_a_lib), so it's reachable via two paths.
transitive_modules_dedup_test(
name = "transitive_modules_dedup_test",
target_under_test = ":module_a_lib",
)

score_needs_info_fields_test(
name = "score_needs_info_fields_test",
target_under_test = ":module_a_lib_needs",
Expand Down Expand Up @@ -1396,6 +1431,7 @@ test_suite(
":test_sphinx_html_merge",
":test_sphinx_module_ext",
":test_trlc_rst_image_rendering",
":transitive_modules_dedup_test",
":unit_component_tests",
"//fixtures/image_srcs:requirements_image_tests",
],
Expand Down
46 changes: 46 additions & 0 deletions bazel/rules/rules_score/test/check_module_d_dedup.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#!/bin/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

# End-to-end regression test for the diamond-dependency HTML-merge
# flattening: module_d_lib is a dep of both module_b_lib and module_c_lib
# (never directly of module_a_lib), so before the fix its HTML was copied
# once per path that reached it -- nested under module_b_lib/module_d_lib/
# AND module_c_lib/module_d_lib/, never at the top level. After the fix it
# must land exactly once, flat, at the top level of the merged site.
# TODO: pass the HTML dir via args instead of using a hardcoded relative path,
# e.g. args = ["$(rootpath :module_a_lib)"] in the sh_test.
html_dir="./module_a_lib/html"

if [[ ! -d "$html_dir" ]]; then
echo "Error: Directory not found: $html_dir" >&2
exit 1
fi

if [[ ! -f "$html_dir/module_d_lib/index.html" ]]; then
echo "Error: Expected flat, top-level $html_dir/module_d_lib/index.html not found" >&2
exit 1
fi

if [[ -d "$html_dir/module_b_lib/module_d_lib" ]]; then
echo "Error: module_d_lib was duplicated, nested under module_b_lib/" >&2
exit 1
fi

if [[ -d "$html_dir/module_c_lib/module_d_lib" ]]; then
echo "Error: module_d_lib was duplicated, nested under module_c_lib/" >&2
exit 1
fi

echo "✓ module_d_lib's HTML appears exactly once, flat, in the merged output"
45 changes: 45 additions & 0 deletions bazel/rules/rules_score/test/fixtures/module_d/index.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
..
# *******************************************************************************
# 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
# *******************************************************************************
Module D Documentation
======================

This is the documentation for Module D.

.. document:: Documentation for Module D
:id: doc__module_fixtures_module_d
:status: valid
:safety: ASIL_B
:security: NO
:realizes:


Overview
--------

Module D is a base module with no dependencies, shared as a diamond
dependency by Module B and Module C (both depend on it, neither directly
via Module A) — used to regression-test that its HTML is merged exactly
once into Module A's published site.
Local need link: :need:`doc__module_fixtures_module_d`

Features
--------

.. needlist::
:tags: module_d

Content
-------

Module D provides foundational functionality shared by Module B and Module C.
27 changes: 27 additions & 0 deletions bazel/rules/rules_score/test/score_module_providers_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,32 @@ def _sphinx_module_info_fields_test_impl(ctx):

sphinx_module_info_fields_test = analysistest.make(_sphinx_module_info_fields_test_impl)

def _transitive_modules_dedup_test_impl(ctx):
"""Regression test: a module reachable via multiple dependency paths (a
diamond — module_d_lib is a dep of both module_b_lib and module_c_lib,
never directly of module_a_lib) must appear exactly once in
transitive_modules, not once per path that reaches it.
"""
env = analysistest.begin(ctx)
target_under_test = analysistest.target_under_test(env)

score_info = target_under_test[SphinxModuleInfo]
transitive_modules = score_info.transitive_modules.to_list()

matches = [m for m in transitive_modules if m.name == "module_d_lib"]
asserts.equals(
env,
1,
len(matches),
"module_d_lib is reachable via two paths (module_b_lib and " +
"module_c_lib) but must appear exactly once in transitive_modules, " +
"got: %s" % matches,
)

return analysistest.end(env)

transitive_modules_dedup_test = analysistest.make(_transitive_modules_dedup_test_impl)

# ============================================================================
# SphinxNeedsInfo Provider Tests
# ============================================================================
Expand Down Expand Up @@ -353,6 +379,7 @@ def sphinx_module_providers_test_suite(name):
tests = [
# Provider field tests
":sphinx_module_info_fields_test",
":transitive_modules_dedup_test",
":score_needs_info_fields_test",
":score_needs_transitive_collection_test",

Expand Down
32 changes: 20 additions & 12 deletions bazel/rules/rules_score/test/test_sphinx_html_merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,21 +157,28 @@ def test_extra_static_copied_after_main(self) -> None:

self.assertEqual((self.output / "_static" / "logo.svg").read_text(), "custom-logo")

def test_nested_sibling_module_copy_is_skipped(self) -> None:
"""Regression test: if a dependency's own HTML tree happens to contain
a subdirectory named after a sibling module (e.g. because that
dependency was itself built with sphinx_html_merge and already
embeds the sibling), that nested copy must be skipped rather than
duplicated into the merged output - the sibling is placed once, at
the site root, by its own top-level merge entry.
def test_deps_are_always_a_flat_transitive_set_no_nested_skip_needed(self) -> None:
"""sphinx_module.bzl always passes --dep as the transitive closure
(SphinxModuleInfo.transitive_modules), each pointing at that module's
own_html_dir — a module's own, unmerged Sphinx output, never another
module's already-merged tree. There is therefore nothing nested to
skip: this merge script has no special-casing for a dep's
subdirectory happening to share a name with another dep, and copies
it verbatim. Guards against reintroducing the old nested-sibling-skip
heuristic, which existed only because deps used to be each other's
*merged* trees (recursively containing further-nested deps) before
the merge was flattened.
"""
main = self.root / "main"
_write(main / "index.html", "<html></html>")

dep_a = self.root / "dep_a"
_write(dep_a / "index.html", "<html></html>")
# dep_a already contains its own nested (stale) copy of dep_b.
_write(dep_a / "dep_b" / "index.html", "<html>stale nested copy</html>")
# Coincidentally named the same as another --dep entry below. Under
# the old nested-merge design this could only happen via actual
# nesting and had to be skipped; under the flat design it's just a
# same-named subdirectory in dep_a's own tree, copied like any other.
_write(dep_a / "dep_b" / "page.html", "<html>dep_a's own dep_b/ subdirectory</html>")

dep_b = self.root / "dep_b"
_write(dep_b / "index.html", "<html>canonical dep_b</html>")
Expand All @@ -182,9 +189,10 @@ def test_nested_sibling_module_copy_is_skipped(self) -> None:
[("dep_a", dep_a), ("dep_b", dep_b)],
)

# The nested copy under dep_a/dep_b/ must not have been copied.
self.assertFalse((self.output / "dep_a" / "dep_b").exists())
# The canonical dep_b, copied from its own top-level entry, is intact.
self.assertEqual(
(self.output / "dep_a" / "dep_b" / "page.html").read_text(),
"<html>dep_a's own dep_b/ subdirectory</html>",
)
self.assertEqual(
(self.output / "dep_b" / "index.html").read_text(),
"<html>canonical dep_b</html>",
Expand Down
Loading