diff --git a/.github/actions/setup-doc-build/action.yml b/.github/actions/setup-doc-build/action.yml index 612345f..56439d1 100644 --- a/.github/actions/setup-doc-build/action.yml +++ b/.github/actions/setup-doc-build/action.yml @@ -22,6 +22,12 @@ description: >- inputs: python-version: default: '3.12' + releases: + description: >- + Also materialise one Clawpack tree per released doc version + (doc/tools/clawpack-release-refs.txt) and export CLAW_VERSION_TREES, so + `make versions` documents each release against the code it shipped. + default: 'false' outputs: claw: @@ -55,6 +61,16 @@ runs: echo "path=$RUNNER_TEMP/claw" >> "$GITHUB_OUTPUT" echo "CLAW=$RUNNER_TEMP/claw" >> "$GITHUB_ENV" + # --reference borrows from the dev tree just cloned: the release commits + # are in its history, so eight more trees cost little more than one. + - name: Materialise the release trees + if: inputs.releases == 'true' + shell: bash + run: | + python doc/tools/fetch_clawpack_src.py "$RUNNER_TEMP/claw-releases" \ + --releases --reference "$RUNNER_TEMP/claw" --quiet + echo "CLAW_VERSION_TREES=$RUNNER_TEMP/claw-releases" >> "$GITHUB_ENV" + # Fails loudly on the class of problem that used to surface only as # warnings nobody read. - name: Check the doc build environment diff --git a/.github/workflows/docs-publish.yml b/.github/workflows/docs-publish.yml index 3a97bf5..74d3282 100644 --- a/.github/workflows/docs-publish.yml +++ b/.github/workflows/docs-publish.yml @@ -38,11 +38,14 @@ name: docs-publish # `make versions` on a PR would show reviewers the site *without* the PR's # changes. # -# * The Clawpack source tree autodoc documents is pinned by commit in -# doc/tools/clawpack-ref.txt and set up by .github/actions/setup-doc-build. -# Every version directory is therefore built against the same source; the -# historical ones get API docs from newer code, which is also what the -# manual `make versions` + rsync_doc.sh procedure always did. +# * Each version is documented against the Clawpack source it shipped. +# dev uses the tree pinned in doc/tools/clawpack-ref.txt; each released +# version uses its release commit from doc/tools/clawpack-release-refs.txt +# (doc/ext/clawpack_source.py swaps it in). Building every version +# against one current tree, as the manual procedure did, documented code +# the releases never had and emptied pages for renamed modules. The +# build fails on any autodoc import failure, and `stage` reports every +# page that loses API entries before anyone approves. on: push: @@ -118,6 +121,8 @@ jobs: persist-credentials: false - uses: ./.github/actions/setup-doc-build + with: + releases: 'true' # actions/checkout leaves only the checked-out ref as a local branch, # and sphinx-multiversion ignores remote-tracking refs, so v5.14.x @@ -243,6 +248,12 @@ jobs: echo "changed=true" >> "$GITHUB_OUTPUT" echo "publish_sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" + # An API page emptied by an import problem is just "M" in the file list + # above; this is how a reviewer sees it before approving. + - name: Report pages that lose API entries + if: steps.commit.outputs.changed == 'true' + run: python3 doc/tools/compare_autodoc.py site "$PREV_SHA" HEAD >> "$GITHUB_STEP_SUMMARY" + - name: Report the diff if: always() run: | diff --git a/.gitignore b/.gitignore index 60d7150..27f0c5a 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,9 @@ api/generated/ # docs can be built against exactly what CI builds against. Deliberately # outside doc/, which is the Sphinx source dir. .claw-pin/ +# `make claw-releases`: one tree per row of doc/tools/clawpack-release-refs.txt, +# which `make versions` builds each released version against. +.claw-releases/ # ----------------------------------------------------------------------------- # Python diff --git a/doc/Makefile b/doc/Makefile index e9c7c12..73967c5 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -22,6 +22,10 @@ VERSIONSDIR = _build # it. Outside the sphinx source dir on purpose -- Sphinx would otherwise walk # it looking for .rst files. CLAWPIN = ../.claw-pin +# One tree per released version (tools/clawpack-release-refs.txt), which +# `make versions` documents each release against; `make claw-releases` fills +# it. $CLAW_VERSION_TREES points the build elsewhere (CI does). +CLAWRELEASES = ../.claw-releases # Internal variables. PAPEROPT_a4 = -D latex_paper_size=a4 @@ -35,7 +39,7 @@ else LAYOUT = _themes/flask_local/layout.html endif -.PHONY: help clean html dirhtml singlehtml pickle json htmlhelp qthelp devhelp epub latex latexpdf text man changes linkcheck doctest checkenv checkwarnings checkwarnings-update checkwarnings-strict check-versions versions clean-versions versions-promote versions-publish claw-pin claw-pin-check tools-tests +.PHONY: help clean html dirhtml singlehtml pickle json htmlhelp qthelp devhelp epub latex latexpdf text man changes linkcheck doctest checkenv checkwarnings checkwarnings-update checkwarnings-strict check-versions versions clean-versions versions-promote versions-publish claw-pin claw-pin-check claw-releases claw-releases-check tools-tests help: @echo "Please use \`make ' where is one of" @@ -61,6 +65,8 @@ help: @echo " clean-versions to remove $(VERSIONSDIR)" @echo " claw-pin to clone the Clawpack source tree pinned in tools/clawpack-ref.txt" @echo " into $(CLAWPIN); build against it with CLAW=\$$(cd $(CLAWPIN) && pwd)" + @echo " claw-releases to clone one tree per released version (tools/clawpack-release-refs.txt)" + @echo " into $(CLAWRELEASES); \`make versions\` documents each release against its own" @echo " checkenv to check that every autodoc target imports from \$$CLAW" @echo " checkwarnings to fail on any NEW reST/docstring warning (vs the baseline)" @echo " checkwarnings-update to regenerate the warning baseline (tools/doc_warnings_baseline.txt)" @@ -209,6 +215,15 @@ claw-pin: claw-pin-check: python tools/fetch_clawpack_src.py $(CLAWPIN) --check +# The release trees for `make versions`. The release commits are all in the +# history of ordinary clones, so borrowing from $CLAW next door makes this +# cheap; check-versions refuses to build if they are missing or stale. +claw-releases: + python tools/fetch_clawpack_src.py $(CLAWRELEASES) --releases --reference ../.. --quiet + +claw-releases-check: + python tools/fetch_clawpack_src.py $(CLAWRELEASES) --releases --check + checkenv: python tools/check_doc_env.py @@ -232,5 +247,6 @@ tools-tests: python tools/test_check_doc_env.py python tools/test_check_doc_warnings.py python tools/test_promote_latest.py + python tools/test_compare_autodoc.py bash tools/test_check_published_tree.sh diff --git a/doc/conf.py b/doc/conf.py index 94ab1e4..61abb24 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -44,17 +44,18 @@ def _safe_fileConfig(*args, **kwargs): # is has to be one decision shared with tools/check_doc_warnings.py, or the # warning baseline stops being comparable -- hence tools/clawroot.py. # Set CLAW to build against the pinned tree (`make claw-pin`). -from clawroot import claw_root +# +# That is the tree for dev and for single-version builds. In a multiversion +# build, ext/clawpack_source.py swaps in each released version's own tree +# (tools/clawpack-release-refs.txt) before autodoc imports anything. +from clawroot import CLAW_SUBPATHS, claw_root clawpack_root = claw_root() # Ahead of site-packages, so a pip-installed or editable clawpack cannot # shadow the tree we mean to document. sys.path.insert(0, clawpack_root) print("clawpack_root = %s" % clawpack_root) -sys.path.append(os.path.join(clawpack_root,'amrclaw/doc')) -sys.path.append(os.path.join(clawpack_root,'visclaw/doc')) -sys.path.append(os.path.join(clawpack_root,'geoclaw/doc')) -sys.path.append(os.path.join(clawpack_root,'pyclaw/src')) -sys.path.append(os.path.join(clawpack_root,'riemann/src/python/riemann')) +for _sub in CLAW_SUBPATHS: + sys.path.append(os.path.join(clawpack_root, _sub)) # General configuration @@ -70,7 +71,8 @@ def _safe_fileConfig(*args, **kwargs): 'sphinx.ext.inheritance_diagram', 'sphinx_multiversion', 'sphinx.ext.mathjax', - 'srclinks'] + 'srclinks', + 'clawpack_source'] # autodoc imports the documented modules at build time, so anything they diff --git a/doc/ext/clawpack_source.py b/doc/ext/clawpack_source.py new file mode 100644 index 0000000..3fc9bee --- /dev/null +++ b/doc/ext/clawpack_source.py @@ -0,0 +1,116 @@ +""" +Document each version against the Clawpack source it shipped, and fail the +build when autodoc cannot import something. + +sphinx-multiversion builds every version with *this* checkout's conf.py, and +conf.py puts one tree on sys.path: $CLAW, pinned in tools/clawpack-ref.txt to +current master. A released version's .rst comes from its own tag, but without +this extension its API pages would be generated from master: renamed modules +fail to import (surge.storm -> met.storm emptied storm_module.html on every +release), and everything else silently documents code the release never had. + +So, on ``config-inited``, a version listed in tools/clawpack-release-refs.txt +gets its own tree (``tools/fetch_clawpack_src.py --releases``) swapped onto +sys.path in place of $CLAW. ``dev`` and plain single-version builds are left +alone. + +autodoc reports an import failure as an ordinary warning, and a build that +emits one still succeeds -- with an empty page. Every page here is expected to +import (tools/check_doc_env.py enforces that for dev before the build), so any +``autodoc: failed to import`` in any version fails the build. +""" + +import importlib +import importlib.util +import logging +import os +import sys +import types + +from sphinx.errors import ExtensionError + +from clawroot import CLAW_SUBPATHS, claw_root, release_root + + +def _claw_paths(root): + return [root] + [os.path.join(root, sub) for sub in CLAW_SUBPATHS] + + +def _install_imp_shim(): + """Python 3.12 removed ``imp``; v5.7.x's pyclaw does ``from imp import reload``. + + Only ``reload`` is provided, which is all any release uses. + """ + if importlib.util.find_spec('imp') is None and 'imp' not in sys.modules: + imp = types.ModuleType('imp') + imp.reload = importlib.reload + sys.modules['imp'] = imp + + +def _use_version_tree(app, config): + version = getattr(config, 'smv_current_version', '') or '' + root = release_root(version) if version else None + if root is None: + app.clawpack_root = claw_root() + return + + root = os.path.realpath(root) + stale = set(_claw_paths(claw_root())) + sys.path[:] = [p for p in sys.path if p not in stale] + sys.path[:0] = [root] + sys.path.extend(_claw_paths(root)[1:]) + for name in [m for m in sys.modules + if m == 'clawpack' or m.startswith('clawpack.')]: + del sys.modules[name] + _install_imp_shim() + app.clawpack_root = root + print(f'clawpack_root ({version}) = {root}') + + +class _ImportFailures(logging.Handler): + """Collects autodoc's import-failure warnings, wherever they are emitted.""" + + def __init__(self): + super().__init__(level=logging.WARNING) + self.failures = set() + + def emit(self, record): + try: + message = record.getMessage() + except Exception: + return + if 'failed to import' in message: + self.failures.add(message.splitlines()[0]) + + +def _check(app, exception, handler): + if exception is not None: + return + root = app.clawpack_root + clawpack = sys.modules.get('clawpack') + if clawpack is not None and getattr(clawpack, '__file__', None): + where = os.path.realpath(clawpack.__file__) + if os.path.commonpath([where, os.path.realpath(root)]) != os.path.realpath(root): + raise ExtensionError( + f'clawpack was imported from {where}, not from the tree this ' + f'version documents ({root}). An installed clawpack is ' + 'shadowing it; build in a virtualenv without one.') + if handler.failures: + raise ExtensionError( + f'autodoc could not import {len(handler.failures)} target(s) ' + f'from {root}; their API pages would be empty:\n ' + + '\n '.join(sorted(handler.failures))) + + +def setup(app): + app.connect('config-inited', _use_version_tree, priority=100) + handler = _ImportFailures() + # Sphinx's loggers are all children of "sphinx"; a handler there sees + # autodoc's warnings, including ones replayed from parallel workers. + logging.getLogger('sphinx').addHandler(handler) + + def check_imports(app, exception): + _check(app, exception, handler) + + app.connect('build-finished', check_imports) + return {'parallel_read_safe': True, 'parallel_write_safe': True} diff --git a/doc/howto_doc.rst b/doc/howto_doc.rst index 4cc867a..b9a71f3 100644 --- a/doc/howto_doc.rst +++ b/doc/howto_doc.rst @@ -186,6 +186,49 @@ able to import what the docs reference:: To bump the pinned Clawpack, edit `tools/clawpack-ref.txt` and repeat the regeneration above in the same commit. +.. _howto_doc_release_trees: + +Released versions: one source tree each +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +`tools/clawpack-ref.txt` is the tree for `dev`. A multiversion build documents +each **released** version against the code that release shipped instead: +`tools/clawpack-release-refs.txt` maps every built release (`v5.7.x`, +`v5.8.x`, ...) to the `clawpack/clawpack` commit of its newest patch tag, and +the subrepos come from that commit's submodule pointers. + +This matters because `sphinx-multiversion` builds every version with *this* +checkout's `conf.py`. Against one current tree, a release's API pages +document code it never had, and a module renamed since the release documents +nothing at all: `storm_module.html` came out empty on every release once +`surge.storm` became a re-export of `met.storm`, with no warning. + +`make claw-releases` clones one tree per row into `$CLAW/doc/.claw-releases` +(borrowing objects from your checkouts, so all of them take about a minute), +and `ext/clawpack_source.py` swaps the right one onto `sys.path` as each +version starts. `$CLAW_VERSION_TREES` points the build at trees somewhere +else. So a full local multiversion build is:: + + make claw-pin + make claw-releases + CLAW=$(cd ../.claw-pin && pwd) make versions-publish + +Three checks keep this honest: + +- `make check-versions` fails if a built release has no row in + `tools/clawpack-release-refs.txt`, a row names a version that is not built, + or a release tree is missing or at the wrong commit. +- The build fails if autodoc cannot import anything, in any version. (That + catches modules that are gone; it cannot catch the re-export case above, + which imports fine and documents nothing.) +- The publish workflow's `stage` summary lists every page that loses API + entries compared with the published site, before anyone approves a publish. + +When cutting a release, add its row to `tools/clawpack-release-refs.txt` (see +:ref:`howto_release_doc`). Releases before 5.8 import the `imp` module, which +Python 3.12 removed; `ext/clawpack_source.py` provides the one function they +use. + .. note:: One warning class is never baselined, however long it has been present: diff --git a/doc/howto_release.rst b/doc/howto_release.rst index 959a549..be4fade 100644 --- a/doc/howto_release.rst +++ b/doc/howto_release.rst @@ -218,6 +218,12 @@ When making changes for a new release, the following pages in the directory `installing_pip.rst`, `installing_fortcodes.rst`, `contents.rst`, `docker_image.rst`. + - Add a row for the new version to `tools/clawpack-release-refs.txt` naming + the `clawpack/clawpack` commit of the release tag (and update the row when + a patch release follows), so its API pages are built from the code it + shipped. `make check-versions` fails until the row is there. See + :ref:`howto_doc_release_trees`. + - Modify the main landing page `_templates/index.html` to cite the proper version number and DOI. (No longer necessary -- This page has been changed to be more generic.) diff --git a/doc/tools/check_versions.py b/doc/tools/check_versions.py index bb0a23a..4b115cb 100644 --- a/doc/tools/check_versions.py +++ b/doc/tools/check_versions.py @@ -55,6 +55,17 @@ TOOLS_DIR = os.path.dirname(os.path.abspath(__file__)) DOC_DIR = os.path.dirname(TOOLS_DIR) +sys.path.insert(0, TOOLS_DIR) +from clawroot import (RELEASE_MANIFEST, read_release_manifest, # noqa: E402 + release_trees_dir) +import fetch_clawpack_src # noqa: E402 + +# Built versions that document the dev pin (tools/clawpack-ref.txt) rather +# than a release tree. Every other built version needs a row in +# tools/clawpack-release-refs.txt, or ext/clawpack_source.py would document it +# against master. +PINNED_VERSIONS = {"dev"} + # Refs that match the whitelists but whose conf.py cannot be loaded by a # supported Sphinx. See "The second trap" above. Their content is already # published and the sync never deletes it, so dropping them is not data loss -- @@ -256,6 +267,27 @@ def main(argv=None): % ", ".join(unexpected) ) + releases = set(read_release_manifest()) + unpinned = sorted(set(built) - PINNED_VERSIONS - releases) + stale = sorted(releases - set(built)) + if unpinned: + sys.exit( + "ERROR: built version(s) with no Clawpack release tree: %s\n" + "Add a row to %s naming the release commit to document them " + "against." % (", ".join(unpinned), os.path.relpath(RELEASE_MANIFEST)) + ) + if stale: + sys.exit( + "ERROR: %s lists version(s) that are not built: %s" + % (os.path.relpath(RELEASE_MANIFEST), ", ".join(stale)) + ) + print("\nrelease trees pinned for %d version(s); %s document(s) the dev pin" + % (len(releases), ", ".join(sorted(PINNED_VERSIONS & set(built))))) + # A tree left at an old commit after the manifest moved would document the + # wrong release without any error, so check the commits, not just presence. + if fetch_clawpack_src.check_releases(release_trees_dir()) != 0: + sys.exit(1) + if recovered: print( "\nNOTE: %s now build(s) successfully; remove from " diff --git a/doc/tools/clawpack-release-refs.txt b/doc/tools/clawpack-release-refs.txt new file mode 100644 index 0000000..1926b63 --- /dev/null +++ b/doc/tools/clawpack-release-refs.txt @@ -0,0 +1,31 @@ +# The Clawpack source tree each *released* documentation version is built +# against: [# tag]. +# +# WHY THIS FILE EXISTS +# +# sphinx-multiversion builds every version with dev's conf.py, so without this +# every version directory would get its API pages from the one tree pinned in +# clawpack-ref.txt -- i.e. current master. A release's .rst names modules as +# they were at that release; where master has since renamed them (surge.storm +# -> met.storm) autodoc fails to import and the page comes out silently empty, +# and everywhere else the release pages document code that release never +# shipped. +# +# Each row pins the super-repo commit of the release's newest patch tag. The +# subrepo commits come from that commit's submodule pointers, exactly as the +# release shipped them; tools/fetch_clawpack_src.py --releases materialises +# one tree per row, and ext/clawpack_source.py selects the tree for the +# version being built. `dev` is not listed: it documents clawpack-ref.txt. +# +# check_versions.py fails if a buildable release version has no row, or a row +# names a version that is not built. When cutting a release, add its row here +# (and bump the row when a patch release follows). + +v5.7.x 5d49f0ab191dc22a6a0dd28daed9907dc8b47bd3 # v5.7.1 +v5.8.x 531a8fb6682d4fb9fab8b0812c83de599117c150 # v5.8.2 +v5.9.x 84be0d766c353753eca52c9fe9a1f15f08966df7 # v5.9.2 +v5.10.x b87c8e74219dd6e8acad8eb753066b2296030d09 # v5.10.0 +v5.11.x 285b7de9837702276213b7deb040495adc60c4cc # v5.11.0 +v5.12.x 98ba40b1ef28c527bcbe16495e075dda5fe8f3f5 # v5.12.0 +v5.13.x 634a9b78a75ba1434be5d6e84fda8dbf096e882b # v5.13.1 +v5.14.x 5ce53b4814a3e03536fa17e8ae05e65960194c56 # v5.14.0 diff --git a/doc/tools/clawroot.py b/doc/tools/clawroot.py index b0c1364..65cc469 100644 --- a/doc/tools/clawroot.py +++ b/doc/tools/clawroot.py @@ -16,16 +16,34 @@ Setting ``CLAW`` overrides it. That is how the docs get built against the pinned tree (``tools/clawpack-ref.txt``, ``make claw-pin``) without anyone having to move their checkout around, and it is what CI uses. + +That answer is for ``dev`` and for single-version builds. A *released* +version in a multiversion build is documented against its own release instead +(``tools/clawpack-release-refs.txt``): ``release_root`` says where that tree +is, and ``ext/clawpack_source.py`` swaps it in. """ from __future__ import annotations import os +import re TOOLS_DIR = os.path.dirname(os.path.abspath(__file__)) SRC_DIR = os.path.dirname(TOOLS_DIR) # doc/doc -- the sphinx source dir +RELEASE_MANIFEST = os.path.join(TOOLS_DIR, 'clawpack-release-refs.txt') + +# Where `make claw-releases` puts the release trees; $CLAW_VERSION_TREES +# overrides it (CI materialises them under $RUNNER_TEMP). +DEFAULT_RELEASE_TREES = os.path.join(SRC_DIR, os.pardir, '.claw-releases') + +_SHA_RE = re.compile(r'^[0-9a-f]{40}$') + +# Paths under a tree that conf.py puts on sys.path besides the root itself. +CLAW_SUBPATHS = ['amrclaw/doc', 'visclaw/doc', 'geoclaw/doc', 'pyclaw/src', + 'riemann/src/python/riemann'] + CLAW_ROOT_HELP = """\ The docs are built against a Clawpack source tree, selected by $CLAW and defaulting to the parent of this repository. To build against the pinned @@ -45,3 +63,51 @@ def claw_root() -> str: if override: return os.path.abspath(override) return os.path.abspath(os.path.join(SRC_DIR, os.pardir, os.pardir)) + + +def read_release_manifest(path: str = RELEASE_MANIFEST) -> dict[str, str]: + """``{doc version: clawpack/clawpack commit}`` from the release manifest.""" + releases: dict[str, str] = {} + with open(path, encoding='utf-8') as fh: + for lineno, line in enumerate(fh, 1): + line = line.split('#', 1)[0].strip() + if not line: + continue + fields = line.split() + if len(fields) != 2: + raise SystemExit(f'{path}:{lineno}: expected ' + f'" ", got: {line!r}') + version, sha = fields + if not _SHA_RE.match(sha): + raise SystemExit(f'{path}:{lineno}: {version} is not pinned to ' + f'a full 40-character SHA: {sha!r}') + if version in releases: + raise SystemExit(f'{path}:{lineno}: {version} listed twice') + releases[version] = sha + return releases + + +def release_trees_dir() -> str: + """Directory holding one materialised tree per released doc version.""" + return os.path.abspath(os.environ.get('CLAW_VERSION_TREES') + or DEFAULT_RELEASE_TREES) + + +def release_root(version: str) -> str | None: + """The tree to document *version* against, or None if it is not a release. + + Raises if *version* is a release whose tree has not been materialised: + falling back to $CLAW there is exactly the bug the manifest exists to + prevent, so it has to be loud. + """ + if version not in read_release_manifest(): + return None + root = os.path.join(release_trees_dir(), version) + if not os.path.exists(os.path.join(root, 'clawpack', '__init__.py')): + raise RuntimeError( + f'{version} is a released version, documented against its own ' + f'Clawpack tree, but there is none at\n {root}\n' + 'Materialise the release trees first:\n\n' + ' make claw-releases\n\n' + 'or point $CLAW_VERSION_TREES at a directory that has them.') + return root diff --git a/doc/tools/compare_autodoc.py b/doc/tools/compare_autodoc.py new file mode 100644 index 0000000..3bfa25e --- /dev/null +++ b/doc/tools/compare_autodoc.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python +# encoding: utf-8 +r""" +Report HTML pages whose autodoc content shrank between two site revisions. + +The publish workflow's ``stage`` job runs this on the site clone, comparing the +published revision with the staged commit, and appends the report to the step +summary -- so whoever approves a publish sees which API pages lose entries +before anything goes live. + +It exists because of the first full-site publish attempt: every release's +storm_module.html went from 22 documented entries to 0, which a file-count +diff shows as an ordinary modification. Here that is a line of its own. + +An "entry" is one ``
``: a documented class, function, +method or attribute. A drop is not necessarily wrong -- v5.7.x legitimately +loses classes its old pages documented from newer code -- so this reports and +does not fail. + +Usage +----- + compare_autodoc.py [--limit N] + +Revisions may be commits or trees. Output is Markdown. +""" + +from __future__ import annotations + +import argparse +import subprocess +import sys + +ENTRY = b'
list[tuple[str, int | None, int | None]]: + """``(page, before, after)`` for every changed page with autodoc entries.""" + pages = changed_pages(repo, before, after) + counts = count_entries(repo, [f'{before}:{p}' for p in pages] + + [f'{after}:{p}' for p in pages]) + rows = [] + for page in pages: + old, new = counts[f'{before}:{page}'], counts[f'{after}:{page}'] + if old or new: + rows.append((page, old, new)) + return rows + + +def report(rows: list[tuple[str, int | None, int | None]], limit: int) -> str: + total_old = sum(old or 0 for _, old, _ in rows) + total_new = sum(new or 0 for _, _, new in rows) + drops = sorted((r for r in rows if (r[2] or 0) < (r[1] or 0)), + key=lambda r: ((r[2] or 0) != 0, (r[2] or 0) - (r[1] or 0), r[0])) + emptied = [r for r in drops if not r[2]] + + lines = ['### Autodoc entries on changed pages', '', + f'{len(rows)} changed page(s) have API entries: ' + f'{total_old} before, {total_new} after.'] + if not drops: + lines += ['', 'No page loses entries.'] + return '\n'.join(lines) + '\n' + + lines += ['', f'**{len(drops)} page(s) lose entries' + + (f', {len(emptied)} of them down to zero' if emptied else '') + + '.** Check these before approving: a page emptied by an ' + 'import failure looks like any other modification in the diff.', + '', '| page | before | after |', '|---|---:|---:|'] + for page, old, new in drops[:limit]: + lines.append(f'| `{page}` | {old if old is not None else "-"} ' + f'| {new if new is not None else "-"} |') + if len(drops) > limit: + lines.append(f'| ... {len(drops) - limit} more | | |') + return '\n'.join(lines) + '\n' + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument('repo') + parser.add_argument('before') + parser.add_argument('after') + parser.add_argument('--limit', type=int, default=40, + help='most pages to list (default 40)') + args = parser.parse_args(argv) + sys.stdout.write(report(compare(args.repo, args.before, args.after), + args.limit)) + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/doc/tools/fetch_clawpack_src.py b/doc/tools/fetch_clawpack_src.py index 026dae8..fb9c7e6 100644 --- a/doc/tools/fetch_clawpack_src.py +++ b/doc/tools/fetch_clawpack_src.py @@ -18,6 +18,7 @@ ----- fetch_clawpack_src.py [--reference DIR] [--quiet] fetch_clawpack_src.py --check + fetch_clawpack_src.py --releases [--reference DIR] [--quiet] [--check] ``--reference DIR`` treats ``DIR`` as a directory of existing Clawpack clones (e.g. an ordinary ``$CLAW``) and borrows their objects, which makes a local @@ -27,6 +28,14 @@ ``--check`` verifies an existing tree matches the manifest and exits non-zero otherwise. ``make checkwarnings-update`` uses it to refuse to write a baseline from an unpinned tree. + +``--releases`` instead builds ``//`` for every row of +``tools/clawpack-release-refs.txt``: the super-repo at the release commit and +each subrepo at the commit that release's submodule pointer names, i.e. the +code the release shipped. ``make versions`` documents each released version +against its own tree (see ``ext/clawpack_source.py``). ``--reference`` is +worth passing here: the release commits are all in the history of ordinary +clones, so eight trees cost little more than one. """ from __future__ import annotations @@ -37,6 +46,8 @@ import subprocess import sys +from clawroot import read_release_manifest + TOOLS_DIR = os.path.dirname(os.path.abspath(__file__)) MANIFEST = os.path.join(TOOLS_DIR, 'clawpack-ref.txt') @@ -103,7 +114,9 @@ def _materialise(repo: str, sha: str, target: str, reference: str | None, os.makedirs(target, exist_ok=True) clone = ['git', 'clone', '--quiet', '--no-checkout'] if reference: - ref_dir = os.path.join(reference, repo) + # In a $CLAW layout the super-repo is the directory itself. + ref_dir = (reference if repo == SUPER_REPO + else os.path.join(reference, repo)) if os.path.isdir(os.path.join(ref_dir, '.git')): # --dissociate copies the borrowed objects in, so the result # does not break if the reference clone is later pruned. @@ -137,6 +150,84 @@ def materialise(dest: str, reference: str | None = None, 'work without the super-repo namespace shim') +def _subrepo_pins(super_dir: str, sha: str, + subrepos: set[str]) -> list[tuple[str, str]]: + """``(subrepo, commit)`` for each submodule pointer in super-repo *sha*. + + Read straight from the tree with ls-tree rather than through `git + submodule`, which would want .gitmodules URLs and network access of its + own. Only the subrepos the dev pin also lists are returned; anything + else a release happened to carry is not documented. + """ + out = subprocess.run(['git', '-C', super_dir, 'ls-tree', sha], + capture_output=True, text=True, check=True).stdout + pins = [] + for line in out.splitlines(): + meta, path = line.split('\t', 1) + _mode, kind, obj = meta.split() + if kind == 'commit' and path in subrepos: + pins.append((path, obj)) + return pins + + +def _release_plan(trees: str) -> list[tuple[str, str, str]]: + """``(version, repo, sha)`` for every repo of every release tree. + + The subrepo pins come from the super-repo, so it has to be present + first; callers materialise row by row in this order. + """ + subrepos = {repo for repo, _ in read_manifest() if repo != SUPER_REPO} + plan = [] + for version, sha in read_release_manifest().items(): + root = os.path.join(trees, version) + plan.append((version, SUPER_REPO, sha)) + if _head(root) is not None: + for repo, sub_sha in _subrepo_pins(root, sha, subrepos): + plan.append((version, repo, sub_sha)) + return plan + + +def materialise_releases(trees: str, reference: str | None = None, + quiet: bool = False) -> None: + """Build ``/`` for every row of the release manifest.""" + trees = os.path.abspath(trees) + subrepos = {repo for repo, _ in read_manifest() if repo != SUPER_REPO} + for version, sha in read_release_manifest().items(): + root = os.path.join(trees, version) + print(f'== {version}') + _materialise(SUPER_REPO, sha, root, reference, quiet) + pins = _subrepo_pins(root, sha, subrepos) + missing = subrepos - {repo for repo, _ in pins} + if missing: + raise SystemExit(f'{version}: super-repo {sha[:8]} has no ' + f'submodule pointer for {", ".join(sorted(missing))}') + for repo, sub_sha in pins: + _materialise(repo, sub_sha, os.path.join(root, repo), reference, + quiet) + + +def check_releases(trees: str) -> int: + """Report whether every release tree under *trees* matches its pins.""" + trees = os.path.abspath(trees) + problems = [] + for version, repo, sha in _release_plan(trees): + root = os.path.join(trees, version) + target = root if repo == SUPER_REPO else os.path.join(root, repo) + head = _head(target) + if head is None: + problems.append(f' {version:<9} {repo:<10} MISSING (want {sha[:8]})') + elif head != sha: + problems.append(f' {version:<9} {repo:<10} {head[:8]} != pinned {sha[:8]}') + if problems: + print(f'{trees} does not match tools/clawpack-release-refs.txt:\n') + print('\n'.join(problems)) + print('\nRun `make claw-releases` to materialise them.') + return 1 + print(f'{trees} matches tools/clawpack-release-refs.txt ' + f'({len(read_release_manifest())} releases).') + return 0 + + def check(dest: str) -> int: """Report whether *dest* matches the manifest. Returns an exit status.""" entries = read_manifest() @@ -172,8 +263,17 @@ def main(argv: list[str] | None = None) -> int: help='verify an existing tree instead of building one') parser.add_argument('--quiet', action='store_true', help='only report the resulting SHAs') + parser.add_argument('--releases', action='store_true', + help='build one tree per row of ' + 'tools/clawpack-release-refs.txt under dest') args = parser.parse_args(argv) + if args.releases: + if args.check: + return check_releases(args.dest) + materialise_releases(args.dest, args.reference, args.quiet) + return 0 + if args.check: return check(args.dest) diff --git a/doc/tools/test_compare_autodoc.py b/doc/tools/test_compare_autodoc.py new file mode 100644 index 0000000..041c4e1 --- /dev/null +++ b/doc/tools/test_compare_autodoc.py @@ -0,0 +1,113 @@ +""" +Tests for tools/compare_autodoc.py. + +Runnable either directly (``python tools/test_compare_autodoc.py``) or under +pytest. The two "revisions" are git trees built with hash-object/mktree in a +throwaway repository, so no commits and no Sphinx are involved. +""" + +import os +import shutil +import subprocess +import sys +import tempfile + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +import compare_autodoc # noqa: E402 + + +def page(entries): + return ''.join('
\n' % i + for i in range(entries)) + '

text

\n' + + +def tree(repo, files): + """Write *files* ({path: text}, one directory level at most) as a tree.""" + def blob(text): + return subprocess.run(['git', '-C', repo, 'hash-object', '-w', '--stdin'], + input=text.encode(), capture_output=True, + check=True).stdout.decode().strip() + + def mktree(entries): + listing = ''.join('%s %s %s\t%s\n' % e for e in entries) + return subprocess.run(['git', '-C', repo, 'mktree'], input=listing.encode(), + capture_output=True, check=True).stdout.decode().strip() + + top, subdirs = [], {} + for path, text in files.items(): + if '/' in path: + d, name = path.split('/', 1) + subdirs.setdefault(d, []).append(('100644', 'blob', blob(text), name)) + else: + top.append(('100644', 'blob', blob(text), path)) + for d, entries in subdirs.items(): + top.append(('040000', 'tree', mktree(entries), d)) + return mktree(top) + + +def with_repo(test): + def run(): + repo = tempfile.mkdtemp() + try: + subprocess.run(['git', 'init', '-q', repo], check=True) + test(repo) + finally: + shutil.rmtree(repo) + run.__name__ = test.__name__ + return run + + +@with_repo +def test_reports_emptied_and_shrunk_pages(repo): + before = tree(repo, {'storm_module.html': page(22), + 'v5.7.x/dtopo.html': page(73), + 'v5.7.x/topo.html': page(34), + 'about.html': '

no api

'}) + after = tree(repo, {'storm_module.html': page(0), + 'v5.7.x/dtopo.html': page(59), + 'v5.7.x/topo.html': page(35), + 'about.html': '

still no api

'}) + rows = compare_autodoc.compare(repo, before, after) + assert sorted(rows) == [('storm_module.html', 22, 0), + ('v5.7.x/dtopo.html', 73, 59), + ('v5.7.x/topo.html', 34, 35)] + text = compare_autodoc.report(rows, limit=40) + assert '2 page(s) lose entries, 1 of them down to zero' in text + # Emptied pages are listed first. + assert text.index('storm_module.html') < text.index('v5.7.x/dtopo.html') + assert 'topo.html` |' not in text.replace('dtopo.html', '') + + +@with_repo +def test_no_drops(repo): + before = tree(repo, {'a.html': page(3)}) + after = tree(repo, {'a.html': page(4) + '

edited

'}) + text = compare_autodoc.report(compare_autodoc.compare(repo, before, after), 40) + assert 'No page loses entries.' in text + + +@with_repo +def test_added_and_deleted_pages(repo): + before = tree(repo, {'gone.html': page(5), 'keep.html': page(1)}) + after = tree(repo, {'new.html': page(2), 'keep.html': page(1)}) + rows = sorted(compare_autodoc.compare(repo, before, after)) + assert rows == [('gone.html', 5, None), ('new.html', None, 2)] + text = compare_autodoc.report(rows, 40) + assert '| `gone.html` | 5 | - |' in text + + +@with_repo +def test_limit(repo): + before = tree(repo, {'p%d.html' % i: page(5) for i in range(5)}) + after = tree(repo, {'p%d.html' % i: page(4) for i in range(5)}) + text = compare_autodoc.report(compare_autodoc.compare(repo, before, after), 2) + assert '... 3 more' in text + + +if __name__ == '__main__': + tests = [v for k, v in sorted(globals().items()) if k.startswith('test_')] + for test in tests: + test() + print('ok %s' % test.__name__) + print('\n%d passed' % len(tests))