refactor: transport external needs labels explicitly

bzl/needs_rules.bzl

@@ -33,8 +33,8 @@ def _sphinx_docs_impl(ctx):
         fail("Sphinx requires a bundle with direct documentation sources")
 
     # File labels provide execroot-relative paths for this action's sandbox.
-    # Pass them through the environment variables already consumed by the CLI
-    # and extensions; reserve the JSON option list for non-path Sphinx overrides.
+    # Pass them through the environment variables consumed by the CLI; reserve
+    # the JSON option list for non-path Sphinx overrides.
     # Encode that list as JSON so spaces, quotes and '=' survive transport.
     # ``config`` is transported separately because the launcher derives
     # Sphinx's ``-c`` directory from its path; it is not just another data file.
@@ -43,7 +43,7 @@ def _sphinx_docs_impl(ctx):
         "SOURCE_DIRECTORY": bundle.source_dir_execroot_path,
         "OUTPUT_DIRECTORY": output.path,
         "SPHINX_CONFIG_FILE": ctx.file.config.path,
-        "DATA": "[]",
+        "EXTERNAL_NEEDS_LABELS": ctx.attr.external_needs_labels,
         "SCORE_SOURCELINKS": (
             ctx.file.score_sourcelinks_json.path if ctx.file.score_sourcelinks_json else ""
         ),
