feat(sphinxdocs): make SphinxDocsLibraryInfo provider public

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",
     ],

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"],

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),

sphinxdocs/sphinxdocs/private/sphinx_docs_library_info.bzl

@@ -1,30 +1,114 @@
 """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 = {
+        "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.
+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 = [],
+        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 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`
+    """
+    direct = []
+    if files:
+        direct.append(SphinxDocsFileset(
+            files = tuple(files),
+            prefix = 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 = transitive_depsets,
+        ),
+    )

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

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,37 @@ gen_directory(
     name = "generated_directory",
 )
 
+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",
+    ],
+)
+
+# 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_transitive",
+    page_name = "custom_transitive_page",
+    prefix = "custom_transitive/",
+)
+
+custom_docs_library(
+    name = "custom_docs_empty",
+)
+
 sphinx_build_binary(
     name = "sphinx-build",
     tags = ["manual"],  # Only needed as part of sphinx doc building

sphinxdocs/tests/sphinx_docs/defs.bzl

@@ -1,5 +1,76 @@
 """Supporting code for tests."""
 
+load(
+    "//sphinxdocs:sphinx_docs_library_info.bzl",
+    "SphinxDocsFileset",
+    "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)
+
+    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 [
+        info,
+        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(),
+        # 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(),
+    },
+)
+
 def _gen_directory_impl(ctx):
     out = ctx.actions.declare_directory(ctx.label.name)
 

sphinxdocs/tests/sphinx_docs/sphinx_docs_output_test.py

@@ -23,6 +23,25 @@ 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}")
+
+    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()