Warning treated as error with -W invalidates the entire incremental build cache

vpoughonEZ commented 2 years ago

Describe the bug

Using -W means that a build following a build with an error will not be incremental.

How to Reproduce

Step 0: Setup a simple sphinx project with three files:

index.rst:

Title
=====

.. toctree::

   file1.rst
   file2.rst

file1.rst:

File 1
======

This is file1.

file2.rst:

File 2
======

This is file2.

Step 1 Build the project with sphinx-build -b html source/ output/ -W -N
Step 2 Add an undefined reference in file1.rst, for example, modify it as such:
```
File 1
======
```

This is file1.

:ref:invalidref

As expected, we get a warning treated as error:

```txt
Warning, treated as error:
/home/vpoughon/ez/tuto-sphinx/source/file1.rst:6:undefined label: invalidref

Step 3 Fix the warning (remove the undefined reference) and rebuild with the same command: sphinx-build -b html source/ output/ -W -N

Observed behavior: Sphinx rebuilds all files (including file2.rst) and the build is not incremental:

reading sources... [ 33%] file1
reading sources... [ 66%] file2
reading sources... [100%] index

looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [ 33%] file1
writing output... [ 66%] file2
writing output... [100%] index

Expected behavior

In Step 3 above, file2.rst is not rebuilt.

Your project

See above

Screenshots

No response

OS

Fedora 33

Python version

3.9.9

Sphinx version

4.2.0

Sphinx extensions

none

Extra tools

No response

Additional context

Note that using --keep-going does not solve my use case, because using it can hide errors on subsequent builds. i.e. if I use --keep-going and build a project with an invalid reference twice, the second build will not show any error.

I would like to use -W only, and keep all builds incremental even across builds with errors.

AA-Turner commented 2 years ago

I don't think -W should retain incremaentalism.

--keep-going could be modified to report errors on every build, though. What happens if you modify the file containing the error on the second run (so that the file is rebuilt)?

A

vpoughonEZ commented 2 years ago

This is a simple example of course, but in a large project that takes several minutes for a clean build, that means that a typo in a reference costs you a full rebuild. I think this use case should be supported if possible.

To answer your question, if I use --keep-going and modify the problematic file (without fixing the invalid ref), the error shows up as expected. But in my use case I want all errors to always be shown and all builds to be incremental.

AA-Turner commented 2 years ago

I want all errors to always be shown and all builds to be incremental.

This seems a contradiction in terms -- an incremental build doesn't re-parse sources (hence incremental), and thus can't report the error. Perhaps the build environment could store a list of errors and re-raise them, but there is also absolutley a use case for saying "I only want new errors to be reported in incremental builds".

May I ask why you want to see all errors on every incremental build, to help me understand you use-case better?

A

vpoughonEZ commented 2 years ago

This seems a contradiction in terms -- an incremental build doesn't re-parse sources (hence incremental), and thus can't report the error.

To clarify what I mean: don't invalidate the cache of already built files that don't have any errors (for example file2.rst in my example above). In other words, I would like: all errors to always be shown and all builds to be incremental (except for files which contain errors, which I would expect to be rebuilt every time - and hence their errors shown every time).

I would like errors to not be hidden on incremental builds mostly for UX. Errors should be visible and alert the user to fix them asap. Also, the sphinx-build process returning zero on incremental builds where a full build of the same source would return 1 is also a bug IMO. It's what happens with -W --keep-going and the procedure above.

In my opinion the following would be best:

-W: don't invalidate build cache of files that don't have errors
-W --keep-going: don't store files that have errors in the build cache

tk0miya commented 2 years ago

Understandable. It would be nice if we can improve the incremental build feature.

Error happened (including -W): don't invalidate build cache of files that don't have errors
-W --keep-going: Invalidate all cache files on the next build

Anyway, we need to improve the whole of the incremental build feature to implement this.

CAM-Gerlach commented 1 year ago

To note, this is also an issue on the CPython docs and PEPs repos as well—unless --keep-going is passed, the entire build cache is invalidated upon any error, and the user is forced to suffer through a many-minute rebuild of hundreds of lengthy documents just to see if they've fixed it. This gets even worse when using -n, as it is even easier and more common to make typos, and is thus rather user-hostile.

In fact, any error causes a complete cache invalidation, which also comes into play for us using environment.default_settings["halt_level"] = 2 to fail on docutils markup warnings but not (yet) Sphinx-level ones, which --keep-going doesn't solve, as these appear as hard errors to Sphinx rather than warnings.

Ideally, I'd suggest that:

Warnings treated as errors (with -W) don't invalidate the build cache
Errors don't invalidate the build cache either, if practical and there isn't a strong reason why not
Only update/don't invalidate the build cache for a file if is successfully built without an error (either hard error, or a warning treated as one)

That way, (--keep-going or not) any files with errors, or warnings treated as such, will still be rebuilt every time (at least with a consistent invocation), but other files will not.

If users do explicitly want a non-incremental build, they can always pass -E.

sphinx-doc / sphinx