Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#9426 closed defect (duplicate)

Docbuilder ignores return code from subprocess.call()

Reported by: Leif Leonhardy Owned by: Minh Van Nguyen
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: documentation Keywords: Sphinx documentation builder error
Cc: Jeroen Demeyer Merged in:
Authors: Reviewers: Minh Van Nguyen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

devel/sage/doc/common/builder.py:

In builder_helper.f():

...
        logger.warning(build_command)
        subprocess.call(build_command, shell=True)

        logger.warning("Build finished.  The built documents can be found in %s", output_dir)
...

In class DocBuilder:

    def pdf(self):
        """
        Builds the PDF files for this document.  This is done by first
        (re)-building the LaTeX output, going into that LaTeX
        directory, and running 'make all-pdf' there.

        EXAMPLES::

            sage: import os, sys; sys.path.append(os.environ['SAGE_DOC']+'/common/'); import builder
            sage: b = builder.DocBuilder('tutorial')
            sage: b.pdf() #not tested
        """
        self.latex()
        os.chdir(self._output_dir('latex'))
        subprocess.call('make all-pdf', shell=True)

        pdf_dir = self._output_dir('pdf')
        for pdf_file in glob.glob('*.pdf'):
            shutil.move(pdf_file, os.path.join(pdf_dir, pdf_file))

        logger.warning("Build finished.  The built documents can be found in %s", pdf_dir)

Change History (12)

comment:1 Changed 12 years ago by Leif Leonhardy

It's trivial to change the message on non-zero return values, but I think errors should somehow be propagated, s.t. doc build errors aren't drowned in the flood of other messages.

comment:2 Changed 12 years ago by Robert Miller

Priority: criticalblocker

Changing this to blocker at least for now...

comment:3 Changed 12 years ago by Robert Miller

Priority: blockermajor

comment:4 Changed 12 years ago by Leif Leonhardy

Cc: Jeroen Demeyer added

comment:5 Changed 12 years ago by Leif Leonhardy

Keywords: Sphinx documentation builder error added

comment:6 Changed 12 years ago by John Palmieri

Should a docbuild error just cause an exception to be raised, thus ending the build? That wouldn't be hard to implement, I think.

comment:7 in reply to:  6 ; Changed 12 years ago by Leif Leonhardy

Replying to jhpalmieri:

Should a docbuild error just cause an exception to be raised, thus ending the build? That wouldn't be hard to implement, I think.

I would have implemented that if I felt it was appropriate. ;-)

An error in a single document should IMHO not stop / break the whole build, but should be propagated and reported (also) at the end, in a summary. Same for warnings, but even harder to implement. (See also #10200.)

I actually do not understand why we do an equivalent of os.system("sphinx ..."); as far as I know, Sphinx is itself written in Python and its functionality should therefore be directly accessible to us from the builder.py script. This way we would easily get the warnings as well.

comment:8 in reply to:  7 Changed 12 years ago by Leif Leonhardy

Replying to leif:

Replying to jhpalmieri:

Should a docbuild error just cause an exception to be raised, thus ending the build? That wouldn't be hard to implement, I think.

I would have implemented that if I felt it was appropriate. ;-)

Looks as if #10191 does exactly that, but only in one of the two cases.

comment:9 Changed 12 years ago by Jeroen Demeyer

I was unaware of this ticket and created #10191 (for the case where Sphinx fails catastrophically and aborts) and #10200 (for the case where Sphinx gives a WARNING or ERROR but continues).

comment:10 in reply to:  7 Changed 12 years ago by Jeroen Demeyer

Replying to leif:

An error in a single document should IMHO not stop / break the whole build, but should be propagated and reported (also) at the end, in a summary. Same for warnings, but even harder to implement. (See also #10200.)

This is actually implemented now in #10200 (needs_review).

I propose to close this ticket as duplicate (given #10200).

comment:11 Changed 12 years ago by Minh Van Nguyen

Milestone: sage-duplicate/invalid/wontfix
Resolution: duplicate
Status: newclosed

Close as duplicate of #10200.

comment:12 Changed 12 years ago by Jeroen Demeyer

Reviewers: Minh Van Nguyen
Note: See TracTickets for help on using tickets.