From d25fb8f68b1131e5e051fcdc5657318a241e2bef Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Thu, 7 Apr 2011 13:40:15 +0000 Subject: [PATCH] Revert r80770 "Switch from xml.dom.minidom to xml.etree" Throws exceptions on mac. R=dpranke@chromium.org BUG= TEST= Review URL: http://codereview.chromium.org/6811020 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@80771 0039d316-1c4b-4281-b951-d872f2087c98 --- fix_encoding.py | 8 -- gclient_utils.py | 25 ++++++ presubmit_canned_checks.py | 32 ++------ rietveld.py | 3 +- scm.py | 152 +++++++++++++++++++----------------- tests/gclient_utils_test.py | 7 +- tests/scm_unittest.py | 5 +- 7 files changed, 119 insertions(+), 113 deletions(-) diff --git a/fix_encoding.py b/fix_encoding.py index 61bd742b4..87e54a579 100644 --- a/fix_encoding.py +++ b/fix_encoding.py @@ -81,8 +81,6 @@ def fix_win_sys_argv(encoding): if _SYS_ARGV_PROCESSED: return False - # These types are available on linux but not Mac. - # pylint: disable=E0611,F0401 from ctypes import byref, c_int, POINTER, windll, WINFUNCTYPE from ctypes.wintypes import LPCWSTR, LPWSTR @@ -188,8 +186,6 @@ class WinUnicodeConsoleOutput(WinUnicodeOutputBase): self._console_handle = console_handle # Loads the necessary function. - # These types are available on linux but not Mac. - # pylint: disable=E0611,F0401 from ctypes import byref, GetLastError, POINTER, windll, WINFUNCTYPE from ctypes.wintypes import BOOL, DWORD, HANDLE, LPWSTR from ctypes.wintypes import LPVOID # pylint: disable=E0611 @@ -270,8 +266,6 @@ class WinUnicodeOutput(WinUnicodeOutputBase): def win_handle_is_a_console(handle): """Returns True if a Windows file handle is a handle to a console.""" - # These types are available on linux but not Mac. - # pylint: disable=E0611,F0401 from ctypes import byref, POINTER, windll, WINFUNCTYPE from ctypes.wintypes import BOOL, DWORD, HANDLE @@ -303,8 +297,6 @@ def win_get_unicode_stream(stream, excepted_fileno, output_handle, encoding): """ old_fileno = getattr(stream, 'fileno', lambda: None)() if old_fileno == excepted_fileno: - # These types are available on linux but not Mac. - # pylint: disable=E0611,F0401 from ctypes import windll, WINFUNCTYPE from ctypes.wintypes import DWORD, HANDLE diff --git a/gclient_utils.py b/gclient_utils.py index 615497482..97c8227c0 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -14,6 +14,8 @@ import subprocess import sys import threading import time +import xml.dom.minidom +import xml.parsers.expat def hack_subprocess(): @@ -112,6 +114,29 @@ def SplitUrlRevision(url): return tuple(components) +def ParseXML(output): + try: + return xml.dom.minidom.parseString(output) + except xml.parsers.expat.ExpatError: + return None + + +def GetNamedNodeText(node, node_name): + child_nodes = node.getElementsByTagName(node_name) + if not child_nodes: + return None + assert len(child_nodes) == 1 and child_nodes[0].childNodes.length == 1 + return child_nodes[0].firstChild.nodeValue + + +def GetNodeNamedAttributeText(node, node_name, attribute_name): + child_nodes = node.getElementsByTagName(node_name) + if not child_nodes: + return None + assert len(child_nodes) == 1 + return child_nodes[0].getAttribute(attribute_name) + + def SyntaxErrorToError(filename, e): """Raises a gclient_utils.Error exception with the human readable message""" try: diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 890b72cd1..e421bd750 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -544,13 +544,8 @@ def RunPylint(input_api, output_api, white_list=None, black_list=None): The default white_list enforces looking only a *.py files. """ - verbose = False white_list = white_list or ['.*\.py$'] black_list = black_list or input_api.DEFAULT_BLACK_LIST - if input_api.is_committing: - error_type = output_api.PresubmitError - else: - error_type = output_api.PresubmitPromptWarning # Only trigger if there is at least one python file affected. src_filter = lambda x: input_api.FilterSourceFile(x, white_list, black_list) @@ -569,6 +564,10 @@ def RunPylint(input_api, output_api, white_list=None, black_list=None): # were listed, try to run pylint. try: from pylint import lint + result = lint.Run(sorted(files)) + except SystemExit, e: + # pylint has the bad habit of calling sys.exit(), trap it here. + result = e.code except ImportError: if input_api.platform == 'win32': return [output_api.PresubmitNotifyResult( @@ -580,31 +579,16 @@ def RunPylint(input_api, output_api, white_list=None, black_list=None): 'sudo easy_install pylint"\n' 'or visit http://pypi.python.org/pypi/setuptools.\n' 'Cannot do static analysis of python files.')] - - def run_lint(files): - try: - lint.Run(files) - assert False - except SystemExit, e: - # pylint has the bad habit of calling sys.exit(), trap it here. - return e.code - - result = None - if not verbose: - result = run_lint(sorted(files)) - else: - for filename in sorted(files): - print('Running pylint on %s' % filename) - out = run_lint([filename]) - if out: - result = out if result: + if input_api.is_committing: + error_type = output_api.PresubmitError + else: + error_type = output_api.PresubmitPromptWarning return [error_type('Fix pylint errors first.')] return [] finally: warnings.filterwarnings('default', category=DeprecationWarning) - # TODO(dpranke): Get the host_url from the input_api instead def CheckRietveldTryJobExecution(input_api, output_api, host_url, platforms, owner): diff --git a/rietveld.py b/rietveld.py index 11f901865..6984d3934 100644 --- a/rietveld.py +++ b/rietveld.py @@ -34,8 +34,7 @@ import patch # Hack out upload logging.info() upload.logging = logging.getLogger('upload') -# Mac pylint choke on this line. -upload.logging.setLevel(logging.WARNING) # pylint: disable=E1103 +upload.logging.setLevel(logging.WARNING) class Rietveld(object): diff --git a/scm.py b/scm.py index 6c1001fec..31b18f677 100644 --- a/scm.py +++ b/scm.py @@ -14,7 +14,7 @@ import subprocess import sys import tempfile import time -from xml.etree import ElementTree +import xml.dom.minidom import gclient_utils import subprocess2 @@ -512,35 +512,38 @@ class SVN(object): """Returns a dictionary from the svn info output for the given file. Throws an exception if svn info fails.""" - result = {} output = SVN.Capture(['info', '--xml', cwd]) - info = ElementTree.XML(output) - if info is None: - return result - entry = info.find('entry') - - # Use .text when the item is not optional. - result['Path'] = entry.attrib['path'] - result['Revision'] = int(entry.attrib['revision']) - result['Node Kind'] = entry.attrib['kind'] - # Differs across versions. - if result['Node Kind'] == 'dir': - result['Node Kind'] = 'directory' - result['URL'] = entry.find('url').text - repository = entry.find('repository') - result['Repository Root'] = repository.find('root').text - result['UUID'] = repository.find('uuid') - wc_info = entry.find('wc-info') - result['Schedule'] = wc_info.find('schedule').text - result['Copied From URL'] = wc_info.find('copy-from-url') - result['Copied From Rev'] = wc_info.find('copy-from-rev') - for key in result.keys(): - if isinstance(result[key], unicode): - # Unicode results interferes with the higher layers matching up things - # in the deps dictionary. - result[key] = result[key].encode() - # Automatic conversion of optional parameters. - result[key] = getattr(result[key], 'text', result[key]) + dom = gclient_utils.ParseXML(output) + result = {} + if dom: + GetNamedNodeText = gclient_utils.GetNamedNodeText + GetNodeNamedAttributeText = gclient_utils.GetNodeNamedAttributeText + def C(item, f): + if item is not None: + return f(item) + # /info/entry/ + # url + # reposityory/(root|uuid) + # wc-info/(schedule|depth) + # commit/(author|date) + # str() the results because they may be returned as Unicode, which + # interferes with the higher layers matching up things in the deps + # dictionary. + result['Repository Root'] = C(GetNamedNodeText(dom, 'root'), str) + result['URL'] = C(GetNamedNodeText(dom, 'url'), str) + result['UUID'] = C(GetNamedNodeText(dom, 'uuid'), str) + result['Revision'] = C(GetNodeNamedAttributeText(dom, 'entry', + 'revision'), + int) + result['Node Kind'] = C(GetNodeNamedAttributeText(dom, 'entry', 'kind'), + str) + # Differs across versions. + if result['Node Kind'] == 'dir': + result['Node Kind'] = 'directory' + result['Schedule'] = C(GetNamedNodeText(dom, 'schedule'), str) + result['Path'] = C(GetNodeNamedAttributeText(dom, 'entry', 'path'), str) + result['Copied From URL'] = C(GetNamedNodeText(dom, 'copy-from-url'), str) + result['Copied From Rev'] = C(GetNamedNodeText(dom, 'copy-from-rev'), str) return result @staticmethod @@ -550,7 +553,9 @@ class SVN(object): Returns: Int base revision """ - return SVN.CaptureInfo(cwd).get('Revision') + info = SVN.Capture(['info', '--xml'], cwd=cwd) + dom = xml.dom.minidom.parseString(info) + return dom.getElementsByTagName('entry')[0].getAttribute('revision') @staticmethod def CaptureStatus(files): @@ -585,50 +590,51 @@ class SVN(object): 'replaced': 'R', 'unversioned': '?', } - dom = ElementTree.XML(SVN.Capture(command)) + dom = gclient_utils.ParseXML(SVN.Capture(command)) results = [] - if dom is None: - return results - # /status/target/entry/(wc-status|commit|author|date) - for target in dom.findall('target'): - for entry in target.findall('entry'): - file_path = entry.attrib['path'] - wc_status = entry.find('wc-status') - # Emulate svn 1.5 status ouput... - statuses = [' '] * 7 - # Col 0 - xml_item_status = wc_status.attrib['item'] - if xml_item_status in status_letter: - statuses[0] = status_letter[xml_item_status] - else: - raise gclient_utils.Error( - 'Unknown item status "%s"; please implement me!' % - xml_item_status) - # Col 1 - xml_props_status = wc_status.attrib['props'] - if xml_props_status == 'modified': - statuses[1] = 'M' - elif xml_props_status == 'conflicted': - statuses[1] = 'C' - elif (not xml_props_status or xml_props_status == 'none' or - xml_props_status == 'normal'): - pass - else: - raise gclient_utils.Error( - 'Unknown props status "%s"; please implement me!' % - xml_props_status) - # Col 2 - if wc_status.attrib.get('wc-locked') == 'true': - statuses[2] = 'L' - # Col 3 - if wc_status.attrib.get('copied') == 'true': - statuses[3] = '+' - # Col 4 - if wc_status.attrib.get('switched') == 'true': - statuses[4] = 'S' - # TODO(maruel): Col 5 and 6 - item = (''.join(statuses), file_path) - results.append(item) + if dom: + # /status/target/entry/(wc-status|commit|author|date) + for target in dom.getElementsByTagName('target'): + #base_path = target.getAttribute('path') + for entry in target.getElementsByTagName('entry'): + file_path = entry.getAttribute('path') + wc_status = entry.getElementsByTagName('wc-status') + assert len(wc_status) == 1 + # Emulate svn 1.5 status ouput... + statuses = [' '] * 7 + # Col 0 + xml_item_status = wc_status[0].getAttribute('item') + if xml_item_status in status_letter: + statuses[0] = status_letter[xml_item_status] + else: + raise gclient_utils.Error( + 'Unknown item status "%s"; please implement me!' % + xml_item_status) + # Col 1 + xml_props_status = wc_status[0].getAttribute('props') + if xml_props_status == 'modified': + statuses[1] = 'M' + elif xml_props_status == 'conflicted': + statuses[1] = 'C' + elif (not xml_props_status or xml_props_status == 'none' or + xml_props_status == 'normal'): + pass + else: + raise gclient_utils.Error( + 'Unknown props status "%s"; please implement me!' % + xml_props_status) + # Col 2 + if wc_status[0].getAttribute('wc-locked') == 'true': + statuses[2] = 'L' + # Col 3 + if wc_status[0].getAttribute('copied') == 'true': + statuses[3] = '+' + # Col 4 + if wc_status[0].getAttribute('switched') == 'true': + statuses[4] = 'S' + # TODO(maruel): Col 5 and 6 + item = (''.join(statuses), file_path) + results.append(item) return results @staticmethod diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index f0d833bd3..b00db3c80 100755 --- a/tests/gclient_utils_test.py +++ b/tests/gclient_utils_test.py @@ -28,12 +28,13 @@ class GclientUtilsUnittest(GclientUtilBase): 'CheckCall', 'CheckCallError', 'CheckCallAndFilter', 'CheckCallAndFilterAndHeader', 'Error', 'ExecutionQueue', 'FileRead', 'FileWrite', 'FindFileUpwards', 'FindGclientRoot', - 'GetGClientRootAndEntries', 'MakeFileAutoFlush', - 'MakeFileAnnotated', 'PathDifference', 'Popen', + 'GetGClientRootAndEntries', 'GetNamedNodeText', 'MakeFileAutoFlush', + 'GetNodeNamedAttributeText', 'MakeFileAnnotated', 'PathDifference', + 'ParseXML', 'Popen', 'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision', 'SyntaxErrorToError', 'WorkItem', 'errno', 'hack_subprocess', 'logging', 'os', 'Queue', 're', 'rmtree', - 'stat', 'subprocess', 'sys','threading', 'time', + 'stat', 'subprocess', 'sys','threading', 'time', 'xml', ] # If this test fails, you should add the relevant test. self.compareMembers(gclient_utils, members) diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 21150f3ff..1c214267c 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -42,10 +42,10 @@ class RootTestCase(BaseSCMTestCase): def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'ElementTree', 'GetCasedPath', 'GenFakeDiff', 'GIT', 'SVN', - 'ValidateEmail', + 'GetCasedPath', 'GenFakeDiff', 'GIT', 'SVN', 'ValidateEmail', 'cStringIO', 'determine_scm', 'gclient_utils', 'glob', 'logging', 'os', 're', 'shutil', 'subprocess', 'subprocess2', 'sys', 'tempfile', 'time', + 'xml', ] # If this test fails, you should add the relevant test. self.compareMembers(scm, members) @@ -104,7 +104,6 @@ class SVNTestCase(BaseSCMTestCase): self.mox.StubOutWithMock(scm, 'GetCasedPath') scm.os.path.abspath = lambda x: x scm.GetCasedPath = lambda x: x - # pylint: disable=E1103 scm.SVN.CaptureInfo(self.root_dir + '/foo/bar').AndReturn({ 'Repository Root': 'svn://svn.chromium.org/chrome', 'URL': 'svn://svn.chromium.org/chrome/trunk/src',