fix(sphinx): resolve module configuration edge cases

bazel/rules/rules_score/private/sphinx_module.bzl

@@ -320,13 +320,27 @@ def _score_needs_impl(ctx):
     )
     transitive_needs = [dep[SphinxNeedsInfo].needs_json_files for dep in ctx.attr.deps if SphinxNeedsInfo in dep]
     needs_json_files = depset([needs_output], transitive = transitive_needs)
+
+    # Self-inclusive union (mirrors SphinxModuleInfo.transitive_modules): each
+    # dep's own needs_modules already contains itself, so unioning deps'
+    # depsets yields the full flat closure without this module needing to add
+    # each dep individually on top. Consumed by _score_html_impl to build
+    # needs_external_needs.json from every module transitively required, not
+    # just direct deps -- otherwise a :need: reference more than one hop away
+    # can never resolve.
+    transitive_needs_modules = [dep[SphinxNeedsInfo].needs_modules for dep in ctx.attr.deps if SphinxNeedsInfo in dep]
+    needs_modules = depset(
+        [struct(name = _needs_output_prefix(ctx.label.name), needs_json_file = needs_output)],
+        transitive = transitive_needs_modules,
+    )
     return [
         DefaultInfo(
             files = needs_json_files,
         ),
         SphinxNeedsInfo(
             needs_json_file = needs_output,  # Direct file only
             needs_json_files = needs_json_files,  # Transitive depset
+            needs_modules = needs_modules,  # Transitive, self-inclusive, keyed by base module name
         ),
     ]
 
@@ -345,17 +359,27 @@ def _score_html_impl(ctx):
     source_prefix = ctx.label.name
 
     sphinx_toolchain = ctx.toolchains["//bazel/rules/rules_score:toolchain_type"].sphinxinfo
