search

sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.19k stars 2.03k forks source link

Compilation errors cause parsed doctrees to be discarded for all files, even the error-free ones #6035

Open cpitclaudel opened 5 years ago

cpitclaudel commented 5 years ago

Describe the bug

Raising an error from an extension (or even a warning when using -W) causes the next build to reprocess all files instead of resuming from the last error-free file. This makes development with -W enabled in a large documentation project painful, because every small error requires a full-rebuild (for us in Coq, the read phase takes about 1 minute).

To Reproduce Steps to reproduce the behavior:

# Create an empty project
$ yes '' | sphinx-quickstart -p test -a me

# Create input files
$ echo test > a.rst
$ echo test > b.rst
$ echo '|error|' > c.rst

# Compile with -W
$ make SPHINXOPTS=-W html
Running Sphinx v1.8.2
making output directory...
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 4 source files that are out of date
updating environment: 4 added, 0 changed, 0 removed
reading sources... [ 25%] a
reading sources... [ 50%] b
reading sources... [ 75%] c
reading sources... [100%] index

Warning, treated as error:
/tmp/test/c.rst:1:Undefined substitution referenced: "error".
Makefile:19: recipe for target 'html' failed
make: *** [html] Error 2

# Compile again:
$ make SPHINXOPTS=-W html
Running Sphinx v1.8.2
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 4 source files that are out of date
updating environment: 4 added, 0 changed, 0 removed
reading sources... [ 25%] a
reading sources... [ 50%] b
reading sources... [ 75%] c
reading sources... [100%] index

Warning, treated as error:
/tmp/test/c.rst:1:Undefined substitution referenced: "error".
Makefile:19: recipe for target 'html' failed
make: *** [html] Error 2

Expected behavior Ideally, the second compilation pass should resume from c.rst, since that's where the error was. Since a.rst and b.rst where successfully read, it would be super if they could be reloaded from disk on the next pass.

Environment info

tk0miya commented 5 years ago

+0 if somebody implement it.

Lysxia commented 5 years ago

I'm kind of interested in fixing this, do you have any suggestions about where to start from?

Lysxia commented 5 years ago

I've traced it back to these lines which are removing environment.pickle, which appears to store the last-read timestamps:

https://github.com/sphinx-doc/sphinx/blob/42c8fbd6f30d040683a56277ba59732c028623ab/sphinx/application.py#L363-L369

Now I'm wondering what a good fix is. Should these lines simply be removed? The comment suggests this behavior makes sense in some settings, I have no idea, in which case we can keep this but hide it behind a flag --clear-on-failure which (I would prefer) is unset by default.

Another (more minor) problem is that the env is saved only on a successful build. So even if I just remove those three lines, on a fresh project, if the build fails from the start, the rebuild still happens.

tk0miya commented 5 years ago

My large concern is how to keep health of env file. I can't say env data is enough healthy when errors are happened. So I also can't say dumping env data to disk is safety. It might be good to dump env data on reading every document.

Note: we need to care for parallel build also.

Lysxia commented 5 years ago

Is the dumped env data useful for other things besides the timestamps?

tk0miya commented 5 years ago

env data is main database of Sphinx. It stores data for cross references, domains and so on. timestamps is a small part of it.

Lysxia commented 5 years ago

I see. Right now I'm content with keeping the env regardless, as a temporary workaround. I'll think about a proper fix when I find some free time, unless someone gets to it before me :)