From bc1a5c5c88e524cfc7549cd7282de06a7a7ddbff Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Fri, 21 Jun 2024 19:29:46 +0200 Subject: [PATCH] [autogen] Add `--remove-old` option (#12456) A common "gotcha" of re-running `sphinx-autogen`, is that if there are changes it will not remove old files, leading to build errors for files not in a `toctree` This commit introduces a `--remove-old` option to remove these files. Note, a key detail here is that we don't want to simply clear the directory before running `sphinx-autogen`, since this would lead to all files having a new `mtime`, and then `sphinx-build` would rebuild all of them even if they have not changed. So we must first collect the list of all correct files, then remove any not in the list. --- CHANGES.rst | 2 + doc/man/sphinx-autogen.rst | 5 ++ sphinx/ext/apidoc.py | 10 ++- sphinx/ext/autosummary/generate.py | 82 +++++++++++++------ tests/test_extensions/test_ext_autosummary.py | 14 ++++ 5 files changed, 85 insertions(+), 28 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index df7f4fa12..0a31a6746 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,6 +13,8 @@ Deprecated Features added -------------- +* #12456: Add :option:`sphinx-autogen --remove-old` option. + Patch by Chris Sewell. * #12448: Add :option:`sphinx-apidoc --remove-old` option. Patch by Chris Sewell. * #12358: Add :attr:`.Sphinx.fresh_env_used`. diff --git a/doc/man/sphinx-autogen.rst b/doc/man/sphinx-autogen.rst index caeb44b12..43cc369b7 100644 --- a/doc/man/sphinx-autogen.rst +++ b/doc/man/sphinx-autogen.rst @@ -43,6 +43,11 @@ Options Document exactly the members in a module's ``__all__`` attribute. +.. option:: --remove-old + + Remove existing files in the output directory + that are not generated anymore. + Example ------- diff --git a/sphinx/ext/apidoc.py b/sphinx/ext/apidoc.py index bafbdf4a5..669387263 100644 --- a/sphinx/ext/apidoc.py +++ b/sphinx/ext/apidoc.py @@ -679,7 +679,15 @@ def main(argv: Sequence[str] = (), /) -> int: if args.remove_old and not args.dryrun: for existing in Path(args.destdir).glob(f'**/*.{args.suffix}'): if existing not in written_files: - existing.unlink() + try: + existing.unlink() + except OSError as exc: + logger.warning( + __('Failed to remove %s: %s'), + existing, + exc.strerror, + type='autodoc', + ) return 0 diff --git a/sphinx/ext/autosummary/generate.py b/sphinx/ext/autosummary/generate.py index 8cfceb9a6..d434d39f8 100644 --- a/sphinx/ext/autosummary/generate.py +++ b/sphinx/ext/autosummary/generate.py @@ -24,6 +24,7 @@ import pydoc import re import sys from os import path +from pathlib import Path from typing import TYPE_CHECKING, Any, NamedTuple from jinja2 import TemplateNotFound @@ -499,10 +500,16 @@ def generate_autosummary_docs( suffix: str = '.rst', base_path: str | os.PathLike[str] | None = None, imported_members: bool = False, - app: Any = None, + app: Sphinx | None = None, overwrite: bool = True, encoding: str = 'utf-8', -) -> None: +) -> list[Path]: + """Generate autosummary documentation for the given sources. + + :returns: list of generated files (both new and existing ones) + """ + assert app is not None, 'app is required' + showed_sources = sorted(sources) if len(showed_sources) > 20: showed_sources = showed_sources[:10] + ['...'] + showed_sources[-10:] @@ -520,12 +527,10 @@ def generate_autosummary_docs( items = find_autosummary_in_files(sources) # keep track of new files - new_files = [] + new_files: list[Path] = [] + all_files: list[Path] = [] - if app: - filename_map = app.config.autosummary_filename_map - else: - filename_map = {} + filename_map = app.config.autosummary_filename_map # write for entry in sorted(set(items), key=str): @@ -559,9 +564,7 @@ def generate_autosummary_docs( ) continue - context: dict[str, Any] = {} - if app: - context.update(app.config.autosummary_context) + context: dict[str, Any] = {**app.config.autosummary_context} content = generate_autosummary_content( name, @@ -577,34 +580,39 @@ def generate_autosummary_docs( qualname, ) - filename = os.path.join(path, filename_map.get(name, name) + suffix) - if os.path.isfile(filename): - with open(filename, encoding=encoding) as f: + file_path = Path(path, filename_map.get(name, name) + suffix) + all_files.append(file_path) + if file_path.is_file(): + with file_path.open(encoding=encoding) as f: old_content = f.read() if content == old_content: continue if overwrite: # content has changed - with open(filename, 'w', encoding=encoding) as f: + with file_path.open('w', encoding=encoding) as f: f.write(content) - new_files.append(filename) + new_files.append(file_path) else: - with open(filename, 'w', encoding=encoding) as f: + with open(file_path, 'w', encoding=encoding) as f: f.write(content) - new_files.append(filename) + new_files.append(file_path) # descend recursively to new files if new_files: - generate_autosummary_docs( - new_files, - output_dir=output_dir, - suffix=suffix, - base_path=base_path, - imported_members=imported_members, - app=app, - overwrite=overwrite, + all_files.extend( + generate_autosummary_docs( + [str(f) for f in new_files], + output_dir=output_dir, + suffix=suffix, + base_path=base_path, + imported_members=imported_members, + app=app, + overwrite=overwrite, + ) ) + return all_files + # -- Finding documented entries in files --------------------------------------- @@ -812,6 +820,13 @@ The format of the autosummary directive is documented in the '(default: %(default)s)' ), ) + parser.add_argument( + '--remove-old', + action='store_true', + dest='remove_old', + default=False, + help=__('Remove existing files in the output directory that were not generated'), + ) return parser @@ -829,14 +844,27 @@ def main(argv: Sequence[str] = (), /) -> None: app.config.templates_path.append(path.abspath(args.templates)) app.config.autosummary_ignore_module_all = not args.respect_module_all - generate_autosummary_docs( + written_files = generate_autosummary_docs( args.source_file, args.output_dir, '.' + args.suffix, imported_members=args.imported_members, - app=app, + app=app, # type: ignore[arg-type] ) + if args.remove_old: + for existing in Path(args.output_dir).glob(f'**/*.{args.suffix}'): + if existing not in written_files: + try: + existing.unlink() + except OSError as exc: + logger.warning( + __('Failed to remove %s: %s'), + existing, + exc.strerror, + type='autosummary', + ) + if __name__ == '__main__': main(sys.argv[1:]) diff --git a/tests/test_extensions/test_ext_autosummary.py b/tests/test_extensions/test_ext_autosummary.py index d761978da..5f767379d 100644 --- a/tests/test_extensions/test_ext_autosummary.py +++ b/tests/test_extensions/test_ext_autosummary.py @@ -684,3 +684,17 @@ def test_autogen(rootdir, tmp_path): args = ['-o', str(tmp_path), '-t', '.', 'autosummary_templating.txt'] autogen_main(args) assert (tmp_path / 'sphinx.application.TemplateBridge.rst').exists() + + +def test_autogen_remove_old(rootdir, tmp_path): + """Test the ``--remove-old`` option.""" + tmp_path.joinpath('other.rst').write_text('old content') + with chdir(rootdir / 'test-templating'): + args = ['-o', str(tmp_path), '-t', '.', 'autosummary_templating.txt'] + autogen_main(args) + assert set(tmp_path.iterdir()) == { + tmp_path / 'sphinx.application.TemplateBridge.rst', + tmp_path / 'other.rst' + } + autogen_main([*args, '--remove-old']) + assert set(tmp_path.iterdir()) == {tmp_path / 'sphinx.application.TemplateBridge.rst'}