Print warnings/errors at the end of the build

[asmeurer]

In the SymPy docs, we've been using this custom Makefile code:

html:
    $(MAKE) -s _html || $(MAKE) -s printwarnings

# Force colors on CI. Set NO_COLOR=1 on the command line to disable this.
_html: export FORCE_COLOR = 1
_html: $(BUILDDIR)/html/pics/*.png
_html: SPHINXOPTS += -W --keep-going
_html: $(BUILDDIR)/logo/sympy-notailtext-favicon.ico
    mkdir -p $(SOURCEDIR)/.static
    mkdir -p $(BUILDDIR)/html
    mkdir -p $(BUILDDIR)/doctrees
    mkdir -p $(SOURCEDIR)/modules
    $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(SPHINXSAVEWARNINGS) $(SOURCEDIR) $(BUILDDIR)/html
    @echo
    @echo "Docs build finished. The HTML pages are in $(BUILDDIR)/html."

# Print the warnings again at the end of the build, to make them easier to find
printwarnings:
    @printf "\033[0;31mThe docs build failed with the following errors:\033[0m\n"
    @cat $(WARNINGSFILE)
    exit 1

This makes it so that when there is an warning/error in the build, it is printed again at the very end. This is very useful for CI builds, where the logs can be quite long and it's difficult to find whatever error or warning caused a build to fail.

Would it be possible for Sphinx to just always do this, so that this little bit of code is unnecessary?

(by the way, the "force color output on CI" thing is another thing that would be useful if I didn't have to manually enable it. I don't know if there's a better thing Sphinx can be doing here. I can open a separate issue for this if you want)

[electric-coder]

Would it be possible for Sphinx to just always do this

I can read makefiles but only when I can't avoid it. I don't think that imposing duplicate error messages is useful for the common user running his local build, I don't think that conceptually we should prioritize CI/CD and deviate from historical norms to only print error messages once, in sequence, by default.

Maybe I didn't understand the FR right :O

"force color output on CI"

Help me avoid some research but is that particular piece of code portable across shells and platforms? Doesn't this propose to make the default makefile platform dependent?

[timhoffm]

@asmeurer do you need all the other output stuff? If not, the --quiet option would be a solution (since 7.3).

Taking the underlying issue

the logs can be quite long and it's difficult to find whatever error or warning caused a build to fail.

I see this as a valid problem. For example the matplotlib doc output is >5000 lines. This needs better user experience. Maybe --silent is enough. If not the first question should be, what output elements and in which order are helpful: warnings/errors inline or at the end or both.One can find arguments for all three, but possible not all are necessary. I don't read the request as literally changing the makefile, but change sphinx-build output, possibly through parameters.

[electric-coder]

what output elements and in which order are helpful: warnings/errors inline or at the end or both

@timhoffm FTR I find this FR reminiscent of a family of Sphinx FAQs that want to suppress output messages by selectively filtering input paths when the objective is to incrementally add docs to existing large code bases.

Besides the previous reservations I'd ask if the right solution isn't to grep a --verbose output?

[asmeurer]

don't think that imposing duplicate error messages is useful for the common user running his local build, I don't think that conceptually we should prioritize CI/CD and deviate from historical norms to only print error messages once, in sequence, by default.

Just to be clear, this change is also 100% useful for local builds too (I say this from experience using my above Makefile thing with building SymPy locally).

I see this as a valid problem. For example the matplotlib doc output is >5000 lines. This needs better user experience. Maybe --silent is enough. If not the first question should be, what output elements and in which order are helpful: warnings/errors inline or at the end or both.One can find arguments for all three, but possible not all are necessary. I don't read the request as literally changing the makefile, but change sphinx-build output, possibly through parameters.

I don't think hiding the output is a good idea, and should definitely not be recommended to people. In a local build, it just prevents you from seeing the build progress, and on CI, it makes it harder to see what went wrong when something goes wrong.

And just to be clear, my request is to make it so that my Makefile hack is no longer necessary, i.e., Sphinx just always re-prints whatever warnings/errors were present in the build again at the very end as part of its default build output. If reading my Makefile code makes your brain hurt, that should be incentive for you to want for this to be implemented so I no longer have to have it!

I see this as a valid problem. For example the matplotlib doc output is >5000 lines. This needs better user experience. Maybe --silent is enough. If not the first question should be, what output elements and in which order are helpful: warnings/errors inline or at the end or both.One can find arguments for all three, but possible not all are necessary. I don't read the request as literally changing the makefile, but change sphinx-build output, possibly through parameters.

Yes, this came up from trying and failing to find where the build errors were in a matplotlib CircleCI job. I added this to SymPy for exactly the same reasons (SymPy's docs, like matplotlib's, are quite large). I was about to send a PR to matplotlib with my above Makefile code, but figured it would make more sense to just request Sphinx to change. But if Sphinx doesn't want to do this, then I would suggest matplotlib copy the SymPy Makefile code, because it's a huge developer experience gain (trust me).

Help me avoid some research but is that particular piece of code portable across shells and platforms? Doesn't this propose to make the default makefile platform dependent?

No, nothing in this issue proposes changing the default Makefiles generated by Sphinx. My Makefile is just showing how I'm working around deficiencies in Sphinx. Again, if you don't like reading Makefile hacks, that's a sign that Sphinx should change so that they become unnecessary.

I'm proposing Sphinx to enable color itself internally when on CI. This would be less platform dependent, because Sphinx could do this conditionally when it detects it is on CI, rather than unconditionally. (to be clear, for the color thing, I'm not completely sure the technical reasons why color output doesn't work on CircleCI. This could actually just be a bug in the way Sphinx detects if it should enable color)

[electric-coder]

100% useful for local builds too

Let me be clear: I'm sure it shouldn't be implemented. None of the other tools out there print the same error X times (why should only Sphinx implement this?).

is to make it so that my Makefile hack is no longer necessary

Most users will have no use for your hack in their Sphinx, it degrades the quality of the output by adding duplication to the otherwise minimalist sphinx-build CLI.

If reading my Makefile code makes your brain hurt

You FR makes for poor default UX because it proposes to add noise (not only in the source) without acknowledging the drawback of duplication. What you're proposing is that the sphinx-build CLI become a redirection tool but that's against the single responsibility principle since whatever system you're building in is sure to already provide the necessary tools for the functionality you propose.

[asmeurer]

You FR makes for poor default UX because it proposes to add noise (not only in the source) without acknowledging the drawback of duplication.

I'm not acknowledging any "drawbacks" of duplicating a few lines at the end of the build because there are none. If the build output is hundreds of lines long, a few extra lines at the end doesn't really add any noise. Indeed, it's the exact opposite, since those few lines are the actual signal of the build output.

If showing each error/warning only once in the output were actually important (it isn't), it would be better to only show them at the end. But there is also benefit to showing them inline as the build progresses.

What you're proposing is that the sphinx-build CLI become a redirection tool but that's against the single responsibility principle since whatever system you're building in is sure to already provide the necessary tools for the functionality you propose.

There is no such "single responsibility principle" for Sphinx. Sphinx is an end-to-end system for building documentation. It should build in high level user experience, not rely on some other tooling built on top of it to do so (because no such other tooling exists).

And note that the Sphinx project even wants to remove Makefiles as a layer on top of sphinx-build (https://github.com/sphinx-doc/sphinx/issues/5618), which is even more motivation for me to remove my one-off workarounds in Makefiles and upstream them into Sphinx itself.

[electric-coder]

I'm not acknowledging any

It's obvious you're wrong and don't acknowledge anything.

Sphinx is an end-to-end system for building documentation. It should (...) not rely on some other tooling

Then why should Sphinx change to satisfy an issue you're having with your CI/CD? That's your problem.

[AA-Turner]

(by the way, the "force color output on CI" thing is another thing that would be useful if I didn't have to manually enable it. I don't know if there's a better thing Sphinx can be doing here. I can open a separate issue for this if you want)

@asmeurer opening a new issue would be useful please, thank you.

A

[AA-Turner]

The request seems pretty reasonable, I've certainly found it frustrating searching our own logs for a failure (and indeed I recently changed CI to be less verbose).

@asmeurer / @timhoffm it seems that scientific Python would make use of this generally. 5000 lines of output does seem pathological though?

In terms of implementation, I see a few options:

1. Change the default to print errors twice, no configuration. This is similar to what e.g. pytest does.
2. Provide a conf.py option. Very explicit and discoverable, but would run for every build (local & CI)
3. Provide a command line argument. Less discoverable, but allows per-run configuration.
4. Use an environment variable. I'd tend to avoid this.
5. A combination of the above.

Thoughts?

A

[electric-coder]

1. Change the default to print errors twice, no configuration.

Absolutely not this one. I've often found myself traveling having to work on a laptop, so duplicating the errors will make the output much noisier and make scrolling it much harder. That's also the case if you're checking the build output locally in an integrated tab.

This was an extremely poor FR as a post: conflating 2 requests, not linking to any example of the behavior in the wild, not linking to the exact doc anchors of the relevant options, nor putting alternatives into proper context.

1. (...) This is similar to what e.g. pytest does.

Here's the issue: what the FR is proposing is not what pytest does by default. Default pytest behavior puts the error stacktrace once at the end and includes a short summary besides marking failures in the ordered test listing. What this FR proposed was complete duplication of the error stacktrace; by default!; - and that is not default behavior in any other tool that I can think off. And again that makes the original post of this FR exceedingly poor (with no concern whatsoever for the reader) because it doesn't demonstrate a parallel with any comparable tool.

[timhoffm]

5000 lines of output does seem pathological though?

Yes. Not claiming that is reasonable, but that's what comes out of sphinx right now (part of the blame goes to sphinx-gallery). Anyway, large documentations currently have substrantial output, e.g. scipy 1000 lines, skimage 1600 lines, matplotlib 5000 lines. As with execution speed, the large projects are the demanding ones and would benefit from improvements.

Thoughts?

I actually like what pytest does: low-volume progress info (1) and a summary of errors at the end (2). This separates the progress and error reporting concerns. Right now, we basically babble out any information when it occurs. Progress is "I'm doing x" and when an error comes up, it's interspersed in between. This is the most simple approach to implement, but it's not necessarily the best way information is presented to the user. Typically, progress is only relevant during the execution, and when looking at errors the context of the progress is not relevant anymore, but you would like to have an overview of what went wrong.

• low-volume progress info: There are many lines reporting operations on single files, e.g.

  copying downloadable files... [  9%] gallery/text_labels_and_annotations/font_table.ipynb
  ...
  writing output... [ 36%] api/_as_gen/matplotlib.pyplot.annotate .. api/_as_gen/matplotlib.pyplot.plot
  ...
  copying images... [ 13%] users/explain/axes/images/thumb/sphx_glr_tight_layout_guide_thumb.png
  ...
  embedding documentation hyperlinks for users/explain... [ 57%] colormaps.html
  ...

  If these could be turned into one-line progress information, that would substantially shorten output. Use tqdm if the dependency is ok, or make a short hand-build progress info otherwise (one dot per file like in pytest)

• summary of errors at the end: This may not be strictly necessary if we can tune down the other output to a few dozen lines (but that may need coordination with extension authors). Still, it would be nice to see the issues collected in one place.

[dbitouze]

Maybe I'm completely wrong here but what about errors/warnings in a .log file?

[timhoffm]

Maybe I'm completely wrong here but what about errors/warnings in a .log file?

One could do that, but I suspect it's not as usable. When run locally from the command line, you'd have to open the file to see the results. When run in CI, they also typically just present stdout/stderr and I wouldn't know how to easily access generated files.

Edit: @dbitouze The option for a logfile already exists: https://www.sphinx-doc.org/en/master/man/sphinx-build.html#cmdoption-sphinx-build-w

[timhoffm]

@electric-coder

It's obvious you're wrong and don't acknowledge anything.

I'd like to remind you on our code of conduct.

I've also noticed that you have strong opinions on several topics. While it's good to have clear ideas, I advise to try and understand why other people are suggesting features or solutions. There's generally an underlying need for a feature, even though it may not be expressed explicitly (here: The need is that it's hard to find error messages, not that one strictly wants duplicate messages). Focusing on the underlying need rather than the request often brings us towards a suitable improvement.

[asmeurer]

@AA-Turner I'm not sure about your options, but another thought I had is maybe this should be tied to the --keep-going flag. This is presumably less important without that flag (although I admit I always use that flag so I'm not sure).

[asmeurer]

Opened https://github.com/sphinx-doc/sphinx/issues/13065 for the color thing.