Switch from xml.dom.minidom to xml.etree

The reason of this change (beside deleting 30 lines) is to fix pylint on Mac.
For an unknown reason, it was dying here trying to process the minidom import.

R=dpranke@chromium.org
BUG=
TEST=

Review URL: http://codereview.chromium.org/6799021

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@80770 0039d316-1c4b-4281-b951-d872f2087c98

fix_encoding.py

@@ -81,6 +81,8 @@ 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
 
@@ -186,6 +188,8 @@ 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
@@ -266,6 +270,8 @@ 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
 
@@ -297,6 +303,8 @@ 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
 

gclient_utils.py

@@ -14,8 +14,6 @@ import subprocess
 import sys
 import threading
 import time
-import xml.dom.minidom
-import xml.parsers.expat
 
 
 def hack_subprocess():
@@ -114,29 +112,6 @@ 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:

presubmit_canned_checks.py

@@ -544,8 +544,13 @@ 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)
@@ -564,10 +569,6 @@ 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(
@@ -579,16 +580,31 @@ 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):

rietveld.py

@@ -34,7 +34,8 @@ import patch
 
 # Hack out upload logging.info()
 upload.logging = logging.getLogger('upload')
-upload.logging.setLevel(logging.WARNING)
+# Mac pylint choke on this line.
+upload.logging.setLevel(logging.WARNING)  # pylint: disable=E1103
 
 
 class Rietveld(object):

scm.py

@@ -14,7 +14,7 @@ import subprocess
 import sys
 import tempfile
 import time
-import xml.dom.minidom
+from xml.etree import ElementTree
 
 import gclient_utils
 import subprocess2
@@ -512,38 +512,35 @@ class SVN(object):
     """Returns a dictionary from the svn info output for the given file.
 
     Throws an exception if svn info fails."""
-    output = SVN.Capture(['info', '--xml', cwd])
-    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)
+    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])
     return result
 
   @staticmethod
@@ -553,9 +550,7 @@ class SVN(object):
     Returns:
       Int base revision
     """
-    info = SVN.Capture(['info', '--xml'], cwd=cwd)
-    dom = xml.dom.minidom.parseString(info)
-    return dom.getElementsByTagName('entry')[0].getAttribute('revision')
+    return SVN.CaptureInfo(cwd).get('Revision')
 
   @staticmethod
   def CaptureStatus(files):
@@ -590,51 +585,50 @@ class SVN(object):
       'replaced': 'R',
       'unversioned': '?',
     }
-    dom = gclient_utils.ParseXML(SVN.Capture(command))
+    dom = ElementTree.XML(SVN.Capture(command))
     results = []
-    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)
+    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)
     return results
 
   @staticmethod

tests/gclient_utils_test.py

@@ -28,13 +28,12 @@ class GclientUtilsUnittest(GclientUtilBase):
         'CheckCall', 'CheckCallError', 'CheckCallAndFilter',
         'CheckCallAndFilterAndHeader', 'Error', 'ExecutionQueue', 'FileRead',
         'FileWrite', 'FindFileUpwards', 'FindGclientRoot',
-        'GetGClientRootAndEntries', 'GetNamedNodeText', 'MakeFileAutoFlush',
-        'GetNodeNamedAttributeText', 'MakeFileAnnotated', 'PathDifference',
-        'ParseXML', 'Popen',
+        'GetGClientRootAndEntries', 'MakeFileAutoFlush',
+        'MakeFileAnnotated', 'PathDifference', 'Popen',
         'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision',
         'SyntaxErrorToError', 'WorkItem',
         'errno', 'hack_subprocess', 'logging', 'os', 'Queue', 're', 'rmtree',
-        'stat', 'subprocess', 'sys','threading', 'time', 'xml',
+        'stat', 'subprocess', 'sys','threading', 'time',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(gclient_utils, members)

tests/scm_unittest.py

@@ -42,10 +42,10 @@ class RootTestCase(BaseSCMTestCase):
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
-        'GetCasedPath', 'GenFakeDiff', 'GIT', 'SVN', 'ValidateEmail',
+        'ElementTree', '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,6 +104,7 @@ 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',