#3311 reopened defect

Update waf.py for Sphinx 1.7 compatibility

Reported by: Richard Campbell Owned by: Chris Johns
Priority: normal Milestone:
Component: doc Version:
Severity: normal Keywords:
Cc: Blocked By:
Blocking:

Description

sphinx-build --version in sphinx 1.7 seems to return its version information in stderr instead of stdout.

Added try/except blocks to the check_sphinx_version method to attempt to get version from sphinx.version_info then from cmd_and_log using the output-Context.BOTH parameter and finally from cmd_and_log with no additional argument.

Deleted unused import from sphinx.util.compat in rtemsdomain.py. This module was removed in sphinx 1.7.

Attachments (2)

0001-RTEMS-Docs-Check-sphinx-version.patch (2.6 KB) - added by Richard Campbell on Feb 22, 2018 at 8:43:06 PM.
Patch to close this ticket
0001-Check-both-stdout-and-stderr-for-Sphinx-version.patch (2.4 KB) - added by Richard Campbell on Mar 12, 2018 at 9:00:20 PM.
Improved patch based on Chris' comments

Download all attachments as: .zip

Change History (8)

Changed on Feb 22, 2018 at 8:43:06 PM by Richard Campbell

Patch to close this ticket

comment:1 Changed on Feb 22, 2018 at 11:22:02 PM by Chris Johns

Owner: set to Chris Johns
Status: newassigned

comment:2 Changed on Mar 9, 2018 at 7:29:11 PM by Richard Campbell <richard.campbell@…>

Resolution: fixed
Status: assignedclosed

In 2a06644/rtems-docs:

RTEMS Docs: Check sphinx version

Sphinx.util.compat module was removed at Sphinx version 1.7.
Imported module was not being used.

Closes #3311.

comment:3 Changed on Mar 11, 2018 at 6:18:15 AM by Chris Johns

I have reviewed the patch. It needs some more work.

Please do not import sphinx. This is a build system and so you either need to know the module is present, ie latex.py which abstracts the latex handling or you add proper checks for a correctly install Python sphinx module, which in this case should not be needed cause the tool should have a command to print the version. Has that changed?

Catching all exceptions like this to handle the case of a python module that is not installed is not a good idea, it may hides other issues that may appear. The previous code wrapped a single conversion line which is an accepted way to do this.

What is wrong with asking Sphinx for it's version?

Why require 2 calls to get the version?

comment:4 Changed on Mar 11, 2018 at 6:18:36 AM by Chris Johns

Resolution: fixed
Status: closedreopened

Changed on Mar 12, 2018 at 9:00:20 PM by Richard Campbell

Improved patch based on Chris' comments

comment:5 Changed on Mar 12, 2018 at 9:05:34 PM by Chris Johns

Excellent and thank you for the prompt follow up. Please push to master.

comment:6 Changed on Mar 12, 2018 at 9:07:46 PM by Richard Campbell

Deleted import of Sphinx
Ask Sphinx for its version with a single call.
Check both stdout and stderr for version string.
Wrap a single conversion line as was done in previous code.

Note: See TracTickets for help on using tickets.