Redo of r56893 with fix to support python 2.5.

Original Description: Remove code duplication and improve style. Create a Popen function to reduce code duplication. Use RemoveDirectory where relevant. Make drover slightly more posix friendly. Review URL: http://codereview.chromium.org/3199011 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@57083 0039d316-1c4b-4281-b951-d872f2087c98
2026-01-11 18:51:29 +00:00 · 2010-08-23 18:54:55 +00:00
parent dd218e5997
commit 3a292688b6
7 changed files with 66 additions and 105 deletions
--- a/drover.py
+++ b/drover.py
@@ -8,10 +8,11 @@ import os
 import re
 import subprocess
 import sys
-import webbrowser

 import breakpad

+import gclient_utils
+
 USAGE = """
 WARNING: Please use this tool in an empty directory
 (or at least one that you don't mind clobbering.)
@@ -46,31 +47,6 @@ files_info_ = None
 delete_map_ = None
 file_pattern_ =  r"[ ]+([MADUC])[ ]+/((?:trunk|branches/.*?)/src(.*)/(.*))"

-def deltree(root):
-  """Removes a given directory"""
-  if (not os.path.exists(root)):
-    return
-
-  if sys.platform == 'win32':
-    os.system('rmdir /S /Q ' + root.replace('/','\\'))
-  else:
-    for name in os.listdir(root):
-      path = os.path.join(root, name)
-      if os.path.isdir(path):
-        deltree(path)
-      else:
-        os.unlink(path)
-    os.rmdir(root)
-
-def clobberDir(dirname):
-  """Removes a given directory"""
-
-  if (os.path.exists(dirname)):
-    print dir + " directory found, deleting"
-    # The following line was removed due to access controls in Windows
-    # which make os.unlink(path) calls impossible.
-    #TODO(laforge) : Is this correct?
-    deltree(dirname)

 def runGcl(subcommand):
  gcl_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), "gcl")
@@ -87,11 +63,9 @@ def gclUpload(revision, author):
  return runGcl(command)

 def getSVNInfo(url, revision):
-  command = 'svn info ' + url + "@"+str(revision)
-  svn_info = subprocess.Popen(command,
-                              shell=True,
-                              stdout=subprocess.PIPE,
-                              stderr=subprocess.PIPE).stdout.readlines()
+  svn_info = gclient_utils.Popen(['svn', 'info', '%s@%s' % (url, revision)],
+                                 stdout=subprocess.PIPE,
+                                 stderr=subprocess.PIPE).stdout.readlines()
  info = {}
  for line in svn_info:
    match = re.search(r"(.*?):(.*)", line)
@@ -101,11 +75,9 @@ def getSVNInfo(url, revision):
  return info

 def isSVNDirty():
-  command = 'svn status'
-  svn_status = subprocess.Popen(command,
-                                shell=True,
-                                stdout=subprocess.PIPE,
-                                stderr=subprocess.PIPE).stdout.readlines()
+  svn_status = gclient_utils.Popen(['svn', 'status'],
+                                   stdout=subprocess.PIPE,
+                                   stderr=subprocess.PIPE).stdout.readlines()
  for line in svn_status:
    match = re.search(r"^[^X?]", line)
    if match:
@@ -145,22 +117,18 @@ def inCheckoutRoot(path):

 def getRevisionLog(url, revision):
  """Takes an svn url and gets the associated revision."""
-  command = 'svn log ' + url + " -r"+str(revision)
-  svn_log = subprocess.Popen(command,
-                             shell=True,
-                             stdout=subprocess.PIPE,
-                             stderr=subprocess.PIPE).stdout.readlines()
+  svn_log = gclient_utils.Popen(['svn', 'log', url, '-r', str(revision)],
+                                stdout=subprocess.PIPE,
+                                stderr=subprocess.PIPE).stdout.readlines()
  # Don't include the header lines and the trailing "---..." line and eliminate
  # any '\r's.
  return ''.join([l.replace('\r','') for l in svn_log[3:-1]])

 def getSVNVersionInfo():
  """Extract version information from SVN"""
-  command = 'svn --version'
-  svn_info = subprocess.Popen(command,
-                              shell=True,
-                              stdout=subprocess.PIPE,
-                              stderr=subprocess.PIPE).stdout.readlines()
+  svn_info = gclient_utils.Popen(['svn', '--version'],
+                                 stdout=subprocess.PIPE,
+                                 stderr=subprocess.PIPE).stdout.readlines()
  info = {}
  for line in svn_info:
    match = re.search(r"svn, version ((\d+)\.(\d+)\.(\d+)) \(r(\d+)\)", line)
@@ -304,16 +272,14 @@ def revertRevision(url, revision):
    os.system(command)

 def getFileInfo(url, revision):
-  global files_info_, file_pattern_
+  global files_info_

  if (files_info_ != None):
    return files_info_

-  command = 'svn log ' + url + " -r " + str(revision) + " -v"
-  svn_log = subprocess.Popen(command,
-                             shell=True,
-                             stdout=subprocess.PIPE,
-                             stderr=subprocess.PIPE).stdout.readlines()
+  svn_log = gclient_utils.Popen(['svn', 'log', url, '-r', str(revision), '-v'],
+                                stdout=subprocess.PIPE,
+                                stderr=subprocess.PIPE).stdout.readlines()

  info = []
  for line in svn_log:
@@ -470,7 +436,7 @@ def drover(options, args):
    if not (options.revertbot or SKIP_CHECK_WORKING or
        prompt("Working directory: '%s' already exists, clobber?" % working)):
      return 0
-    deltree(working)
+    gclient_utils.RemoveDirectory(working)

  if not options.local:
    os.makedirs(working)
--- a/gcl.py
+++ b/gcl.py
@@ -208,14 +208,8 @@ def ErrorExit(msg):

 def RunShellWithReturnCode(command, print_output=False):
  """Executes a command and returns the output and the return code."""
-  # Use a shell for subcommands on Windows to get a PATH search, and because svn
-  # may be a batch file.
-  use_shell = sys.platform.startswith("win")
-  env = os.environ.copy()
-  env['LANGUAGE'] = 'en'
-  p = subprocess.Popen(command, stdout=subprocess.PIPE,
-                       stderr=subprocess.STDOUT, shell=use_shell, env=env,
-                       universal_newlines=True)
+  p = gclient_utils.Popen(command, stdout=subprocess.PIPE,
+                          stderr=subprocess.STDOUT, universal_newlines=True)
  if print_output:
    output_array = []
    while True:
--- a/gclient_scm.py
+++ b/gclient_scm.py
@@ -653,7 +653,7 @@ class GitWrapper(SCMWrapper):
    cmd.extend(args)
    logging.debug(cmd)
    try:
-      sp = subprocess.Popen(cmd, cwd=cwd, stdout=stdout)
+      sp = gclient_utils.Popen(cmd, cwd=cwd, stdout=stdout)
      output = sp.communicate()[0]
    except OSError:
      raise gclient_utils.Error("git command '%s' failed to run." %
--- a/gclient_utils.py
+++ b/gclient_utils.py
@@ -23,7 +23,6 @@ import subprocess
 import sys
 import threading
 import time
-import threading
 import xml.dom.minidom
 import xml.parsers.expat

@@ -39,8 +38,31 @@ class CheckCallError(OSError):
    self.stderr = stderr


+def Popen(*args, **kwargs):
+  """Calls subprocess.Popen() with hacks to work around certain behaviors.
+
+  Ensure English outpout for svn and make it work reliably on Windows.
+  """
+  copied = False
+  if not 'env' in kwargs:
+    copied = True
+    kwargs = kwargs.copy()
+    # It's easier to parse the stdout if it is always in English.
+    kwargs['env'] = os.environ.copy()
+    kwargs['env']['LANGUAGE'] = 'en'
+  if not 'shell' in kwargs:
+    if not copied:
+      kwargs = kwargs.copy()
+    # *Sigh*:  Windows needs shell=True, or else it won't search %PATH% for the
+    # executable, but shell=True makes subprocess on Linux fail when it's called
+    # with a list because it only tries to execute the first item in the list.
+    kwargs['shell'] = (sys.platform=='win32')
+  return subprocess.Popen(*args, **kwargs)
+
+
 def CheckCall(command, cwd=None, print_error=True):
-  """Like subprocess.check_call() but returns stdout.
+  """Similar subprocess.check_call() but redirects stdout and
+  returns (stdout, stderr).

  Works on python 2.4
  """
