From 4c097725917e0d267b9c3c87a74f4407d6b11cbc Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:02:11 +0100 Subject: [PATCH 01/12] Always ``--keep-going`` --- doc/man/sphinx-build.rst | 18 ++++++++++++++++-- sphinx/application.py | 5 +++-- sphinx/cmd/build.py | 1 + sphinx/testing/fixtures.py | 2 +- sphinx/testing/util.py | 2 +- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/doc/man/sphinx-build.rst b/doc/man/sphinx-build.rst index a8a21f8ce93..b99b933291b 100644 --- a/doc/man/sphinx-build.rst +++ b/doc/man/sphinx-build.rst @@ -253,11 +253,17 @@ Options .. option:: -W, --fail-on-warning - Turn warnings into errors. This means that the build stops at the first - warning and ``sphinx-build`` exits with exit status 1. + Turn warnings into errors. + This means that :program:`sphinx-build` exits with exit status 1 + if any warnings are generated during the build. .. versionchanged:: 7.3 Add ``--fail-on-warning`` long option. + .. versionchanged:: 8.1 + :program:`sphinx-build` no longer exits on the first warning, + but instead runs the entire build and exits with exit status 1 + if any warnings were generated. + This behaviour was previously enabled with :option:`--keep-going`. .. option:: --keep-going @@ -267,6 +273,14 @@ Options and exits with exit status 1 if errors are encountered. .. versionadded:: 1.8 + .. versionchanged:: 8.1 + :program:`sphinx-build` no longer exits on the first warning, + meaning that in effect :option:`!--fail-on-warning` is always enabled. + The option is retained for compatibility, but may be removed at some + later date. + + .. xref RemovedInSphinx10Warning: deprecate this option in Sphinx 10 + or no earlier than 2026-01-01. .. option:: -T, --show-traceback diff --git a/sphinx/application.py b/sphinx/application.py index a1589fb230c..37681d24120 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -143,7 +143,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st status: IO[str] | None = sys.stdout, warning: IO[str] | None = sys.stderr, freshenv: bool = False, warningiserror: bool = False, tags: Sequence[str] = (), - verbosity: int = 0, parallel: int = 0, keep_going: bool = False, + verbosity: int = 0, parallel: int = 0, keep_going: bool = True, pdb: bool = False) -> None: """Initialize the Sphinx application. @@ -163,7 +163,8 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st :param verbosity: The verbosity level. :param parallel: The maximum number of parallel jobs to use when reading/writing documents. - :param keep_going: If true, continue processing when an error occurs. + :param keep_going: If false, fail on the first warning. + Only used when warningiserror is true. :param pdb: If true, enable the Python debugger on an exception. """ self.phase = BuildPhase.INITIALIZATION diff --git a/sphinx/cmd/build.py b/sphinx/cmd/build.py index 02fd99a8ccc..f62f55d2b86 100644 --- a/sphinx/cmd/build.py +++ b/sphinx/cmd/build.py @@ -202,6 +202,7 @@ def get_parser() -> argparse.ArgumentParser: group.add_argument('--fail-on-warning', '-W', action='store_true', dest='warningiserror', help=__('turn warnings into errors')) group.add_argument('--keep-going', action='store_true', dest='keep_going', + default=True, help=__("with --fail-on-warning, keep going when getting warnings")) group.add_argument('--show-traceback', '-T', action='store_true', dest='traceback', help=__('show full traceback on exception')) diff --git a/sphinx/testing/fixtures.py b/sphinx/testing/fixtures.py index 388e5f6e3cc..24254a975ac 100644 --- a/sphinx/testing/fixtures.py +++ b/sphinx/testing/fixtures.py @@ -28,7 +28,7 @@ 'testroot="root", srcdir=None, ' 'confoverrides=None, freshenv=False, ' 'warningiserror=False, tags=None, verbosity=0, parallel=0, ' - 'keep_going=False, builddir=None, docutils_conf=None' + 'keep_going=True, builddir=None, docutils_conf=None' '): arguments to initialize the sphinx test application.' ), 'test_params(shared_result=...): test parameters.', diff --git a/sphinx/testing/util.py b/sphinx/testing/util.py index 53e55d6d5ce..79ceec08590 100644 --- a/sphinx/testing/util.py +++ b/sphinx/testing/util.py @@ -117,7 +117,7 @@ def __init__( parallel: int = 0, # additional arguments at the end to keep the signature verbosity: int = 0, # argument is not in the same order as in the superclass - keep_going: bool = False, + keep_going: bool = True, warningiserror: bool = False, # argument is not in the same order as in the superclass # unknown keyword arguments **extras: Any, From 4d7fab816a97ef82c1edbf683bea6831ae9fc6f0 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:43:51 +0100 Subject: [PATCH 02/12] Simplify things --- .github/workflows/builddoc.yml | 1 - CHANGES.rst | 6 +++ doc/internals/contributing.rst | 2 +- doc/man/sphinx-build.rst | 2 +- sphinx/application.py | 54 +++++++++++++------------- sphinx/builders/linkcheck.py | 4 +- sphinx/cmd/build.py | 6 +-- sphinx/ext/autodoc/__init__.py | 6 +-- sphinx/ext/autodoc/importer.py | 15 +++---- sphinx/ext/coverage.py | 8 ++-- sphinx/ext/doctest.py | 2 +- sphinx/testing/fixtures.py | 2 +- sphinx/testing/util.py | 3 +- sphinx/util/logging.py | 58 +--------------------------- tests/test_util/test_util_logging.py | 42 -------------------- 15 files changed, 57 insertions(+), 154 deletions(-) diff --git a/.github/workflows/builddoc.yml b/.github/workflows/builddoc.yml index b5f82592360..e252665cd2d 100644 --- a/.github/workflows/builddoc.yml +++ b/.github/workflows/builddoc.yml @@ -39,4 +39,3 @@ jobs: --jobs=auto --show-traceback --fail-on-warning - --keep-going diff --git a/CHANGES.rst b/CHANGES.rst index 4b920ca3911..cceab8e429e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,6 +13,12 @@ Deprecated Features added -------------- +* #12743: No longer exit on the first warning when + :option:`--fail-on-warning ` is used. + Instead, exit with a non-zero status if any warnings were generated + during the build. + Patch by Adam Turner. + Bugs fixed ---------- diff --git a/doc/internals/contributing.rst b/doc/internals/contributing.rst index e2981c0a1e6..410f7057cbc 100644 --- a/doc/internals/contributing.rst +++ b/doc/internals/contributing.rst @@ -266,7 +266,7 @@ To build the documentation, run the following command: .. code-block:: shell - sphinx-build -M html ./doc ./build/sphinx --fail-on-warning --keep-going + sphinx-build -M html ./doc ./build/sphinx --fail-on-warning This will parse the Sphinx documentation's source files and generate HTML for you to preview in :file:`build/sphinx/html`. diff --git a/doc/man/sphinx-build.rst b/doc/man/sphinx-build.rst index b99b933291b..0a189bc3dc1 100644 --- a/doc/man/sphinx-build.rst +++ b/doc/man/sphinx-build.rst @@ -43,7 +43,7 @@ Options the source and output directories, before any other options are passed. For example:: - sphinx-build -M html ./source ./build --fail-on-warning --keep-going + sphinx-build -M html ./source ./build --fail-on-warning The *make-mode* provides the same build functionality as a default :ref:`Makefile or Make.bat `, diff --git a/sphinx/application.py b/sphinx/application.py index 37681d24120..7e9600b16d5 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -41,6 +41,8 @@ from sphinx.util.tags import Tags if TYPE_CHECKING: + from typing import Final + from docutils import nodes from docutils.nodes import Element, Node from docutils.parsers import Parser @@ -134,7 +136,7 @@ class Sphinx: :ivar outdir: Directory for storing build documents. """ - warningiserror: bool + warningiserror: Final = False _warncount: int def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[str] | None, @@ -143,7 +145,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st status: IO[str] | None = sys.stdout, warning: IO[str] | None = sys.stderr, freshenv: bool = False, warningiserror: bool = False, tags: Sequence[str] = (), - verbosity: int = 0, parallel: int = 0, keep_going: bool = True, + verbosity: int = 0, parallel: int = 0, pdb: bool = False) -> None: """Initialize the Sphinx application. @@ -163,8 +165,6 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st :param verbosity: The verbosity level. :param parallel: The maximum number of parallel jobs to use when reading/writing documents. - :param keep_going: If false, fail on the first warning. - Only used when warningiserror is true. :param pdb: If true, enable the Python debugger on an exception. """ self.phase = BuildPhase.INITIALIZATION @@ -204,11 +204,8 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st else: self._warning = warning self._warncount = 0 - self.keep_going = warningiserror and keep_going - if self.keep_going: - self.warningiserror = False - else: - self.warningiserror = warningiserror + self.keep_going = bool(warningiserror) + self._warning_is_error = bool(warningiserror) self.pdb = pdb logging.setup(self, self._status, self._warning) @@ -387,26 +384,29 @@ def build(self, force_all: bool = False, filenames: list[str] | None = None) -> self.events.emit('build-finished', err) raise - if self._warncount and self.keep_going: - self.statuscode = 1 - - status = (__('succeeded') if self.statuscode == 0 - else __('finished with problems')) - if self._warncount: - if self.warningiserror: - if self._warncount == 1: - msg = __('build %s, %s warning (with warnings treated as errors).') - else: - msg = __('build %s, %s warnings (with warnings treated as errors).') + if self._warncount == 0: + if self.statuscode != 0: + logger.info(bold(__('build finished with problems.'))) else: - if self._warncount == 1: - msg = __('build %s, %s warning.') - else: - msg = __('build %s, %s warnings.') - - logger.info(bold(msg), status, self._warncount) + logger.info(bold(__('build succeeded.'))) + elif self._warncount == 1: + if self._warning_is_error: + self.statuscode = 1 + msg = __('build finished with problems, %s warning (with warnings treated as errors).') + elif self.statuscode != 0: + msg = __('build finished with problems, %s warning.') + else: + msg = __('build succeeded, %s warning.') + logger.info(bold(msg)) else: - logger.info(bold(__('build %s.')), status) + if self._warning_is_error: + self.statuscode = 1 + msg = __('build finished with problems, %s warnings (with warnings treated as errors).') + elif self.statuscode != 0: + msg = __('build finished with problems, %s warnings.') + else: + msg = __('build succeeded, %s warnings.') + logger.info(bold(msg), self._warncount) if self.statuscode == 0 and self.builder.epilog: logger.info('') diff --git a/sphinx/builders/linkcheck.py b/sphinx/builders/linkcheck.py index 5352b25936b..e9b07164eaa 100644 --- a/sphinx/builders/linkcheck.py +++ b/sphinx/builders/linkcheck.py @@ -106,7 +106,7 @@ def process_result(self, result: CheckResult) -> None: elif result.status == 'working': logger.info(darkgreen('ok ') + result.uri + result.message) elif result.status == 'timeout': - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning('timeout ' + result.uri + result.message, location=(result.docname, result.lineno)) else: @@ -115,7 +115,7 @@ def process_result(self, result: CheckResult) -> None: result.uri + ': ' + result.message) self.timed_out_hyperlinks += 1 elif result.status == 'broken': - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning(__('broken link: %s (%s)'), result.uri, result.message, location=(result.docname, result.lineno)) else: diff --git a/sphinx/cmd/build.py b/sphinx/cmd/build.py index f62f55d2b86..88f4eae018d 100644 --- a/sphinx/cmd/build.py +++ b/sphinx/cmd/build.py @@ -201,9 +201,7 @@ def get_parser() -> argparse.ArgumentParser: help=__('write warnings (and errors) to given file')) group.add_argument('--fail-on-warning', '-W', action='store_true', dest='warningiserror', help=__('turn warnings into errors')) - group.add_argument('--keep-going', action='store_true', dest='keep_going', - default=True, - help=__("with --fail-on-warning, keep going when getting warnings")) + group.add_argument('--keep-going', action='store_true', help=argparse.SUPPRESS) group.add_argument('--show-traceback', '-T', action='store_true', dest='traceback', help=__('show full traceback on exception')) group.add_argument('--pdb', '-P', action='store_true', dest='pdb', @@ -333,7 +331,7 @@ def build_main(argv: Sequence[str]) -> int: app = Sphinx(args.sourcedir, args.confdir, args.outputdir, args.doctreedir, args.builder, args.confoverrides, args.status, args.warning, args.freshenv, args.warningiserror, - args.tags, args.verbosity, args.jobs, args.keep_going, + args.tags, args.verbosity, args.jobs, args.pdb) app.build(args.force_all, args.filenames) return app.statuscode diff --git a/sphinx/ext/autodoc/__init__.py b/sphinx/ext/autodoc/__init__.py index e318d83a00e..5d19f5e88fc 100644 --- a/sphinx/ext/autodoc/__init__.py +++ b/sphinx/ext/autodoc/__init__.py @@ -417,7 +417,7 @@ def import_object(self, raiseerror: bool = False) -> bool: try: ret = import_object(self.modname, self.objpath, self.objtype, attrgetter=self.get_attr, - warningiserror=self.config.autodoc_warningiserror) + ) self.module, self.parent, self.object_name, self.object = ret if ismock(self.object): self.object = undecorate(self.object) @@ -2457,7 +2457,7 @@ def import_object(self, raiseerror: bool = False) -> bool: with mock(self.config.autodoc_mock_imports): ret = import_object(self.modname, self.objpath[:-1], 'class', attrgetter=self.get_attr, # type: ignore[attr-defined] - warningiserror=self.config.autodoc_warningiserror) + ) parent = ret[3] if self.is_runtime_instance_attribute(parent): self.object = self.RUNTIME_INSTANCE_ATTRIBUTE @@ -2511,7 +2511,7 @@ def import_object(self, raiseerror: bool = False) -> bool: try: ret = import_object(self.modname, self.objpath[:-1], 'class', attrgetter=self.get_attr, # type: ignore[attr-defined] - warningiserror=self.config.autodoc_warningiserror) + ) parent = ret[3] if self.is_uninitialized_instance_attribute(parent): self.object = UNINITIALIZED_ATTR diff --git a/sphinx/ext/autodoc/importer.py b/sphinx/ext/autodoc/importer.py index dd28146b0dd..ebdaa984888 100644 --- a/sphinx/ext/autodoc/importer.py +++ b/sphinx/ext/autodoc/importer.py @@ -137,24 +137,22 @@ def unmangle(subject: Any, name: str) -> str | None: return name -def import_module(modname: str, warningiserror: bool = False) -> Any: +def import_module(modname: str) -> Any: """Call importlib.import_module(modname), convert exceptions to ImportError.""" try: - with logging.skip_warningiserror(not warningiserror): - return importlib.import_module(modname) + return importlib.import_module(modname) except BaseException as exc: # Importing modules may cause any side effects, including # SystemExit, so we need to catch all errors. raise ImportError(exc, traceback.format_exc()) from exc -def _reload_module(module: ModuleType, warningiserror: bool = False) -> Any: +def _reload_module(module: ModuleType) -> Any: """ Call importlib.reload(module), convert exceptions to ImportError """ try: - with logging.skip_warningiserror(not warningiserror): - return importlib.reload(module) + return importlib.reload(module) except BaseException as exc: # Importing modules may cause any side effects, including # SystemExit, so we need to catch all errors. @@ -162,8 +160,7 @@ def _reload_module(module: ModuleType, warningiserror: bool = False) -> Any: def import_object(modname: str, objpath: list[str], objtype: str = '', - attrgetter: Callable[[Any, str], Any] = safe_getattr, - warningiserror: bool = False) -> Any: + attrgetter: Callable[[Any, str], Any] = safe_getattr) -> Any: if objpath: logger.debug('[autodoc] from %s import %s', modname, '.'.join(objpath)) else: @@ -176,7 +173,7 @@ def import_object(modname: str, objpath: list[str], objtype: str = '', while module is None: try: original_module_names = frozenset(sys.modules) - module = import_module(modname, warningiserror=warningiserror) + module = import_module(modname) if os.environ.get('SPHINX_AUTODOC_RELOAD_MODULES'): new_modules = [m for m in sys.modules if m not in original_module_names] # Try reloading modules with ``typing.TYPE_CHECKING == True``. diff --git a/sphinx/ext/coverage.py b/sphinx/ext/coverage.py index f7ce3beaa65..7790cf31663 100644 --- a/sphinx/ext/coverage.py +++ b/sphinx/ext/coverage.py @@ -241,7 +241,7 @@ def write_c_coverage(self) -> None: for typ, name in sorted(undoc): op.write(' * %-50s [%9s]\n' % (name, typ)) if self.config.coverage_show_missing_items: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning(__('undocumented c api: %s [%s] in file %s'), name, typ, filename) else: @@ -423,7 +423,7 @@ def write_py_coverage(self) -> None: op.write('Functions:\n') op.writelines(' * %s\n' % x for x in undoc['funcs']) if self.config.coverage_show_missing_items: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: for func in undoc['funcs']: logger.warning( __('undocumented python function: %s :: %s'), @@ -440,7 +440,7 @@ def write_py_coverage(self) -> None: if not methods: op.write(' * %s\n' % class_name) if self.config.coverage_show_missing_items: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning( __('undocumented python class: %s :: %s'), name, class_name) @@ -452,7 +452,7 @@ def write_py_coverage(self) -> None: op.write(' * %s -- missing methods:\n\n' % class_name) op.writelines(' - %s\n' % x for x in methods) if self.config.coverage_show_missing_items: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: for meth in methods: logger.warning( __('undocumented python method:' + diff --git a/sphinx/ext/doctest.py b/sphinx/ext/doctest.py index 19f01f4cab6..ba451208a5e 100644 --- a/sphinx/ext/doctest.py +++ b/sphinx/ext/doctest.py @@ -322,7 +322,7 @@ def _out(self, text: str) -> None: self.outfile.write(text) def _warn_out(self, text: str) -> None: - if self.app.quiet or self.app.warningiserror: + if self.app.quiet: logger.warning(text) else: logger.info(text, nonl=True) diff --git a/sphinx/testing/fixtures.py b/sphinx/testing/fixtures.py index 24254a975ac..414b03879d5 100644 --- a/sphinx/testing/fixtures.py +++ b/sphinx/testing/fixtures.py @@ -28,7 +28,7 @@ 'testroot="root", srcdir=None, ' 'confoverrides=None, freshenv=False, ' 'warningiserror=False, tags=None, verbosity=0, parallel=0, ' - 'keep_going=True, builddir=None, docutils_conf=None' + 'builddir=None, docutils_conf=None' '): arguments to initialize the sphinx test application.' ), 'test_params(shared_result=...): test parameters.', diff --git a/sphinx/testing/util.py b/sphinx/testing/util.py index 79ceec08590..30bad74c91f 100644 --- a/sphinx/testing/util.py +++ b/sphinx/testing/util.py @@ -117,7 +117,6 @@ def __init__( parallel: int = 0, # additional arguments at the end to keep the signature verbosity: int = 0, # argument is not in the same order as in the superclass - keep_going: bool = True, warningiserror: bool = False, # argument is not in the same order as in the superclass # unknown keyword arguments **extras: Any, @@ -170,7 +169,7 @@ def __init__( srcdir, confdir, outdir, doctreedir, buildername, confoverrides=confoverrides, status=status, warning=warning, freshenv=freshenv, warningiserror=warningiserror, tags=tags, - verbosity=verbosity, parallel=parallel, keep_going=keep_going, + verbosity=verbosity, parallel=parallel, pdb=False, ) except Exception: diff --git a/sphinx/util/logging.py b/sphinx/util/logging.py index 4816bcbbe00..3d19a04093f 100644 --- a/sphinx/util/logging.py +++ b/sphinx/util/logging.py @@ -11,7 +11,6 @@ from docutils import nodes from docutils.utils import get_source_line -from sphinx.errors import SphinxWarning from sphinx.util.console import colorize from sphinx.util.osutil import abspath @@ -324,22 +323,8 @@ def pending_logging() -> Iterator[MemoryHandler]: @contextmanager def skip_warningiserror(skip: bool = True) -> Iterator[None]: - """Context manager to skip WarningIsErrorFilter temporarily.""" - logger = logging.getLogger(NAMESPACE) - - if skip is False: - yield - else: - try: - disabler = DisableWarningIsErrorFilter() - for handler in logger.handlers: - # use internal method; filters.insert() directly to install disabler - # before WarningIsErrorFilter - handler.filters.insert(0, disabler) - yield - finally: - for handler in logger.handlers: - handler.removeFilter(disabler) + # Deprecate in Sphinx 10 + yield @contextmanager @@ -445,44 +430,6 @@ def filter(self, record: logging.LogRecord) -> bool: return True -class WarningIsErrorFilter(logging.Filter): - """Raise exception if warning emitted.""" - - def __init__(self, app: Sphinx) -> None: - self.app = app - super().__init__() - - def filter(self, record: logging.LogRecord) -> bool: - if getattr(record, 'skip_warningsiserror', False): - # disabled by DisableWarningIsErrorFilter - return True - elif self.app.warningiserror: - location = getattr(record, 'location', '') - try: - message = record.msg % record.args - except (TypeError, ValueError): - message = record.msg # use record.msg itself - - if location: - exc = SphinxWarning(location + ":" + str(message)) - else: - exc = SphinxWarning(message) - if record.exc_info is not None: - raise exc from record.exc_info[1] - else: - raise exc - else: - return True - - -class DisableWarningIsErrorFilter(logging.Filter): - """Disable WarningIsErrorFilter if this filter installed.""" - - def filter(self, record: logging.LogRecord) -> bool: - record.skip_warningsiserror = True - return True - - class MessagePrefixFilter(logging.Filter): """Prepend prefix to all log records.""" @@ -655,7 +602,6 @@ def setup(app: Sphinx, status: IO, warning: IO) -> None: warning_handler = WarningStreamHandler(SafeEncodingWriter(warning)) warning_handler.addFilter(WarningSuppressor(app)) warning_handler.addFilter(WarningLogRecordTranslator(app)) - warning_handler.addFilter(WarningIsErrorFilter(app)) warning_handler.addFilter(OnceFilter()) warning_handler.setLevel(logging.WARNING) warning_handler.setFormatter(ColorizeFormatter()) diff --git a/tests/test_util/test_util_logging.py b/tests/test_util/test_util_logging.py index aa868007161..bc7bf3e1b2c 100644 --- a/tests/test_util/test_util_logging.py +++ b/tests/test_util/test_util_logging.py @@ -169,24 +169,6 @@ def test_suppress_warnings(app): assert app._warncount == 8 -def test_warningiserror(app): - logging.setup(app, app.status, app.warning) - logger = logging.getLogger(__name__) - - # if False, warning is not error - app.warningiserror = False - logger.warning('message') - - # if True, warning raises SphinxWarning exception - app.warningiserror = True - with pytest.raises(SphinxWarning): - logger.warning('message: %s', 'arg') - - # message contains format string (refs: #4070) - with pytest.raises(SphinxWarning): - logger.warning('%s') - - def test_info_location(app): logging.setup(app, app.status, app.warning) logger = logging.getLogger(__name__) @@ -339,30 +321,6 @@ def write(self, object): assert app.status.getvalue() == "unicode ?...\n" -def test_skip_warningiserror(app): - logging.setup(app, app.status, app.warning) - logger = logging.getLogger(__name__) - - app.warningiserror = True - with logging.skip_warningiserror(): - logger.warning('message') - - # if False, warning raises SphinxWarning exception - with logging.skip_warningiserror(False): # NoQA: SIM117 - with pytest.raises(SphinxWarning): - logger.warning('message') - - # It also works during pending_warnings. - with logging.pending_warnings(): # NoQA: SIM117 - with logging.skip_warningiserror(): - logger.warning('message') - - with pytest.raises(SphinxWarning): # NoQA: PT012,SIM117 - with logging.pending_warnings(): - with logging.skip_warningiserror(False): - logger.warning('message') - - def test_prefixed_warnings(app): logging.setup(app, app.status, app.warning) logger = logging.getLogger(__name__) From af40e776781e4cda8d9164a22795c8c64e51b650 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:51:17 +0100 Subject: [PATCH 03/12] ruff --- sphinx/application.py | 6 ++++-- tests/test_util/test_util_logging.py | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sphinx/application.py b/sphinx/application.py index 7e9600b16d5..a994e4bd57b 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -392,7 +392,8 @@ def build(self, force_all: bool = False, filenames: list[str] | None = None) -> elif self._warncount == 1: if self._warning_is_error: self.statuscode = 1 - msg = __('build finished with problems, %s warning (with warnings treated as errors).') + msg = __('build finished with problems, %s warning ' + '(with warnings treated as errors).') elif self.statuscode != 0: msg = __('build finished with problems, %s warning.') else: @@ -401,7 +402,8 @@ def build(self, force_all: bool = False, filenames: list[str] | None = None) -> else: if self._warning_is_error: self.statuscode = 1 - msg = __('build finished with problems, %s warnings (with warnings treated as errors).') + msg = __('build finished with problems, %s warnings ' + '(with warnings treated as errors).') elif self.statuscode != 0: msg = __('build finished with problems, %s warnings.') else: diff --git a/tests/test_util/test_util_logging.py b/tests/test_util/test_util_logging.py index bc7bf3e1b2c..0b40d50977a 100644 --- a/tests/test_util/test_util_logging.py +++ b/tests/test_util/test_util_logging.py @@ -7,7 +7,6 @@ import pytest from docutils import nodes -from sphinx.errors import SphinxWarning from sphinx.util import logging, osutil from sphinx.util.console import colorize, strip_colors from sphinx.util.logging import is_suppressed_warning, prefixed_warnings From 7dbe321d5e379a99fdec0437f2699a05b38a3207 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:55:48 +0100 Subject: [PATCH 04/12] autodoc_warningiserror --- doc/usage/extensions/autodoc.rst | 4 ++++ sphinx/ext/autodoc/__init__.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/usage/extensions/autodoc.rst b/doc/usage/extensions/autodoc.rst index 7495d3a9681..289218f8093 100644 --- a/doc/usage/extensions/autodoc.rst +++ b/doc/usage/extensions/autodoc.rst @@ -770,6 +770,10 @@ There are also config values that you can set: If ``False`` is given, autodoc forcedly suppresses the error if the imported module emits warnings. By default, ``True``. + .. versionchanged:: 8.1 + This option now has no effect as :option:`!--fail-on-warning` + no longer exits early. + .. confval:: autodoc_inherit_docstrings This value controls the docstrings inheritance. diff --git a/sphinx/ext/autodoc/__init__.py b/sphinx/ext/autodoc/__init__.py index 5d19f5e88fc..391248b9427 100644 --- a/sphinx/ext/autodoc/__init__.py +++ b/sphinx/ext/autodoc/__init__.py @@ -1960,7 +1960,7 @@ def import_object(self, raiseerror: bool = False) -> bool: # annotation only instance variable (PEP-526) try: with mock(self.config.autodoc_mock_imports): - parent = import_module(self.modname, self.config.autodoc_warningiserror) + parent = import_module(self.modname) annotations = get_type_hints(parent, None, self.config.autodoc_type_aliases, include_extras=True) From d14b2e0eb427a7259680aecfc662c04405451962 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:58:07 +0100 Subject: [PATCH 05/12] _fail_on_warnings --- sphinx/application.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sphinx/application.py b/sphinx/application.py index a994e4bd57b..3d7c95f6738 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -205,7 +205,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st self._warning = warning self._warncount = 0 self.keep_going = bool(warningiserror) - self._warning_is_error = bool(warningiserror) + self._fail_on_warnings = bool(warningiserror) self.pdb = pdb logging.setup(self, self._status, self._warning) @@ -390,7 +390,7 @@ def build(self, force_all: bool = False, filenames: list[str] | None = None) -> else: logger.info(bold(__('build succeeded.'))) elif self._warncount == 1: - if self._warning_is_error: + if self._fail_on_warnings: self.statuscode = 1 msg = __('build finished with problems, %s warning ' '(with warnings treated as errors).') @@ -400,7 +400,7 @@ def build(self, force_all: bool = False, filenames: list[str] | None = None) -> msg = __('build succeeded, %s warning.') logger.info(bold(msg)) else: - if self._warning_is_error: + if self._fail_on_warnings: self.statuscode = 1 msg = __('build finished with problems, %s warnings ' '(with warnings treated as errors).') From 9146d8dc42cd65c54afd3ffa6eb5a11613ac0eb8 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Fri, 9 Aug 2024 23:47:59 +0100 Subject: [PATCH 06/12] Add --debug-warnings Previously, warnings emitted during reading and writing were deferred, which made --pdb ineffective for debugging them. With this change, warnings are no longer deferred when --pdb and --debug-warnings are both specified. Co-authored-by: Jeremy Maitin-Shepard --- CHANGES.rst | 3 +++ doc/man/sphinx-build.rst | 7 +++++++ sphinx/application.py | 4 +++- sphinx/builders/__init__.py | 5 +++-- sphinx/cmd/build.py | 2 ++ sphinx/testing/util.py | 4 +++- sphinx/util/logging.py | 19 +++++++++++++++++++ tests/test_builders/test_build_warnings.py | 19 +++++++++++++++++++ 8 files changed, 59 insertions(+), 4 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index cceab8e429e..eebde9b3a9b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -18,6 +18,9 @@ Features added Instead, exit with a non-zero status if any warnings were generated during the build. Patch by Adam Turner. +* #12743: Add :option:`sphinx-build --debug-warnings` to debug warnings when + :option:`sphinx-build --pdb` is specified. + Patch by Adam Turner and Jeremy Maitin-Shepard. Bugs fixed ---------- diff --git a/doc/man/sphinx-build.rst b/doc/man/sphinx-build.rst index 0a189bc3dc1..5c066c56924 100644 --- a/doc/man/sphinx-build.rst +++ b/doc/man/sphinx-build.rst @@ -301,6 +301,13 @@ Options .. versionchanged:: 7.3 Add ``--pdb`` long option. +.. option:: --debug-warnings + + (Requires :option:`--pdb`.) + Run the Python debugger, :mod:`pdb`, for warnings emitted while building. + + .. versionadded:: 8.1 + .. option:: -h, --help, --version Display usage summary or Sphinx version. diff --git a/sphinx/application.py b/sphinx/application.py index 3d7c95f6738..34a17b860f5 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -146,7 +146,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st freshenv: bool = False, warningiserror: bool = False, tags: Sequence[str] = (), verbosity: int = 0, parallel: int = 0, - pdb: bool = False) -> None: + pdb: bool = False, debug_warnings: bool = False) -> None: """Initialize the Sphinx application. :param srcdir: The path to the source directory. @@ -166,6 +166,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st :param parallel: The maximum number of parallel jobs to use when reading/writing documents. :param pdb: If true, enable the Python debugger on an exception. + :param debug_warnings: If true, enable the Python debugger on warnings. """ self.phase = BuildPhase.INITIALIZATION self.verbosity = verbosity @@ -207,6 +208,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st self.keep_going = bool(warningiserror) self._fail_on_warnings = bool(warningiserror) self.pdb = pdb + self._debug_warnings = pdb and debug_warnings logging.setup(self, self._status, self._warning) self.events = EventManager(self) diff --git a/sphinx/builders/__init__.py b/sphinx/builders/__init__.py index 151df6aeb18..c5799da0dd5 100644 --- a/sphinx/builders/__init__.py +++ b/sphinx/builders/__init__.py @@ -6,6 +6,7 @@ import pickle import re import time +from contextlib import nullcontext from os import path from typing import TYPE_CHECKING, Any, Literal, final @@ -313,7 +314,7 @@ def build( logger.info(bold(__('building [%s]: ')) + summary, self.name) # while reading, collect all warnings from docutils - with logging.pending_warnings(): + with nullcontext() if self.app._debug_warnings else logging.pending_warnings(): updated_docnames = set(self.read()) doccount = len(updated_docnames) @@ -613,7 +614,7 @@ def write( self._write_serial(sorted(docnames)) def _write_serial(self, docnames: Sequence[str]) -> None: - with logging.pending_warnings(): + with nullcontext() if self.app._debug_warnings else logging.pending_warnings(): for docname in status_iterator(docnames, __('writing output... '), "darkgreen", len(docnames), self.app.verbosity): self.app.phase = BuildPhase.RESOLVING diff --git a/sphinx/cmd/build.py b/sphinx/cmd/build.py index 88f4eae018d..865c73f45b2 100644 --- a/sphinx/cmd/build.py +++ b/sphinx/cmd/build.py @@ -206,6 +206,8 @@ def get_parser() -> argparse.ArgumentParser: help=__('show full traceback on exception')) group.add_argument('--pdb', '-P', action='store_true', dest='pdb', help=__('run Pdb on exception')) + group.add_argument('--debug-warnings', action='store_true', + help=__('run Pdb on warnings (requires --pdb)')) return parser diff --git a/sphinx/testing/util.py b/sphinx/testing/util.py index 30bad74c91f..2b33d255cd9 100644 --- a/sphinx/testing/util.py +++ b/sphinx/testing/util.py @@ -118,6 +118,8 @@ def __init__( # additional arguments at the end to keep the signature verbosity: int = 0, # argument is not in the same order as in the superclass warningiserror: bool = False, # argument is not in the same order as in the superclass + pdb: bool = False, + debug_warnings: bool = False, # unknown keyword arguments **extras: Any, ) -> None: @@ -170,7 +172,7 @@ def __init__( confoverrides=confoverrides, status=status, warning=warning, freshenv=freshenv, warningiserror=warningiserror, tags=tags, verbosity=verbosity, parallel=parallel, - pdb=False, + pdb=pdb, debug_warnings=debug_warnings, ) except Exception: self.cleanup() diff --git a/sphinx/util/logging.py b/sphinx/util/logging.py index 3d19a04093f..c6fe00a2bdf 100644 --- a/sphinx/util/logging.py +++ b/sphinx/util/logging.py @@ -11,11 +11,13 @@ from docutils import nodes from docutils.utils import get_source_line +from sphinx.errors import SphinxWarning from sphinx.util.console import colorize from sphinx.util.osutil import abspath if TYPE_CHECKING: from collections.abc import Iterator, Sequence, Set + from typing import NoReturn from docutils.nodes import Node @@ -392,6 +394,21 @@ def filter(self, record: logging.LogRecord) -> bool: return record.levelno < logging.WARNING +class _RaiseOnWarningFilter(logging.Filter): + """Raise exception if a warning is emitted.""" + + def filter(self, record: logging.LogRecord) -> NoReturn: + try: + message = record.msg % record.args + except (TypeError, ValueError): + message = record.msg # use record.msg itself + if location := getattr(record, 'location', ''): + message = f"{location}:{message}" + if record.exc_info is not None: + raise SphinxWarning(message) from record.exc_info[1] + raise SphinxWarning(message) + + def is_suppressed_warning( warning_type: str, sub_type: str, suppress_warnings: Set[str] | Sequence[str], ) -> bool: @@ -600,6 +617,8 @@ def setup(app: Sphinx, status: IO, warning: IO) -> None: info_handler.setFormatter(ColorizeFormatter()) warning_handler = WarningStreamHandler(SafeEncodingWriter(warning)) + if app._debug_warnings: + warning_handler.addFilter(_RaiseOnWarningFilter()) warning_handler.addFilter(WarningSuppressor(app)) warning_handler.addFilter(WarningLogRecordTranslator(app)) warning_handler.addFilter(OnceFilter()) diff --git a/tests/test_builders/test_build_warnings.py b/tests/test_builders/test_build_warnings.py index 0ccade75e07..6606d464e4f 100644 --- a/tests/test_builders/test_build_warnings.py +++ b/tests/test_builders/test_build_warnings.py @@ -1,9 +1,11 @@ import os import re import sys +import traceback import pytest +from sphinx.errors import SphinxError from sphinx.util.console import strip_colors ENV_WARNINGS = """\ @@ -58,6 +60,23 @@ def test_html_warnings(app): _check_warnings(warnings_exp, app.warning.getvalue()) +@pytest.mark.sphinx( + 'html', + testroot='warnings', + freshenv=True, + pdb=True, + debug_warnings=True, +) +def test_html_warnings_pdb(app): + try: + app.build(force_all=True) + pytest.fail('Expected an exception to be raised') + except SphinxError: + tb = traceback.format_exc() + assert 'unindent_warning' in tb + assert 'pending_warnings' not in tb + + @pytest.mark.sphinx('latex', testroot='warnings', freshenv=True) def test_latex_warnings(app): app.build(force_all=True) From cbabdeb01bae1ac25e81d914dd603675f58c4f00 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Fri, 9 Aug 2024 23:55:42 +0100 Subject: [PATCH 07/12] DummyApplication --- sphinx/ext/autosummary/generate.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sphinx/ext/autosummary/generate.py b/sphinx/ext/autosummary/generate.py index 356043e1e68..41ca280ad13 100644 --- a/sphinx/ext/autosummary/generate.py +++ b/sphinx/ext/autosummary/generate.py @@ -72,6 +72,7 @@ def __init__(self, translator: NullTranslations) -> None: self.verbosity = 0 self._warncount = 0 self.warningiserror = False + self._debug_warnings = False self.config.add('autosummary_context', {}, 'env', ()) self.config.add('autosummary_filename_map', {}, 'env', ()) From 227685c3d643701f502bf7e69d00aa38890cf7ca Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Tue, 13 Aug 2024 16:10:25 +0100 Subject: [PATCH 08/12] fixups --- CHANGES.rst | 2 +- sphinx/application.py | 9 +++++---- sphinx/cmd/build.py | 16 ++++++++++------ sphinx/ext/autodoc/__init__.py | 21 ++++++++++++--------- tests/test_quickstart.py | 10 +++++----- 5 files changed, 33 insertions(+), 25 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index eebde9b3a9b..20ac3d3a21c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -14,7 +14,7 @@ Features added -------------- * #12743: No longer exit on the first warning when - :option:`--fail-on-warning ` is used. + :option:`--fail-on-warning ` is used. Instead, exit with a non-zero status if any warnings were generated during the build. Patch by Adam Turner. diff --git a/sphinx/application.py b/sphinx/application.py index 34a17b860f5..188a2d156e9 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -145,7 +145,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st status: IO[str] | None = sys.stdout, warning: IO[str] | None = sys.stderr, freshenv: bool = False, warningiserror: bool = False, tags: Sequence[str] = (), - verbosity: int = 0, parallel: int = 0, + verbosity: int = 0, parallel: int = 0, keep_going: bool = False, pdb: bool = False, debug_warnings: bool = False) -> None: """Initialize the Sphinx application. @@ -165,6 +165,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st :param verbosity: The verbosity level. :param parallel: The maximum number of parallel jobs to use when reading/writing documents. + :param keep_going: Unused. :param pdb: If true, enable the Python debugger on an exception. :param debug_warnings: If true, enable the Python debugger on warnings. """ @@ -394,12 +395,12 @@ def build(self, force_all: bool = False, filenames: list[str] | None = None) -> elif self._warncount == 1: if self._fail_on_warnings: self.statuscode = 1 - msg = __('build finished with problems, %s warning ' + msg = __('build finished with problems, 1 warning ' '(with warnings treated as errors).') elif self.statuscode != 0: - msg = __('build finished with problems, %s warning.') + msg = __('build finished with problems, 1 warning.') else: - msg = __('build succeeded, %s warning.') + msg = __('build succeeded, 1 warning.') logger.info(bold(msg)) else: if self._fail_on_warnings: diff --git a/sphinx/cmd/build.py b/sphinx/cmd/build.py index 865c73f45b2..b63713671b3 100644 --- a/sphinx/cmd/build.py +++ b/sphinx/cmd/build.py @@ -206,7 +206,7 @@ def get_parser() -> argparse.ArgumentParser: help=__('show full traceback on exception')) group.add_argument('--pdb', '-P', action='store_true', dest='pdb', help=__('run Pdb on exception')) - group.add_argument('--debug-warnings', action='store_true', + group.add_argument('--debug-warnings', action='store_true', dest='debug_warnings', help=__('run Pdb on warnings (requires --pdb)')) return parser @@ -330,11 +330,15 @@ def build_main(argv: Sequence[str]) -> int: try: confdir = args.confdir or args.sourcedir with patch_docutils(confdir), docutils_namespace(): - app = Sphinx(args.sourcedir, args.confdir, args.outputdir, - args.doctreedir, args.builder, args.confoverrides, args.status, - args.warning, args.freshenv, args.warningiserror, - args.tags, args.verbosity, args.jobs, - args.pdb) + app = Sphinx( + srcdir=args.sourcedir, confdir=args.confdir, + outdir=args.outputdir, doctreedir=args.doctreedir, + buildername=args.builder, confoverrides=args.confoverrides, + status=args.status, warning=args.warning, + freshenv=args.freshenv, warningiserror=args.warningiserror, + tags=args.tags, verbosity=args.verbosity, parallel=args.jobs, keep_going=False, + pdb=args.pdb, debug_warnings=args.debug_warnings, + ) app.build(args.force_all, args.filenames) return app.statuscode except (Exception, KeyboardInterrupt) as exc: diff --git a/sphinx/ext/autodoc/__init__.py b/sphinx/ext/autodoc/__init__.py index 391248b9427..28559e0eccc 100644 --- a/sphinx/ext/autodoc/__init__.py +++ b/sphinx/ext/autodoc/__init__.py @@ -415,9 +415,10 @@ def import_object(self, raiseerror: bool = False) -> bool: """ with mock(self.config.autodoc_mock_imports): try: - ret = import_object(self.modname, self.objpath, self.objtype, - attrgetter=self.get_attr, - ) + ret = import_object( + self.modname, self.objpath, self.objtype, + attrgetter=self.get_attr, + ) self.module, self.parent, self.object_name, self.object = ret if ismock(self.object): self.object = undecorate(self.object) @@ -2455,9 +2456,10 @@ def import_object(self, raiseerror: bool = False) -> bool: except ImportError as exc: try: with mock(self.config.autodoc_mock_imports): - ret = import_object(self.modname, self.objpath[:-1], 'class', - attrgetter=self.get_attr, # type: ignore[attr-defined] - ) + ret = import_object( + self.modname, self.objpath[:-1], 'class', + attrgetter=self.get_attr, # type: ignore[attr-defined] + ) parent = ret[3] if self.is_runtime_instance_attribute(parent): self.object = self.RUNTIME_INSTANCE_ATTRIBUTE @@ -2509,9 +2511,10 @@ def import_object(self, raiseerror: bool = False) -> bool: return super().import_object(raiseerror=True) # type: ignore[misc] except ImportError as exc: try: - ret = import_object(self.modname, self.objpath[:-1], 'class', - attrgetter=self.get_attr, # type: ignore[attr-defined] - ) + ret = import_object( + self.modname, self.objpath[:-1], 'class', + attrgetter=self.get_attr, # type: ignore[attr-defined] + ) parent = ret[3] if self.is_uninitialized_instance_attribute(parent): self.object = UNINITIALIZED_ATTR diff --git a/tests/test_quickstart.py b/tests/test_quickstart.py index 671aed469e3..5db07f0d62b 100644 --- a/tests/test_quickstart.py +++ b/tests/test_quickstart.py @@ -204,11 +204,11 @@ def test_quickstart_and_build(tmp_path): qs.generate(d) app = application.Sphinx( - tmp_path, # srcdir - tmp_path, # confdir - (tmp_path / '_build' / 'html'), # outdir - (tmp_path / '_build' / '.doctree'), # doctreedir - 'html', # buildername + srcdir=tmp_path, + confdir=tmp_path, + outdir=(tmp_path / '_build' / 'html'), + doctreedir=(tmp_path / '_build' / '.doctree'), + buildername='html', # buildername status=StringIO(), warning=warnfile) app.build(force_all=True) From 57ebb2294f9ee0f27a5bd6b554d9991d5047e11a Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Tue, 13 Aug 2024 16:18:44 +0100 Subject: [PATCH 09/12] more fixups --- doc/man/sphinx-build.rst | 5 +++-- sphinx/application.py | 2 +- sphinx/cmd/build.py | 3 ++- tox.ini | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/doc/man/sphinx-build.rst b/doc/man/sphinx-build.rst index 5c066c56924..6b6a4a4dd5d 100644 --- a/doc/man/sphinx-build.rst +++ b/doc/man/sphinx-build.rst @@ -267,8 +267,9 @@ Options .. option:: --keep-going - Only applicable whilst using :option:`--fail-on-warning`, - which by default exits :program:`sphinx-build` on the first warning. + From Sphinx 8.1, :option:`!--keep-going` is always enabled. + Previously, it was only applicable whilst using :option:`--fail-on-warning`, + which by default exited :program:`sphinx-build` on the first warning. Using :option:`!--keep-going` runs :program:`!sphinx-build` to completion and exits with exit status 1 if errors are encountered. diff --git a/sphinx/application.py b/sphinx/application.py index 188a2d156e9..217121053e2 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -206,7 +206,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st else: self._warning = warning self._warncount = 0 - self.keep_going = bool(warningiserror) + self.keep_going = bool(warningiserror) # Unused self._fail_on_warnings = bool(warningiserror) self.pdb = pdb self._debug_warnings = pdb and debug_warnings diff --git a/sphinx/cmd/build.py b/sphinx/cmd/build.py index b63713671b3..dab46cd448b 100644 --- a/sphinx/cmd/build.py +++ b/sphinx/cmd/build.py @@ -336,7 +336,8 @@ def build_main(argv: Sequence[str]) -> int: buildername=args.builder, confoverrides=args.confoverrides, status=args.status, warning=args.warning, freshenv=args.freshenv, warningiserror=args.warningiserror, - tags=args.tags, verbosity=args.verbosity, parallel=args.jobs, keep_going=False, + tags=args.tags, + verbosity=args.verbosity, parallel=args.jobs, keep_going=False, pdb=args.pdb, debug_warnings=args.debug_warnings, ) app.build(args.force_all, args.filenames) diff --git a/tox.ini b/tox.ini index b6f1daac1f4..13b43827b39 100644 --- a/tox.ini +++ b/tox.ini @@ -48,7 +48,7 @@ extras = docs commands = python -c "import shutil; shutil.rmtree('./build/sphinx', ignore_errors=True) if '{env:CLEAN:}' else None" - sphinx-build -M {env:BUILDER:html} ./doc ./build/sphinx -nW --keep-going {posargs} + sphinx-build -M {env:BUILDER:html} ./doc ./build/sphinx --fail-on-warning {posargs} [testenv:docs-live] description = From ac1b8bb8986a593d35ab64b40560f413fa416c49 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Tue, 13 Aug 2024 16:29:50 +0100 Subject: [PATCH 10/12] Alias skip_warningiserror to nullcontext --- sphinx/util/logging.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sphinx/util/logging.py b/sphinx/util/logging.py index c6fe00a2bdf..65589a5c968 100644 --- a/sphinx/util/logging.py +++ b/sphinx/util/logging.py @@ -5,7 +5,7 @@ import logging import logging.handlers from collections import defaultdict -from contextlib import contextmanager +from contextlib import contextmanager, nullcontext from typing import IO, TYPE_CHECKING, Any from docutils import nodes @@ -323,10 +323,7 @@ def pending_logging() -> Iterator[MemoryHandler]: memhandler.flushTo(logger) -@contextmanager -def skip_warningiserror(skip: bool = True) -> Iterator[None]: - # Deprecate in Sphinx 10 - yield +skip_warningiserror = nullcontext # Deprecate in Sphinx 10 @contextmanager From d7dac4c314d64aafddc7220a7ba4e1b5ff243f57 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Tue, 13 Aug 2024 16:35:16 +0100 Subject: [PATCH 11/12] DummyApplication --- sphinx/ext/autosummary/generate.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sphinx/ext/autosummary/generate.py b/sphinx/ext/autosummary/generate.py index e26b630cce9..7990daf4da0 100644 --- a/sphinx/ext/autosummary/generate.py +++ b/sphinx/ext/autosummary/generate.py @@ -71,7 +71,6 @@ def __init__(self, translator: NullTranslations) -> None: self.translator = translator self.verbosity = 0 self._warncount = 0 - self.warningiserror = False self._debug_warnings = False self.config.add('autosummary_context', {}, 'env', ()) From 202c9079767bd39fe839f815ee157c6d6bcab361 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Tue, 13 Aug 2024 17:05:09 +0100 Subject: [PATCH 12/12] rename to `--exception-on-warning` --- CHANGES.rst | 4 ++-- doc/man/sphinx-build.rst | 6 +++--- sphinx/application.py | 6 +++--- sphinx/builders/__init__.py | 4 ++-- sphinx/cmd/build.py | 7 ++++--- sphinx/ext/autosummary/generate.py | 2 +- sphinx/testing/util.py | 4 ++-- sphinx/util/logging.py | 2 +- tests/test_builders/test_build_warnings.py | 5 ++--- 9 files changed, 20 insertions(+), 20 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 24674a2f175..7de6ab68dfe 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -35,8 +35,8 @@ Features added Instead, exit with a non-zero status if any warnings were generated during the build. Patch by Adam Turner. -* #12743: Add :option:`sphinx-build --debug-warnings` to debug warnings when - :option:`sphinx-build --pdb` is specified. +* #12743: Add :option:`sphinx-build --exception-on-warning`, + to raise an exception when warnings are emitted during the build. Patch by Adam Turner and Jeremy Maitin-Shepard. Bugs fixed diff --git a/doc/man/sphinx-build.rst b/doc/man/sphinx-build.rst index 6b6a4a4dd5d..a9e3a09cbfd 100644 --- a/doc/man/sphinx-build.rst +++ b/doc/man/sphinx-build.rst @@ -302,10 +302,10 @@ Options .. versionchanged:: 7.3 Add ``--pdb`` long option. -.. option:: --debug-warnings +.. option:: --exception-on-warning - (Requires :option:`--pdb`.) - Run the Python debugger, :mod:`pdb`, for warnings emitted while building. + Raise an exception when a warning is emitted during the build. + This can be useful in combination with :option:`--pdb` to debug warnings. .. versionadded:: 8.1 diff --git a/sphinx/application.py b/sphinx/application.py index 217121053e2..142e1929400 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -146,7 +146,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st freshenv: bool = False, warningiserror: bool = False, tags: Sequence[str] = (), verbosity: int = 0, parallel: int = 0, keep_going: bool = False, - pdb: bool = False, debug_warnings: bool = False) -> None: + pdb: bool = False, exception_on_warning: bool = False) -> None: """Initialize the Sphinx application. :param srcdir: The path to the source directory. @@ -167,7 +167,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st when reading/writing documents. :param keep_going: Unused. :param pdb: If true, enable the Python debugger on an exception. - :param debug_warnings: If true, enable the Python debugger on warnings. + :param exception_on_warning: If true, raise an exception on warnings. """ self.phase = BuildPhase.INITIALIZATION self.verbosity = verbosity @@ -209,7 +209,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st self.keep_going = bool(warningiserror) # Unused self._fail_on_warnings = bool(warningiserror) self.pdb = pdb - self._debug_warnings = pdb and debug_warnings + self._exception_on_warning = exception_on_warning logging.setup(self, self._status, self._warning) self.events = EventManager(self) diff --git a/sphinx/builders/__init__.py b/sphinx/builders/__init__.py index 253e0400088..76e7e230cdc 100644 --- a/sphinx/builders/__init__.py +++ b/sphinx/builders/__init__.py @@ -328,7 +328,7 @@ def build( logger.info(bold(__('building [%s]: ')) + summary, self.name) # while reading, collect all warnings from docutils - with nullcontext() if self.app._debug_warnings else logging.pending_warnings(): + with nullcontext() if self.app._exception_on_warning else logging.pending_warnings(): updated_docnames = set(self.read()) doccount = len(updated_docnames) @@ -628,7 +628,7 @@ def write( self._write_serial(sorted(docnames)) def _write_serial(self, docnames: Sequence[str]) -> None: - with nullcontext() if self.app._debug_warnings else logging.pending_warnings(): + with nullcontext() if self.app._exception_on_warning else logging.pending_warnings(): for docname in status_iterator(docnames, __('writing output... '), "darkgreen", len(docnames), self.app.verbosity): self.app.phase = BuildPhase.RESOLVING diff --git a/sphinx/cmd/build.py b/sphinx/cmd/build.py index dab46cd448b..1773fa670a7 100644 --- a/sphinx/cmd/build.py +++ b/sphinx/cmd/build.py @@ -206,8 +206,9 @@ def get_parser() -> argparse.ArgumentParser: help=__('show full traceback on exception')) group.add_argument('--pdb', '-P', action='store_true', dest='pdb', help=__('run Pdb on exception')) - group.add_argument('--debug-warnings', action='store_true', dest='debug_warnings', - help=__('run Pdb on warnings (requires --pdb)')) + group.add_argument('--exception-on-warning', action='store_true', + dest='exception_on_warning', + help=__('raise an exception on warnings')) return parser @@ -338,7 +339,7 @@ def build_main(argv: Sequence[str]) -> int: freshenv=args.freshenv, warningiserror=args.warningiserror, tags=args.tags, verbosity=args.verbosity, parallel=args.jobs, keep_going=False, - pdb=args.pdb, debug_warnings=args.debug_warnings, + pdb=args.pdb, exception_on_warning=args.exception_on_warning, ) app.build(args.force_all, args.filenames) return app.statuscode diff --git a/sphinx/ext/autosummary/generate.py b/sphinx/ext/autosummary/generate.py index 7990daf4da0..b6f31be9b3b 100644 --- a/sphinx/ext/autosummary/generate.py +++ b/sphinx/ext/autosummary/generate.py @@ -71,7 +71,7 @@ def __init__(self, translator: NullTranslations) -> None: self.translator = translator self.verbosity = 0 self._warncount = 0 - self._debug_warnings = False + self._exception_on_warning = False self.config.add('autosummary_context', {}, 'env', ()) self.config.add('autosummary_filename_map', {}, 'env', ()) diff --git a/sphinx/testing/util.py b/sphinx/testing/util.py index 1452dcaec4d..54c3a6f66e0 100644 --- a/sphinx/testing/util.py +++ b/sphinx/testing/util.py @@ -119,7 +119,7 @@ def __init__( verbosity: int = 0, # argument is not in the same order as in the superclass warningiserror: bool = False, # argument is not in the same order as in the superclass pdb: bool = False, - debug_warnings: bool = False, + exception_on_warning: bool = False, # unknown keyword arguments **extras: Any, ) -> None: @@ -172,7 +172,7 @@ def __init__( confoverrides=confoverrides, status=status, warning=warning, freshenv=freshenv, warningiserror=warningiserror, tags=tags, verbosity=verbosity, parallel=parallel, - pdb=pdb, debug_warnings=debug_warnings, + pdb=pdb, exception_on_warning=exception_on_warning, ) except Exception: self.cleanup() diff --git a/sphinx/util/logging.py b/sphinx/util/logging.py index 65589a5c968..804ef62ccb4 100644 --- a/sphinx/util/logging.py +++ b/sphinx/util/logging.py @@ -614,7 +614,7 @@ def setup(app: Sphinx, status: IO, warning: IO) -> None: info_handler.setFormatter(ColorizeFormatter()) warning_handler = WarningStreamHandler(SafeEncodingWriter(warning)) - if app._debug_warnings: + if app._exception_on_warning: warning_handler.addFilter(_RaiseOnWarningFilter()) warning_handler.addFilter(WarningSuppressor(app)) warning_handler.addFilter(WarningLogRecordTranslator(app)) diff --git a/tests/test_builders/test_build_warnings.py b/tests/test_builders/test_build_warnings.py index 1d7d4db670b..c89e33b93d5 100644 --- a/tests/test_builders/test_build_warnings.py +++ b/tests/test_builders/test_build_warnings.py @@ -77,10 +77,9 @@ def test_html_warnings(app): 'html', testroot='warnings', freshenv=True, - pdb=True, - debug_warnings=True, + exception_on_warning=True, ) -def test_html_warnings_pdb(app): +def test_html_warnings_exception_on_warning(app): try: app.build(force_all=True) pytest.fail('Expected an exception to be raised')