Opened 5 years ago

Closed 4 years ago

#6970 enhancement closed worksforme (worksforme)

Clean build for documentation

Reported by: Adi Roiban Owned by: Adi Roiban
Priority: normal Milestone: New Logging System
Component: core Keywords:
Cc: Branch: branches/clean-build-for-documentation-6970
branch-diff, diff-cov, branch-cov, buildbot
Author: glyph

Description

I got the following errors while building the documentation.

The might not be related to this task, but I think that we should fix them in order to have a clean build system.

Maybe the CI system could include a build to recompil all documentaion and have Sphinx running in nitpick mode and turn warning into errors. Otherwise there is a risk of accumulating to many errors/warning into to documentation and making it hard to detect problems introduced by new code.

/home/adi/chevah/twisted/docs/index.rst:11: ERROR: Error in "toctree" directive:
unknown option: "includehidden".

.. toctree::
    :maxdepth: 2
    :includehidden:

    projects/core/index
    projects/conch/index
    projects/lore/index
    projects/mail/index
    projects/names/index
    projects/pair/index
    projects/web/index
    projects/words/index
    projects/historic/index
looking for now-outdated files... none found
pickling environment... done
checking consistency... /home/adi/chevah/twisted/docs/projects/conch/index.rst:: WARNING: document isn't included in any toctree
/home/adi/chevah/twisted/docs/projects/core/index.rst:: WARNING: document isn't included in any toctree
/home/adi/chevah/twisted/docs/projects/historic/index.rst:: WARNING: document isn't included in any toctree
/home/adi/chevah/twisted/docs/projects/lore/index.rst:: WARNING: document isn't included in any toctree
/home/adi/chevah/twisted/docs/projects/mail/index.rst:: WARNING: document isn't included in any toctree
/home/adi/chevah/twisted/docs/projects/names/index.rst:: WARNING: document isn't included in any toctree
/home/adi/chevah/twisted/docs/projects/pair/index.rst:: WARNING: document isn't included in any toctree
/home/adi/chevah/twisted/docs/projects/web/index.rst:: WARNING: document isn't included in any toctree
/home/adi/chevah/twisted/docs/projects/words/index.rst:: WARNING: document isn't included in any toctree
done
preparing documents... done
/home/adi/chevah/twisted/docs/projects/web/howto/web-in-60/access-logging.rst:30: WARNING: Pygments lexer name u'shell' is not known

Attachments (5)

6970-1.diff (1.7 KB) - added by Adi Roiban 5 years ago.
6970-2.diff (586 bytes) - added by Adi Roiban 5 years ago.
6970-3.diff (687 bytes) - added by Adi Roiban 5 years ago.
clean-build-for-documentation-6970-4.patch (1.9 KB) - added by Trevor Bramwell 5 years ago.
Fixed other warnings, updated patch to trunk, and added NEWS file.
clean-build-for-documentation-6970-5.patch (1.9 KB) - added by Trevor Bramwell 5 years ago.
Same patch, but creates two seperate misc NEWS entries.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 5 years ago by Adi Roiban

Keywords: review added
Owner: changed from Adi Roiban to hawkowl

Ok. The main problems was because makefile was using the old sphinx version available in Ubuntu 12.04... I have installed sphinx in venv and all looks good.

I have fixed the shell -> sh syntax highliting in blocks.

Maybe documentation should be updated to inform users that they need to use the latest sphinx (or greater than 1.2.1)... and not the one provided by their OS.

I have not updated the README file to inform users how to test documentation build since it is somehow in conflict with the work from #6953

I have included a sample target for testing HTML documentation which gives a simple PASS/FAIL result and which can be integrated with a buildbot builder.

Diff is here https://github.com/chevah/twisted/compare/6970-clean-docs-build

I will attach the patch

Thanks!

Changed 5 years ago by Adi Roiban

Attachment: 6970-1.diff added

comment:2 Changed 5 years ago by hawkowl

Owner: hawkowl deleted

comment:3 Changed 5 years ago by Glyph

Keywords: review removed
Owner: set to Adi Roiban

Hi adi,

Our CI system doesn't use make html; that Makefile is just there as a convenience to Sphinx users. You want to update bin/admin/build-docs if you want to change how the docs get built.

I know the patches are very small, but could you separate out the warnings fixes from the makefile changes? I'd rather leave the sphinx conf and makefile alone as much as possible, and isolate doc/format changes to separate tickets.

Thanks a bunch.

comment:4 Changed 5 years ago by Adi Roiban

Keywords: review added
Owner: Adi Roiban deleted

Thanks for the review.

I have attached a new diff which only changes the documentation.

I will open a new ticket for bin/admin/build-docs

Changed 5 years ago by Adi Roiban

Attachment: 6970-2.diff added

comment:5 Changed 5 years ago by Jonathan Ballet

