From 09b3ba21604c85b197c3356253721170d5e8495e Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Tue, 12 May 2009 00:37:02 +0000 Subject: [PATCH] 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 | 53 +++++++++++++++++++++++-------------------- tests/gclient_test.py | 24 +++++++++----------- 2 files changed, 39 insertions(+), 38 deletions(-) diff --git a/gclient.py b/gclient.py index f48f13b332..43a1a4c2aa 100755 --- a/gclient.py +++ b/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, diff --git a/tests/gclient_test.py b/tests/gclient_test.py index 9d3719dd9b..e7337fd4c2 100644 --- a/tests/gclient_test.py +++ b/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): """ % 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): """ % (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',