Fix case where wc_info is not present.

Reapply r80770 "Switch from xml.dom.minidom to xml.etree".

TBR=dpranke@chromium.org
BUG=
TEST=

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@80785 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,40 @@ 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')
+    if wc_info is not None:
+      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')
+    else:
+      result['Schedule'] = None
+      result['Copied From URL'] = None
+      result['Copied From Rev'] = None
+    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 +555,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 +590,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',