Stop exiting early with --fail-on-warnings; add --exception-on-warning

.github/workflows/builddoc.yml

@@ -39,4 +39,3 @@ jobs:
         --jobs=auto
         --show-traceback
         --fail-on-warning
-        --keep-going

CHANGES.rst

@@ -13,6 +13,15 @@ Deprecated
 Features added
 --------------
 
+* #12743: No longer exit on the first warning when
+  :option:`--fail-on-warning  <sphinx-build --fail-on-warning>` is used.
+  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
 ----------
 

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`.

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 <makefile_options>`,
@@ -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
 
@@ -287,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.

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.

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,8 +145,8 @@ 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,
-                 pdb: bool = False) -> None:
+                 verbosity: int = 0, parallel: int = 0,
+                 pdb: bool = False, debug_warnings: bool = False) -> None:
         """Initialize the Sphinx application.
 
         :param srcdir: The path to the source directory.
@@ -163,8 +165,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 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
@@ -203,12 +205,10 @@ 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._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)
@@ -386,26 +386,31 @@ 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._fail_on_warnings:
+                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._fail_on_warnings:
+                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('')

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

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:

sphinx/cmd/build.py

@@ -201,12 +201,13 @@ 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',
-                       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',
                        help=__('run Pdb on exception'))
+    group.add_argument('--debug-warnings', action='store_true',
+                       help=__('run Pdb on warnings (requires --pdb)'))
 
     return parser
 
@@ -332,7 +333,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

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

sphinx/ext/autodoc/importer.py

@@ -137,33 +137,30 @@ 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.
         raise ImportError(exc, traceback.format_exc()) from exc
 
 
 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``.

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