Cleanup gclient.py use of options where unnecessary.

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

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

gclient.py

@@ -475,7 +475,7 @@ def IsUsingGit(root, paths):
 # SVN utils:
 
 
-def RunSVN(options, args, in_directory):
+def RunSVN(args, in_directory):
   """Runs svn, sending output to stdout.
 
   Args:
@@ -491,7 +491,7 @@ def RunSVN(options, args, in_directory):
   SubprocessCall(c, in_directory, options.stdout)
 
 
-def CaptureSVN(options, args, in_directory):
+def CaptureSVN(args, in_directory=None, print_error=True):
   """Runs svn, capturing output sent to stdout as a string.
 
   Args:
@@ -508,8 +508,14 @@ def CaptureSVN(options, args, in_directory):
   # 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").
-  return subprocess.Popen(c, cwd=in_directory, shell=(sys.platform == 'win32'),
-                          stdout=subprocess.PIPE).communicate()[0]
+  stderr = None
+  if print_error:
+    stderr = subprocess.PIPE
+  return subprocess.Popen(c,
+                          cwd=in_directory,
+                          shell=(sys.platform == 'win32'),
+                          stdout=subprocess.PIPE,
+                          stderr=stderr).communicate()[0]
 
 
 def RunSVNAndGetFileList(options, args, in_directory, file_list):
@@ -556,7 +562,7 @@ def RunSVNAndGetFileList(options, args, in_directory, file_list):
                            pattern=pattern, capture_list=file_list)
 
 
-def CaptureSVNInfo(options, relpath, in_directory):
+def CaptureSVNInfo(relpath, in_directory=None, print_error=True):
   """Returns a dictionary from the svn info output for the given file.
 
   Args:
@@ -564,7 +570,8 @@ def CaptureSVNInfo(options, relpath, in_directory):
       the directory given by in_directory.
     in_directory: The directory where svn is to be run.
   """
-  dom = ParseXML(CaptureSVN(options, ["info", "--xml", relpath], in_directory))
+  output = CaptureSVN(["info", "--xml", relpath], in_directory, print_error)
+  dom = ParseXML(output)
   result = {}
   if dom:
     def C(item, f):
@@ -592,13 +599,13 @@ def CaptureSVNInfo(options, relpath, in_directory):
   return result
 
 
-def CaptureSVNHeadRevision(options, url):
+def CaptureSVNHeadRevision(url):
   """Get the head revision of a SVN repository.
 
   Returns:
     Int head revision
   """
-  info = CaptureSVN(options, ["info", "--xml", url], os.getcwd())
+  info = CaptureSVN(["info", "--xml", url], os.getcwd())
   dom = xml.dom.minidom.parseString(info)
   return int(dom.getElementsByTagName('entry')[0].getAttribute('revision'))
 
@@ -617,7 +624,7 @@ class FileStatus:
             self.path)
 
 
-def CaptureSVNStatus(options, path):
+def CaptureSVNStatus(path):
   """Runs 'svn status' on an existing path.
 
   Args:
@@ -626,7 +633,7 @@ def CaptureSVNStatus(options, path):
   Returns:
     An array of FileStatus corresponding to the emulated output of 'svn status'
     version 1.5."""
-  dom = ParseXML(CaptureSVN(options, ["status", "--xml"], path))
+  dom = ParseXML(CaptureSVN(["status", "--xml"], path))
   results = []
   if dom:
     # /status/target/entry/(wc-status|commit|author|date)
@@ -728,13 +735,13 @@ class SCMWrapper(object):
     """Cleanup working copy."""
     command = ['cleanup']
     command.extend(args)
-    RunSVN(options, command, os.path.join(self._root_dir, self.relpath))
+    RunSVN(command, os.path.join(self._root_dir, self.relpath))
 
   def diff(self, options, args, file_list):
     # NOTE: This function does not currently modify file_list.
     command = ['diff']
     command.extend(args)
