From 11e0fd6997a744d808276df959dd31376f22e71d Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Tue, 12 May 2009 00:47:31 +0000 Subject: [PATCH] Revert changes 15823 and 15824 because they broke gclient revert. TBR=sgk Review URL: http://codereview.chromium.org/115217 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@15827 0039d316-1c4b-4281-b951-d872f2087c98 --- gcl.py | 64 +++++++++++++++++++++++------- gclient.py | 53 ++++++++++++------------- tests/gcl_unittest.py | 77 +++++++++++++++++-------------------- tests/gclient_test.py | 24 ++++++------ tests/trychange_unittest.py | 72 ---------------------------------- trychange.py | 3 +- 6 files changed, 126 insertions(+), 167 deletions(-) delete mode 100644 tests/trychange_unittest.py diff --git a/gcl.py b/gcl.py index 6e64702d8..dd2b63979 100755 --- a/gcl.py +++ b/gcl.py @@ -18,8 +18,6 @@ import upload import urllib2 import xml.dom.minidom -# gcl now depends on gclient. -import gclient __version__ = '1.0' @@ -50,16 +48,60 @@ MISSING_TEST_MSG = "Change contains new or modified methods, but no new tests!" read_gcl_info = False +### Simplified XML processing functions. + +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) + + ### SVN Functions def IsSVNMoved(filename): """Determine if a file has been added through svn mv""" - info = gclient.CaptureSVNInfo(filename) + info = GetSVNFileInfo(filename) return (info.get('Copied From URL') and info.get('Copied From Rev') and info.get('Schedule') == 'add') +def GetSVNFileInfo(file): + """Returns a dictionary from the svn info output for the given file.""" + dom = ParseXML(RunShell(["svn", "info", "--xml", file])) + result = {} + if dom: + # /info/entry/ + # url + # reposityory/(root|uuid) + # wc-info/(schedule|depth) + # commit/(author|date) + result['Node Kind'] = GetNodeNamedAttributeText(dom, 'entry', 'kind') + result['Repository Root'] = GetNamedNodeText(dom, 'root') + result['Schedule'] = GetNamedNodeText(dom, 'schedule') + result['URL'] = GetNamedNodeText(dom, 'url') + result['Path'] = GetNodeNamedAttributeText(dom, 'entry', 'path') + result['Copied From URL'] = GetNamedNodeText(dom, 'copy-from-url') + result['Copied From Rev'] = GetNamedNodeText(dom, 'copy-from-rev') + return result + + def GetSVNFileProperty(file, property_name): """Returns the value of an SVN property for the given file. @@ -105,7 +147,7 @@ def GetSVNStatus(file): 'unversioned': '?', # TODO(maruel): Find the corresponding strings for X, ~ } - dom = gclient.ParseXML(RunShell(command)) + dom = ParseXML(RunShell(command)) results = [] if dom: # /status/target/entry/(wc-status|commit|author|date) @@ -160,16 +202,14 @@ def GetRepositoryRoot(): """ global repository_root if not repository_root: - infos = gclient.CaptureSVNInfo(os.getcwd(), print_error=False) - cur_dir_repo_root = infos.get("Repository Root") + cur_dir_repo_root = GetSVNFileInfo(os.getcwd()).get("Repository Root") if not cur_dir_repo_root: raise Exception("gcl run outside of repository") repository_root = os.getcwd() while True: parent = os.path.dirname(repository_root) - if (gclient.CaptureSVNInfo(parent).get("Repository Root") != - cur_dir_repo_root): + if GetSVNFileInfo(parent).get("Repository Root") != cur_dir_repo_root: break repository_root = parent return repository_root @@ -192,7 +232,7 @@ def GetCodeReviewSetting(key): cached_settings_file = os.path.join(GetInfoDir(), CODEREVIEW_SETTINGS_FILE) if (not os.path.exists(cached_settings_file) or os.stat(cached_settings_file).st_mtime > 60*60*24*3): - dir_info = gclient.CaptureSVNInfo(".") + dir_info = GetSVNFileInfo(".") repo_root = dir_info["Repository Root"] url_path = dir_info["URL"] settings = "" @@ -294,13 +334,11 @@ class ChangeInfo: files: a list of 2 tuple containing (status, filename) of changed files, with paths being relative to the top repository directory. """ - def __init__(self, name="", issue="", description="", files=None): + def __init__(self, name="", issue="", description="", files=[]): self.name = name self.issue = issue self.description = description self.files = files - if self.files is None: - self.files = [] self.patch = None def FileList(self): @@ -712,7 +750,7 @@ def GenerateDiff(files, root=None): for file in files: # Use svn info output instead of os.path.isdir because the latter fails # when the file is deleted. - if gclient.CaptureSVNInfo(file).get("Node Kind") in ("dir", "directory"): + if GetSVNFileInfo(file).get("Node Kind") in ("dir", "directory"): continue # If the user specified a custom diff command in their svn config file, # then it'll be used when we do svn diff, which we don't want to happen diff --git a/gclient.py b/gclient.py index 43a1a4c2a..f48f13b33 100755 --- a/gclient.py +++ b/gclient.py @@ -475,7 +475,7 @@ def IsUsingGit(root, paths): # SVN utils: -def RunSVN(args, in_directory): +def RunSVN(options, args, in_directory): """Runs svn, sending output to stdout. Args: @@ -491,7 +491,7 @@ def RunSVN(args, in_directory): SubprocessCall(c, in_directory, options.stdout) -def CaptureSVN(args, in_directory=None, print_error=True): +def CaptureSVN(options, args, in_directory): """Runs svn, capturing output sent to stdout as a string. Args: @@ -508,14 +508,8 @@ def CaptureSVN(args, in_directory=None, print_error=True): # 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 print_error: - stderr = subprocess.PIPE - return subprocess.Popen(c, - cwd=in_directory, - shell=(sys.platform == 'win32'), - stdout=subprocess.PIPE, - stderr=stderr).communicate()[0] + return subprocess.Popen(c, cwd=in_directory, shell=(sys.platform == 'win32'), + stdout=subprocess.PIPE).communicate()[0] def RunSVNAndGetFileList(options, args, in_directory, file_list): @@ -562,7 +556,7 @@ def RunSVNAndGetFileList(options, args, in_directory, file_list): pattern=pattern, capture_list=file_list) -def CaptureSVNInfo(relpath, in_directory=None, print_error=True): +def CaptureSVNInfo(options, relpath, in_directory): """Returns a dictionary from the svn info output for the given file. Args: @@ -570,8 +564,7 @@ def CaptureSVNInfo(relpath, in_directory=None, print_error=True): the directory given by in_directory. in_directory: The directory where svn is to be run. """ - output = CaptureSVN(["info", "--xml", relpath], in_directory, print_error) - dom = ParseXML(output) + dom = ParseXML(CaptureSVN(options, ["info", "--xml", relpath], in_directory)) result = {} if dom: def C(item, f): @@ -599,13 +592,13 @@ def CaptureSVNInfo(relpath, in_directory=None, print_error=True): return result -def CaptureSVNHeadRevision(url): +def CaptureSVNHeadRevision(options, url): """Get the head revision of a SVN repository. Returns: Int head revision """ - info = CaptureSVN(["info", "--xml", url], os.getcwd()) + info = CaptureSVN(options, ["info", "--xml", url], os.getcwd()) dom = xml.dom.minidom.parseString(info) return int(dom.getElementsByTagName('entry')[0].getAttribute('revision')) @@ -624,7 +617,7 @@ class FileStatus: self.path) -def CaptureSVNStatus(path): +def CaptureSVNStatus(options, path): """Runs 'svn status' on an existing path. Args: @@ -633,7 +626,7 @@ def CaptureSVNStatus(path): Returns: An array of FileStatus corresponding to the emulated output of 'svn status' version 1.5.""" - dom = ParseXML(CaptureSVN(["status", "--xml"], path)) + dom = ParseXML(CaptureSVN(options, ["status", "--xml"], path)) results = [] if dom: # /status/target/entry/(wc-status|commit|author|date) @@ -735,13 +728,13 @@ class SCMWrapper(object): """Cleanup working copy.""" command = ['cleanup'] command.extend(args) - RunSVN(command, os.path.join(self._root_dir, self.relpath)) + RunSVN(options, 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(command, os.path.join(self._root_dir, self.relpath)) + RunSVN(options, 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. @@ -787,18 +780,19 @@ class SCMWrapper(object): return # Get the existing scm url and the revision number of the current checkout. - from_info = CaptureSVNInfo(os.path.join(self._root_dir, self.relpath, '.'), + from_info = CaptureSVNInfo(options, + 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(from_info['URL'], '.') + from_info_live = CaptureSVNInfo(options, from_info['URL'], '.') revision = int(from_info_live['Revision']) rev_str = ' at %d' % revision if from_info['URL'] != components[0]: - to_info = CaptureSVNInfo(url, '.') + to_info = CaptureSVNInfo(options, 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. @@ -820,7 +814,7 @@ class SCMWrapper(object): from_info['Repository Root'], to_info['Repository Root'], self.relpath] - RunSVN(command, self._root_dir) + RunSVN(options, command, self._root_dir) from_info['URL'] = from_info['URL'].replace( from_info['Repository Root'], to_info['Repository Root']) @@ -853,7 +847,7 @@ class SCMWrapper(object): # Don't reuse the args. return self.update(options, [], file_list) - files = CaptureSVNStatus(path) + files = CaptureSVNStatus(options, path) # Batch the command. files_to_revert = [] for file in files: @@ -882,7 +876,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(command + accumulated_paths, + RunSVN(options, command + accumulated_paths, os.path.join(self._root_dir, self.relpath)) accumulated_paths = [] accumulated_length = 0 @@ -890,7 +884,7 @@ class SCMWrapper(object): accumulated_paths.append(p) accumulated_length += len(p) if accumulated_paths: - RunSVN(command + accumulated_paths, + RunSVN(options, command + accumulated_paths, os.path.join(self._root_dir, self.relpath)) def status(self, options, args, file_list): @@ -1331,7 +1325,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(e_dir): + if CaptureSVNStatus(self._options, e_dir): # There are modified files in this entry entries[entry] = None # Keep warning until removed. print >> self._options.stdout, ( @@ -1389,7 +1383,8 @@ class GClient(object): return (original_url, int(revision_overrides[name])) else: # TODO(aharper): SVN/SCMWrapper cleanup (non-local commandset) - return (original_url, CaptureSVNHeadRevision(original_url)) + return (original_url, CaptureSVNHeadRevision(self._options, + original_url)) else: url_components = original_url.split("@") if revision_overrides.has_key(name): @@ -1406,6 +1401,7 @@ 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, @@ -1433,6 +1429,7 @@ 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/gcl_unittest.py b/tests/gcl_unittest.py index 54e6d70be..b36dd9a57 100755 --- a/tests/gcl_unittest.py +++ b/tests/gcl_unittest.py @@ -5,9 +5,7 @@ """Unit tests for gcl.py.""" -import StringIO import os -import sys import unittest # Local imports @@ -48,14 +46,15 @@ class GclUnittest(GclTestsBase): 'ErrorExit', 'GenerateChangeName', 'GenerateDiff', 'GetCLs', 'GetChangelistInfoFile', 'GetCodeReviewSetting', 'GetEditor', 'GetFilesNotInCL', 'GetInfoDir', 'GetIssueDescription', - 'GetModifiedFiles', 'GetRepositoryRoot', 'GetSVNStatus', + 'GetModifiedFiles', 'GetNamedNodeText', 'GetNodeNamedAttributeText', + 'GetRepositoryRoot', 'GetSVNFileInfo', 'GetSVNStatus', 'GetSVNFileProperty', 'Help', 'IGNORE_PATHS', 'IsSVNMoved', 'IsTreeOpen', 'Lint', 'LoadChangelistInfo', 'LoadChangelistInfoForMultiple', - 'MISSING_TEST_MSG', 'Opened', 'PresubmitCL', 'ReadFile', + 'MISSING_TEST_MSG', 'Opened', 'ParseXML', 'PresubmitCL', 'ReadFile', 'RunShell', 'RunShellWithReturnCode', 'SEPARATOR', 'SendToRietveld', 'TryChange', - 'UnknownFiles', 'UploadCL', 'Warn', 'WriteFile', 'gclient', - 'gcl_info_dir', 'getpass', 'main', 'os', 'random', 're', 'read_gcl_info', + 'UnknownFiles', 'UploadCL', 'Warn', 'WriteFile', 'gcl_info_dir', + 'getpass', 'main', 'os', 'random', 're', 'read_gcl_info', 'repository_root', 'string', 'subprocess', 'sys', 'tempfile', 'upload', 'urllib2', 'use_shell', 'xml', @@ -63,6 +62,37 @@ class GclUnittest(GclTestsBase): # If this test fails, you should add the relevant test. self.compareMembers(gcl, members) + def testGetSVNFileInfo(self): + def RunShellMock(command): + return r""" + + +http://src.chromium.org/svn/trunk/src/chrome/app/d +http://src.chromium.org/svn + +add +infinity +http://src.chromium.org/svn/trunk/src/chrome/app/DEPS +14628 +369f59057ba0e6d9017e28f8bdfb1f43 + + + +""" % command[3] + gcl.RunShell = RunShellMock + filename = os.path.join('app', 'd') + info = gcl.GetSVNFileInfo(filename) + expected = { + 'URL': 'http://src.chromium.org/svn/trunk/src/chrome/app/d', + 'Repository Root': 'http://src.chromium.org/svn', + 'Schedule': 'add', + 'Copied From URL': 'http://src.chromium.org/svn/trunk/src/chrome/app/DEPS', + 'Copied From Rev': '14628', + 'Path': filename, + 'Node Kind': 'file', + } + self.assertEquals(sorted(info.items()), sorted(expected.items())) + def testGetSVNStatus(self): def RunShellMock(command): return r""" @@ -126,41 +156,6 @@ class GclUnittest(GclTestsBase): info = gcl.GetSVNStatus(None) self.assertEquals(info, []) - def testHelp(self): - stdout = sys.stdout - dummy = StringIO.StringIO() - sys.stdout = dummy - gcl.Help() - sys.stdout = stdout - self.assertEquals(len(dummy.getvalue()), 1718) - - def testGetRepositoryRoot(self): - try: - gcl.GetRepositoryRoot() - except Exception,e: - self.assertEquals(e.args[0], "gcl run outside of repository") - - -class ChangeInfoUnittest(GclTestsBase): - def testChangeInfoMembers(self): - members = [ - 'CloseIssue', 'Delete', 'FileList', 'MissingTests', 'Save', - 'UpdateRietveldDescription', 'description', 'files', 'issue', 'name', - 'patch' - ] - # If this test fails, you should add the relevant test. - self.compareMembers(gcl.ChangeInfo(), members) - - def testChangeInfoBase(self): - files = [('M', 'foo'), ('A', 'bar')] - o = gcl.ChangeInfo('name2', 'issue2', 'description2', files) - self.assertEquals(o.name, 'name2') - self.assertEquals(o.issue, 'issue2') - self.assertEquals(o.description, 'description2') - self.assertEquals(o.files, files) - self.assertEquals(o.patch, None) - self.assertEquals(o.FileList(), ['foo', 'bar']) - if __name__ == '__main__': unittest.main() diff --git a/tests/gclient_test.py b/tests/gclient_test.py index e7337fd4c..9d3719dd9 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(base_path).AndReturn([]) + gclient.CaptureSVNStatus(options, 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(base_path).AndReturn(items) + gclient.CaptureSVNStatus(options, base_path).AndReturn(items) print >>options.stdout, os.path.join(base_path, 'a') print >>options.stdout, os.path.join(base_path, 'b') - gclient.RunSVN(['revert', 'a', 'b'], base_path) + gclient.RunSVN(options, ['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(os.path.join(base_path, "."), '.' + gclient.CaptureSVNInfo(options, os.path.join(base_path, "."), '.' ).AndReturn(file_info) # Cheat a bit here. - gclient.CaptureSVNInfo(file_info['URL'], '.').AndReturn(file_info) + gclient.CaptureSVNInfo(options, file_info['URL'], '.').AndReturn(file_info) additional_args = [] if options.manually_grab_svn_rev: additional_args = ['--revision', str(file_info['Revision'])] @@ -1276,8 +1276,9 @@ class SCMWrapperTestCase(BaseTestCase): """ % self.url - gclient.CaptureSVN(['info', '--xml', self.url], - '.', True).AndReturn(xml_text) + options = self.Options(verbose=True) + gclient.CaptureSVN(options, ['info', '--xml', self.url], + '.').AndReturn(xml_text) expected = { 'URL': 'http://src.chromium.org/svn/trunk/src/chrome/app/d', 'UUID': None, @@ -1290,7 +1291,7 @@ class SCMWrapperTestCase(BaseTestCase): 'Node Kind': 'file', } self.mox.ReplayAll() - file_info = self._CaptureSVNInfo(self.url, '.', True) + file_info = self._CaptureSVNInfo(options, self.url, '.') self.assertEquals(sorted(file_info.items()), sorted(expected.items())) self.mox.VerifyAll() @@ -1318,10 +1319,11 @@ class SCMWrapperTestCase(BaseTestCase): """ % (self.url, self.root_dir) - gclient.CaptureSVN(['info', '--xml', self.url], - '.', True).AndReturn(xml_text) + options = self.Options(verbose=True) + gclient.CaptureSVN(options, ['info', '--xml', self.url], + '.').AndReturn(xml_text) self.mox.ReplayAll() - file_info = self._CaptureSVNInfo(self.url, '.', True) + file_info = self._CaptureSVNInfo(options, self.url, '.') expected = { 'URL': self.url, 'UUID': '7b9385f5-0452-0410-af26-ad4892b7a1fb', diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py deleted file mode 100644 index 1e68dfc3e..000000000 --- a/tests/trychange_unittest.py +++ /dev/null @@ -1,72 +0,0 @@ -#!/usr/bin/python -# Copyright (c) 2009 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -"""Unit tests for trychange.py.""" - -import os -import unittest - -# Local imports -import trychange - - -class TryChangeTestsBase(unittest.TestCase): - """Setups and tear downs the mocks but doesn't test anything as-is.""" - def setUp(self): - pass - - def tearDown(self): - pass - - def compareMembers(self, object, members): - """If you add a member, be sure to add the relevant test!""" - # Skip over members starting with '_' since they are usually not meant to - # be for public use. - actual_members = [x for x in sorted(dir(object)) - if not x.startswith('_')] - expected_members = sorted(members) - if actual_members != expected_members: - diff = ([i for i in actual_members if i not in expected_members] + - [i for i in expected_members if i not in actual_members]) - print diff - self.assertEqual(actual_members, expected_members) - - -class TryChangeUnittest(TryChangeTestsBase): - """General trychange.py tests.""" - def testMembersChanged(self): - members = [ - 'EscapeDot', 'ExecuteTryServerScript', 'GIT', 'GetSourceRoot', 'GuessVCS', - 'HELP_STRING', 'InvalidScript', 'NoTryServerAccess', 'PathDifference', - 'RunCommand', 'SCM', 'SCRIPT_PATH', 'SVN', 'TryChange', 'USAGE', - 'datetime', 'gcl', 'gclient', 'getpass', 'logging', 'optparse', 'os', - 'shutil', 'sys', 'tempfile', 'traceback', 'urllib', - ] - # If this test fails, you should add the relevant test. - self.compareMembers(trychange, members) - - -class SVNUnittest(TryChangeTestsBase): - """General trychange.py tests.""" - def testMembersChanged(self): - members = [ - 'GenerateDiff', 'ProcessOptions', 'options' - ] - # If this test fails, you should add the relevant test. - self.compareMembers(trychange.SVN(None), members) - - -class TryChangeUnittest(TryChangeTestsBase): - """General trychange.py tests.""" - def testMembersChanged(self): - members = [ - 'GenerateDiff', 'GetEmail', 'GetPatchName', 'ProcessOptions', 'options' - ] - # If this test fails, you should add the relevant test. - self.compareMembers(trychange.GIT(None), members) - - -if __name__ == '__main__': - unittest.main() diff --git a/trychange.py b/trychange.py index 3666b984e..990f57525 100755 --- a/trychange.py +++ b/trychange.py @@ -20,7 +20,6 @@ import traceback import urllib import gcl -import gclient __version__ = '1.1' @@ -133,7 +132,7 @@ class SVN(SCM): for file in files: # Use svn info output instead of os.path.isdir because the latter fails # when the file is deleted. - if gclient.CaptureSVNInfo(file).get("Node Kind") in ("dir", "directory"): + if gcl.GetSVNFileInfo(file).get("Node Kind") == "directory": continue # If the user specified a custom diff command in their svn config file, # then it'll be used when we do svn diff, which we don't want to happen