I tested the change (with the flags from #6980) and the doc now builds without warning.

I don't have commit privileges, but the change looks reasonably safe... That can be merged to trunk.

comment:6 Changed 5 years ago by Jonathan Ballet

Keywords: review removed

You'll probably need a topfile (.doc?) though.

comment:7 Changed 5 years ago by Adi Roiban

Keywords: review added

Thanks for the review. I have added the topfile... in core.

Changed 5 years ago by Adi Roiban

Attachment: 6970-3.diff added

Changed 5 years ago by Trevor Bramwell

Fixed other warnings, updated patch to trunk, and added NEWS file.

comment:8 Changed 5 years ago by Trevor Bramwell

adiroiban,

Great work on this! I hope I'm not stepping on your toes by doing this, but I have updated your patch to apply to the current trunk, along with fixing other warnings that have since popped up. I have also added a NEWS entry under twisted/core .

Here is an output of the issues I fixed after running make html on the current trunk.

sphinx-build -b html -d _build/doctrees   . _build/html
Running Sphinx v1.2
loading pickled environment... done
building [html]: targets for 2 source files that are out of date
updating environment: 0 added, 2 changed, 0 removed
reading sources... [100%] web/howto/web-in-60/access-logging                                                        
/home/bramwelt/scm/github/twisted/Twisted/docs/core/howto/logger.rst:9: WARNING: malformed hyperlink target.
/home/bramwelt/scm/github/twisted/Twisted/docs/core/howto/logger.rst:247: WARNING: Explicit markup ends without a blank line; unexpected unindent.
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
/home/bramwelt/scm/github/twisted/Twisted/docs/web/howto/web-in-60/access-logging.rst:30: WARNING: Pygments lexer name u'shell' is not known
writing output... [100%] web/howto/web-in-60/index                                                                  
/home/bramwelt/scm/github/twisted/Twisted/docs/core/howto/logger.rst:172: WARNING: undefined label: core-howto-logger-saving-events-for-later (if the link has no caption the label must precede a section header)
writing additional files... genindex search

comment:9 Changed 5 years ago by Jean-Paul Calderone

Just a quick technical point - the web howto fix belongs to the Twisted Web documentation which belongs to the Twisted Web subproject: therefore, it should be described in a news fragment in the twisted/web/topfiles directory. That means that in this case the patch should have two news fragments (similar but not quite identical since they describe fixes in different places). Also, consider whether this is a significant enough change to warrant a description in the release notes (will people reading about a new Twisted release be less excited if they don't read about this?). If not, then a .misc is good enough.

Changed 5 years ago by Trevor Bramwell

Same patch, but creates two seperate misc NEWS entries.

comment:10 Changed 5 years ago by Trevor Bramwell

exarkun, thank you for the point! I had the feeling it wasn't that significant to warrant a change, but I wanted to err on the side of caution. Your comment definitely cleared up the ambiguity. I have updated the patch, and created two misc NEWS entries. One under twisted/web/topfiles and one under twisted/topfiles .

comment:11 Changed 5 years ago by Adi Roiban

Don't worry. Feel free to update and take over.

The patch is simple and since it was sent 3 months ago I lost hope in Twisted review process :(

without a test on buildbot, there is a risk that documentation will always accumulate warnings.

comment:12 Changed 5 years ago by Glyph

Author: glyph
Branch: branches/clean-build-for-documentation-6970

(In [42958]) Branching to clean-build-for-documentation-6970.

comment:13 Changed 5 years ago by Glyph

Resolution: fixed
Status: newclosed

(In [42961]) Merge clean-build-for-documentation-6970: Make the documentation build without warnings.

Author: bramwelt, adiroiban

Reviewer: glyph, exarkun

Fixes: #6970

Just fix a couple of warnings introduced in the logger docs.

comment:14 Changed 5 years ago by Glyph

Keywords: review removed

Thanks for your contributions, adiroiban, bramwelt. Those warnings were annoying, glad they're fixed ;).

comment:15 Changed 4 years ago by hawkowl

Resolution: fixed
Status: closedreopened

Reopening because it relies on #6750 which is being reverted.

Also, this branch had two topfiles?

comment:16 Changed 4 years ago by hawkowl

Milestone: New Logging System

comment:17 Changed 4 years ago by Adi Roiban

Owner: set to Adi Roiban
Status: reopenednew

I lost this ticked. I will try to continue the work. Does it need to be on the new logging milestone?

comment:18 Changed 4 years ago by Adi Roiban

Resolution: worksforme
Status: newclosed

I tried this on latest trunk

$ sphinx-build --version
Sphinx (sphinx-build) 1.2.3
$ cd doc
$ sphinx-build -Wa -b html -d _build/doctrees   . _build/html
build succeeded.
}}

so it looks like we no longer have this errors.

I will close this ticket.
Note: See TracTickets for help on using tickets.