-    RunSVN(options, command, os.path.join(self._root_dir, self.relpath))
+    RunSVN(command, os.path.join(self._root_dir, self.relpath))
 
   def update(self, options, args, file_list):
     """Runs SCM to update or transparently checkout the working copy.
@@ -780,19 +787,18 @@ class SCMWrapper(object):
       return
 
     # Get the existing scm url and the revision number of the current checkout.
-    from_info = CaptureSVNInfo(options,
-                               os.path.join(self._root_dir, self.relpath, '.'),
+    from_info = CaptureSVNInfo(os.path.join(self._root_dir, self.relpath, '.'),
                                '.')
 
     if options.manually_grab_svn_rev:
       # Retrieve the current HEAD version because svn is slow at null updates.
       if not revision:
-        from_info_live = CaptureSVNInfo(options, from_info['URL'], '.')
+        from_info_live = CaptureSVNInfo(from_info['URL'], '.')
         revision = int(from_info_live['Revision'])
         rev_str = ' at %d' % revision
 
     if from_info['URL'] != components[0]:
-      to_info = CaptureSVNInfo(options, url, '.')
+      to_info = CaptureSVNInfo(url, '.')
       if from_info['Repository Root'] != to_info['Repository Root']:
         # We have different roots, so check if we can switch --relocate.
         # Subversion only permits this if the repository UUIDs match.
@@ -814,7 +820,7 @@ class SCMWrapper(object):
                    from_info['Repository Root'],
                    to_info['Repository Root'],
                    self.relpath]
-        RunSVN(options, command, self._root_dir)
+        RunSVN(command, self._root_dir)
         from_info['URL'] = from_info['URL'].replace(
             from_info['Repository Root'],
             to_info['Repository Root'])
@@ -847,7 +853,7 @@ class SCMWrapper(object):
       # Don't reuse the args.
       return self.update(options, [], file_list)
 
-    files = CaptureSVNStatus(options, path)
+    files = CaptureSVNStatus(path)
     # Batch the command.
     files_to_revert = []
     for file in files:
@@ -876,7 +882,7 @@ class SCMWrapper(object):
       for p in files_to_revert:
         # Some shell have issues with command lines too long.
         if accumulated_length and accumulated_length + len(p) > 3072:
-          RunSVN(options, command + accumulated_paths,
+          RunSVN(command + accumulated_paths,
                  os.path.join(self._root_dir, self.relpath))
           accumulated_paths = []
           accumulated_length = 0
@@ -884,7 +890,7 @@ class SCMWrapper(object):
           accumulated_paths.append(p)
           accumulated_length += len(p)
       if accumulated_paths:
-        RunSVN(options, command + accumulated_paths,
+        RunSVN(command + accumulated_paths,
                os.path.join(self._root_dir, self.relpath))
 
   def status(self, options, args, file_list):
@@ -1325,7 +1331,7 @@ class GClient(object):
       for entry in prev_entries:
         e_dir = os.path.join(self._root_dir, entry)
         if entry not in entries and self._options.path_exists(e_dir):
-          if CaptureSVNStatus(self._options, e_dir):
+          if CaptureSVNStatus(e_dir):
             # There are modified files in this entry
             entries[entry] = None  # Keep warning until removed.
             print >> self._options.stdout, (
@@ -1383,8 +1389,7 @@ class GClient(object):
           return (original_url, int(revision_overrides[name]))
         else:
           # TODO(aharper): SVN/SCMWrapper cleanup (non-local commandset)
-          return (original_url, CaptureSVNHeadRevision(self._options,
-                                                       original_url))
+          return (original_url, CaptureSVNHeadRevision(original_url))
       else:
         url_components = original_url.split("@")
         if revision_overrides.has_key(name):
@@ -1401,7 +1406,6 @@ class GClient(object):
       entries[name] = "%s@%d" % (url, rev)
       # TODO(aharper): SVN/SCMWrapper cleanup (non-local commandset)
       entries_deps_content[name] = CaptureSVN(
-                                     self._options,
                                      ["cat",
                                       "%s/%s@%d" % (url,
                                                     self._options.deps_file,
@@ -1429,7 +1433,6 @@ class GClient(object):
         deps_parent_url_components = deps_parent_url.split("@")
         # TODO(aharper): SVN/SCMWrapper cleanup (non-local commandset)
         deps_parent_content = CaptureSVN(
-                                self._options,
                                 ["cat",
                                  "%s/%s@%s" % (deps_parent_url_components[0],
                                                self._options.deps_file,

tests/gclient_test.py

@@ -1145,7 +1145,7 @@ class SCMWrapperTestCase(BaseTestCase):
     base_path = os.path.join(self.root_dir, self.relpath)
     gclient.os.path.isdir = self.mox.CreateMockAnything()
     gclient.os.path.isdir(base_path).AndReturn(True)
-    gclient.CaptureSVNStatus(options, base_path).AndReturn([])
+    gclient.CaptureSVNStatus(base_path).AndReturn([])
 
     self.mox.ReplayAll()
     scm = gclient.SCMWrapper(url=self.url, root_dir=self.root_dir,
@@ -1164,11 +1164,11 @@ class SCMWrapperTestCase(BaseTestCase):
       gclient.FileStatus('a', 'M', ' ', ' ', ' '), 
       gclient.FileStatus('b', 'A', ' ', ' ', ' '),
     ]
-    gclient.CaptureSVNStatus(options, base_path).AndReturn(items)
+    gclient.CaptureSVNStatus(base_path).AndReturn(items)
 
     print >>options.stdout, os.path.join(base_path, 'a')
     print >>options.stdout, os.path.join(base_path, 'b')
-    gclient.RunSVN(options, ['revert', 'a', 'b'], base_path)
+    gclient.RunSVN(['revert', 'a', 'b'], base_path)
 
     self.mox.ReplayAll()
     scm = gclient.SCMWrapper(url=self.url, root_dir=self.root_dir,
@@ -1229,10 +1229,10 @@ class SCMWrapperTestCase(BaseTestCase):
     options.path_exists(os.path.join(base_path, '.git')).AndReturn(False)
     # Checkout or update.
     options.path_exists(base_path).AndReturn(True)
-    gclient.CaptureSVNInfo(options, os.path.join(base_path, "."), '.'
+    gclient.CaptureSVNInfo(os.path.join(base_path, "."), '.'
         ).AndReturn(file_info)
     # Cheat a bit here.
-    gclient.CaptureSVNInfo(options, file_info['URL'], '.').AndReturn(file_info)
+    gclient.CaptureSVNInfo(file_info['URL'], '.').AndReturn(file_info)
     additional_args = []
     if options.manually_grab_svn_rev:
       additional_args = ['--revision', str(file_info['Revision'])]
@@ -1276,9 +1276,8 @@ class SCMWrapperTestCase(BaseTestCase):
 </entry>
 </info>
 """ % self.url