@@ -93,6 +93,7 @@ sphinx_docs = rule(
         "score_sourcelinks_json": attr.label(allow_single_file = True),
         "mounts_manifest": attr.label(allow_single_file = True),
         "score_metamodel_yaml": attr.label(allow_single_file = True),
+        "external_needs_labels": attr.string(default = "[]"),
         "extra_opts": attr.string_list(),
         # The launcher runs on the build host and carries extension runfiles.
         "sphinx": attr.label(cfg = "exec", executable = True, mandatory = True),

docs.bzl

@@ -79,7 +79,6 @@ def _sphinx_define(name, value):
 
 def _needs_sphinx_extra_opts(
         master_doc,
-        external_needs_source,
         score_bundle_needs_export,
         score_source_code_linker_plain_links):
     """Return per-target Sphinx configuration defines for a Needs build."""
@@ -90,7 +89,6 @@ def _needs_sphinx_extra_opts(
         option
         for name, value in [
         ("master_doc", master_doc),
-        ("external_needs_source", external_needs_source),
         ("score_bundle_needs_export", score_bundle_needs_export),
         ("score_source_code_linker_plain_links", score_source_code_linker_plain_links),
         ]
@@ -119,7 +117,7 @@ def _needs_sphinx_docs(
         sphinx_build_deps,
         bundle,
         master_doc = None,
-        external_needs_source = None,
+        external_needs_labels = "[]",
         score_bundle_needs_export = None,
         score_sourcelinks_json = None,
         score_source_code_linker_plain_links = None,
@@ -152,13 +150,13 @@ def _needs_sphinx_docs(
         data = sphinx_build_data,
         extra_opts = _needs_sphinx_extra_opts(
             master_doc,
-            external_needs_source,
             score_bundle_needs_export,
             score_source_code_linker_plain_links,
         ),
         # Keep these as labels rather than path strings in ``extra_opts``. The
         # private rule declares them as action inputs and provides execroot
         # paths directly through the environment.
+        external_needs_labels = external_needs_labels,
         score_sourcelinks_json = score_sourcelinks_json,
         mounts_manifest = mounts_manifest,
         score_metamodel_yaml = score_metamodel_yaml,
@@ -369,7 +367,7 @@ def _declare_bundle_local_needs(
         sphinx_build_deps = sphinx_build_deps,
         sphinx_build_data = data,
         master_doc = entry_doc,
-        external_needs_source = "[]",
+        external_needs_labels = "[]",
         score_bundle_needs_export = "1",
         score_sourcelinks_json = sourcelinks_json,
         score_source_code_linker_plain_links = "1",
@@ -701,7 +699,7 @@ def docs(
         config = sphinx_config,
         sphinx_build_deps = deps,
         sphinx_build_data = data + external_needs + metamodel_label + [":docs_bundle"],
-        external_needs_source = str(data + external_needs),
+        external_needs_labels = str(data + external_needs),
         score_sourcelinks_json = ":sourcelinks_json",
         score_source_code_linker_plain_links = "1",
         mounts_manifest = mounts_manifest,

src/docs_cli/README.md

@@ -49,6 +49,8 @@ linking can traverse this Bazel package boundary.
 `docs.bzl` provides `SOURCE_DIRECTORY`, `PACKAGE_DIR`, `DATA`, and optional
 configuration such as `SPHINX_CONFIG_FILE`, `SCORE_METAMODEL_YAML`,
 `MOUNTS_MANIFEST`, `EXTERNAL_NEEDS_FILES`, `TEST_SOURCES` and `KNOWN_GOOD_JSON`.
+The sandboxed Needs action passes its external-needs labels separately through
+the internal `EXTERNAL_NEEDS_LABELS` variable.
 Bazel provides the workspace and runfiles locations. The CLI resolves source
 and output paths relative to the package containing the `docs()` call; generated
 configuration is resolved through runfiles.

src/docs_cli/cli.py

@@ -40,15 +40,23 @@
 env = Environment()
 
 
-def _merged_external_needs() -> str:
-    """Combine DATA and EXTERNAL_NEEDS_FILES into one JSON label list.
-
-    Both env vars hold JSON lists of Bazel labels; the extension parses the
-    resulting `external_needs_source` define uniformly.
+def _build_external_needs_source_config() -> str:
+    """Build the JSON label list for Sphinx's ``external_needs_source`` config.
+
+    ``DATA`` contains all data dependencies of the documentation target,
+    ``EXTERNAL_NEEDS_FILES`` contains explicitly declared external-needs
+    dependencies, and the sandboxed Needs action uses
+    ``EXTERNAL_NEEDS_LABELS`` for its explicitly declared label list. All
+    variables contain JSON arrays of Bazel labels.
+
+    The metamodel extension filters ordinary data dependencies and resolves
+    supported labels against the runfiles directory supplied as a separate
+    Sphinx configuration value.
     """
-    data = env.string_list("DATA")
+    data = env.string_list("DATA", "[]")
     external = env.string_list("EXTERNAL_NEEDS_FILES", "[]")
-    return json.dumps(data + external)
+    labels = env.string_list("EXTERNAL_NEEDS_LABELS", "[]")
+    return json.dumps(data + external + labels)
 
 
 def _compute_hash(files: list[Path]) -> str:
@@ -159,6 +167,7 @@ def sphinx_arguments(
     """Build Sphinx arguments from the resolved launcher configuration."""
     output_dir = config.output_dir
     mounts_manifest = env.optional_path("MOUNTS_MANIFEST")
+    runfiles_dir = env.optional_path("RUNFILES_DIR")
     if mounts_manifest:
         mounts_manifest = _resolve_runfiles_relative_path(config, mounts_manifest)
 
@@ -170,13 +179,15 @@ def sphinx_arguments(
         "-T",  # show details in case of errors in extensions
         "--jobs",
         "auto",
-        # Merge DATA (:needs_json / :docs_sources) with EXTERNAL_NEEDS_FILES
-        # (:needs_json_file) into one define consumed by the Sphinx extensions.
-        f"--define=external_needs_source={_merged_external_needs()}",
+        # Forward Bazel data dependencies to the score_metamodel extension.
+        f"--define=external_needs_source={_build_external_needs_source_config()}",
         f"--define=testcase_source_dirs={env.get('TEST_SOURCES', '[]')}",
         # Path to the Bazel-emitted mounts manifest (empty when no mounts are
         # configured); consumed by the score_mounts extension.
         f"--define=mounts_manifest={mounts_manifest or ''}",
+        # The external-needs extension uses this root to resolve label-based
+        # inputs without reading the process environment itself.
+        f"--define=runfiles_dir={runfiles_dir.absolute() if runfiles_dir else ''}",
     ]
 
     if config.is_bazel_build:

src/docs_cli/main_test.py

@@ -11,6 +11,7 @@
 # SPDX-License-Identifier: Apache-2.0
 # *******************************************************************************
 
+import json
 from pathlib import Path
 from unittest.mock import Mock
 
@@ -31,6 +32,7 @@ def workspace(fs: FFS, monkeypatch: pytest.MonkeyPatch) -> Path:
     # optional values left behind by the test runner before setting the basics.
     ENVIRONMENT_OVERRIDES = (
         "EXTERNAL_NEEDS_FILES",
+        "EXTERNAL_NEEDS_LABELS",
         "TEST_SOURCES",
         "MOUNTS_MANIFEST",
         "SPHINX_CONFIG_FILE",
@@ -195,8 +197,8 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_
     monkeypatch.setenv("SPHINX_CONFIG_FILE", "config/conf.py")
     monkeypatch.setenv("SCORE_METAMODEL_YAML", "config/metamodel.yaml")
     monkeypatch.setenv("MOUNTS_MANIFEST", "mounts.json")
-    monkeypatch.setenv("DATA", '[":bundle"]')
-    monkeypatch.setenv("EXTERNAL_NEEDS_FILES", '["@vendor//:needs"]')
+    monkeypatch.setenv("DATA", '["//:needs_json"]')
+    monkeypatch.setenv("EXTERNAL_NEEDS_FILES", '["@vendor//:needs_json"]')
     monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo")
     monkeypatch.setenv("KNOWN_GOOD_JSON", "baseline.json")
     monkeypatch.setenv("ACTION", "incremental")
@@ -205,14 +207,16 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_
     arguments = sphinx_arguments(DocsCliConfig.from_environment())
 
     # Assert
+    external_needs = json.dumps(["//:needs_json", "@vendor//:needs_json"])
     expected_arguments = {
         # Generated configuration and metamodel paths use the runfiles tree.
         "-c",
         str(workspace / "runfiles/config"),
         f"--define=score_metamodel_yaml={workspace}/runfiles/config/metamodel.yaml",
         f"--define=mounts_manifest={workspace}/runfiles/mounts.json",
+        f"--define=runfiles_dir={workspace}/runfiles",
         # DATA and EXTERNAL_NEEDS_FILES are passed as one Sphinx define.
-        '--define=external_needs_source=[":bundle", "@vendor//:needs"]',
+        f"--define=external_needs_source={external_needs}",
         # GitHub metadata must keep edit links repository-relative.
         "-A=github_user=owner",
         "-A=github_repo=repo",
@@ -249,6 +253,24 @@ def test_bazel_build_resolves_mount_manifest_from_execroot(
     )
 
 
+def test_bazel_needs_action_uses_external_needs_labels_channel(
+    workspace: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    # Arrange
+    monkeypatch.delenv("BUILD_WORKSPACE_DIRECTORY")
+    monkeypatch.chdir(workspace)
+    monkeypatch.setenv("ACTION", "build_needs_json")
+    monkeypatch.setenv("OUTPUT_DIRECTORY", "outputs/needs")
+    monkeypatch.setenv("EXTERNAL_NEEDS_LABELS", '["//:needs_json"]')
+
+    # Act
+    arguments = sphinx_arguments(DocsCliConfig.from_environment())
+
+    # Assert
+    assert '--define=external_needs_source=["//:needs_json"]' in arguments
+
+
 def test_direct_invocation_resolves_paths_relative_to_cwd(
     workspace: Path,
     monkeypatch: pytest.MonkeyPatch,

src/extensions/score_metamodel/__init__.py

@@ -243,6 +243,13 @@ def _clear_needs_defaults(app: Sphinx):
 
 def setup(app: Sphinx) -> dict[str, str | bool]:
     app.add_config_value("external_needs_source", "", rebuild="env")
+    app.add_config_value(
+        "runfiles_dir",
+        "",
+        rebuild="env",
+        types=str,
+        description="Bazel runfiles root supplied by the documentation CLI.",
+    )
     app.add_config_value("score_metamodel_yaml", "", rebuild="env")
     app.add_config_value("required_in_id", [], rebuild="env")
     app.add_config_value("score_bundle_needs_export", False, rebuild="env")
@@ -278,11 +285,7 @@ def setup(app: Sphinx) -> dict[str, str | bool]:
     config_setdefault(app.config, "needs_reproducible_json", True)
     config_setdefault(app.config, "needs_json_remove_defaults", True)
 
-    # sphinx-collections runs on default prio 500.
-    # We need to populate the sphinx-collections config before that happens.
-    # If we put it anywhere higher it seems that other things already lock the needs
-    # To ensure that this runs first before locking happens priot is => 425
-    # The lower the number the higher priority it has (runs earlier)
+    # Populate external Needs before Sphinx-Needs locks its configuration.
     _ = app.connect("config-inited", connect_external_needs, priority=425)
 
     discover_checks()