Joho sphinx upstream adoption

bazel/rules/rules_score/BUILD

@@ -14,6 +14,7 @@
 load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
 load("@pip_rules_score//:requirements.bzl", "requirement")
 load("@rules_python//python:pip.bzl", "compile_pip_requirements")
+load("@rules_python//sphinxdocs:sphinx.bzl", "sphinx_build_binary")
 load("@rules_python//sphinxdocs:sphinx_docs_library.bzl", "sphinx_docs_library")
 load(
     "//bazel/rules/rules_score:rules_score.bzl",
@@ -32,7 +33,6 @@ config_setting(
 
 exports_files([
     "src/bazel_sphinx_needs.py",
-    "src/sphinx_wrapper.py",
     "templates/conf.template.py",
     "templates/dependable_element_index.template.rst",
     "templates/section_page.template.rst",
@@ -135,6 +135,7 @@ py_library(
     visibility = ["//visibility:public"],
     deps = [
         ":bazel_sphinx_needs",
+        ":sphinx_conf_helpers",
         requirement("sphinx"),
     ],
 )
@@ -264,31 +265,13 @@ py_library(
     ],
 )
 
-py_binary(
+# Built from rules_python's own sphinx_build.py (persistent-worker/param-file
+# support included) rather than a score_tooling wrapper -- see
+# sphinx_toolchain.bzl's score_sphinx_toolchain docstring.
+sphinx_build_binary(
     name = "raw_build",
-    srcs = ["src/sphinx_wrapper.py"],
-    data = ["@rules_python//sphinxdocs/private:sphinx_build.py"],
-    main = "src/sphinx_wrapper.py",
     visibility = ["//visibility:public"],
-    deps = [
-        ":sphinx_base_deps",
-        "@rules_python//python/runfiles",
-    ],
-)
-
-# Same srcs as :raw_build, exposed as a py_library so test/test_sphinx_wrapper.py
-# can import the wrapper's functions directly (mirrors :sphinx_html_merge_lib's
-# role for :sphinx_html_merge above).
-py_library(
-    name = "sphinx_wrapper_lib",
-    srcs = ["src/sphinx_wrapper.py"],
-    data = ["@rules_python//sphinxdocs/private:sphinx_build.py"],
-    imports = ["src"],
-    visibility = ["//visibility:public"],
-    deps = [
-        ":sphinx_base_deps",
-        "@rules_python//python/runfiles",
-    ],
+    deps = [":sphinx_base_deps"],
 )
 
 sphinx_toolchain(

bazel/rules/rules_score/private/sphinx_module.bzl

@@ -20,9 +20,10 @@ load("//bazel/rules/rules_score:providers.bzl", "FilteredExecpathInfo", "SphinxI
 load("//bazel/rules/rules_score/private:verbosity.bzl", "VERBOSITY_ATTR", "get_log_level")
 
 # Maps the //bazel/rules/rules_score:verbosity build setting (see
-# verbosity.bzl) to the sphinx-build CLI flags that achieve it. Kept in
-# Starlark (not sphinx_wrapper.py) now that the wrapper is a thin,
-# argv-passthrough shim -- see sphinx_wrapper.py's module docstring.
+# verbosity.bzl) to the sphinx-build CLI flags that achieve it. Lives here in
+# Starlark rather than in a wrapper script: the Sphinx build binary is
+# rules_python's own sphinx_build.py (see score_sphinx_toolchain), which
+# takes plain sphinx-build argv with no score-specific flags of its own.
 _SPHINX_VERBOSITY_FLAGS = {
     "warn": ["-q"],
     "info": [],

bazel/rules/rules_score/sphinx_toolchain.bzl

@@ -13,7 +13,8 @@
 """Sphinx toolchain: the SphinxInfo provider, the sphinx_toolchain rule, and
 the score_sphinx_toolchain() convenience macro for consumers."""
 
-load("@aspect_rules_py//py:defs.bzl", "py_binary")
+load("@rules_python//python:py_binary.bzl", "py_binary")
+load("@rules_python//sphinxdocs:sphinx.bzl", "sphinx_build_binary")
 
 SphinxInfo = provider(
     doc = "Provider for Sphinx Toolchain",
@@ -51,14 +52,19 @@ def score_sphinx_toolchain(
         deps = None,
         extra_data = [],
         conf_template = None,
-        package_collisions = "warning",
+        package_collisions = None,
+        py_binary_rule = py_binary,
         **kwargs):
     """Declares a sphinx_toolchain, reusing score_tooling's default build binary.
 
-    Emits `<name>_binary` (the Sphinx build py_binary), `<name>_info` (the
-    `sphinx_toolchain` target), and `<name>` (the `toolchain()` itself).
-    Register the result yourself so the root module's registration takes
-    precedence over score_tooling's own default:
+    Emits `<name>_binary` (the Sphinx build py_binary, built via
+    @rules_python//sphinxdocs:sphinx.bzl's `sphinx_build_binary` from
+    rules_python's own sphinx_build.py -- persistent-worker/param-file
+    support come from there, not from anything score_tooling ports or
+    re-implements), `<name>_info` (the `sphinx_toolchain` target), and
+    `<name>` (the `toolchain()` itself). Register the result yourself so the
+    root module's registration takes precedence over score_tooling's own
+    default:
 
         register_toolchains("//:<name>")
 
@@ -75,30 +81,48 @@ def score_sphinx_toolchain(
         extra_data: Extra data files/targets for the Sphinx build binary.
         conf_template: Label of a conf.py template. Defaults to score_tooling's
             generic template if not given.
-        package_collisions: Forwarded to the generated py_binary.
+        package_collisions: Forwarded to the generated py_binary, if
+            `py_binary_rule` accepts it (e.g. aspect_rules_py's `py_binary`,
+            which uses it to control venv-collision handling). The default
+            `py_binary_rule` (rules_python's own) has no such attr --
+            passing this without also overriding `py_binary_rule` fails with
+            a clear error instead of a raw "unexpected keyword" from Bazel.
+        py_binary_rule: {type}`callable` A `py_binary`-compatible rule/macro
+            used to build the Sphinx build binary, forwarded to
+            `sphinx_build_binary`. Defaults to rules_python's own `py_binary`.
+            Pass aspect_rules_py's `py_binary` (or another py_binary-compatible
+            rule) here if a consumer's deps need its venv semantics -- e.g.
+            deps whose imports rely on aspect's runfiles layout rather than
+            rules_python's.
         **kwargs: Forwarded to the `toolchain()` target (e.g. `visibility`,
             `exec_compatible_with`, `target_compatible_with`).
     """
     if deps != None and extra_deps:
         fail("score_sphinx_toolchain: pass either `deps` (replace mode) or " +
              "`extra_deps` (extend mode) for target '%s', not both" % name)
+    if package_collisions != None and py_binary_rule == py_binary:
+        fail("score_sphinx_toolchain: package_collisions has no effect with " +
+             "the default py_binary_rule (rules_python's py_binary has no " +
+             "such attr) for target '%s' -- pass py_binary_rule explicitly " % name +
+             "(e.g. aspect_rules_py's py_binary) alongside package_collisions, " +
+             "or drop package_collisions.")
 
     binary_deps = deps if deps != None else (
         ["@score_tooling//bazel/rules/rules_score:sphinx_base_deps"] + extra_deps
     )
 
-    py_binary(
-        name = name + "_binary",
-        srcs = ["@score_tooling//bazel/rules/rules_score:src/sphinx_wrapper.py"],
-        main = "@score_tooling//bazel/rules/rules_score:src/sphinx_wrapper.py",
-        # sphinx_build.py is rules_python's persistent-worker entry point,
-        # loaded at runtime via runfiles (not ported/copied). Must always be
-        # present so the import succeeds if Bazel ever invokes this binary
-        # with --persistent_worker, regardless of `deps`/`extra_deps` mode.
-        data = extra_data + ["@rules_python//sphinxdocs/private:sphinx_build.py"],
-        package_collisions = package_collisions,
+    binary_kwargs = dict(
+        data = extra_data,
+        deps = binary_deps,
         visibility = ["//visibility:private"],
-        deps = binary_deps + ["@rules_python//python/runfiles"],
+    )
+    if package_collisions != None:
+        binary_kwargs["package_collisions"] = package_collisions
+
+    sphinx_build_binary(
+        name = name + "_binary",
+        py_binary_rule = py_binary_rule,
+        **binary_kwargs
     )
 
     toolchain_kwargs = {}

bazel/rules/rules_score/src/sphinx_conf_helpers.py

@@ -197,6 +197,37 @@ def resolve_plantuml_command(required: bool = True, graphviz_dot_path: Optional[
     return "%s%s%s" % (plantuml_path, include_flag, layout_flag)
 
 
+def init_hermetic_tools(app: Any, config: Any) -> None:
+    """Sphinx "config-inited" listener: resolve the hermetic PlantUML/Graphviz
+    tool paths into config.graphviz_dot / config.plantuml.
+
+    Must run as a "config-inited" listener, not a module-level conf.py
+    assignment. Sphinx's chdir(confdir) (see sphinx.config.eval_config_file)
+    only wraps evaluating conf.py itself, so cwd is back at the execroot by
+    the time "config-inited" fires -- which is exactly what
+    resolve_graphviz_dot()/resolve_plantuml_command()'s internal
+    os.path.abspath() calls need, since sphinx_module.bzl's
+    _hermetic_tool_env() hands them execroot-relative paths. A module-level
+    call running inside the chdir would resolve those paths against confdir
+    instead, silently producing a wrong (but plausible-looking) absolute
+    path. Mirrors sphinx_module_ext.py's init_external_needs, which fixes
+    the identical cwd mismatch for needs_external_needs.json.
+
+    config.graphviz_dot / config.plantuml are config values already
+    registered by sphinx.ext.graphviz / sphinxcontrib.plantuml's own
+    setup() (via app.add_config_value); this only overrides their value,
+    the same pattern sphinx_module_ext.py uses for config.needs_external_needs.
+
+    Args:
+        app: Sphinx application object (unused; kept for the "config-inited"
+            listener signature).
+        config: Sphinx configuration object.
+    """
+    graphviz_dot = resolve_graphviz_dot()
+    config.graphviz_dot = graphviz_dot
+    config.plantuml = resolve_plantuml_command(graphviz_dot_path=graphviz_dot)
+
+
 # ---------------------------------------------------------------------------
 # sphinx-needs schema, loaded from the upstream S-CORE metamodel
 # ---------------------------------------------------------------------------
@@ -260,6 +291,7 @@ def load_metamodel_needs_schema() -> Dict[str, Any]:
     "resolve_graphviz_dot",
     "resolve_fta_metamodel_dir",
     "resolve_plantuml_command",
+    "init_hermetic_tools",
     "load_metamodel_needs_schema",
     # Re-exported so consumers only need one import for both needs-loading
     # and hermetic-tool concerns.

bazel/rules/rules_score/src/sphinx_module_ext.py

@@ -11,7 +11,8 @@
 # SPDX-License-Identifier: Apache-2.0
 # *******************************************************************************
 #
-"""Sphinx extension entry point for loading external (cross-module) needs.
+"""Sphinx extension entry point for loading external (cross-module) needs
+and resolving hermetic tool paths.
 
 Registered by listing "sphinx_module_ext" in conf.py's `extensions = [...]`;
 Sphinx then auto-invokes `setup(app)` below, no manual wiring required. This
@@ -25,6 +26,7 @@
 from typing import Any, Dict
 
 from bazel_sphinx_needs import load_external_needs
+from sphinx_conf_helpers import init_hermetic_tools
 
 
 def init_external_needs(app: Any, config: Any) -> None:
@@ -55,6 +57,7 @@ def setup(app: Any) -> Dict[str, Any]:
         Extension metadata dictionary
     """
     app.connect("config-inited", init_external_needs)
+    app.connect("config-inited", init_hermetic_tools)
 
     return {
         "version": "1.0",