diff --git a/bzl/needs_rules.bzl b/bzl/needs_rules.bzl index 1c6fbd045..9756095a0 100644 --- a/bzl/needs_rules.bzl +++ b/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), diff --git a/docs.bzl b/docs.bzl index fbf84aa50..f364ec77d 100644 --- a/docs.bzl +++ b/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,6 +618,11 @@ 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(), @@ -625,9 +630,9 @@ def docs( "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 @@ -635,11 +640,11 @@ def docs( 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"], ) diff --git a/score_pytest.bzl b/score_pytest.bzl index a82cc0aaa..adcf95ce1 100644 --- a/score_pytest.bzl +++ b/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, diff --git a/score_pytest/main.py b/score_pytest/main.py index 8c5afd553..6d1f2559e 100644 --- a/score_pytest/main.py +++ b/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()) diff --git a/score_pytest/tests/test_rules_are_working_correctly.py b/score_pytest/tests/test_rules_are_working_correctly.py index 29598f4a7..224f994b4 100644 --- a/score_pytest/tests/test_rules_are_working_correctly.py +++ b/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", + ] + ) diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index b0a69cb43..40efec82e 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -30,7 +30,7 @@ ) from src.extensions.score_mounts._resolver import load_mounts_manifest, resolve_walk_dir -from src.helper_lib import Environment, get_runfiles_dir +from src.helper_lib import Environment from src.helper_lib.config import DocsCliConfig logger = logging.getLogger(__name__) @@ -90,9 +90,7 @@ def update_module_hash(build_dir: Path, sentinel_files: list[Path]) -> None: (build_dir / _MODULE_HASH_FILE).write_text(_compute_hash(sentinel_files)) -def mounted_watch_dirs( - manifest_path: Path, ws_root: Path | None, runfiles_dir: Path | None = None -) -> list[str]: +def mounted_watch_dirs(manifest_path: Path, config: DocsCliConfig) -> list[str]: """Return the directories provided by docs bundles for ``sphinx-autobuild``. This deliberately uses the same manifest and path-resolution rules as the @@ -116,23 +114,10 @@ def add_watch_dir(path: Path) -> None: # which makes sphinx-autobuild observe unrelated files (including its # own output). Watch the generated data directories instead. if spec.src_root: - add_watch_dir(resolve_walk_dir(manifest, spec, ws_root, runfiles_dir)) + add_watch_dir(resolve_walk_dir(spec, config)) for data_file in spec.data: - if ws_root is not None and runfiles_dir is not None: - runfiles_str = str(runfiles_dir) - if "/bazel-out/" in runfiles_str: - # The runfiles path points into the execroot's output - # tree. Use the execroot prefix just like score_mounts. - walk_file = Path(runfiles_str.split("/bazel-out/")[0]) / data_file - else: - walk_file = ( - ws_root - / "bazel-bin" - / data_file.removeprefix("bazel-out/k8-fastbuild/bin/") - ) - else: - walk_file = Path.cwd() / data_file + walk_file = config.resolve_bazel_output_path(data_file) add_watch_dir(walk_file.parent) return watch_dirs @@ -143,8 +128,9 @@ def sphinx_arguments( ) -> list[str]: """Build Sphinx arguments from the resolved launcher configuration.""" output_dir = config.output_dir + source_dir = config.package_dir / env.required_path("SOURCE_DIRECTORY") base_arguments = [ - str(config.source_dir), + str(source_dir), str(output_dir), "-W", # treat warning as errors "--keep-going", # do not abort after one error @@ -167,9 +153,9 @@ def sphinx_arguments( # mixing action state into the declared output. base_arguments.extend(["-d", str(output_dir) + "_doctrees"]) - # The sandboxed Needs rule transports options as JSON so spaces, quotes and - # equals signs survive the environment boundary. Append them last so an - # action-specific value can override one of the shared defaults above. + # The sandboxed Needs rule transports non-path Sphinx overrides as JSON + # so spaces, quotes and '=' survive the environment boundary. Append + # them last so action-specific values can override shared defaults. base_arguments.extend(env.string_list("SPHINX_EXTRA_OPTS", "[]")) else: # Interactive builds keep warnings in the workspace so developers can @@ -179,32 +165,18 @@ def sphinx_arguments( base_arguments.extend(["--warning-file", str(output_dir / "warnings.txt")]) if config_file := env.optional_path("SPHINX_CONFIG_FILE"): - # The action receives ctx.file.config.path, which is interpreted from - # the action's execution-root working directory. Resolve it locally - # instead of using runfiles lookup; interactive targets receive a - # runfiles-relative path and need that lookup before Sphinx gets the - # containing directory. - if config.is_bazel_build: - config_file = config_file.absolute() - elif not config_file.is_absolute(): - config_file = get_runfiles_dir() / config_file + # Bazel run targets pass an rlocationpath, build actions pass the + # declared input's execution-root path, and direct callers use + # cwd-relative paths. The config owns those execution-mode differences. + config_file = config.resolve_input_path(config_file) + if config_file is None: + raise ValueError("Could not resolve SPHINX_CONFIG_FILE") base_arguments.extend(["-c", str(config_file.parent)]) if metamodel_yaml := env.optional_path("SCORE_METAMODEL_YAML"): - # Under ``bazel run``, this environment variable is runfiles-relative - # and must be resolved through RUNFILES_DIR. A sandboxed Needs action - # instead expands the metamodel label to an execution-root path in - # SPHINX_EXTRA_OPTS; applying runfiles lookup there would escape the - # action's declared inputs. - if not config.is_bazel_build and not metamodel_yaml.is_absolute(): - runfiles_dir = env.optional_path("RUNFILES_DIR") - ws_root = config.ws_root or Path() - metamodel_yaml = ( - runfiles_dir / metamodel_yaml - if runfiles_dir is not None - else ws_root / metamodel_yaml - ) - metamodel_yaml = metamodel_yaml.absolute() + metamodel_yaml = config.resolve_input_path(metamodel_yaml) + if metamodel_yaml is None: + raise ValueError("Could not resolve SCORE_METAMODEL_YAML") base_arguments.append(f"--define=score_metamodel_yaml={metamodel_yaml}") if github_repository := env.get("GITHUB_REPOSITORY", ""): @@ -217,7 +189,13 @@ def sphinx_arguments( base_arguments.append("-A=github_version=main") # doc_path must be repo-relative so the edit URL does not contain the # absolute runner filesystem path (e.g. /home/runner/work/…/docs). - relative_doc_path = config.source_dir_relative_to_ws + if config.is_bazel_run: + assert config.ws_root + relative_doc_path = source_dir.relative_to(config.ws_root) + else: + # Direct calls and sandbox actions already use cwd-relative source + # paths; only bazel run needs its workspace prefix stripped. + relative_doc_path = source_dir base_arguments.append(f"-A=doc_path={relative_doc_path}") if known_good_json := env.optional_path("KNOWN_GOOD_JSON"): @@ -231,17 +209,14 @@ def watch_arguments(config: DocsCliConfig) -> list[str]: mounts_manifest = env.optional_path("MOUNTS_MANIFEST") watch_arguments: list[str] = [] if mounts_manifest: - # ``MOUNTS_MANIFEST`` is runfiles-relative under ``bazel run`` and - # an ordinary path for direct invocations, matching score_mounts. - manifest_path = ( - get_runfiles_dir() / mounts_manifest - if config.is_bazel_run - else mounts_manifest - ) + # The manifest is an rlocationpath under ``bazel run`` and a regular + # path for direct calls. Build actions do not invoke sphinx-autobuild. + manifest_path = config.resolve_input_path(mounts_manifest) + if manifest_path is None: + raise ValueError("Could not resolve MOUNTS_MANIFEST") for watch_dir in mounted_watch_dirs( manifest_path, - config.ws_root, - get_runfiles_dir() if config.is_bazel_run else None, + config, ): watch_arguments.extend(["--watch", watch_dir]) return watch_arguments @@ -276,7 +251,7 @@ def main(argv: list[str] | None = None) -> int: logger.info("Waiting for client to connect on port: " + str(args.debug_port)) debugpy.wait_for_client() - config = DocsCliConfig.from_environment(env) + config = DocsCliConfig(env) ws_root = config.ws_root or Path() package_dir = config.package_dir output_dir = config.output_dir diff --git a/src/docs_cli/dirty_build_test.py b/src/docs_cli/dirty_build_test.py index f8e82281c..d57ee080c 100644 --- a/src/docs_cli/dirty_build_test.py +++ b/src/docs_cli/dirty_build_test.py @@ -13,6 +13,7 @@ import json from pathlib import Path +from unittest.mock import Mock import pytest from pyfakefs.fake_filesystem import FakeFilesystem as FFS @@ -195,8 +196,20 @@ def test_mounted_watch_dirs_match_sphinx_mount_paths(tmp_path: Path) -> None: ) workspace = tmp_path / "workspace" runfiles_dir = tmp_path / "runfiles" + config = Mock(is_bazel_run=True) + config.ws_root = workspace - assert mounted_watch_dirs(manifest_path, workspace, runfiles_dir) == [ + def resolve_input_path(path: Path) -> Path: + # In bazel run, DocsCliConfig interprets every relative input as a + # runfiles address; callers do not need to select that mode themselves. + relative_path = Path( + *(part for part in Path(path).parts if part not in {"_main", ".."}) + ) + return runfiles_dir / relative_path + + config.resolve_input_path.side_effect = resolve_input_path + + assert mounted_watch_dirs(manifest_path, config) == [ str(workspace / "extensions/local/docs"), str(runfiles_dir / "vendor+" / "docs"), ] @@ -222,8 +235,11 @@ def test_mounted_watch_dirs_use_data_directories_for_pure_data_bundles( encoding="utf-8", ) workspace = tmp_path / "workspace" - runfiles_dir = tmp_path / "runfiles" + config = Mock() + config.resolve_bazel_output_path.return_value = ( + workspace / "bazel-bin/pkg/generated/index.rst" + ) - assert mounted_watch_dirs(manifest_path, workspace, runfiles_dir) == [ + assert mounted_watch_dirs(manifest_path, config) == [ str(workspace / "bazel-bin/pkg/generated") ] diff --git a/src/docs_cli/main_test.py b/src/docs_cli/main_test.py index a63a685c4..61b1aa5eb 100644 --- a/src/docs_cli/main_test.py +++ b/src/docs_cli/main_test.py @@ -33,8 +33,10 @@ def workspace(fs: FFS, monkeypatch: pytest.MonkeyPatch) -> Path: "EXTERNAL_NEEDS_FILES", "TEST_SOURCES", "MOUNTS_MANIFEST", + "SCORE_SOURCELINKS", "SPHINX_CONFIG_FILE", "SCORE_METAMODEL_YAML", + "SPHINX_EXTRA_OPTS", "GITHUB_REPOSITORY", "KNOWN_GOOD_JSON", "RUNFILES_DIR", @@ -108,6 +110,48 @@ def test_build_action_selects_sphinx_builder( assert arguments[-2:] == ["-b", builder] +def test_needs_build_uses_environment_for_file_inputs_and_json_for_other_options( + workspace: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Needs actions pass file paths separately from Sphinx option overrides.""" + + # Arrange + monkeypatch.delenv("BUILD_WORKSPACE_DIRECTORY") + monkeypatch.setenv("ACTION", "build_needs_json") + monkeypatch.setenv("OUTPUT_DIRECTORY", "outputs/needs") + monkeypatch.chdir(workspace) + mounts_manifest = Path("bazel-out/k8-fastbuild/bin/pkg/mounts.json") + source_links = Path("bazel-out/k8-fastbuild/bin/pkg/sourcelinks.json") + metamodel = Path("bazel-out/k8-fastbuild/bin/pkg/metamodel.yaml") + monkeypatch.setenv("MOUNTS_MANIFEST", str(mounts_manifest)) + monkeypatch.setenv("SCORE_SOURCELINKS", str(source_links)) + monkeypatch.setenv("SCORE_METAMODEL_YAML", str(metamodel)) + monkeypatch.setenv( + "SPHINX_EXTRA_OPTS", + '["--define=master_doc=custom-index", "--define=score_source_code_linker_plain_links=1"]', + ) + noop_sphinx = Mock(return_value=0) + monkeypatch.setattr(docs_cli, "sphinx_main", noop_sphinx) + monkeypatch.setattr(docs_cli, "update_module_hash", Mock()) + + # Act + assert docs_cli.main([]) == 0 + + # Assert + arguments = noop_sphinx.call_args.args[0] + # The launcher converts the two environment paths that are Sphinx config + # values into defines; the source-linker consumes its path from the env. + assert f"--define=mounts_manifest={mounts_manifest}" in arguments + assert f"--define=score_metamodel_yaml={workspace / metamodel}" in arguments + assert not any( + arg.startswith("--define=score_sourcelinks_json=") for arg in arguments + ) + # Non-path Sphinx overrides retain their JSON transport. + assert "--define=master_doc=custom-index" in arguments + assert "--define=score_source_code_linker_plain_links=1" in arguments + + def test_failed_build_returns_exit_code_and_forces_next_build_clean( workspace: Path, monkeypatch: pytest.MonkeyPatch, @@ -200,9 +244,27 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_ monkeypatch.setenv("ACTION", "incremental") # Act - arguments = sphinx_arguments(DocsCliConfig.from_environment()) + config = DocsCliConfig() + arguments = sphinx_arguments(config) # Assert + assert config.resolve_input_path(Path("config/conf.py")) == ( + workspace / "runfiles/config/conf.py" + ) + # Runfile addresses, including external repository short paths, resolve + # through the config instead of requiring callers to join RUNFILES_DIR. + assert config.resolve_input_path( + Path("_main/../vendor+/docs"), runfiles_relative=True + ) == (workspace / "runfiles/vendor+/docs") + assert config.relative_to_runfiles(workspace / "runfiles/vendor+/docs") == Path( + "vendor+/docs" + ) + # Bazel's execroot-relative output spelling maps to the workspace-visible + # bazel-bin path in an interactive run. + assert ( + config.resolve_bazel_output_path("bazel-out/k8-fastbuild/bin/pkg/generated") + == workspace / "bazel-bin/pkg/generated" + ) expected_arguments = { # Generated configuration and metamodel paths use the runfiles tree. "-c", @@ -239,9 +301,10 @@ def test_direct_invocation_resolves_paths_relative_to_cwd( monkeypatch.chdir(workspace) monkeypatch.setenv("SCORE_METAMODEL_YAML", "metamodel.yaml") monkeypatch.setenv("ACTION", "incremental") + monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo") # Act - config = DocsCliConfig.from_environment() + config = DocsCliConfig() arguments = sphinx_arguments(config) # Assert @@ -251,3 +314,98 @@ def test_direct_invocation_resolves_paths_relative_to_cwd( assert f"--define=score_metamodel_yaml={workspace}/metamodel.yaml" in arguments # A direct invocation has no generated Sphinx config to resolve. assert "-c" not in arguments + # Direct callers do not have BUILD_WORKSPACE_DIRECTORY, so keep their + # cwd-relative source path instead of trying to relativize it to a workspace. + assert "-A=doc_path=docs" in arguments + + +def test_config_defaults_make_all_paths_available_without_launcher_values( + workspace: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Standalone extensions can inspect config without docs.bzl variables.""" + + # Arrange + for name in ( + "ACTION", + "BUILD_WORKSPACE_DIRECTORY", + "PACKAGE_DIR", + "SOURCE_DIRECTORY", + "OUTPUT_DIRECTORY", + ): + monkeypatch.delenv(name, raising=False) + monkeypatch.delenv("RUNFILES_DIR", raising=False) + monkeypatch.delenv("RUNFILES_MANIFEST_FILE", raising=False) + monkeypatch.setattr( + config_module.Runfiles, + "Create", + staticmethod(lambda: None), + ) + monkeypatch.chdir(workspace) + + # Act + config = DocsCliConfig() + + # Assert + assert config.is_direct + assert config.action is None + assert config.package_dir == Path() + assert config.output_dir == Path("_build") + + +def test_config_finds_ide_support_runfiles_for_direct_invocations( + workspace: Path, + fs: FFS, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Direct Sphinx launched from the IDE venv can still locate Bazel runfiles.""" + + # Arrange + for name in ( + "ACTION", + "BUILD_WORKSPACE_DIRECTORY", + "PACKAGE_DIR", + "RUNFILES_DIR", + "RUNFILES_MANIFEST_FILE", + ): + monkeypatch.delenv(name, raising=False) + monkeypatch.setattr( + config_module.Runfiles, + "Create", + staticmethod(lambda: None), + ) + monkeypatch.chdir(workspace) + fs.create_dir(workspace / ".git") + ide_runfiles_dir = workspace / "bazel-bin/ide_support.runfiles" + fs.create_dir(ide_runfiles_dir) + fs.create_file(ide_runfiles_dir / "_main/src/config/conf.py") + + # Act + config = DocsCliConfig() + + # Assert + assert config.is_direct + assert config.uses_ide_support_runfiles + assert config.resolve_input_path( + Path("_main/src/config/conf.py"), runfiles_relative=True + ) == (ide_runfiles_dir / "_main/src/config/conf.py") + + +def test_config_recovers_execroot_for_runfiles_output_paths( + workspace: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + execroot = workspace / "sandbox/execroot/_main" + monkeypatch.setenv( + "RUNFILES_DIR", + str(execroot / "bazel-out/k8-fastbuild/bin/docs.runfiles"), + ) + + config = DocsCliConfig() + + assert ( + config.resolve_bazel_output_path( + "bazel-out/k8-fastbuild/bin/pkg/generated/index.rst" + ) + == execroot / "bazel-out/k8-fastbuild/bin/pkg/generated/index.rst" + ) diff --git a/src/extensions/score_cross_module_compatibility/__init__.py b/src/extensions/score_cross_module_compatibility/__init__.py index 96c6fb075..5bb422cec 100644 --- a/src/extensions/score_cross_module_compatibility/__init__.py +++ b/src/extensions/score_cross_module_compatibility/__init__.py @@ -22,7 +22,8 @@ from sphinx.util import logging from sphinx_needs.need_item import NeedItem -from src.helper_lib import Environment, find_ws_root, get_runfiles_dir +from src.helper_lib import Environment +from src.helper_lib.config import DocsCliConfig _VERSION_CONDITION = re.compile(r"^\s*version\s*==\s*(\d+)\s*$") logger = logging.getLogger(__name__) @@ -224,15 +225,18 @@ def _manifest_path(app: Sphinx) -> Path | None: if not isinstance(raw, str) or not raw.strip(): return None direct = Path(raw) - runfiles = get_runfiles_dir() / raw + cli_config = DocsCliConfig() # ``mounts_manifest`` may be an execroot path, while the environment value - # passed to ``bazel run`` is runfiles-relative. Prefer an existing path so - # the policy does not depend on the current working directory. + # passed to ``bazel run`` is runfiles-relative. Prefer an existing direct + # path, then let the execution-mode config resolve the launcher value. if direct.is_file(): return direct - if runfiles.is_file(): - return runfiles - return runfiles if find_ws_root() else direct + resolved = cli_config.resolve_input_path(direct) + if resolved is not None and resolved.is_file(): + return resolved + if cli_config.is_bazel_run: + return resolved + return direct def get_reporter(app: Sphinx) -> CompatibilityReporter: diff --git a/src/extensions/score_layout/__init__.py b/src/extensions/score_layout/__init__.py index dfd49eb9f..4f35b99bd 100644 --- a/src/extensions/score_layout/__init__.py +++ b/src/extensions/score_layout/__init__.py @@ -20,10 +20,10 @@ import sphinx_options from sphinx.application import Sphinx -from src.helper_lib import Environment, config_setdefault +from src.helper_lib import config_setdefault +from src.helper_lib.config import DocsCliConfig logger = logging.getLogger(__name__) -env = Environment() # TEMP UNTIL UPSTREAM FIX - BEGIN # Bug ref: https://github.com/useblocks/sphinx-needs/issues/1913 @@ -119,7 +119,7 @@ def configure_mounted_source_controls( return source_path = source_path.resolve() - workspace_directory = env.optional_path("BUILD_WORKSPACE_DIRECTORY") + workspace_directory = DocsCliConfig().ws_root if workspace_directory: workspace_root = workspace_directory.resolve() if ( diff --git a/src/extensions/score_metamodel/__init__.py b/src/extensions/score_metamodel/__init__.py index 8a6a35777..75b210ac6 100644 --- a/src/extensions/score_metamodel/__init__.py +++ b/src/extensions/score_metamodel/__init__.py @@ -34,10 +34,10 @@ load_metamodel_data as load_metamodel_data, validate_mandatory_regexes as validate_mandatory_regexes, ) -from src.helper_lib import Environment, config_setdefault +from src.helper_lib import config_setdefault +from src.helper_lib.config import DocsCliConfig logger = logging.get_logger(__name__) -env = Environment() local_check_function = Callable[[Sphinx, NeedItem, CheckLogger], None] graph_check_function = Callable[[Sphinx, NeedsView, CheckLogger], None] @@ -110,7 +110,7 @@ def _run_checks(app: Sphinx) -> None: logger.debug(f"Running checks for {len(needs_all_needs)} needs") - ws_root = env.optional_path("BUILD_WORKSPACE_DIRECTORY") + ws_root = DocsCliConfig().ws_root cwd_or_ws_root = ws_root if ws_root else Path.cwd() prefix = str(Path(app.srcdir).relative_to(cwd_or_ws_root)) diff --git a/src/extensions/score_metamodel/docs/BUILD b/src/extensions/score_metamodel/docs/BUILD index f1e75b7af..49b2975d7 100644 --- a/src/extensions/score_metamodel/docs/BUILD +++ b/src/extensions/score_metamodel/docs/BUILD @@ -15,6 +15,9 @@ load("//:docs.bzl", "docs_bundle") load("@aspect_rules_py//py:defs.bzl", "py_binary") load("@docs_as_code_hub_env//:requirements.bzl", "all_requirements") +# TEMPORARY DIFF NOTE: The generator, YAML input, and declared outputs are all +# addressed from this genrule's execution root. ``execpath`` makes that action +# path contract explicit for both its executable and file arguments. genrule( name = "generate_metamodel_rst", srcs = [ @@ -24,7 +27,7 @@ genrule( "generated/index.rst", "generated/metamodel_classes.mmd", ], - cmd = "$(location :generate_metamodel_rst_bin) --rst-output $(location generated/index.rst) --mmd-output $(location generated/metamodel_classes.mmd) $(location //src/extensions/score_metamodel:metamodel_yaml)", + cmd = "$(execpath :generate_metamodel_rst_bin) --rst-output $(execpath generated/index.rst) --mmd-output $(execpath generated/metamodel_classes.mmd) $(execpath //src/extensions/score_metamodel:metamodel_yaml)", tools = [":generate_metamodel_rst_bin"], visibility = ["//visibility:private"], ) diff --git a/src/extensions/score_metamodel/external_needs.py b/src/extensions/score_metamodel/external_needs.py index cd89a6566..bba387312 100644 --- a/src/extensions/score_metamodel/external_needs.py +++ b/src/extensions/score_metamodel/external_needs.py @@ -21,7 +21,7 @@ from sphinx.util import logging from sphinx_needs.needsfile import NeedsList -from src.helper_lib import get_runfiles_dir +from src.helper_lib.config import DocsCliConfig logger = logging.getLogger(__name__) @@ -187,8 +187,12 @@ def add_external_needs_json(e: ExternalNeedsSource, config: Config): / "_build/needs/needs.json" ) - r = get_runfiles_dir() - json_file = r / json_file_raw + json_file = DocsCliConfig().resolve_input_path( + json_file_raw, runfiles_relative=True + ) + if json_file is None: + logger.error("Could not resolve external needs JSON runfile %s", json_file_raw) + return logger.debug(f"External needs.json: {json_file}") try: needs_json_data = json.loads(Path(json_file).read_text(encoding="utf-8")) # pyright: ignore[reportAny] @@ -215,11 +219,20 @@ def add_external_docs_sources(e: ExternalNeedsSource, config: Config): # The runfiles layout mirrors the original git layout: same-repo mounts live # under `_main/…`, cross-module mounts under `{e.bazel_module}+/…` # (see _runfiles_module_dir). - r = get_runfiles_dir() - if "ide_support.runfiles" in str(r): + cli_config = DocsCliConfig() + if cli_config.uses_ide_support_runfiles: logger.error("Combo builds are currently only supported with Bazel.") return - docs_source_path = Path(r) / _runfiles_module_dir(e) / e.path_to_target + docs_source_path = cli_config.resolve_input_path( + Path(_runfiles_module_dir(e)) / e.path_to_target, + runfiles_relative=True, + ) + if docs_source_path is None: + logger.error( + "Could not resolve external documentation source runfile %s", + Path(_runfiles_module_dir(e)) / e.path_to_target, + ) + return # A cross-module root mount keeps its module name as the collection key # (unchanged). Sub-package / same-repo mounts disambiguate via the path. @@ -274,8 +287,12 @@ def _add_needs_json_file(ext_needs: ExternalNeedsSource, config: Config) -> None json_file_raw = ( Path(_runfiles_module_dir(ext_needs)) / ext_needs.path_to_target / "needs.json" ) - r = get_runfiles_dir() - json_file = r / json_file_raw + json_file = DocsCliConfig().resolve_input_path( + json_file_raw, runfiles_relative=True + ) + if json_file is None: + logger.error("Could not resolve external needs JSON runfile %s", json_file_raw) + return logger.debug(f"External needs_json_file: {json_file}") try: needs_json_data = json.loads( diff --git a/src/extensions/score_metamodel/log.py b/src/extensions/score_metamodel/log.py index 52bf6a03a..801923a8f 100644 --- a/src/extensions/score_metamodel/log.py +++ b/src/extensions/score_metamodel/log.py @@ -18,12 +18,11 @@ from sphinx_needs.logging import SphinxLoggerAdapter from sphinx_needs.need_item import NeedItem -from src.helper_lib import Environment +from src.helper_lib.config import DocsCliConfig Location = str | tuple[str | None, int | None] | Node | None NewCheck = tuple[str, Location] logger = logging.get_logger(__name__) -env = Environment() class CheckLogger: @@ -39,9 +38,10 @@ def __init__( self._prefix = prefix self._new_checks: list[NewCheck] = [] self._compatibility = compatibility + cli_config = DocsCliConfig() + self._has_bazel_runfiles = cli_config.is_bazel_run or cli_config.is_bazel_build - @staticmethod - def _location(need: NeedItem, prefix: str): + def _location(self, need: NeedItem): def get(key: str) -> Any: return need.get(key, None) @@ -49,12 +49,10 @@ def get(key: str) -> Any: # Note: passing the location as a string allows us to use # readable relative paths, passing as a tuple results # in absolute paths to ~/.cache/.../bazel-out/.. - if env.optional_path("RUNFILES_DIR") or env.optional_path( - "RUNFILES_MANIFEST_FILE" - ): + if self._has_bazel_runfiles: matching_file = f"{need['docname']}{need['doctype']}" else: - matching_file = f"{prefix}/{need['docname']}{need['doctype']}" + matching_file = f"{self._prefix}/{need['docname']}{need['doctype']}" return f"{matching_file}:{need['lineno']}" return None @@ -63,7 +61,7 @@ def warning_for_option( self, need: NeedItem, option: str, msg: str, is_new_check: bool = False ): full_msg = f"{need['id']}.{option} ({need.get(option, None)}): {msg}" - location = CheckLogger._location(need, self._prefix) + location = self._location(need) self._log_message(full_msg, location, is_new_check, need) def warning_for_link( @@ -94,7 +92,7 @@ def warning_for_need( category: str = "need", ): full_msg = f"{need['id']}: {msg}" - location = CheckLogger._location(need, self._prefix) + location = self._location(need) self._log_message(full_msg, location, is_new_check, need, category) def _log_message( diff --git a/src/extensions/score_metamodel/tests/test_external_needs.py b/src/extensions/score_metamodel/tests/test_external_needs.py index 4e301e160..51d7ffc57 100644 --- a/src/extensions/score_metamodel/tests/test_external_needs.py +++ b/src/extensions/score_metamodel/tests/test_external_needs.py @@ -200,7 +200,11 @@ def test_add_external_needs_json_appends_entry( json.dumps({"project_url": "https://example.test/repo"}), encoding="utf-8" ) - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: runfiles_dir) + monkeypatch.setattr( + ext_needs.DocsCliConfig, + "resolve_input_path", + lambda _, path, *, runfiles_relative: runfiles_dir / path, + ) add_external_needs_json(e, config) @@ -234,7 +238,11 @@ def test_add_external_needs_json_appends_entry_local( json.dumps({"project_url": "https://example.test/local"}), encoding="utf-8" ) - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: runfiles_dir) + monkeypatch.setattr( + ext_needs.DocsCliConfig, + "resolve_input_path", + lambda _, path, *, runfiles_relative: runfiles_dir / path, + ) add_external_needs_json(e, config) @@ -261,7 +269,11 @@ def test_add_needs_json_file_appends_entry( config = Config() config.needs_external_needs = [] - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: runfiles_dir) + monkeypatch.setattr( + ext_needs.DocsCliConfig, + "resolve_input_path", + lambda _, path, *, runfiles_relative: runfiles_dir / path, + ) # Act e = ExternalNeedsSource( @@ -291,7 +303,11 @@ def test_add_external_needs_json_missing_file_keeps_list_empty( config = Config() config.needs_external_needs = [] - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: tmp_path) + monkeypatch.setattr( + ext_needs.DocsCliConfig, + "resolve_input_path", + lambda _, path, *, runfiles_relative: tmp_path / path, + ) add_external_needs_json(e, config) @@ -309,7 +325,11 @@ def test_add_external_docs_sources_adds_collection( config = Config() config.collections = {} - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: tmp_path) + monkeypatch.setattr( + ext_needs.DocsCliConfig, + "resolve_input_path", + lambda _, path, *, runfiles_relative: tmp_path / path, + ) add_external_docs_sources(e, config) @@ -340,7 +360,11 @@ def test_add_external_docs_sources_local_sub_package( config = Config() config.collections = {} - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: tmp_path) + monkeypatch.setattr( + ext_needs.DocsCliConfig, + "resolve_input_path", + lambda _, path, *, runfiles_relative: tmp_path / path, + ) add_external_docs_sources(e, config) @@ -373,7 +397,11 @@ def test_add_external_docs_sources_local_root_key_fallback( config = Config() config.collections = {} - monkeypatch.setattr(ext_needs, "get_runfiles_dir", lambda: tmp_path) + monkeypatch.setattr( + ext_needs.DocsCliConfig, + "resolve_input_path", + lambda _, path, *, runfiles_relative: tmp_path / path, + ) add_external_docs_sources(e, config) @@ -396,7 +424,9 @@ def test_add_external_docs_sources_ide_support_returns_without_changes( config.collections = {} monkeypatch.setattr( - ext_needs, "get_runfiles_dir", lambda: Path("/tmp/ide_support.runfiles") + ext_needs.DocsCliConfig, + "uses_ide_support_runfiles", + property(lambda _: True), ) add_external_docs_sources(e, config) diff --git a/src/extensions/score_mounts/__init__.py b/src/extensions/score_mounts/__init__.py index 72b1de106..9250803c6 100644 --- a/src/extensions/score_mounts/__init__.py +++ b/src/extensions/score_mounts/__init__.py @@ -46,38 +46,39 @@ resolve_source_files, resolve_walk_dir, ) -from src.helper_lib import Environment, find_ws_root, get_runfiles_dir +from src.helper_lib import Environment +from src.helper_lib.config import DocsCliConfig env = Environment() logger = logging.getLogger(__name__) -def _read_manifest(config: Config): +def _read_manifest(config: Config, cli_config: DocsCliConfig): """Locate and load the mounts manifest, or return ``None`` when unset. The manifest path is passed by Bazel either via the ``mounts_manifest`` config value or the ``MOUNTS`` env var. Its interpretation depends on the build - context: under ``bazel run`` it is a runfiles-relative path - (``$(rlocationpath)``) resolved against the runfiles dir; in a sandbox build - it is relative to the exec root (``$(location)``). Resolving the path here - keeps that context branch out of the pure ``_resolver`` module. + context: under ``bazel run`` it is a runfiles address; in a sandbox build + it is relative to the execution root. The launcher config resolves either + spelling before the pure ``_resolver`` module reads the manifest. """ raw = getattr(config, "mounts_manifest", None) or env.get("MOUNTS_MANIFEST", "") if not raw or not raw.strip() or not isinstance(raw, str): return None - # ``bazel run`` passes an rlocation-relative path; ``sphinx_docs`` in a - # sandbox passes its execroot-relative ``$(location)`` path directly. - manifest_path = get_runfiles_dir() / raw if find_ws_root() else Path(raw) + # ``bazel run`` passes a runfiles address; ``sphinx_docs`` in a sandbox + # passes the declared input's execution-root-relative path directly. + manifest_path = cli_config.resolve_input_path(Path(raw)) + if manifest_path is None: + raise ValueError(f"Could not resolve mounts manifest: {raw}") return load_mounts_manifest(manifest_path) def _resolve_data_mounts( manifest: MountsManifest, - ws_root: Path | None, - runfiles_dir: Path | None, + cli_config: DocsCliConfig, ) -> dict[str, MountSpec]: """Resolve data file mounts from the manifest. @@ -87,20 +88,7 @@ def _resolve_data_mounts( data_mounts: dict[str, MountSpec] = {} for spec in manifest.mounts: for data_file in spec.data: - if ws_root is not None and runfiles_dir is not None: - runfiles_str = str(runfiles_dir) - if "/bazel-out/" in runfiles_str: - # Execroot = runfiles path before the first /bazel-out/ occurrence - # e.g. runfiles=execroot/_main/bazel-out/... => execroot=execroot/_main - walk_file = Path(runfiles_str.split("/bazel-out/")[0]) / data_file - else: - walk_file = ( - ws_root - / "bazel-bin" - / data_file.removeprefix("bazel-out/k8-fastbuild/bin/") - ) - else: - walk_file = Path.cwd() / data_file + walk_file = cli_config.resolve_bazel_output_path(data_file) if not walk_file.is_file(): raise ValueError( "score_mounts: resolved data file does not exist: " @@ -286,8 +274,7 @@ def _exclude_mounted_primary_sources( def _resolve_source_mounts( manifest: MountsManifest, - ws_root: Path | None, - runfiles_dir: Path | None, + cli_config: DocsCliConfig, ) -> list[tuple[MountSpec, Path]]: """Resolve and validate the directory mounts used for ownership checks. @@ -302,7 +289,7 @@ def _resolve_source_mounts( for spec in manifest.mounts: if not spec.src_root or spec.files: continue - walk_dir = resolve_walk_dir(manifest, spec, ws_root, runfiles_dir) + walk_dir = resolve_walk_dir(spec, cli_config) if not walk_dir.is_dir(): raise ValueError( "score_mounts: resolved mount dir does not exist: " @@ -321,13 +308,11 @@ def _on_config_inited(app: Sphinx, config: Config) -> None: walks, and writes the assembled list to ``config.mounts``. A missing or empty manifest is a no-op. """ - manifest = _read_manifest(config) + cli_config = DocsCliConfig() + manifest = _read_manifest(config, cli_config) if manifest is None or not manifest.mounts: return - ws_root = find_ws_root() - runfiles_dir = get_runfiles_dir() if ws_root is not None else None - # In every context sphinx_mounts reads the bundle's original files (no copy # is made); directory mounts are walked while explicit source mounts use # their declared file list. Only where those files are staged differs: @@ -343,7 +328,7 @@ def _on_config_inited(app: Sphinx, config: Config) -> None: # Directory mounts need to be resolved as a group before runtime entries are # assembled. Only then can their physical roots be compared for nesting and # can both Sphinx's primary walk and each parent mount be given exclusions. - source_mounts = _resolve_source_mounts(manifest, ws_root, runfiles_dir) + source_mounts = _resolve_source_mounts(manifest, cli_config) primary_exclusions, nested_exclusions = _mount_exclusions( Path(app.srcdir).resolve(), source_mounts ) @@ -369,7 +354,7 @@ def _on_config_inited(app: Sphinx, config: Config) -> None: if spec.files: # Explicit source bundles use sphinx-mounts' file-list mode so the # original files are read directly without discovering siblings. - source_files = resolve_source_files(manifest, spec, ws_root, runfiles_dir) + source_files = resolve_source_files(spec, cli_config) source_suffixes = _configured_source_suffixes(config) document_files = [ source_file @@ -402,10 +387,10 @@ def _on_config_inited(app: Sphinx, config: Config) -> None: # Resolve data (e.g. genrule outputs in bazel-out). # Data paths are execroot-relative (e.g. bazel-out/.../bin/src/.../index.rst). - # During bazel run: compute execroot from RUNFILES_DIR; during sandboxed build: + # During bazel run: compute execroot from the runfiles tree; in a sandboxed build: # cwd IS the execroot. # Only the parent directories of resolved files are added to mounts. - data_mounts = _resolve_data_mounts(manifest, ws_root, runfiles_dir) + data_mounts = _resolve_data_mounts(manifest, cli_config) for walk_dir_str, spec in data_mounts.items(): config.mounts.append(_make_mount_entry(Path(walk_dir_str), spec)) logger.info("score_mounts: added %d data mount(s)", len(data_mounts)) diff --git a/src/extensions/score_mounts/_resolver.py b/src/extensions/score_mounts/_resolver.py index a769578b9..06fe510cb 100644 --- a/src/extensions/score_mounts/_resolver.py +++ b/src/extensions/score_mounts/_resolver.py @@ -21,11 +21,12 @@ from __future__ import annotations import json -import os from dataclasses import dataclass, field from pathlib import Path from typing import cast +from src.helper_lib.config import DocsCliConfig + @dataclass(frozen=True) class MountSpec: @@ -110,16 +111,16 @@ def load_mounts_manifest(manifest_path: str | Path) -> MountsManifest: def resolve_walk_dir( - manifest: MountsManifest, spec: MountSpec, - ws_root: Path | None, - runfiles_dir: Path | None = None, + cli_config: DocsCliConfig, ) -> Path: - """Resolve a mount directory for either ``bazel run`` or a sandbox build. + """Resolve a mount directory through the launcher execution config. Generated source roots are recorded with their execroot-relative bazel-out path, while ``bazel run`` exposes the same artifacts below ``bazel-bin`` in - the workspace. The ``generated`` flag selects that translation. + the workspace. The ``generated`` flag selects that translation. External + source roots use their runfiles address under ``bazel run``; all other source + roots are relative to the visible workspace or current execution root. For example, a generated ``bazel-out/k8-fastbuild/bin/pkg/docs`` root resolves to ``/bazel-bin/pkg/docs`` under ``bazel run`` and to @@ -128,37 +129,27 @@ def resolve_walk_dir( and ``/`` in a sandbox. """ if spec.generated: - if ws_root is not None: - # Generated source files are exposed through bazel-bin at runtime, - # while their manifest paths are execroot-relative bazel-out paths. - output_parts = spec.src_root.split("/") - if ( - # A generated file may be directly below the configuration's - # ``bin`` directory, so the source root itself can end there. - len(output_parts) >= 3 - and output_parts[0] == "bazel-out" - and output_parts[2] == "bin" - ): - return ws_root / "bazel-bin" / "/".join(output_parts[3:]) - return ws_root / spec.src_root - return Path.cwd() / spec.src_root - if spec.external and ws_root is not None: - if runfiles_dir is None: - raise ValueError("external mounts under bazel run require RUNFILES_DIR") + return cli_config.resolve_bazel_output_path(spec.src_root) + + if spec.external and cli_config.is_bazel_run: # External short paths begin with ``../+`` relative to the - # runfiles ``_main`` directory, not relative to a manifest nested in a - # Bazel package. Prefixing ``_main`` preserves that Bazel convention. - return Path(os.path.abspath(runfiles_dir / "_main" / spec.runtime_path)) - if ws_root is not None: - return ws_root / spec.src_root - return Path.cwd() / spec.src_root + # runfiles ``_main`` directory, not relative to the manifest package. + # This branch is specific to bazel run, where relative inputs already + # resolve through runfiles in DocsCliConfig. + runfile_path = cli_config.resolve_input_path(Path("_main") / spec.runtime_path) + if runfile_path is None: + raise ValueError( + "score_mounts: cannot resolve external mount from runfiles: " + f"{spec.runtime_path} (mount_at={spec.mount_at})" + ) + return runfile_path + + return (cli_config.ws_root or Path.cwd()) / spec.src_root def resolve_source_files( - manifest: MountsManifest, spec: MountSpec, - ws_root: Path | None, - runfiles_dir: Path | None = None, + cli_config: DocsCliConfig, ) -> list[Path]: """Resolve an explicit source allowlist below its original parent. @@ -166,7 +157,7 @@ def resolve_source_files( mounts. The manifest's relative file names then identify only the Bazel artifacts declared by ``docs_bundle(srcs = [...])``. """ - walk_dir = resolve_walk_dir(manifest, spec, ws_root, runfiles_dir) + walk_dir = resolve_walk_dir(spec, cli_config) resolved_files: list[Path] = [] for relative_path in spec.files: source_file = walk_dir / relative_path diff --git a/src/extensions/score_mounts/tests/test_data_mounts.py b/src/extensions/score_mounts/tests/test_data_mounts.py index b5a16a9b0..5495efe08 100644 --- a/src/extensions/score_mounts/tests/test_data_mounts.py +++ b/src/extensions/score_mounts/tests/test_data_mounts.py @@ -13,6 +13,7 @@ """Tests for ``_resolve_data_mounts`` in the ``score_mounts`` extension.""" from pathlib import Path +from unittest.mock import Mock import pytest @@ -36,8 +37,11 @@ def test_missing_data_file_raises(tmp_path: Path) -> None: ] ) + cli_config = Mock() + cli_config.resolve_bazel_output_path.side_effect = lambda path: tmp_path / path + with pytest.raises(ValueError, match="resolved data file does not exist"): - _resolve_data_mounts(manifest, tmp_path, tmp_path) + _resolve_data_mounts(manifest, cli_config) def test_existing_data_file_resolved(tmp_path: Path) -> None: @@ -57,7 +61,11 @@ def test_existing_data_file_resolved(tmp_path: Path) -> None: ] ) - mounts = _resolve_data_mounts(manifest, tmp_path, tmp_path / "runfiles") + cli_config = Mock() + cli_config.resolve_bazel_output_path.side_effect = lambda path: ( + tmp_path / "bazel-bin" / Path(path).name + ) + mounts = _resolve_data_mounts(manifest, cli_config) assert str(tmp_path / "bazel-bin") in mounts diff --git a/src/extensions/score_mounts/tests/test_resolver.py b/src/extensions/score_mounts/tests/test_resolver.py index 326942fc3..25801b7b3 100644 --- a/src/extensions/score_mounts/tests/test_resolver.py +++ b/src/extensions/score_mounts/tests/test_resolver.py @@ -18,6 +18,7 @@ import json from pathlib import Path +from unittest.mock import Mock import pytest @@ -138,10 +139,17 @@ def test_external_mount_uses_execroot_path_in_sandbox( }, ) spec = load_mounts_manifest(manifest).mounts[0] - assert resolve_walk_dir(load_mounts_manifest(manifest), spec, None) == ( + cli_config = Mock(is_bazel_run=False) + cli_config.ws_root = tmp_path + cli_config.resolve_input_path.return_value = ( tmp_path / "external" / "score_process_description+" / "docs_as_mount" ) + assert resolve_walk_dir(spec, cli_config) == ( + tmp_path / "external" / "score_process_description+" / "docs_as_mount" + ) + assert cli_config.ws_root == tmp_path + def test_external_mount_uses_runfiles_root_under_bazel_run(tmp_path: Path) -> None: manifest = _write_manifest( @@ -158,12 +166,17 @@ def test_external_mount_uses_runfiles_root_under_bazel_run(tmp_path: Path) -> No }, ) spec = load_mounts_manifest(manifest).mounts[0] - assert resolve_walk_dir( - load_mounts_manifest(manifest), - spec, - tmp_path / "workspace", - tmp_path, - ) == (tmp_path / "score_process_description+" / "docs_as_mount") + cli_config = Mock(is_bazel_run=True) + cli_config.resolve_input_path.return_value = ( + tmp_path / "score_process_description+" / "docs_as_mount" + ) + + assert resolve_walk_dir(spec, cli_config) == ( + tmp_path / "score_process_description+" / "docs_as_mount" + ) + cli_config.resolve_input_path.assert_called_once_with( + Path("_main") / spec.runtime_path + ) def test_generated_source_mount_uses_bazel_bin_under_bazel_run(tmp_path: Path) -> None: @@ -183,16 +196,16 @@ def test_generated_source_mount_uses_bazel_bin_under_bazel_run(tmp_path: Path) - ) spec = load_mounts_manifest(manifest).mounts[0] - assert ( - resolve_walk_dir( - load_mounts_manifest(manifest), - spec, - tmp_path / "workspace", - tmp_path / "workspace" / "docs.runfiles", - ) - == tmp_path / "workspace" / "bazel-bin" / "pkg" / "generated" + cli_config = Mock() + cli_config.resolve_bazel_output_path.return_value = ( + tmp_path / "workspace" / "bazel-bin" / "pkg" / "generated" ) + assert resolve_walk_dir(spec, cli_config) == ( + tmp_path / "workspace" / "bazel-bin" / "pkg" / "generated" + ) + cli_config.resolve_bazel_output_path.assert_called_once_with(spec.src_root) + def test_generated_root_source_mount_uses_bazel_bin_under_bazel_run( tmp_path: Path, @@ -213,10 +226,10 @@ def test_generated_root_source_mount_uses_bazel_bin_under_bazel_run( ) spec = load_mounts_manifest(manifest).mounts[0] - assert ( - resolve_walk_dir(load_mounts_manifest(manifest), spec, tmp_path / "workspace") - == tmp_path / "workspace" / "bazel-bin" - ) + cli_config = Mock() + cli_config.resolve_bazel_output_path.return_value = tmp_path / "workspace/bazel-bin" + + assert resolve_walk_dir(spec, cli_config) == tmp_path / "workspace/bazel-bin" def test_explicit_source_files_resolve_below_original_root(tmp_path: Path) -> None: @@ -240,11 +253,13 @@ def test_explicit_source_files_resolve_below_original_root(tmp_path: Path) -> No ) spec = load_mounts_manifest(manifest).mounts[0] - assert resolve_source_files( - load_mounts_manifest(manifest), - spec, - tmp_path / "workspace", - ) == [source_root / "index.rst", source_root / "guide.rst"] + cli_config = Mock() + cli_config.ws_root = source_root.parent + + assert resolve_source_files(spec, cli_config) == [ + source_root / "index.rst", + source_root / "guide.rst", + ] def test_generated_source_mount_uses_execroot_in_sandbox( @@ -267,6 +282,11 @@ def test_generated_source_mount_uses_execroot_in_sandbox( ) spec = load_mounts_manifest(manifest).mounts[0] - assert resolve_walk_dir(load_mounts_manifest(manifest), spec, None) == ( + cli_config = Mock() + cli_config.resolve_bazel_output_path.return_value = ( + tmp_path / "bazel-out" / "k8-fastbuild" / "bin" / "pkg" / "generated" + ) + + assert resolve_walk_dir(spec, cli_config) == ( tmp_path / "bazel-out" / "k8-fastbuild" / "bin" / "pkg" / "generated" ) diff --git a/src/extensions/score_plantuml.py b/src/extensions/score_plantuml.py index 43211c211..5dbafa96c 100644 --- a/src/extensions/score_plantuml.py +++ b/src/extensions/score_plantuml.py @@ -33,7 +33,8 @@ from sphinx.application import Sphinx from sphinx.util import logging -from src.helper_lib import config_setdefault, get_runfiles_dir +from src.helper_lib import config_setdefault +from src.helper_lib.config import DocsCliConfig logger = logging.getLogger(__name__) @@ -72,22 +73,22 @@ def use_document_source_as_plantuml_cwd( node["incdir"] = str(source.parent) -def find_correct_path(runfiles: Path) -> Path: +def find_correct_path(cli_config: DocsCliConfig) -> Path: """ This ensures that the 'plantuml' binary path is found in local 'score_docs_as_code' and module use. """ - if (Path(runfiles) / "score_docs_as_code+").exists(): - # Docs-as-code used as a module with bazel 8 - module = "score_docs_as_code+" - elif (Path(runfiles) / "score_docs_as_code~").exists(): - # Docs-as-code used as a module with bazel 7 - module = "score_docs_as_code~" - else: - # Docs-as-code is the current module - module = "_main" + # Bazel 8 and 7 use different canonical names for a module repository; + # local workspace use places this module under ``_main``. + for module in ("score_docs_as_code+", "score_docs_as_code~", "_main"): + plantuml_path = cli_config.resolve_input_path( + Path(module) / "src" / "plantuml", + runfiles_relative=True, + ) + if plantuml_path is not None and plantuml_path.exists(): + return plantuml_path - return runfiles / module / "src" / "plantuml" + raise FileNotFoundError("Could not locate PlantUML files in Bazel runfiles") def check_graphviz(app: Sphinx) -> None: @@ -116,7 +117,7 @@ def check_graphviz(app: Sphinx) -> None: def setup(app: Sphinx): # we must overwrite the plantuml path due to Bazel - app.config.plantuml = str(find_correct_path(get_runfiles_dir())) + app.config.plantuml = str(find_correct_path(DocsCliConfig())) config_setdefault(app.config, "plantuml_output_format", "svg_obj") config_setdefault(app.config, "plantuml_syntax_error_image", True) config_setdefault(app.config, "needs_build_needumls", "_plantuml_sources") diff --git a/src/extensions/score_source_code_linker/__init__.py b/src/extensions/score_source_code_linker/__init__.py index 6cab5c507..c5fe5e10e 100644 --- a/src/extensions/score_source_code_linker/__init__.py +++ b/src/extensions/score_source_code_linker/__init__.py @@ -59,7 +59,8 @@ construct_and_add_need, run_xml_parser, ) -from src.helper_lib import Environment, find_ws_root +from src.helper_lib import Environment +from src.helper_lib.config import DocsCliConfig env = Environment() @@ -93,7 +94,15 @@ def build_and_save_combined_file(outdir: Path, app: Sphinx | None = None): getattr(app.config, "score_sourcelinks_json", "") or "" ).strip() if source_code_links_path: - source_code_links_json = Path(source_code_links_path) + raw_source_code_links_json = Path(source_code_links_path) + source_code_links_json = DocsCliConfig().resolve_input_path( + raw_source_code_links_json + ) + if source_code_links_json is None: + raise FileNotFoundError( + "Could not resolve pre-generated source-code links file: " + f"{raw_source_code_links_json}" + ) try: source_code_links = load_source_code_links_json(source_code_links_json) except FileNotFoundError as exc: @@ -174,14 +183,14 @@ def register_test_code_linker(app: Sphinx): app.connect("env-updated", setup_test_code_linker, priority=505) -def setup_test_code_linker(app: Sphinx, env: BuildEnvironment): +def setup_test_code_linker(app: Sphinx, build_env: BuildEnvironment): # TODO instead of implementing our own caching here, we should rely on Bazel tl_cache_json = get_cache_filename(app.outdir, "score_xml_parser_cache.json") if ( not tl_cache_json.exists() or not app.config.skip_rescanning_via_source_code_linker ): - ws_root = find_ws_root() + ws_root = DocsCliConfig().ws_root if not ws_root: return LOGGER.debug( @@ -204,7 +213,7 @@ def setup_test_code_linker(app: Sphinx, env: BuildEnvironment): LOGGER.info(f"{'=' * 80}", type="score_source_code_linker") return - run_xml_parser(app, env) + run_xml_parser(app, build_env) return tcn_cache = get_cache_filename(app.outdir, "score_testcaseneeds_cache.json") assert tcn_cache.exists(), ( diff --git a/src/extensions/score_source_code_linker/needlinks.py b/src/extensions/score_source_code_linker/needlinks.py index 4b8c616d6..144c1d0d4 100644 --- a/src/extensions/score_source_code_linker/needlinks.py +++ b/src/extensions/score_source_code_linker/needlinks.py @@ -17,9 +17,7 @@ from pathlib import Path from typing import Any, TypedDict, TypeGuard -from src.helper_lib import Environment - -env = Environment() +from src.helper_lib.config import DocsCliConfig class MetaData(TypedDict): @@ -183,7 +181,7 @@ def load_source_code_links_with_metadata_json(file: Path) -> list[NeedLink]: This normally should be the one called 'locally' => :docs target """ if not file.is_absolute(): - ws_root = env.get("BUILD_WORKSPACE_DIRECTORY", "") + ws_root = DocsCliConfig().ws_root if ws_root: file = Path(ws_root) / file @@ -224,7 +222,7 @@ def load_source_code_links_json(file: Path) -> list[NeedLink]: """ if not file.is_absolute(): # use env variable set by Bazel - ws_root = env.optional_path("BUILD_WORKSPACE_DIRECTORY") + ws_root = DocsCliConfig().ws_root if ws_root: file = ws_root / file diff --git a/src/extensions/score_source_code_linker/tests/test_codelink.py b/src/extensions/score_source_code_linker/tests/test_codelink.py index 8b3d9bf61..2ead5769a 100644 --- a/src/extensions/score_source_code_linker/tests/test_codelink.py +++ b/src/extensions/score_source_code_linker/tests/test_codelink.py @@ -58,6 +58,7 @@ from src.helper_lib import ( get_current_git_hash, ) +from src.helper_lib.config import DocsCliConfig def test_need(**kwargs: Any) -> NeedItem: @@ -370,6 +371,25 @@ def test_combining_without_source_links_continues_with_empty_code_links( assert json.loads(grouped_cache.read_text(encoding="utf-8")) == [] +def test_combining_resolves_source_links_runfile( + temp_dir: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Resolve Bazel's logical runfiles address before opening the JSON input.""" + source_links = temp_dir / "source_links.json" + source_links.write_text("[]", encoding="utf-8") + monkeypatch.setenv("SCORE_SOURCELINKS", "_main/generated/source_links.json") + monkeypatch.setattr( + DocsCliConfig, + "resolve_input_path", + lambda _self, _path: source_links, + ) + + build_and_save_combined_file(temp_dir) + + grouped_cache = temp_dir / "score_scl_grouped_cache.json" + assert json.loads(grouped_cache.read_text(encoding="utf-8")) == [] + + def test_combining_with_missing_source_links_reports_configured_path( temp_dir: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/src/extensions/score_source_code_linker/tests/test_xml_parser.py b/src/extensions/score_source_code_linker/tests/test_xml_parser.py index bd1d1e5e5..e4789bab4 100644 --- a/src/extensions/score_source_code_linker/tests/test_xml_parser.py +++ b/src/extensions/score_source_code_linker/tests/test_xml_parser.py @@ -35,6 +35,7 @@ import src.extensions.score_source_code_linker.xml_parser as xml_parser from src.extensions.score_source_code_linker.testlink import DataOfTestCase +from src.helper_lib.config import DocsCliConfig # Unsure if I should make these last a session or not @@ -584,6 +585,26 @@ def test_get_metadata_from_test_path_combo_with_hash( assert md["url"] == "https://github.com/eclipse-score/docs-as-code" +def test_get_metadata_from_test_path_resolves_known_good_runfile( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + """Resolve the Bazel runfiles address before reading known-good metadata.""" + json_file = tmp_path / "known_good.json" + json_file.write_text(json.dumps(_KNOWN_GOOD_WITH_HASH), encoding="utf-8") + monkeypatch.setenv("KNOWN_GOOD_JSON", "_main/config/known_good.json") + monkeypatch.setattr( + DocsCliConfig, + "resolve_input_path", + lambda _self, _path: json_file, + ) + + md = xml_parser.get_metadata_from_test_path(_COMBO_TEST_PATH) + + assert md["repo_name"] == "score_docs_as_code" + assert md["hash"] == "abc123hashvalue" + assert md["url"] == "https://github.com/eclipse-score/docs-as-code" + + def test_get_metadata_from_test_path_combo_with_version( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ): diff --git a/src/extensions/score_source_code_linker/xml_parser.py b/src/extensions/score_source_code_linker/xml_parser.py index fde71ee7b..abd875f5a 100644 --- a/src/extensions/score_source_code_linker/xml_parser.py +++ b/src/extensions/score_source_code_linker/xml_parser.py @@ -50,6 +50,7 @@ store_test_xml_parsed_json, ) from src.helper_lib import Environment, find_ws_root +from src.helper_lib.config import DocsCliConfig env = Environment() @@ -147,6 +148,12 @@ def get_metadata_from_test_path(raw_filepath: Path) -> MetaData: """ # print("THIs IS FILEPATH IN GET MD FROm TestPATH: ", raw_filepath) known_good_json = env.optional_path("KNOWN_GOOD_JSON") + if known_good_json is not None: + # docs.bzl supplies this file as a runfiles address to interactive + # Bazel targets; resolve it before the JSON reader opens the file. + known_good_json = DocsCliConfig().resolve_input_path(known_good_json) + if known_good_json is None: + raise FileNotFoundError("Could not resolve KNOWN_GOOD_JSON runfile") clean_filepath = clean_test_file_name(raw_filepath) # print(f"This is the cleaned filepath: {clean_filepath}") repo_name = parse_repo_name_from_path(clean_filepath) diff --git a/src/extensions/score_sync_toml/__init__.py b/src/extensions/score_sync_toml/__init__.py index cf868ba1a..dc743fe90 100644 --- a/src/extensions/score_sync_toml/__init__.py +++ b/src/extensions/score_sync_toml/__init__.py @@ -15,7 +15,8 @@ from sphinx.application import Sphinx from src.extensions.score_sync_toml._mounts import register_mounts -from src.helper_lib import config_setdefault, find_git_root +from src.helper_lib import config_setdefault +from src.helper_lib.config import DocsCliConfig def setup(app: Sphinx) -> dict[str, str | bool]: @@ -28,7 +29,7 @@ def setup(app: Sphinx) -> dict[str, str | bool]: # A Bazel build action has no Git worktree. In that context this extension # must be inactive: writing a fallback file into the sandbox is useless and # can make generated configuration appear to work when it is discarded. - git_root = find_git_root() + git_root = DocsCliConfig().git_root if git_root is None: app.config.suppress_warnings += [ "needs_config_writer.unsupported_type", diff --git a/src/extensions/score_sync_toml/_mounts.py b/src/extensions/score_sync_toml/_mounts.py index ab6ae9253..403ae1dba 100644 --- a/src/extensions/score_sync_toml/_mounts.py +++ b/src/extensions/score_sync_toml/_mounts.py @@ -13,41 +13,41 @@ from sphinx.config import Config -from src.helper_lib import find_git_root, get_runfiles_dir +from src.helper_lib.config import DocsCliConfig def _toml_string(value: str) -> str: return '"' + value.replace("\\", "\\\\").replace('"', '\\"') + '"' -def _toml_path(path: Path) -> str: +def _toml_path(path: Path, cli_config: DocsCliConfig) -> str: """Derive a stable TOML path from a resolved runtime path.""" resolved_path = path.resolve() - git_root = find_git_root() + git_root = cli_config.git_root if git_root is not None: try: return str(resolved_path.relative_to(git_root)) except ValueError: pass - try: - external_path = resolved_path.relative_to(get_runfiles_dir()) - except ValueError: + runfiles_path = cli_config.relative_to_runfiles(resolved_path) + if runfiles_path is None or not runfiles_path.parts: return str(resolved_path) - if external_path.parts[0] == "_main": - return str(Path(*external_path.parts[1:])) - return "bazel-bin/external/" + str(external_path) + if runfiles_path.parts[0] == "_main": + return str(Path(*runfiles_path.parts[1:])) + return "bazel-bin/external/" + str(runfiles_path) -def _toml_dir(entry: dict[str, Any]) -> str: +def _toml_dir(entry: dict[str, Any], cli_config: DocsCliConfig) -> str: """Derive a stable TOML directory from a resolved mount entry.""" - return _toml_path(Path(entry["dir"])) + return _toml_path(Path(entry["dir"]), cli_config) def materialize_mounts(entries: list[dict[str, Any]]) -> Path | None: """Write resolved mounts as a temporary, Git-root-relative TOML merge file.""" if not entries: return None + cli_config = DocsCliConfig() lines: list[str] = [] for entry in entries: source_files = entry.get("files", []) @@ -60,12 +60,12 @@ def materialize_mounts(entries: list[dict[str, Any]]) -> Path | None: # Preserve explicit source mounts as a file allowlist in the # generated TOML instead of widening them back to a directory. files = ", ".join( - _toml_string(_toml_path(Path(source_file))) + _toml_string(_toml_path(Path(source_file), cli_config)) for source_file in source_files ) lines.append(f"files = [{files}]") else: - lines.append(f"dir = {_toml_string(_toml_dir(entry))}") + lines.append(f"dir = {_toml_string(_toml_dir(entry, cli_config))}") lines.append(f"mount_at = {_toml_string(entry['mount_at'])}") if entry.get("attach_to"): lines.append(f"attach_to = {_toml_string(entry['attach_to'])}") diff --git a/src/extensions/score_sync_toml/test_mounts.py b/src/extensions/score_sync_toml/test_mounts.py index d718b0b8a..12010641e 100644 --- a/src/extensions/score_sync_toml/test_mounts.py +++ b/src/extensions/score_sync_toml/test_mounts.py @@ -12,6 +12,7 @@ from src.extensions import score_sync_toml from src.extensions.score_sync_toml import _mounts from src.extensions.score_sync_toml._mounts import materialize_mounts +from src.helper_lib import config as config_module def test_materialize_mounts_serializes_structured_entries(): @@ -54,8 +55,12 @@ def test_materialize_mounts_maps_external_runfiles_path_to_bazel_bin( runfiles_dir = tmp_path / "runfiles" walk_dir = runfiles_dir / "score_process_description+" / "process" walk_dir.mkdir(parents=True) - monkeypatch.setattr(_mounts, "find_git_root", lambda: None) - monkeypatch.setattr(_mounts, "get_runfiles_dir", lambda: runfiles_dir) + monkeypatch.setattr(config_module, "find_git_root", lambda: None) + monkeypatch.setattr( + _mounts.DocsCliConfig, + "relative_to_runfiles", + lambda _, path: path.relative_to(runfiles_dir), + ) fragment = materialize_mounts( [ @@ -78,7 +83,11 @@ def test_materialize_mounts_preserves_explicit_source_files( """Serialize explicit mounts as files instead of widening them to dirs.""" git_root = tmp_path / "workspace" git_root.mkdir() - monkeypatch.setattr(_mounts, "find_git_root", lambda: git_root) + monkeypatch.setattr( + _mounts.DocsCliConfig, + "git_root", + property(lambda _: git_root), + ) fragment = materialize_mounts( [ @@ -120,7 +129,7 @@ def connect(self, *args: Any, **kwargs: Any) -> None: "setup must not register TOML sync without a Git worktree" ) - monkeypatch.setattr(score_sync_toml, "find_git_root", lambda: None) + monkeypatch.setattr(config_module, "find_git_root", lambda: None) metadata = score_sync_toml.setup(cast(Sphinx, AppWithoutGitWorktree())) diff --git a/src/helper_lib/config.py b/src/helper_lib/config.py index 63ab1a172..286577b87 100644 --- a/src/helper_lib/config.py +++ b/src/helper_lib/config.py @@ -12,6 +12,7 @@ # ******************************************************************************* import logging +import os from enum import Enum from functools import cached_property from pathlib import Path @@ -34,9 +35,9 @@ class DocsCliConfig: Keeping environment parsing in one place lets the launcher operate on a stable configuration object. Paths stored on this object are resolved to - the filesystem visible to the current process. The logical package and - source paths remain available for repository metadata such as GitHub edit - links. + the filesystem visible to the current process. Paths inside the Sphinx + source tree belong to Sphinx itself; this config handles launcher paths, + including inputs located through Bazel runfiles. """ def _identify_environment(self) -> ExecutionEnvironment: @@ -62,22 +63,26 @@ def is_direct(self): """Whether the launcher was started outside Bazel.""" return self.environment == ExecutionEnvironment.DIRECT - @classmethod - def from_environment(cls, env: Environment | None = None) -> "DocsCliConfig": - """Load configuration from the process environment or a test mapping.""" - return cls(env if env is not None else Environment()) + @property + def uses_ide_support_runfiles(self) -> bool: + """Whether direct invocation found the dedicated IDE runfiles tree.""" + return ( + self.is_direct + and self._runfiles_dir is not None + and self._runfiles_dir.name == "ide_support.runfiles" + ) - def __init__(self, env: Environment): + def __init__(self, env: Environment | None = None): """ Load launcher configuration from the current Bazel environment. Specifically, this method handles bazel build and run differences. """ - self._env = env + self._env = env if env is not None else Environment() # These three must be queried first: - self.ws_root = env.optional_path("BUILD_WORKSPACE_DIRECTORY") + self.ws_root = self._env.optional_path("BUILD_WORKSPACE_DIRECTORY") self._runfiles = Runfiles.Create() self.environment = self._identify_environment() @@ -85,18 +90,21 @@ def __init__(self, env: Environment): if self.ws_root: self._require_directory(self.ws_root, "BUILD_WORKSPACE_DIRECTORY") - self.action = env.get("ACTION") - logger.debug( - "Resolved documentation paths: environment=%s, cwd=%s, " - "package_dir=%s, source_directory=%s, output_dir=%s", + "Resolved documentation environment: environment=%s, cwd=%s, " + "workspace_root=%s, action=%s", self.environment.value, Path.cwd(), - self.package_dir, - self.source_dir_relative_to_ws, - self.output_dir, + self.ws_root, + self.action, ) + @cached_property + def action(self) -> str | None: + # Environment.get uses an empty string as its optional default; expose + # a missing or empty action as None to distinguish it from real actions. + return self._env.get("ACTION", "") or None + @cached_property def git_root(self) -> Path | None: """Return the Git root when it is visible from the current process.""" @@ -146,46 +154,118 @@ def output_dir(self) -> Path: return self.package_dir / "_build" @cached_property - def source_dir(self) -> Path: - """Return the source directory in the current execution context.""" - # SOURCE_DIRECTORY is relative in every mode. package_dir is empty for - # build and direct modes because those paths are already relative to - # their execution directory; bazel run adds its workspace package - # prefix here. - source_dir_relative = self._env.required_path("SOURCE_DIRECTORY") - assert not source_dir_relative.is_absolute() - return self.package_dir / source_dir_relative + def _runfiles_dir(self) -> Path | None: + """Return the runfiles tree when one is available to this process.""" + if self._runfiles: + # RUNFILES_DIR is optional when Bazel uses a manifest-only layout; + # resolve_input_path can still use the runfiles library in that case. + runfiles_dir = Environment(self._runfiles.EnvVars()).optional_path( + "RUNFILES_DIR" + ) + if runfiles_dir is not None: + return runfiles_dir - @cached_property - def source_dir_relative_to_ws(self) -> Path: - """Return the source directory relative to the workspace when running.""" - if self.is_bazel_run: - # bazel run has an absolute workspace root, so remove that prefix - # to produce the workspace-relative path used by GitHub edit links. - assert self.ws_root - return self.source_dir.relative_to(self.ws_root) + if self.is_direct: + # IDE builds run outside Bazel but still consume the runfiles tree + # produced by the dedicated ide_support target. + if self.git_root is not None: + ide_runfiles_dir = self.git_root / "bazel-bin" / "ide_support.runfiles" + if ide_runfiles_dir.is_dir(): + return ide_runfiles_dir + else: + return None else: - # Build actions intentionally have no workspace root in their - # sandbox, and direct invocations use cwd as their path base. Their - # source path is therefore already in the most useful relative - # form available here. - assert not self.ws_root - return self.source_dir - - def _resolve_input_path(self, path: Path) -> Path | None: + # A sandboxed process can resolve runfiles through its manifest + # even when no directory-form runfiles root is exposed. + return None + + return None + + def _resolve_runfiles_path(self, path: Path | str) -> Path | None: + """Resolve a runfiles-relative input through Bazel or the IDE runfiles tree. + + ``Rlocation`` supports manifest-only runfiles layouts. Direct IDE + invocations do not have a runfiles library instance, so they use the + runfiles directory discovered from the local ``ide_support`` target. """ - Resolve an optional config input in its current execution context. + path = Path(path) + if path.is_absolute(): + return path + + # Bazel's external repository paths can be expressed as + # ``_main/..//...``. Normalize that intentional parent + # traversal before asking the runfiles library to resolve the key. + normalized_path = Path(os.path.normpath(path.as_posix())) + if normalized_path.parts and normalized_path.parts[0] == "..": + return None + + if self._runfiles: + location = self._runfiles.Rlocation(normalized_path.as_posix()) + if location: + return Path(location).absolute() + + if self._runfiles_dir is not None: + return Path(os.path.abspath(self._runfiles_dir / normalized_path)) + return None + + def relative_to_runfiles(self, path: Path) -> Path | None: + """Return a path's runfiles-relative spelling when it lies in runfiles.""" + if self._runfiles_dir is None: + return None + try: + return path.resolve().relative_to(self._runfiles_dir.resolve()) + except ValueError: + return None + + def resolve_bazel_output_path(self, path: Path | str) -> Path: + """Resolve an execroot-relative Bazel output in the current context. + + ``bazel run`` exposes generated outputs below ``bazel-bin``. Some + runfiles layouts instead point into the execroot's ``bazel-out`` tree, + in which case the execroot prefix is recovered from that runfiles path. + Sandboxed builds already run from the execroot. """ - if self.is_bazel_build or self.is_bazel_run: - # Interactive Bazel targets receive runfiles-relative paths from - # ``rlocationpath``. The runfiles tree is the only stable location - # for generated files and external repository inputs. - assert self._runfiles - loc = self._runfiles.Rlocation(str(path)) - return Path(loc).absolute() if loc else None + path = Path(path) + if path.is_absolute(): + return path + + runfiles_dir = self._runfiles_dir + if self.is_bazel_run and runfiles_dir is not None: + runfiles_spelling = runfiles_dir.as_posix() + if "/bazel-out/" in runfiles_spelling: + execroot = Path(runfiles_spelling.split("/bazel-out/", 1)[0]) + return execroot / path + + if self.is_bazel_run and self.ws_root is not None: + parts = path.parts + if len(parts) >= 3 and parts[0] == "bazel-out" and parts[2] == "bin": + return self.ws_root / "bazel-bin" / Path(*parts[3:]) + + return Path.cwd() / path + + def resolve_input_path( + self, path: Path, *, runfiles_relative: bool = False + ) -> Path | None: + """Resolve an input path for the current mode or explicitly from runfiles. + + ``runfiles_relative`` is for callers that already know the value is a + runfiles address, including direct IDE invocations using ide_support. + Otherwise the path origin follows the active launcher mode. + """ + if path.is_absolute(): + return path + + if runfiles_relative or self.is_bazel_run: + # docs.bzl passes rlocationpath values to interactive Bazel targets; + # this also handles manifest-only layouts and direct IDE callers. + return self._resolve_runfiles_path(path) + elif self.is_bazel_build: + # Build actions pass declared input paths relative to their + # execution root, not runfiles-relative paths. + return path.absolute() else: - # Direct invocations resolve relative inputs from the workspace or - # current working directory. + # Direct invocations resolve relative inputs from the workspace + # when present, or otherwise from the caller's current directory. base = self.ws_root or Path.cwd() return (base / path).absolute() diff --git a/src/tests/docs_bzl/scenarios/data_files_runfiles/BUILD b/src/tests/docs_bzl/scenarios/data_files_runfiles/BUILD index e9e5d12f4..8c1f40299 100644 --- a/src/tests/docs_bzl/scenarios/data_files_runfiles/BUILD +++ b/src/tests/docs_bzl/scenarios/data_files_runfiles/BUILD @@ -29,17 +29,19 @@ genrule( "generated/index.rst", "generated/generated_data.mmd", ], + # TEMPORARY DIFF NOTE: The command writes declared outputs from the + # execution root, so ``execpath`` states the output-path contract explicitly. # The RST and Mermaid file are both declared sources, but only the RST - # should be registered as a Sphinx document. The Mermaid file is a - # companion asset that must remain readable next to the original RST. - cmd = """cat > $(location generated/index.rst) <<'EOF' + # should be registered as a Sphinx document. The Mermaid file is a companion + # asset that must remain readable next to the original RST. + cmd = """cat > $(execpath generated/index.rst) <<'EOF' Generated Data Page =================== .. mermaid:: generated_data.mmd :name: generated-data-diagram EOF -cat > $(location generated/generated_data.mmd) <<'EOF' +cat > $(execpath generated/generated_data.mmd) <<'EOF' classDiagram class GeneratedData EOF""",