SpecificationTreeGraphView.renderGraphvizGraph() can fail without properly logging errors and warnings from subprocess

Bug #1087314 reported by Abel Deuring
4
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Launchpad itself
Triaged
High
Unassigned

Bug Description

Running "./bin/test blueprints -vv" in noticed failures in these doc tests

lib/lp/blueprints/doc/specgraph.txt
lib/lp/blueprints/stories/blueprints/xx-non-ascii-imagemap.txt
lib/lp/blueprints/stories/blueprints/xx-dependencies.txt

The errors were not very enlightening; trying to access http://blueprints.launchpad.dev/firefox/+spec/canvas from a web browser revealed that the rendered dependency graphs were not displayed.

A bit of debugging showed that SpecificationTreeGraphView.renderGraphvizGraph() raised an ProblemRenderingGraph because the subprocess simply send a warning about a deprecated font configuration to stderr, so err was not empty:

    def renderGraphvizGraph(self, format):
        """Return graph data in the appropriate format.

        Shell out to `dot` to do the work.
        Raise ProblemRenderingGraph exception if `dot` gives any error output.
        """
        assert format in ('png', 'cmapx')
        input = self.getDotFileText().encode('UTF-8')
        # XXX sinzui 2008-04-03 bug=211568:
        # This use of subprocess.Popen is far from ideal. There is extra
        # risk of getting an OSError, or an command line issue that we
        # represent as a ProblemRenderingGraph. We need python bindings
        # to make the PNG/cmapx.
        cmd = 'unflatten -l 2 | dot -T%s' % format
        process = Popen(
            cmd, shell=True, stdin=PIPE, stdout=PIPE, stderr=PIPE,
            close_fds=True)
        process.stdin.write(input)
        process.stdin.close()
        output = process.stdout.read()
        err = process.stderr.read()
        if err:
            raise ProblemRenderingGraph(err, output)

We should at least log the content of err, if only to make debugging of real problems easier. Also, it might make sense to check if output contains anything that could be an image.And to raise an error only when this is not the case.

The fix of the failures in my case just required a small change of /etc/fonts/conf.d/50-user.conf:

<?xml version="1.0"?>
<!DOCTYPE fontconfig SYSTEM "fonts.dtd">
<fontconfig>
        <!-- Load per-user customization file -->
        <include ignore_missing="yes" prefix="xdg">fontconfig/conf.d</include>
        <include ignore_missing="yes" prefix="xdg">fontconfig/fonts.conf</include>
        <!-- the following elements will be removed in the future -->
        <include ignore_missing="yes" deprecated="yes">~/.fonts.conf.d</include>
        <include ignore_missing="yes" deprecated="yes">~/.fonts.conf</include>
</fontconfig>

I just had to remove 'deprecated="yes"'

Changed in launchpad:
status: New → Triaged
importance: Undecided → Critical
importance: Critical → High
Curtis Hovey (sinzui)
tags: added: lp-blueprints specifications
tags: added: tech-debt
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.