docs: show PR warning banner and fix links doc source pages (#1521)

This fixes a few issues with the RTD doc building:

* Warning banner is now shown for PR requests
* Pages now link to the github source
* The footer now shows the git commit they were built at

This works by passing the RTD environment variables to the sphinx build
process, which allows the conf.py file to get their values. Env vars are
passed by a new flag, `--//sphinxdocs:extra_env`, which allows passing
arbitrary environment variable values into the sphinx build process. To
make future usage of the RTD env vars easier, the build process passes
along all the `READTHEDOCS*` environment variables.

Fixes #1516

Author: rickeylev

.readthedocs.yml

@@ -8,4 +8,7 @@ build:
   commands:
     - env
     - npm install -g @bazel/bazelisk
-    - bazel run --config=rtd --//sphinxdocs:extra_defines=version=$READTHEDOCS_VERSION //docs/sphinx:readthedocs_install
+    - bazel version
+    # Put the actual build behind a shell script because its easier to modify than
+    # the yaml config.
+    - docs/sphinx/readthedocs_build.sh

docs/sphinx/conf.py

@@ -1,5 +1,7 @@
 # Configuration file for the Sphinx documentation builder.
 
+import os
+
 # -- Project information
 project = "rules_python"
 copyright = "2023, The Bazel Authors"
@@ -27,6 +29,29 @@
     "sphinx_rtd_theme",  # Necessary to get jquery to make flyout work
 ]
 
+# Adapted from the template code:
+# https://github.com/readthedocs/readthedocs.org/blob/main/readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl
+if os.environ.get("READTHEDOCS") == "True":
+  # Must come first because it can interfere with other extensions, according
+  # to the original conf.py template comments
+  extensions.insert(0, "readthedocs_ext.readthedocs")
+
+  if os.environ.get("READTHEDOCS_VERSION_TYPE") == "external":
+    # Insert after the main extension
+    extensions.insert(1, "readthedocs_ext.external_version_warning")
+    readthedocs_vcs_url = "http://github.com/bazelbuild/rules_python/pull/{}".format(
+        os.environ.get("READTHEDOCS_VERSION", "")
+    )
+    # The build id isn't directly available, but it appears to be encoded
+    # into the host name, so we can parse it from that. The format appears
+    # to be `build-X-project-Y-Z`, where:
+    # * X is an integer build id
+    # * Y is an integer project id
+    # * Z is the project name
+    _build_id = os.environ.get("HOSTNAME", "build-0-project-0-rules-python")
+    _build_id = _build_id.split("-")[1]
+    readthedocs_build_url = f"https://readthedocs.org/projects/rules-python/builds/{_build_id}"
+
 exclude_patterns = ["_includes/*"]
 templates_path = ["_templates"]
 primary_domain = None  # The default is 'py', which we don't make much use of
@@ -69,6 +94,33 @@
 html_theme = "sphinx_rtd_theme"
 html_theme_options = {}
 
+# The html_context settings are part of the jinja context used by the themes.
+html_context = {
+    # This controls whether the flyout menu is shown. It is always false
+    # because:
+    # * For local builds, the flyout menu is empty and doesn't show in the
+    #   same place as for RTD builds. No point in showing it locally.
+    # * For RTD builds, the flyout menu is always automatically injected,
+    #   so having it be True makes the flyout show up twice.
+    "READTHEDOCS": False,
+    'PRODUCTION_DOMAIN': "readthedocs.org",
+    # This is the path to a page's source (after the github user/repo/commit)
+    "conf_py_path": "/docs/sphinx/",
+    'github_user': 'bazelbuild',
+    'github_repo': 'rules_python',
+    # The git version that was checked out, e.g. the tag or branch name
+    'github_version': os.environ.get("READTHEDOCS_GIT_IDENTIFIER", ""),
+    # For local builds, the github link won't work. Disabling it replaces
+    # it with a "view source" link to view the source Sphinx saw, which
+    # is useful for local development.
+    'display_github': os.environ.get("READTHEDOCS") == "True",
+    'commit': os.environ.get("READTHEDOCS_GIT_COMMIT_HASH", "unknown commit"),
+
+    # Used by readthedocs_ext.external_version_warning extension
+    # This is the PR number being built
+    'current_version': os.environ.get("READTHEDOCS_VERSION", ""),
+}
+
 # Keep this in sync with the stardoc templates
 html_permalinks_icon = "¶"
 
