refactor: centralize Bazel path handling for docs builds

bzl/needs_rules.bzl

@@ -26,15 +26,20 @@ def _sphinx_docs_impl(ctx):
     output = ctx.actions.declare_directory(ctx.label.name + "/_build/needs")
 
     bundle = ctx.attr.bundle[DocsBundleInfo]
+
     # The bundle owns both the direct inputs and their execution-root-relative
     # source root. Nested sources are provided separately for score_mounts, so
     # local exports retain their bundle ownership.
     if not bundle.own_source_files.to_list():
         fail("Sphinx requires a bundle with direct documentation sources")
 
-    # Expand file labels at analysis time, then encode the argument list as
-    # JSON so spaces, quotes and '=' in Sphinx options survive the environment
-    # transport unchanged. The launcher adds these after its default options.
+    # 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.
+    # Encode that list as JSON so spaces, quotes and '=' survive transport.
+    # TEMPORARY DIFF NOTE: Keeping file paths out of ``SPHINX_EXTRA_OPTS``
+    # avoids manufacturing ``--define`` strings for values that the launcher
+    # and extensions already accept as environment variables.
     # ``config`` is transported separately because the launcher derives
     # Sphinx's ``-c`` directory from its path; it is not just another data file.
     env = {
@@ -43,10 +48,16 @@ def _sphinx_docs_impl(ctx):
         "OUTPUT_DIRECTORY": output.path,
         "SPHINX_CONFIG_FILE": ctx.file.config.path,
         "DATA": "[]",
-        "SPHINX_EXTRA_OPTS": json.encode([
-            ctx.expand_location(option, targets = ctx.attr.tools)
-            for option in ctx.attr.extra_opts
-        ]),
+        "SCORE_SOURCELINKS": (
+            ctx.file.score_sourcelinks_json.path if ctx.file.score_sourcelinks_json else ""
+        ),
+        "MOUNTS_MANIFEST": (
+            ctx.file.mounts_manifest.path if ctx.file.mounts_manifest else ""
+        ),
+        "SCORE_METAMODEL_YAML": (
+            ctx.file.score_metamodel_yaml.path if ctx.file.score_metamodel_yaml else ""
+        ),
+        "SPHINX_EXTRA_OPTS": json.encode(ctx.attr.extra_opts),
     }
 
     # Data and mounted sources must be present at their execution-root paths.
@@ -56,7 +67,15 @@ def _sphinx_docs_impl(ctx):
         executable = ctx.executable.sphinx,
         env = env,
         inputs = depset(
-            [ctx.file.config] + ctx.files.data + ctx.files.tools,
+            [ctx.file.config] + ctx.files.data + ctx.files.tools + [
+                file
+                for file in [
+                    ctx.file.score_sourcelinks_json,
+                    ctx.file.mounts_manifest,
+                    ctx.file.score_metamodel_yaml,
+                ]
+                if file
+            ],
             transitive = [bundle.own_source_files],
         ),
         outputs = [output],
@@ -73,6 +92,12 @@ sphinx_docs = rule(
         "bundle": attr.label(providers = [DocsBundleInfo], mandatory = True),
         "data": attr.label_list(allow_files = True),
         "tools": attr.label_list(allow_files = True),
+        # TEMPORARY DIFF NOTE: These values used to be embedded in Sphinx
+        # defines. Typed labels let the action pass their execroot paths through
+        # the existing environment contract and still declare sandbox inputs.
+        "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),
         "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

@@ -81,10 +81,7 @@ def _needs_sphinx_extra_opts(
         master_doc,
         external_needs_source,
         score_bundle_needs_export,
-        score_sourcelinks_json,
-        score_source_code_linker_plain_links,
-        mounts_manifest,
-        score_metamodel_yaml):
+        score_source_code_linker_plain_links):
     """Return per-target Sphinx configuration defines for a Needs build."""
     # The launcher supplies diagnostics shared by every builder. Keep only
     # target-specific defines here so the action does not receive duplicate
@@ -95,10 +92,7 @@ def _needs_sphinx_extra_opts(
         ("master_doc", master_doc),
         ("external_needs_source", external_needs_source),
         ("score_bundle_needs_export", score_bundle_needs_export),
-        ("score_sourcelinks_json", score_sourcelinks_json),
         ("score_source_code_linker_plain_links", score_source_code_linker_plain_links),
-        ("mounts_manifest", mounts_manifest),
-        ("score_metamodel_yaml", score_metamodel_yaml),
         ]
         for option in _sphinx_define(name, value)
     ]
@@ -152,11 +146,14 @@ def _needs_sphinx_docs(
             master_doc,
             external_needs_source,
             score_bundle_needs_export,
-            score_sourcelinks_json,
             score_source_code_linker_plain_links,
-            mounts_manifest,
-            score_metamodel_yaml,
         ),
+        # TEMPORARY DIFF NOTE: Keep these as labels rather than path strings in
+        # ``extra_opts``. The private rule can then declare them as action
+        # inputs and provide execroot paths directly through the environment.
+        score_sourcelinks_json = score_sourcelinks_json,
+        mounts_manifest = mounts_manifest,
+        score_metamodel_yaml = score_metamodel_yaml,
         sphinx = sphinx_build,
         tools = tools,
         visibility = visibility,