-    options = self.Options(verbose=True)
-    gclient.CaptureSVN(options, ['info', '--xml', self.url],
-                       '.').AndReturn(xml_text)
+    gclient.CaptureSVN(['info', '--xml', self.url],
+                       '.', True).AndReturn(xml_text)
     expected = {
       'URL': 'http://src.chromium.org/svn/trunk/src/chrome/app/d',
       'UUID': None,
@@ -1291,7 +1290,7 @@ class SCMWrapperTestCase(BaseTestCase):
       'Node Kind': 'file',
     }
     self.mox.ReplayAll()
-    file_info = self._CaptureSVNInfo(options, self.url, '.')
+    file_info = self._CaptureSVNInfo(self.url, '.', True)
     self.assertEquals(sorted(file_info.items()), sorted(expected.items()))
     self.mox.VerifyAll()
 
@@ -1319,11 +1318,10 @@ class SCMWrapperTestCase(BaseTestCase):
 </entry>
 </info>
 """ % (self.url, self.root_dir)
-    options = self.Options(verbose=True)
-    gclient.CaptureSVN(options, ['info', '--xml', self.url],
-                       '.').AndReturn(xml_text)
+    gclient.CaptureSVN(['info', '--xml', self.url],
+                       '.', True).AndReturn(xml_text)
     self.mox.ReplayAll()
-    file_info = self._CaptureSVNInfo(options, self.url, '.')
+    file_info = self._CaptureSVNInfo(self.url, '.', True)
     expected = {
       'URL': self.url,
       'UUID': '7b9385f5-0452-0410-af26-ad4892b7a1fb',