Changeset 58059 in webkit


Ignore:
Timestamp:
Apr 21, 2010 11:18:56 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-04-21 Eric Seidel <eric@webkit.org>

Unreviewed. Another attempt to fix NRWT for chromium.

REGRESSION(57531): the commit-queue still hates Tor Arne Vestbø
https://bugs.webkit.org/show_bug.cgi?id=37765

  • Scripts/webkitpy/layout_tests/port/base.py:
    • wdiff_text was returning a byte array instead of a unicode string. The simple fix was to just decode the result. However, seeing so much duplicated code with Executive made me cry, so I re-wrote the function to be more like pretty_patch_text and use run_command (which already knows how to handle unicode).
Location:
trunk/WebKitTools
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebKitTools/ChangeLog

    r58056 r58059  
     12010-04-21  Eric Seidel  <eric@webkit.org>
     2
     3        Unreviewed.  Another attempt to fix NRWT for chromium.
     4
     5        REGRESSION(57531): the commit-queue still hates Tor Arne Vestbø
     6        https://bugs.webkit.org/show_bug.cgi?id=37765
     7
     8        * Scripts/webkitpy/layout_tests/port/base.py:
     9         - wdiff_text was returning a byte array instead of a
     10           unicode string.  The simple fix was to just decode
     11           the result.  However, seeing so much duplicated code
     12           with Executive made me cry, so I re-wrote the function
     13           to be more like pretty_patch_text and use run_command
     14           (which already knows how to handle unicode).
     15
    1162010-04-21  Adam Barth  <abarth@webkit.org>
    217
  • trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py

    r57860 r58059  
    5050
    5151
    52 # Python bug workaround.  See Port.wdiff_text() for an explanation.
     52# Python's Popen has a bug that causes any pipes opened to a
     53# process that can't be executed to be leaked.  Since this
     54# code is specifically designed to tolerate exec failures
     55# to gracefully handle cases where wdiff is not installed,
     56# the bug results in a massive file descriptor leak. As a
     57# workaround, if an exec failure is ever experienced for
     58# wdiff, assume it's not available.  This will leak one
     59# file descriptor but that's better than leaking each time
     60# wdiff would be run.
     61#
     62# http://mail.python.org/pipermail/python-list/
     63#    2008-August/505753.html
     64# http://bugs.python.org/issue3210
    5365_wdiff_available = True
    5466_pretty_patch_available = True
     
    520532        raise NotImplementedError('Port.version')
    521533
     534    _WDIFF_DEL = '##WDIFF_DEL##'
     535    _WDIFF_ADD = '##WDIFF_ADD##'
     536    _WDIFF_END = '##WDIFF_END##'
     537
     538    def _format_wdiff_output_as_html(self, wdiff):
     539        wdiff = cgi.escape(wdiff)
     540        wdiff = wdiff.replace(_WDIFF_DEL, '<span class=del>')
     541        wdiff = wdiff.replace(_WDIFF_ADD, '<span class=add>')
     542        wdiff = wdiff.replace(_WDIFF_END, '</span>')
     543        html = '<head><style>.del { background: #faa; } '
     544        html += '.add { background: #afa; }</style></head>'
     545        html += "<pre>%s</pre>" % wdiff
     546        return result
     547
     548    def _wdiff_command(self):
     549        executable = self._path_to_wdiff()
     550        return [executable,
     551                "--start-delete=%s" % _WDIFF_DEL,
     552                "--end-delete=%s" % _WDIFF_END,
     553                "--start-insert=%s" % _WDIFF_ADD,
     554                "--end-insert=%s" % _WDIFF_END,
     555                actual_filename,
     556                expected_filename]
     557
    522558    def wdiff_text(self, actual_filename, expected_filename):
    523559        """Returns a string of HTML indicating the word-level diff of the
    524560        contents of the two filenames. Returns an empty string if word-level
    525561        diffing isn't available."""
    526         executable = self._path_to_wdiff()
    527         cmd = [executable,
    528                '--start-delete=##WDIFF_DEL##',
    529                '--end-delete=##WDIFF_END##',
    530                '--start-insert=##WDIFF_ADD##',
    531                '--end-insert=##WDIFF_END##',
    532                actual_filename,
    533                expected_filename]
    534         # FIXME: Why not just check os.exists(executable) once?
     562
    535563        global _wdiff_available
    536         result = ''
     564        if not _wdiff_available:
     565            return ""
    537566        try:
    538             # Python's Popen has a bug that causes any pipes opened to a
    539             # process that can't be executed to be leaked.  Since this
    540             # code is specifically designed to tolerate exec failures
    541             # to gracefully handle cases where wdiff is not installed,
    542             # the bug results in a massive file descriptor leak. As a
    543             # workaround, if an exec failure is ever experienced for
    544             # wdiff, assume it's not available.  This will leak one
    545             # file descriptor but that's better than leaking each time
    546             # wdiff would be run.
    547             #
    548             # http://mail.python.org/pipermail/python-list/
    549             #    2008-August/505753.html
    550             # http://bugs.python.org/issue3210
    551             #
    552             # It also has a threading bug, so we don't output wdiff if
    553             # the Popen raises a ValueError.
    554             # http://bugs.python.org/issue1236
    555             if _wdiff_available:
    556                 try:
    557                     # FIXME: Use Executive() here.
    558                     wdiff = subprocess.Popen(cmd,
    559                         stdout=subprocess.PIPE).communicate()[0]
    560                 except ValueError, e:
    561                     # Working around a race in Python 2.4's implementation
    562                     # of Popen().
    563                     wdiff = ''
    564                 wdiff = cgi.escape(wdiff)
    565                 wdiff = wdiff.replace('##WDIFF_DEL##', '<span class=del>')
    566                 wdiff = wdiff.replace('##WDIFF_ADD##', '<span class=add>')
    567                 wdiff = wdiff.replace('##WDIFF_END##', '</span>')
    568                 result = '<head><style>.del { background: #faa; } '
    569                 result += '.add { background: #afa; }</style></head>'
    570                 result += '<pre>' + wdiff + '</pre>'
     567            return self._executive.run_command(self._wdiff_command())
    571568        except OSError, e:
    572             if (e.errno == errno.ENOENT or e.errno == errno.EACCES or
    573                 e.errno == errno.ECHILD):
    574                 _wdiff_available = False
    575             else:
    576                 raise e
    577         return result
     569            # If the system is missing wdiff stop trying.
     570            _wdiff_available = False
     571            return ''
     572        except ScriptError, e:
     573            # If wdiff failed to run for some reason, stop trying.
     574            _wdiff_available = False
     575            return ""
     576
    578577
    579578    _pretty_patch_error_html = "Failed to run PrettyPatch, see error console."
Note: See TracChangeset for help on using the changeset viewer.