@@ -354,6 +351,9 @@ def _declare_bundle_local_needs(
     sphinx_build_deps = _sphinx_runtime_deps(deps)
 
     needs_local = _bundle_internal_target(name, "needs_local")
+    # TEMPORARY DIFF NOTE: Keep the generated source-links target typed as a
+    # label here; the private Needs rule now owns translating it to an action
+    # environment path and declaring it as an input.
     _needs_sphinx_docs(
         name = needs_local,
         bundle = ":" + name,
@@ -363,7 +363,7 @@ def _declare_bundle_local_needs(
         master_doc = entry_doc,
         external_needs_source = "[]",
         score_bundle_needs_export = "1",
-        score_sourcelinks_json = "$(location " + str(sourcelinks_json) + ")" if sourcelinks_json else None,
+        score_sourcelinks_json = sourcelinks_json,
         score_source_code_linker_plain_links = "1",
         tools = [sourcelinks_json] if sourcelinks_json else [],
         visibility = visibility,
@@ -618,28 +618,33 @@ def docs(
         # generated configuration must be present in the runfiles tree.
         docs_data += [sphinx_config]
 
+    # TEMPORARY DIFF NOTE: Interactive file dependencies use runfiles keys so
+    # the Python runfiles library can resolve them in directory- and
+    # manifest-based layouts; these are not build-action execroot paths.
+    # SOURCE_DIRECTORY is intentionally different: it points at the checkout
+    # read by the developer, not at a copy of the local sources in runfiles.
     docs_env = {
         "SOURCE_DIRECTORY": source_dir,
         "PACKAGE_DIR": native.package_name(),
         "TEST_SOURCES": str(test_sources),
         "DATA": str(data),
         "EXTERNAL_NEEDS_FILES": str(external_needs),
         # `bazel run` starts from a runfiles tree, so this logical path is
-        # resolved by score_mounts through ``RUNFILES_DIR``.
+        # resolved by score_mounts through DocsCliConfig's runfiles resolver.
         "MOUNTS_MANIFEST": "$(rlocationpath :_mounts_manifest)" if bundles else "",
-        "SCORE_SOURCELINKS": "$(location :sourcelinks_json)",
+        "SCORE_SOURCELINKS": "$(rlocationpath :sourcelinks_json)",
     }
     if config_is_generated:
         # The generated file is named conf.py. Run targets pass its containing
         # directory to Sphinx via -c.
         docs_env["SPHINX_CONFIG_FILE"] = "$(rlocationpath " + sphinx_config + ")"
     if metamodel:
         # The interactive ``py_binary`` targets run from a runfiles tree.
-        # docs_cli resolves this logical path through ``RUNFILES_DIR``.
+        # docs_cli resolves this logical path through DocsCliConfig.
         docs_env["SCORE_METAMODEL_YAML"] = "$(rlocationpath " + str(metamodel) + ")"
     if known_good_label:
         known_good_str = str(known_good_label[0])
-        docs_env["KNOWN_GOOD_JSON"] = "$(location " + known_good_str + ")"
+        docs_env["KNOWN_GOOD_JSON"] = "$(rlocationpath " + known_good_str + ")"
         docs_data += known_good_label
 
     # Generated documentation artifacts may live below ``docs/``.  A
@@ -697,21 +702,24 @@ def docs(
         sphinx_build_deps = deps,
         sphinx_build_data = data + external_needs + metamodel_label + [":docs_bundle"],
         external_needs_source = str(data + external_needs),
-        score_sourcelinks_json = "$(location :sourcelinks_json)",
+        score_sourcelinks_json = ":sourcelinks_json",
         score_source_code_linker_plain_links = "1",
-        # The build action runs in a sandbox, so it needs the action-input path
-        # rather than the runfiles-relative spelling.
-        mounts_manifest = "$(location :_mounts_manifest)" if bundles else None,
-        score_metamodel_yaml = "$(location " + str(metamodel) + ")" if metamodel else None,
+        # TEMPORARY DIFF NOTE: Pass labels, not ``$(location ...)`` strings, so
+        # the action can declare each input and use its execution-root path
+        # directly, without string expansion or placeholder substitution.
+        mounts_manifest = ":_mounts_manifest" if bundles else None,
+        score_metamodel_yaml = metamodel if metamodel else None,
         tools = external_needs + metamodel_label + [":sourcelinks_json", ":docs_bundle"] + mounts_manifest_label,
         visibility = ["//visibility:public"],
     )
 
+    # TEMPORARY DIFF NOTE: These commands consume build outputs from the
+    # execution root. ``execpath`` states that action-path contract explicitly.
     native.genrule(
         name = "metrics_json",
         srcs = [":needs_json"],
         outs = ["metrics.json"],
-        cmd = "cp $(location :needs_json)/metrics.json $@",
+        cmd = "cp $(execpath :needs_json)/metrics.json $@",
         visibility = ["//visibility:public"],
         tags = ["manual"],
     )
@@ -722,7 +730,7 @@ def docs(
         name = "needs_json_file",
         srcs = [":needs_json"],
         outs = ["needs.json"],
-        cmd = "cp $(location :needs_json)/needs.json $@",
+        cmd = "cp $(execpath :needs_json)/needs.json $@",
         visibility = ["//visibility:public"],
         tags = ["manual"],
     )

score_pytest.bzl

@@ -42,8 +42,11 @@ def score_pytest(name, srcs, args = [], data = [], deps = [], env = {}, plugins
             pytest_bootstrap,
         ] + srcs,
         main = pytest_bootstrap,
+        # TEMPORARY DIFF NOTE: Config and test-file arguments now use runfiles
+        # keys instead of location expansions. The bootstrap resolves them
+        # before pytest starts, including in manifest-only runfiles layouts.
         args = [
-                   "-c $(location %s)" % pytest_config,
+                   "-c $(rlocationpath %s)" % pytest_config,
                    "-p no:cacheprovider",
 
                    # XML_OUTPUT_FILE: Location to which test actions should write a test
@@ -54,8 +57,14 @@ def score_pytest(name, srcs, args = [], data = [], deps = [], env = {}, plugins
                ] +
                args +
                plugins +
-               ["$(location %s)" % x for x in srcs],
-        deps = deps + ["@score_docs_as_code//score_pytest:attribute_plugin"],
+               ["$(rlocationpath %s)" % x for x in srcs],
+        # pytest consumes filesystem paths, not runfiles addresses. The
+        # bootstrap resolves these rlocationpaths before handing arguments to
+        # pytest.
+        deps = deps + [
+            "@score_docs_as_code//score_pytest:attribute_plugin",
+            "@rules_python//python/runfiles",
+        ],
         data = [
             pytest_config,
         ] + data,

score_pytest/main.py

@@ -11,9 +11,45 @@
 # SPDX-License-Identifier: Apache-2.0
 # *******************************************************************************
 import sys
+from typing import cast
 
 import pytest
+from python.runfiles import Runfiles
+
+
+def _resolve_runfiles_paths(args: list[str]) -> list[str]:
+    """Turn runfiles addresses in pytest arguments into filesystem paths."""
+    runfiles = Runfiles.Create()
+    if runfiles is None:
+        return args
+
+    resolved_args: list[str] = []
+    for arg in args:
+        # pytest flags and their ordinary values are not runfiles addresses.
+        # rlocationpath values always include a repository and use `/` as the
+        # separator, so only those path-like arguments need a lookup.
+        if arg.startswith("-") or "=" in arg or "/" not in arg:
+            resolved_args.append(arg)
+            continue
+
+        # pytest expects ordinary paths for its config and test-file arguments;
+        # rlocationpath values also work with manifest-only runfiles layouts.
+        try:
+            resolved_path = cast(str | None, runfiles.Rlocation(arg))
+            resolved_args.append(resolved_path or arg)
+        except ValueError:
+            # Preserve non-normalized user arguments; only Bazel's generated
+            # rlocationpath spellings are guaranteed to be normalized.
+            resolved_args.append(arg)
+    return resolved_args
+
+
+def main(argv: list[str] | None = None) -> int:
+    """Run pytest after resolving any Bazel runfiles arguments."""
+    if argv is None:
+        argv = sys.argv[1:]
+    return pytest.main(_resolve_runfiles_paths(argv))
+
 
 if __name__ == "__main__":
-    args = sys.argv[1:]
-    sys.exit(pytest.main(args))
+    sys.exit(main())

score_pytest/tests/test_rules_are_working_correctly.py

@@ -10,5 +10,48 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 # *******************************************************************************
+
+from unittest.mock import Mock
+
+import pytest
+
+from score_pytest import main
+
+
 def test_score_pytest_loads_conftest(fixture42):  # pyright: ignore[reportMissingParameterType]
     assert fixture42 == 42
+
+
+def test_score_pytest_resolves_runfiles_arguments(monkeypatch: pytest.MonkeyPatch):
+    """Resolve config and test-file addresses while leaving pytest options intact."""
+    runfiles = Mock()
+    runfiles.Rlocation.side_effect = lambda path: {
+        "_main/pyproject.toml": "/runfiles/pyproject.toml",
+        "_main/tests/test_example.py": "/runfiles/tests/test_example.py",
+    }.get(path)
+    monkeypatch.setattr(main.Runfiles, "Create", Mock(return_value=runfiles))
+    pytest_main = Mock(return_value=0)
+    monkeypatch.setattr(main.pytest, "main", pytest_main)
+
+    result = main.main(
+        [
+            "-c",
+            "_main/pyproject.toml",
+            "-p",
+            "no:cacheprovider",
+            "--junitxml=/tmp/test.xml",
+            "_main/tests/test_example.py",
+        ]
+    )
+
+    assert result == 0
+    pytest_main.assert_called_once_with(
+        [
+            "-c",
+            "/runfiles/pyproject.toml",
+            "-p",
+            "no:cacheprovider",
+            "--junitxml=/tmp/test.xml",
+            "/runfiles/tests/test_example.py",
+        ]
+    )