@@ -86,7 +138,6 @@
 
 suppress_warnings = ["myst.header", "myst.xref_missing"]
 
-
 def setup(app):
   # Pygments says it supports starlark, but it doesn't seem to actually
   # recognize `starlark` as a name. So just manually map it to python.

docs/sphinx/readthedocs_build.sh

@@ -0,0 +1,19 @@
+#!/bin/bash
+
+set -eou pipefail
+
+declare -a extra_env
+while IFS='=' read -r -d '' name value; do
+  if [[ "$name" == READTHEDOCS* ]]; then
+    extra_env+=("--//sphinxdocs:extra_env=$name=$value")
+  fi
+done < <(env -0)
+
+# In order to get the build number, we extract it from the host name
+extra_env+=("--//sphinxdocs:extra_env=HOSTNAME=$HOSTNAME")
+
+set -x
+bazel run \
+  "--//sphinxdocs:extra_defines=version=$READTHEDOCS_VERSION" \
+  "${extra_env[@]}" \
+  //docs/sphinx:readthedocs_install

sphinxdocs/BUILD.bazel

@@ -13,19 +13,24 @@
 # limitations under the License.
 
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
-load("//sphinxdocs/private:sphinx.bzl", "sphinx_defines_flag")
+load("//sphinxdocs/private:sphinx.bzl", "repeated_string_list_flag")
 
 package(
     default_visibility = ["//:__subpackages__"],
 )
 
 # Additional -D values to add to every Sphinx build.
 # This is usually used to override the version when building
-sphinx_defines_flag(
+repeated_string_list_flag(
     name = "extra_defines",
     build_setting_default = [],
 )
 
+repeated_string_list_flag(
+    name = "extra_env",
+    build_setting_default = [],
+)
+
 bzl_library(
     name = "sphinx_bzl",
     srcs = ["sphinx.bzl"],

sphinxdocs/private/sphinx.bzl

@@ -155,6 +155,7 @@ _sphinx_docs = rule(
         ),
         "strip_prefix": attr.string(doc = "Prefix to remove from input file paths."),
         "_extra_defines_flag": attr.label(default = "//sphinxdocs:extra_defines"),
+        "_extra_env_flag": attr.label(default = "//sphinxdocs:extra_env"),
     },
 )
 
@@ -201,30 +202,36 @@ def _run_sphinx(ctx, format, source_path, inputs, output_prefix):
     args.add("-E")  # Don't try to use cache files. Bazel can't make use of them.
     args.add("-a")  # Write all files; don't try to detect "changed" files
     args.add_all(ctx.attr.extra_opts)
-    args.add_all(ctx.attr._extra_defines_flag[_SphinxDefinesInfo].value, before_each = "-D")
+    args.add_all(ctx.attr._extra_defines_flag[_FlagInfo].value, before_each = "-D")
     args.add(source_path)
     args.add(output_dir.path)
 
+    env = dict([
+        v.split("=", 1)
+        for v in ctx.attr._extra_env_flag[_FlagInfo].value
+    ])
+
     ctx.actions.run(
         executable = ctx.executable.sphinx,
         arguments = [args],
         inputs = inputs,
         outputs = [output_dir],
         mnemonic = "SphinxBuildDocs",
         progress_message = "Sphinx building {} for %{{label}}".format(format),
+        env = env,
     )
     return output_dir
 
-_SphinxDefinesInfo = provider(
-    doc = "Provider for the extra_defines flag value",
+_FlagInfo = provider(
+    doc = "Provider for a flag value",
     fields = ["value"],
 )
 
-def _sphinx_defines_flag_impl(ctx):
-    return _SphinxDefinesInfo(value = ctx.build_setting_value)
+def _repeated_string_list_flag_impl(ctx):
+    return _FlagInfo(value = ctx.build_setting_value)
 
-sphinx_defines_flag = rule(
-    implementation = _sphinx_defines_flag_impl,
+repeated_string_list_flag = rule(
+    implementation = _repeated_string_list_flag_impl,
     build_setting = config.string_list(flag = True, repeatable = True),
 )