-    needs_external_needs = {}
-    for dep in ctx.attr.needs:
-        if SphinxNeedsInfo in dep:
-            dep_name = _needs_output_prefix(dep.label.name)
-            needs_external_needs[dep.label.name] = {
-                "base_url": dep_name,  # Relative path to the subdirectory where dep HTML is copied
-                "json_path": dep[SphinxNeedsInfo].needs_json_file.path,  # Use direct file
-                "id_prefix": "",
-                "css_class": "",
-                "version": "1.0",
-            }
+
+    # Built from the full transitive closure (each direct needs-dep's own
+    # SphinxNeedsInfo.needs_modules is already self-inclusive), not just
+    # direct deps -- a :need: reference more than one hop away could
+    # otherwise never resolve. base_url = module.name (no path prefix) is
+    # only truthful because the HTML merge below is flat: every transitive
+    # module lands at that exact depth-1 path in the published site,
+    # regardless of how many dependency hops away it is.
+    transitive_needs_modules = depset(
+        transitive = [dep[SphinxNeedsInfo].needs_modules for dep in ctx.attr.needs if SphinxNeedsInfo in dep],
+    ).to_list()
+    needs_external_needs = {
+        module.name: {
+            "base_url": module.name,  # Relative path to the subdirectory where dep HTML is copied
+            "json_path": module.needs_json_file.path,
+            "id_prefix": "",
+            "css_class": "",
+            "version": "1.0",
+        }
+        for module in transitive_needs_modules
+    }
     needs_external_needs_json = ctx.actions.declare_file(ctx.label.name + "/needs_external_needs.json")
     ctx.actions.write(
         output = needs_external_needs_json,
@@ -400,12 +424,43 @@ def _score_html_impl(ctx):
     # Because plain files and generated files are in different directories,
     # we need to merge the two into a single directory.
     index_source_file = _get_index_file(ctx)
+
+    # An index also listed in renamed_srcs would relocate to two different
+    # destinations under the two relocation formulas below (srcs: strip
+    # ctx.attr.strip_prefix from short_path; renamed_srcs: the dict's own
+    # explicit destination path), so --index_file would end up pointing at
+    # the wrong one -- surfacing as an opaque "index file does not exist"
+    # from the Sphinx build action instead of a clear build-time error.
+    # docs_library_deps can't be checked here: its relocation is
+    # provider-driven, not visible until the srcs loop below has already
+    # run, so that combination remains an unguarded gap.
+    for renamed_src_target in ctx.attr.renamed_srcs.keys():
+        if renamed_src_target.label == ctx.attr.index.label:
+            fail(
+                "sphinx_module '{}': 'index' ({}) must not also appear as a renamed_srcs key -- ".format(
+                    ctx.label.name,
+                    ctx.attr.index.label,
+                ) +
+                "srcs and renamed_srcs relocate a file to two different destinations, so " +
+                "--index_file would point at the wrong one. Remove it from renamed_srcs.",
+            )
+
     relocated_index_file = ""
     for orig_file in ctx.files.srcs:
         dest = _relocate(orig_file)
         if orig_file.path == index_source_file.path:
             relocated_index_file = dest.path
 
+    if not relocated_index_file:
+        fail(
+            "sphinx_module '{}': 'index' ({}) did not resolve to a relocated path -- ".format(
+                ctx.label.name,
+                ctx.attr.index.label,
+            ) +
+            "its file must also appear in 'srcs'. An index reachable only via 'renamed_srcs' " +
+            "or 'docs_library_deps' is not currently supported.",
+        )
+
     sphinx_html_output = ctx.actions.declare_directory(ctx.label.name + "/_html")
 
     # The HTML pass reads a relocated tree generated under bazel-out (built
@@ -579,7 +634,7 @@ def sphinx_module(
         deps = [],
         docs_library_deps = [],
         renamed_srcs = {},
-        strip_prefix = "",
+        strip_prefix = None,
         extra_opts = [],
         extra_opts_targets = [],
         allow_persistent_workers = False,
@@ -598,10 +653,13 @@ def sphinx_module(
         docs_library_deps: {type}`list[label]` of {obj}`sphinx_docs_library` targets.
         renamed_srcs: {type}`dict[label, str]` Doc source files that are renamed
                     on their way into the Sphinx source tree.
-        strip_prefix: {type}`str` A prefix to remove from the file paths of the
+        strip_prefix: {type}`str | None` A prefix to remove from the file paths of the
                     source files. e.g., given `//sphinxdocs/docs:foo.md`, stripping `docs/` makes
                     Sphinx see `foo.md` in its generated source directory. If not
-                    specified, then {any}`native.package_name` is used.
+                specified (None, the default), {any}`native.package_name` + "/" is
+                used. Pass "" explicitly to strip nothing -- unlike a plain string
+                default, None lets that explicit "" survive, since "" and "not
+                specified" are different intents.
         extra_opts: {type}`list[str]` Additional string options to pass onto Sphinx building.
                     On each provided option, a location expansion is performed.
                     See {any}`ctx.expand_location`.
@@ -616,7 +674,7 @@ def sphinx_module(
         visibility: Bazel visibility
     """
     package = native.package_name()
-    resolved_strip_prefix = strip_prefix if strip_prefix else (package + "/" if package else "")
+    resolved_strip_prefix = strip_prefix if strip_prefix != None else (package + "/" if package else "")
 
     # conf.py generation is a private implementation detail consumed only by
     # the sibling _score_needs/_score_html targets below (same package) --

bazel/rules/rules_score/providers.bzl

@@ -281,6 +281,14 @@ SphinxNeedsInfo = provider(
     fields = {
         "needs_json_file": "Direct needs.json file for this module",
         "needs_json_files": "Depset of needs.json files including transitive dependencies",
+        "needs_modules": "Depset of struct(name, needs_json_file), one entry per " +
+                         "module transitively required by this one, including this " +
+                         "module itself. Self-inclusive union, same shape/rationale " +
+                         "as SphinxModuleInfo.transitive_modules -- used to build " +
+                         "needs_external_needs.json from the full transitive closure " +
+                         "(direct deps only would miss a need defined more than one " +
+                         "hop away), with base_url = <module_name> truthful for every " +
+                         "entry only because the HTML merge is flat.",
     },
 )
 FilteredExecpathInfo = provider(

bazel/rules/rules_score/test/BUILD

@@ -189,6 +189,20 @@ sphinx_module(
     ],
 )
 
+# Regression fixture for two-hop needs resolution: module_e_lib depends only
+# on module_b_lib, with no direct edge to module_c_lib or module_d_lib, yet
+# its index.rst references a need defined in module_d_lib (reachable only via
+# module_b_lib -> module_d_lib). needs_external_needs.json must be built from
+# the full transitive closure of needs modules (SphinxNeedsInfo.needs_modules)
+# for this to resolve -- building it from direct deps only (the pre-fix
+# behavior) can never see past one hop.
+sphinx_module(
+    name = "module_e_lib",
+    srcs = glob(["fixtures/module_e/*.rst"]),
+    index = "fixtures/module_e/index.rst",
+    deps = [":module_b_lib"],
+)
+
 # Fixture with allow_persistent_workers = True, used only to assert on the
 # resulting Sphinx-build action argv: the HTML pass must drop "--jobs auto"
 # (see worker_enabled in sphinx_module.bzl's _add_sphinx_args), while the
@@ -901,13 +915,17 @@ explicit_config_test(
 # HTML Content Validation Tests
 # ============================================================================
 
-# Test that generated HTML does not contain "Unknown need" statements
-# TODO: fix hardcoded path in check_unknown_needs.sh — pass the HTML output dir
-# via args = ["$(rootpath :module_a_lib)"] and update the script to use "$1/index.html".
+# Test that generated HTML does not contain "Unknown need" statements.
+# Covers both direct-dep needs resolution (module_a_lib) and the two-hop case
+# (module_e_lib) -- see check_unknown_needs.sh's TODO for the hardcoded-path
+# cleanup this could still use.
 sh_test(
     name = "no_unknown_needs_test",
     srcs = ["check_unknown_needs.sh"],
-    data = [":module_a_lib"],
+    data = [
+        ":module_a_lib",
+        ":module_e_lib",
+    ],
     tags = ["manual"],
 )
 

bazel/rules/rules_score/test/check_unknown_needs.sh

@@ -14,18 +14,29 @@
 set -euo pipefail
 
 # Test if the html output contains unknown needs.
-# 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 and read as "$1/index.html".
-html_file="./module_a_lib/html/index.html"
+# TODO: pass the HTML dirs via args instead of using hardcoded relative paths,
+# e.g. args = ["$(rootpath :module_a_lib)", "$(rootpath :module_e_lib)"] in the
+# sh_test and read as "$1/index.html", "$2/index.html".
+#
+# module_a_lib covers direct-dep needs resolution; module_e_lib covers the
+# two-hop case (its :need: reference resolves only through module_b_lib ->
+# module_d_lib, with no direct edge of its own to the defining module) --
+# see needs_modules in sphinx_module.bzl for the mechanism this exercises.
+html_files=(
+    "./module_a_lib/html/index.html"
+    "./module_e_lib/html/index.html"
+)
 
-if [[ ! -f "$html_file" ]]; then
-    echo "Error: File not found: $html_file" >&2
-    exit 1
-fi
+for html_file in "${html_files[@]}"; do
+    if [[ ! -f "$html_file" ]]; then
+        echo "Error: File not found: $html_file" >&2
+        exit 1
+    fi
 
-if grep -q "Unknown need" "$html_file"; then
-    echo "Error: Found 'Unknown need' in $html_file" >&2
-    exit 1
-fi
+    if grep -q "Unknown need" "$html_file"; then
+        echo "Error: Found 'Unknown need' in $html_file" >&2
+        exit 1
+    fi
+done
 
 echo "✓ No unknown needs found"

bazel/rules/rules_score/test/fixtures/module_e/index.rst

@@ -0,0 +1,43 @@
+..
+   # *******************************************************************************
+   # 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 E Documentation
+======================
+
+This is the documentation for Module E.
+
+.. document:: Documentation for Module E
+   :id: doc__module_fixtures_module_e
+   :status: valid
+   :safety: ASIL_B
+   :security: NO
+   :realizes:
+
+
+Overview
+--------
+
+Module E depends only on Module B (no direct edge to Module C or Module D).
+Its need reference below to a need defined in Module D is reachable only
+via Module B -> Module D (and Module B -> Module C -> Module D) -- a
+regression fixture for two-hop needs resolution: needs_external_needs.json
+must be built from the full transitive closure of needs modules, not just
+direct deps, or this reference can never resolve.
+
+Two-hop need reference to Module D :need:`doc__module_fixtures_module_d`.
+
+Features
+--------
+
+.. needlist::
+   :tags: module_e