@@ -49,13 +71,7 @@ def CheckCall(command, cwd=None, print_error=True):
    stderr = None
    if not print_error:
      stderr = subprocess.PIPE
-    env = os.environ.copy()
-    env['LANGUAGE'] = 'en'
-    process = subprocess.Popen(command, cwd=cwd,
-                               shell=sys.platform.startswith('win'),
-                               stdout=subprocess.PIPE,
-                               stderr=stderr,
-                               env=env)
+    process = Popen(command, cwd=cwd, stdout=subprocess.PIPE, stderr=stderr)
    std_out, std_err = process.communicate()
  except OSError, e:
    raise CheckCallError(command, cwd, e.errno, None)
@@ -275,15 +291,9 @@ def SubprocessCallAndFilter(command,
  if print_messages:
    print('\n________ running \'%s\' in \'%s\''
          % (' '.join(command), in_directory))
-  env = os.environ.copy()
-  env['LANGUAGE'] = 'en'

-  # *Sigh*:  Windows needs shell=True, or else it won't search %PATH% for the
-  # executable, but shell=True makes subprocess on Linux fail when it's called
-  # with a list because it only tries to execute the first item in the list.
-  kid = subprocess.Popen(command, bufsize=0, cwd=in_directory,
-      shell=(sys.platform == 'win32'), stdout=subprocess.PIPE,
-      stderr=subprocess.STDOUT, env=env)
+  kid = Popen(command, bufsize=0, cwd=in_directory,
+              stdout=subprocess.PIPE, stderr=subprocess.STDOUT)

  # Do a flush of sys.stdout before we begin reading from the subprocess's
  # stdout.
@@ -327,7 +337,7 @@ def SubprocessCallAndFilter(command,
    msg = 'failed to run command: %s' % ' '.join(command)

    if fail_status != None:
-      print >>sys.stderr, msg
+      print >> sys.stderr, msg
      sys.exit(fail_status)

    raise Error(msg)
@@ -337,10 +347,10 @@ def FindGclientRoot(from_dir, filename='.gclient'):
  """Tries to find the gclient root."""
  path = os.path.realpath(from_dir)
  while not os.path.exists(os.path.join(path, filename)):
-    next = os.path.split(path)
-    if not next[1]:
+    split_path = os.path.split(path)
+    if not split_path[1]:
      return None
-    path = next[0]
+    path = split_path[0]
  logging.info('Found gclient root at ' + path)
  return path

--- a/scm.py
+++ b/scm.py
@@ -215,7 +215,7 @@ class GIT(object):
      # pipe at a time.
      # The -100 is an arbitrary limit so we don't search forever.
      cmd = ['git', 'log', '-100', '--pretty=medium']
-      proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, cwd=cwd)
+      proc = gclient_utils.Popen(cmd, stdout=subprocess.PIPE, cwd=cwd)
      for line in proc.stdout:
        match = git_svn_re.match(line)
        if match:
@@ -371,19 +371,11 @@ class SVN(object):
    """
    c = [SVN.COMMAND]
    c.extend(args)
-
-    # *Sigh*:  Windows needs shell=True, or else it won't search %PATH% for
-    # the svn.exe executable, but shell=True makes subprocess on Linux fail
-    # when it's called with a list because it only tries to execute the
-    # first string ("svn").
    stderr = None
    if not print_error:
      stderr = subprocess.PIPE
-    return subprocess.Popen(c,
-                            cwd=in_directory,
-                            shell=(sys.platform == 'win32'),
-                            stdout=subprocess.PIPE,
-                            stderr=stderr).communicate()[0]
+    return gclient_utils.Popen(c, cwd=in_directory, stdout=subprocess.PIPE,
+        stderr=stderr).communicate()[0]

  @staticmethod
  def RunAndGetFileList(verbose, args, in_directory, file_list):
--- a/tests/gclient_utils_test.py
+++ b/tests/gclient_utils_test.py
@@ -24,7 +24,7 @@ class GclientUtilsUnittest(GclientUtilBase):
        'CheckCall', 'CheckCallError', 'Error', 'ExecutionQueue', 'FileRead',
        'FileWrite', 'FindFileUpwards', 'FindGclientRoot',
        'GetGClientRootAndEntries', 'GetNamedNodeText',
-        'GetNodeNamedAttributeText', 'PathDifference', 'ParseXML',
+        'GetNodeNamedAttributeText', 'PathDifference', 'ParseXML', 'Popen',
        'PrintableObject', 'RemoveDirectory', 'SplitUrlRevision',
        'SubprocessCall', 'SubprocessCallAndFilter', 'SyntaxErrorToError',
        'WorkItem',
--- a/tests/scm_unittest.py
+++ b/tests/scm_unittest.py
@@ -28,6 +28,7 @@ class BaseTestCase(SuperMoxTestBase):
 class BaseSCMTestCase(BaseTestCase):
  def setUp(self):
    BaseTestCase.setUp(self)
+    self.mox.StubOutWithMock(scm.gclient_utils, 'Popen')
    self.mox.StubOutWithMock(scm.gclient_utils, 'SubprocessCall')
    self.mox.StubOutWithMock(scm.gclient_utils, 'SubprocessCallAndFilter')

@@ -295,11 +296,10 @@ class SVNTestCase(BaseSCMTestCase):
 </status>
 """
    proc = self.mox.CreateMockAnything()
-    scm.subprocess.Popen(['svn', 'status', '--xml', '.'],
-                         cwd=None,
-                         shell=scm.sys.platform.startswith('win'),
-                         stderr=None,
-                         stdout=scm.subprocess.PIPE).AndReturn(proc)
+    scm.gclient_utils.Popen(['svn', 'status', '--xml', '.'],
+                            cwd=None,
+                            stderr=None,
+                            stdout=scm.subprocess.PIPE).AndReturn(proc)
    proc.communicate().AndReturn((text, 0))

    self.mox.ReplayAll()
@@ -328,11 +328,10 @@ class SVNTestCase(BaseSCMTestCase):
       </target>
       </status>"""
    proc = self.mox.CreateMockAnything()
-    scm.subprocess.Popen(['svn', 'status', '--xml'],
-                         cwd=None,
-                         shell=scm.sys.platform.startswith('win'),
-                         stderr=None,
-                         stdout=scm.subprocess.PIPE).AndReturn(proc)
+    scm.gclient_utils.Popen(['svn', 'status', '--xml'],
+                            cwd=None,
+                            stderr=None,
+                            stdout=scm.subprocess.PIPE).AndReturn(proc)
    proc.communicate().AndReturn((text, 0))
    self.mox.ReplayAll()
    info = scm.SVN.CaptureStatus(None)