From fe291bf109ab78e87db390c17a80782863a1576f Mon Sep 17 00:00:00 2001 From: Jo-Hoenk <162738563+JoHoenk@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:58:14 +0200 Subject: [PATCH 1/2] feat(sphinxdocs): make SphinxDocsLibraryInfo provider public Custom rules that produce doc files for sphinx_docs currently must depend on the private sphinx_docs_library rule implementation, since SphinxDocsLibraryInfo lives under sphinxdocs/private. Expose it via //sphinxdocs:sphinx_docs_library_info.bzl so custom rules can supply docs without that dependency. * Define a SphinxDocsFileset provider for the entries of the transitive field, so it types as depset[SphinxDocsFileset] instead of depset[struct]. * Add create_sphinx_docs_library_info(), which builds the provider from direct files plus deps, so callers don't require any inside knowledge. * Add a custom_docs_library test rule and output tests verifying a non-sphinx_docs_library rule can supply docs to sphinx_docs through the public provider, both directly and via deps --- sphinxdocs/docs/BUILD.bazel | 1 + sphinxdocs/sphinxdocs/BUILD.bazel | 7 ++ .../private/sphinx_docs_library.bzl | 27 +++---- .../private/sphinx_docs_library_info.bzl | 70 ++++++++++++++++--- .../sphinxdocs/sphinx_docs_library_info.bzl | 34 +++++++++ sphinxdocs/tests/sphinx_docs/BUILD.bazel | 24 ++++++- sphinxdocs/tests/sphinx_docs/defs.bzl | 36 ++++++++++ .../sphinx_docs/sphinx_docs_output_test.py | 13 ++++ 8 files changed, 184 insertions(+), 28 deletions(-) create mode 100644 sphinxdocs/sphinxdocs/sphinx_docs_library_info.bzl diff --git a/sphinxdocs/docs/BUILD.bazel b/sphinxdocs/docs/BUILD.bazel index 58c9e7fc18..b6c176beb9 100644 --- a/sphinxdocs/docs/BUILD.bazel +++ b/sphinxdocs/docs/BUILD.bazel @@ -51,6 +51,7 @@ sphinx_stardocs( "//sphinxdocs:readthedocs", "//sphinxdocs:sphinx", "//sphinxdocs:sphinx_docs_library", + "//sphinxdocs:sphinx_docs_library_info", "//sphinxdocs:sphinx_stardoc", "//sphinxdocs/private:sphinx_docs_library", ], diff --git a/sphinxdocs/sphinxdocs/BUILD.bazel b/sphinxdocs/sphinxdocs/BUILD.bazel index 9a871f5268..148d12739d 100644 --- a/sphinxdocs/sphinxdocs/BUILD.bazel +++ b/sphinxdocs/sphinxdocs/BUILD.bazel @@ -63,6 +63,13 @@ bzl_library( deps = ["//sphinxdocs/private:sphinx_docs_library_macro"], ) +bzl_library( + name = "sphinx_docs_library_info", + srcs = ["sphinx_docs_library_info.bzl"], + visibility = ["//visibility:public"], + deps = ["//sphinxdocs/private:sphinx_docs_library_info"], +) + bzl_library( name = "sphinx_stardoc", srcs = ["sphinx_stardoc.bzl"], diff --git a/sphinxdocs/sphinxdocs/private/sphinx_docs_library.bzl b/sphinxdocs/sphinxdocs/private/sphinx_docs_library.bzl index 076ed72254..4ad36ddbe4 100644 --- a/sphinxdocs/sphinxdocs/private/sphinx_docs_library.bzl +++ b/sphinxdocs/sphinxdocs/private/sphinx_docs_library.bzl @@ -1,27 +1,18 @@ """Implementation of sphinx_docs_library.""" -load(":sphinx_docs_library_info.bzl", "SphinxDocsLibraryInfo") +load( + ":sphinx_docs_library_info.bzl", + "SphinxDocsLibraryInfo", + "create_sphinx_docs_library_info", +) def _sphinx_docs_library_impl(ctx): - strip_prefix = ctx.attr.strip_prefix or (ctx.label.package + "/") - direct_entries = [] - if ctx.files.srcs: - entry = struct( - strip_prefix = strip_prefix, - prefix = ctx.attr.prefix, - files = ctx.files.srcs, - ) - direct_entries.append(entry) - return [ - SphinxDocsLibraryInfo( - strip_prefix = strip_prefix, - prefix = ctx.attr.prefix, + create_sphinx_docs_library_info( files = ctx.files.srcs, - transitive = depset( - direct = direct_entries, - transitive = [t[SphinxDocsLibraryInfo].transitive for t in ctx.attr.deps], - ), + prefix = ctx.attr.prefix, + strip_prefix = ctx.attr.strip_prefix or (ctx.label.package + "/"), + deps = ctx.attr.deps, ), DefaultInfo( files = depset(ctx.files.srcs), diff --git a/sphinxdocs/sphinxdocs/private/sphinx_docs_library_info.bzl b/sphinxdocs/sphinxdocs/private/sphinx_docs_library_info.bzl index de40d8deed..cc49d452dc 100644 --- a/sphinxdocs/sphinxdocs/private/sphinx_docs_library_info.bzl +++ b/sphinxdocs/sphinxdocs/private/sphinx_docs_library_info.bzl @@ -1,30 +1,82 @@ """Provider for collecting doc files as libraries.""" +SphinxDocsFileset = provider( + doc = "A set of doc files sharing the same path manipulation.", + fields = { + "files": """ +:type: tuple[File] + +The documentation files. A tuple because depset elements must be immutable. +""", + "prefix": """ +:type: str + +Prefix to prepend to file paths in `files`. Added after `strip_prefix` is removed. +""", + "strip_prefix": """ +:type: str + +Prefix to remove from file paths in `files`. Removed before `prefix` is prepended. +""", + }, +) SphinxDocsLibraryInfo = provider( doc = "Information about a collection of doc files.", fields = { "files": """ -:type: depset[File] +:type: list[File] -The documentation files for the library. +The direct documentation files for the library. """, "prefix": """ :type: str -Prefix to prepend to file paths in `files`. It is added after `strip_prefix` -is removed. +Prefix to prepend to file paths in `files`. Added after `strip_prefix` is removed. """, "strip_prefix": """ :type: str -Prefix to remove from file paths in `files`. It is removed before `prefix` -is prepended. +Prefix to remove from file paths in `files`. Removed before `prefix` is prepended. """, "transitive": """ -:type: depset[struct] +:type: depset[SphinxDocsFileset] + +This library's own files and those of its deps. -Depset of transitive library information. Each entry in the depset is a struct -with fields matching the fields of this provider. +The only field consumers read, so a rule must include its own +{obj}`SphinxDocsFileset` here or its files are silently ignored. Use +{obj}`create_sphinx_docs_library_info` to construct the provider correctly. """, }, ) + +def create_sphinx_docs_library_info(*, files = [], prefix = "", strip_prefix = "", deps = []): + """Creates a {obj}`SphinxDocsLibraryInfo`, populating the `transitive` field. + + Args: + files: {type}`list[File]` the direct doc files. + prefix: {type}`str` prefix to prepend to `files` paths. Not applied to `deps`. + strip_prefix: {type}`str` prefix to remove from `files` paths. Not applied to `deps`. + deps: {type}`list[Target]` targets with {obj}`SphinxDocsLibraryInfo` whose + files are included as-is. + + Returns: + {type}`SphinxDocsLibraryInfo` + """ + direct = [] + if files: + direct.append(SphinxDocsFileset( + files = tuple(files), + prefix = prefix, + strip_prefix = strip_prefix, + )) + + return SphinxDocsLibraryInfo( + files = files, + prefix = prefix, + strip_prefix = strip_prefix, + transitive = depset( + direct = direct, + transitive = [d[SphinxDocsLibraryInfo].transitive for d in deps], + ), + ) diff --git a/sphinxdocs/sphinxdocs/sphinx_docs_library_info.bzl b/sphinxdocs/sphinxdocs/sphinx_docs_library_info.bzl new file mode 100644 index 0000000000..236be66d4f --- /dev/null +++ b/sphinxdocs/sphinxdocs/sphinx_docs_library_info.bzl @@ -0,0 +1,34 @@ +"""Public entry point for SphinxDocsLibraryInfo. + +Lets custom rules supply doc files to `sphinx_docs` without depending on the +`sphinx_docs_library` rule implementation: + +```starlark +load( + "@sphinxdocs//sphinxdocs:sphinx_docs_library_info.bzl", + "create_sphinx_docs_library_info", +) + +def _my_docs_impl(ctx): + return [create_sphinx_docs_library_info( + files = ctx.files.srcs, + prefix = "my_docs/", + strip_prefix = ctx.label.package + "/", + deps = ctx.attr.deps, + )] +``` +""" + +load( + "//sphinxdocs/private:sphinx_docs_library_info.bzl", + _SphinxDocsFileset = "SphinxDocsFileset", + _SphinxDocsLibraryInfo = "SphinxDocsLibraryInfo", + _create_sphinx_docs_library_info = "create_sphinx_docs_library_info", +) + +# buildifier: disable=name-conventions +SphinxDocsFileset = _SphinxDocsFileset + +SphinxDocsLibraryInfo = _SphinxDocsLibraryInfo + +create_sphinx_docs_library_info = _create_sphinx_docs_library_info diff --git a/sphinxdocs/tests/sphinx_docs/BUILD.bazel b/sphinxdocs/tests/sphinx_docs/BUILD.bazel index 71bc1f3d79..e088a76fdd 100644 --- a/sphinxdocs/tests/sphinx_docs/BUILD.bazel +++ b/sphinxdocs/tests/sphinx_docs/BUILD.bazel @@ -1,7 +1,7 @@ load("@bazel_skylib//rules:build_test.bzl", "build_test") load("@rules_python//python:py_test.bzl", "py_test") load("//sphinxdocs:sphinx.bzl", "sphinx_build_binary", "sphinx_docs") -load(":defs.bzl", "gen_directory") +load(":defs.bzl", "custom_docs_library", "gen_directory") # We only build for Linux and Mac because: # 1. The actual doc process only runs on Linux @@ -28,6 +28,7 @@ sphinx_docs( sphinx = ":sphinx-build", strip_prefix = package_name() + "/", target_compatible_with = _TARGET_COMPATIBLE_WITH, + deps = [":custom_docs"], ) genrule( @@ -40,6 +41,27 @@ gen_directory( name = "generated_directory", ) +custom_docs_library( + name = "custom_docs", + page_name = "custom_page", + prefix = "custom/", + deps = [ + ":custom_docs_dep", + ":custom_docs_empty", + ], +) + +# The parent's prefix must not be applied to a dep's files. +custom_docs_library( + name = "custom_docs_dep", + page_name = "custom_dep_page", + prefix = "custom_dep/", +) + +custom_docs_library( + name = "custom_docs_empty", +) + sphinx_build_binary( name = "sphinx-build", tags = ["manual"], # Only needed as part of sphinx doc building diff --git a/sphinxdocs/tests/sphinx_docs/defs.bzl b/sphinxdocs/tests/sphinx_docs/defs.bzl index 36fd1dba4f..4939fe56ed 100644 --- a/sphinxdocs/tests/sphinx_docs/defs.bzl +++ b/sphinxdocs/tests/sphinx_docs/defs.bzl @@ -1,5 +1,41 @@ """Supporting code for tests.""" +load( + "//sphinxdocs:sphinx_docs_library_info.bzl", + "SphinxDocsLibraryInfo", + "create_sphinx_docs_library_info", +) + +def _custom_docs_library_impl(ctx): + files = [] + if ctx.attr.page_name: + out = ctx.actions.declare_file(ctx.attr.page_name + ".md") + ctx.actions.write(out, "# {}\n".format(ctx.attr.page_name)) + files.append(out) + + return [ + create_sphinx_docs_library_info( + files = files, + prefix = ctx.attr.prefix, + strip_prefix = ctx.label.package + "/", + deps = ctx.attr.deps, + ), + DefaultInfo(files = depset(files)), + ] + +# Verifies a rule that isn't sphinx_docs_library can supply doc files to +# sphinx_docs using only the public SphinxDocsLibraryInfo entry point. +custom_docs_library = rule( + implementation = _custom_docs_library_impl, + attrs = { + "deps": attr.label_list(providers = [SphinxDocsLibraryInfo]), + # When unset, the rule produces no direct files, which exercises the + # empty-files path of create_sphinx_docs_library_info. + "page_name": attr.string(), + "prefix": attr.string(), + }, +) + def _gen_directory_impl(ctx): out = ctx.actions.declare_directory(ctx.label.name) diff --git a/sphinxdocs/tests/sphinx_docs/sphinx_docs_output_test.py b/sphinxdocs/tests/sphinx_docs/sphinx_docs_output_test.py index 5d00817926..ab6c004f27 100644 --- a/sphinxdocs/tests/sphinx_docs/sphinx_docs_output_test.py +++ b/sphinxdocs/tests/sphinx_docs/sphinx_docs_output_test.py @@ -23,6 +23,19 @@ def test_directory_artifact_relative_xref(self): break self.assertEqual("dir_page2.html", actual) + def test_custom_sphinx_docs_library_info_provider(self): + page_path = importlib.resources.files(sphinx_docs).joinpath( + "docs/_build/html/custom/custom_page.html" + ) + self.assertTrue(os.path.exists(str(page_path)), f"Not found at {page_path}") + + def test_custom_sphinx_docs_library_info_deps(self): + # The dep's own prefix applies; the parent's prefix does not. + page_path = importlib.resources.files(sphinx_docs).joinpath( + "docs/_build/html/custom_dep/custom_dep_page.html" + ) + self.assertTrue(os.path.exists(str(page_path)), f"Not found at {page_path}") + if __name__ == "__main__": absltest.main() From 8721ea4d195af14979949bc999fda11850323af6 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 15 Aug 2026 23:28:39 +0000 Subject: [PATCH 2/2] feat(sphinxdocs): support transitives and filter provider-less deps Allow rules to supply transitives directly to create_sphinx_docs_library_info as a list or depset of SphinxDocsFileset objects, and ignore targets in deps that do not have SphinxDocsLibraryInfo. * Add transitives parameter to create_sphinx_docs_library_info accepting list[SphinxDocsFileset] | depset[SphinxDocsFileset]. * Filter deps in create_sphinx_docs_library_info to ignore targets without SphinxDocsLibraryInfo. * Add tests verifying provider types and transitives propagation. --- .../private/sphinx_docs_library_info.bzl | 56 +++++++++++++++---- sphinxdocs/tests/sphinx_docs/BUILD.bazel | 10 ++++ sphinxdocs/tests/sphinx_docs/defs.bzl | 49 +++++++++++++--- .../sphinx_docs/sphinx_docs_output_test.py | 6 ++ 4 files changed, 102 insertions(+), 19 deletions(-) diff --git a/sphinxdocs/sphinxdocs/private/sphinx_docs_library_info.bzl b/sphinxdocs/sphinxdocs/private/sphinx_docs_library_info.bzl index cc49d452dc..a441c803f2 100644 --- a/sphinxdocs/sphinxdocs/private/sphinx_docs_library_info.bzl +++ b/sphinxdocs/sphinxdocs/private/sphinx_docs_library_info.bzl @@ -1,4 +1,8 @@ """Provider for collecting doc files as libraries.""" + +# NOTE: A provider is used for memory efficiency because providers perform key +# sharing. +# buildifier: disable=name-conventions SphinxDocsFileset = provider( doc = "A set of doc files sharing the same path manipulation.", fields = { @@ -10,12 +14,14 @@ The documentation files. A tuple because depset elements must be immutable. "prefix": """ :type: str -Prefix to prepend to file paths in `files`. Added after `strip_prefix` is removed. +Prefix to prepend to file paths in `files`. Added after `strip_prefix` is +removed. """, "strip_prefix": """ :type: str -Prefix to remove from file paths in `files`. Removed before `prefix` is prepended. +Prefix to remove from file paths in `files`. Removed before `prefix` is +prepended. """, }, ) @@ -31,34 +37,49 @@ The direct documentation files for the library. "prefix": """ :type: str -Prefix to prepend to file paths in `files`. Added after `strip_prefix` is removed. +Prefix to prepend to file paths in `files`. Added after `strip_prefix` is +removed. """, "strip_prefix": """ :type: str -Prefix to remove from file paths in `files`. Removed before `prefix` is prepended. +Prefix to remove from file paths in `files`. Removed before `prefix` is +prepended. """, "transitive": """ :type: depset[SphinxDocsFileset] This library's own files and those of its deps. -The only field consumers read, so a rule must include its own -{obj}`SphinxDocsFileset` here or its files are silently ignored. Use +A rule must include its own {obj}`SphinxDocsFileset` here or its files won't be +propagated (and thus silently dropped). Use {obj}`create_sphinx_docs_library_info` to construct the provider correctly. """, }, ) -def create_sphinx_docs_library_info(*, files = [], prefix = "", strip_prefix = "", deps = []): +def create_sphinx_docs_library_info( + *, + files = [], + prefix = "", + strip_prefix = "", + deps = [], + transitives = []): """Creates a {obj}`SphinxDocsLibraryInfo`, populating the `transitive` field. Args: files: {type}`list[File]` the direct doc files. - prefix: {type}`str` prefix to prepend to `files` paths. Not applied to `deps`. - strip_prefix: {type}`str` prefix to remove from `files` paths. Not applied to `deps`. - deps: {type}`list[Target]` targets with {obj}`SphinxDocsLibraryInfo` whose - files are included as-is. + prefix: {type}`str` prefix to prepend to `files` paths. Not applied to + `deps`. + strip_prefix: {type}`str` prefix to remove from `files` paths. Not + applied to `deps`. + deps: {type}`list[Target]` targets whose {obj}`SphinxDocsLibraryInfo` + files are added as transitive (not direct) files. It is not + required that targets have the provider; targets without it are + ignored. + transitives: {type}`list[SphinxDocsFileset] | depset[SphinxDocsFileset]` + {obj}`SphinxDocsFileset` objects whose files are added as + transitive (not direct) files. Returns: {type}`SphinxDocsLibraryInfo` @@ -71,12 +92,23 @@ def create_sphinx_docs_library_info(*, files = [], prefix = "", strip_prefix = " strip_prefix = strip_prefix, )) + transitive_depsets = [ + d[SphinxDocsLibraryInfo].transitive + for d in deps + if SphinxDocsLibraryInfo in d + ] + if transitives: + if type(transitives) == "depset": + transitive_depsets.append(transitives) + else: + direct.extend(transitives) + return SphinxDocsLibraryInfo( files = files, prefix = prefix, strip_prefix = strip_prefix, transitive = depset( direct = direct, - transitive = [d[SphinxDocsLibraryInfo].transitive for d in deps], + transitive = transitive_depsets, ), ) diff --git a/sphinxdocs/tests/sphinx_docs/BUILD.bazel b/sphinxdocs/tests/sphinx_docs/BUILD.bazel index e088a76fdd..99b84ce801 100644 --- a/sphinxdocs/tests/sphinx_docs/BUILD.bazel +++ b/sphinxdocs/tests/sphinx_docs/BUILD.bazel @@ -45,9 +45,13 @@ custom_docs_library( name = "custom_docs", page_name = "custom_page", prefix = "custom/", + transitive_deps = [ + ":custom_docs_transitive", + ], deps = [ ":custom_docs_dep", ":custom_docs_empty", + ":gen_binary_asset", ], ) @@ -58,6 +62,12 @@ custom_docs_library( prefix = "custom_dep/", ) +custom_docs_library( + name = "custom_docs_transitive", + page_name = "custom_transitive_page", + prefix = "custom_transitive/", +) + custom_docs_library( name = "custom_docs_empty", ) diff --git a/sphinxdocs/tests/sphinx_docs/defs.bzl b/sphinxdocs/tests/sphinx_docs/defs.bzl index 4939fe56ed..40a79776cd 100644 --- a/sphinxdocs/tests/sphinx_docs/defs.bzl +++ b/sphinxdocs/tests/sphinx_docs/defs.bzl @@ -2,6 +2,7 @@ load( "//sphinxdocs:sphinx_docs_library_info.bzl", + "SphinxDocsFileset", "SphinxDocsLibraryInfo", "create_sphinx_docs_library_info", ) @@ -13,13 +14,46 @@ def _custom_docs_library_impl(ctx): ctx.actions.write(out, "# {}\n".format(ctx.attr.page_name)) files.append(out) + transitives = [] + for d in ctx.attr.transitive_deps: + if SphinxDocsLibraryInfo in d: + transitives.append(d[SphinxDocsLibraryInfo].transitive) + + info = create_sphinx_docs_library_info( + files = files, + prefix = ctx.attr.prefix, + strip_prefix = ctx.label.package + "/", + deps = ctx.attr.deps, + transitives = transitives[0] if len(transitives) == 1 else transitives, + ) + if type(info.files) != "list": + fail("Expected SphinxDocsLibraryInfo.files to be a list, got: {}".format( + type(info.files), + )) + + # Also test passing a list of SphinxDocsFileset objects to transitives: + test_fileset_info = create_sphinx_docs_library_info( + transitives = [ + SphinxDocsFileset( + files = tuple(files), + prefix = "fileset/", + strip_prefix = "", + ), + ], + ) + if type(test_fileset_info.files) != "list": + fail("Expected SphinxDocsLibraryInfo.files to be a list, got: {}".format( + type(test_fileset_info.files), + )) + if test_fileset_info.transitive.to_list(): + first_fileset = test_fileset_info.transitive.to_list()[0] + if type(first_fileset.files) != "tuple": + fail("Expected SphinxDocsFileset.files to be a tuple, got: {}".format( + type(first_fileset.files), + )) + return [ - create_sphinx_docs_library_info( - files = files, - prefix = ctx.attr.prefix, - strip_prefix = ctx.label.package + "/", - deps = ctx.attr.deps, - ), + info, DefaultInfo(files = depset(files)), ] @@ -28,11 +62,12 @@ def _custom_docs_library_impl(ctx): custom_docs_library = rule( implementation = _custom_docs_library_impl, attrs = { - "deps": attr.label_list(providers = [SphinxDocsLibraryInfo]), + "deps": attr.label_list(), # When unset, the rule produces no direct files, which exercises the # empty-files path of create_sphinx_docs_library_info. "page_name": attr.string(), "prefix": attr.string(), + "transitive_deps": attr.label_list(), }, ) diff --git a/sphinxdocs/tests/sphinx_docs/sphinx_docs_output_test.py b/sphinxdocs/tests/sphinx_docs/sphinx_docs_output_test.py index ab6c004f27..f4e91048d3 100644 --- a/sphinxdocs/tests/sphinx_docs/sphinx_docs_output_test.py +++ b/sphinxdocs/tests/sphinx_docs/sphinx_docs_output_test.py @@ -36,6 +36,12 @@ def test_custom_sphinx_docs_library_info_deps(self): ) self.assertTrue(os.path.exists(str(page_path)), f"Not found at {page_path}") + def test_custom_sphinx_docs_library_info_transitives(self): + page_path = importlib.resources.files(sphinx_docs).joinpath( + "docs/_build/html/custom_transitive/custom_transitive_page.html" + ) + self.assertTrue(os.path.exists(str(page_path)), f"Not found at {page_path}") + if __name__ == "__main__": absltest.main()