From f7ae6d550e444eed32a09995743794350dc9ea53 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Tue, 22 Dec 2009 20:49:04 +0000 Subject: [PATCH] Add --sup_rep support to trychange.py. This adds the functionality to try a job spread across multiple checkouts. TEST=fixed unit tests and tested with hybrid svn-git checkouts BUG=none Review URL: http://codereview.chromium.org/504085 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@35168 0039d316-1c4b-4281-b951-d872f2087c98 --- scm.py | 1 + tests/gcl_unittest.py | 21 ++++------ tests/scm_unittest.py | 12 ++++++ tests/trychange_unittest.py | 16 +++---- trychange.py | 83 ++++++++++++++++++++++--------------- 5 files changed, 78 insertions(+), 55 deletions(-) diff --git a/scm.py b/scm.py index 533716d63e..c06049cd73 100644 --- a/scm.py +++ b/scm.py @@ -662,6 +662,7 @@ class SVN(object): The directory is returned as an absolute path. """ + directory = os.path.abspath(directory) infos = SVN.CaptureInfo(directory, print_error=False) cur_dir_repo_root = infos.get("Repository Root") if not cur_dir_repo_root: diff --git a/tests/gcl_unittest.py b/tests/gcl_unittest.py index 64f5f63427..e7ba592384 100755 --- a/tests/gcl_unittest.py +++ b/tests/gcl_unittest.py @@ -17,10 +17,12 @@ class GclTestsBase(SuperMoxTestBase): self.fake_root_dir = self.RootDir() self.mox.StubOutWithMock(gcl, 'RunShell') self.mox.StubOutWithMock(gcl.SVN, 'CaptureInfo') + self.mox.StubOutWithMock(gcl.SVN, 'GetCheckoutRoot') self.mox.StubOutWithMock(gcl, 'tempfile') self.mox.StubOutWithMock(gcl.upload, 'RealMain') self.mox.StubOutWithMock(gcl.gclient_utils, 'FileRead') self.mox.StubOutWithMock(gcl.gclient_utils, 'FileWrite') + gcl.REPOSITORY_ROOT = None class GclUnittest(GclTestsBase): @@ -60,26 +62,17 @@ class GclUnittest(GclTestsBase): pass def testGetRepositoryRootNone(self): - gcl.REPOSITORY_ROOT = None - gcl.os.getcwd().AndReturn("/bleh/prout") - result = { - "Repository Root": "" - } - gcl.SVN.CaptureInfo("/bleh/prout", print_error=False).AndReturn(result) + gcl.os.getcwd().AndReturn(self.fake_root_dir) + gcl.SVN.GetCheckoutRoot(self.fake_root_dir).AndReturn(None) self.mox.ReplayAll() - self.assertRaises(Exception, gcl.GetRepositoryRoot) + self.assertRaises(gcl.gclient_utils.Error, gcl.GetRepositoryRoot) def testGetRepositoryRootGood(self): - gcl.REPOSITORY_ROOT = None root_path = gcl.os.path.join('bleh', 'prout', 'pouet') gcl.os.getcwd().AndReturn(root_path) - result1 = { "Repository Root": "Some root" } - gcl.SVN.CaptureInfo(root_path, print_error=False).AndReturn(result1) - results2 = { "Repository Root": "A different root" } - gcl.SVN.CaptureInfo(gcl.os.path.dirname(root_path), - print_error=False).AndReturn(results2) + gcl.SVN.GetCheckoutRoot(root_path).AndReturn(root_path + '.~') self.mox.ReplayAll() - self.assertEquals(gcl.GetRepositoryRoot(), root_path) + self.assertEquals(gcl.GetRepositoryRoot(), root_path + '.~') def testGetCachedFile(self): # TODO(maruel): TEST ME diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 235f76b2c6..a9c71c165b 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -151,6 +151,18 @@ class SVNTestCase(BaseSCMTestCase): # If this test fails, you should add the relevant test. self.compareMembers(scm.SVN, members) + def testGetCheckoutRoot(self): + self.mox.StubOutWithMock(scm.SVN, 'CaptureInfo') + scm.os.path.abspath(self.root_dir + 'x').AndReturn(self.root_dir) + result1 = { "Repository Root": "Some root" } + scm.SVN.CaptureInfo(self.root_dir, print_error=False).AndReturn(result1) + results2 = { "Repository Root": "A different root" } + scm.SVN.CaptureInfo(scm.os.path.dirname(self.root_dir), + print_error=False).AndReturn(results2) + self.mox.ReplayAll() + self.assertEquals(scm.SVN.GetCheckoutRoot(self.root_dir + 'x'), + self.root_dir) + def testGetFileInfo(self): xml_text = r""" diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py index f972040b5d..779a99bea0 100644 --- a/tests/trychange_unittest.py +++ b/tests/trychange_unittest.py @@ -54,43 +54,43 @@ class SVNUnittest(TryChangeTestsBase): """trychange.SVN tests.""" def testMembersChanged(self): members = [ - 'GetBots', 'GetFileNames', 'GetLocalRoot', + 'GenerateDiff', 'GetBots', 'GetFileNames', 'GetLocalRoot', ] # If this test fails, you should add the relevant test. self.compareMembers(trychange.SVN, members) def testBasic(self): - trychange.os.getcwd().AndReturn(self.fake_root) trychange.scm.SVN.GetCheckoutRoot(self.fake_root).AndReturn(self.fake_root) - trychange.scm.SVN.GenerateDiff(['foo.txt', 'bar.txt'], + trychange.scm.SVN.GenerateDiff(['foo.txt', 'bar.txt'], self.fake_root, full_move=True).AndReturn('A diff') trychange.scm.SVN.GetEmail(self.fake_root).AndReturn('georges@example.com') self.mox.ReplayAll() - svn = trychange.SVN(self.options) + svn = trychange.SVN(self.options, self.fake_root) self.assertEqual(svn.GetFileNames(), self.expected_files) self.assertEqual(svn.GetLocalRoot(), self.fake_root) + self.assertEqual(svn.GenerateDiff(), 'A diff') class GITUnittest(TryChangeTestsBase): """trychange.GIT tests.""" def testMembersChanged(self): members = [ - 'GetBots', 'GetFileNames', 'GetLocalRoot', + 'GenerateDiff', 'GetBots', 'GetFileNames', 'GetLocalRoot', ] # If this test fails, you should add the relevant test. self.compareMembers(trychange.GIT, members) def testBasic(self): - trychange.os.getcwd().AndReturn(self.fake_root) trychange.scm.GIT.GetCheckoutRoot(self.fake_root).AndReturn(self.fake_root) trychange.scm.GIT.GenerateDiff(self.fake_root, - full_move=True).AndReturn('a diff') + full_move=True).AndReturn('A diff') trychange.scm.GIT.GetPatchName(self.fake_root).AndReturn('bleh-1233') trychange.scm.GIT.GetEmail(self.fake_root).AndReturn('georges@example.com') self.mox.ReplayAll() - git = trychange.GIT(self.options) + git = trychange.GIT(self.options, self.fake_root) self.assertEqual(git.GetFileNames(), self.expected_files) self.assertEqual(git.GetLocalRoot(), self.fake_root) + self.assertEqual(git.GenerateDiff(), 'A diff') if __name__ == '__main__': diff --git a/trychange.py b/trychange.py index 38d817ca8e..abae97182e 100755 --- a/trychange.py +++ b/trychange.py @@ -72,42 +72,42 @@ def EscapeDot(name): class SCM(object): """Simplistic base class to implement one function: ProcessOptions.""" - def __init__(self, options): + def __init__(self, options, cwd): + self.checkout_root = cwd self.options = options + self.files = self.options.files + self.options.files = None def GetFileNames(self): """Return the list of files in the diff.""" - return self.options.files + return self.files class SVN(SCM): """Gathers the options and diff for a subversion checkout.""" def __init__(self, *args, **kwargs): SCM.__init__(self, *args, **kwargs) - self.checkout_root = scm.SVN.GetCheckoutRoot(os.getcwd()) - if not self.options.diff: - # Generate the diff from the scm. - self.options.diff = self._GenerateDiff() + self.checkout_root = scm.SVN.GetCheckoutRoot(self.checkout_root) if not self.options.email: # Assumes the svn credential is an email address. self.options.email = scm.SVN.GetEmail(self.checkout_root) - def _GenerateDiff(self): + def GenerateDiff(self): """Returns a string containing the diff for the given file list. The files in the list should either be absolute paths or relative to the given root. """ - if not self.options.files: + if not self.files: previous_cwd = os.getcwd() os.chdir(self.checkout_root) excluded = ['!', '?', 'X', ' ', '~'] - self.options.files = [ + self.files = [ f[1] for f in scm.SVN.CaptureStatus(self.checkout_root) if f[0][0] not in excluded ] os.chdir(previous_cwd) - return scm.SVN.GenerateDiff(self.options.files, full_move=True) + return scm.SVN.GenerateDiff(self.files, self.checkout_root, full_move=True) def GetLocalRoot(self): """Return the path of the repository root.""" @@ -130,10 +130,7 @@ class GIT(SCM): """Gathers the options and diff for a git checkout.""" def __init__(self, *args, **kwargs): SCM.__init__(self, *args, **kwargs) - self.checkout_root = scm.GIT.GetCheckoutRoot(os.getcwd()) - if not self.options.diff: - self.options.diff = scm.GIT.GenerateDiff(self.checkout_root, - full_move=True) + self.checkout_root = scm.GIT.GetCheckoutRoot(self.checkout_root) if not self.options.name: self.options.name = scm.GIT.GetPatchName(self.checkout_root) if not self.options.email: @@ -143,6 +140,10 @@ class GIT(SCM): """Return the path of the repository root.""" return self.checkout_root + def GenerateDiff(self): + # For now, ignores self.files + return scm.GIT.GenerateDiff(self.checkout_root, full_move=True) + def GetBots(self): try: # A git checkout is always a full checkout. @@ -291,7 +292,7 @@ def _SendChangeSVN(options): shutil.rmtree(temp_dir, True) -def GuessVCS(options): +def GuessVCS(options, cwd): """Helper to guess the version control system. NOTE: Very similar to upload.GuessVCS. Doesn't look for hg since we don't @@ -306,16 +307,16 @@ def GuessVCS(options): """ __pychecker__ = 'no-returnvalues' # Subversion has a .svn in all working directories. - if os.path.isdir('.svn'): + if os.path.isdir(os.path.join(cwd, '.svn')): logging.info("Guessed VCS = Subversion") - return SVN(options) + return SVN(options, cwd) # Git has a command to test if you're in a git tree. # Try running it, but don't die if we don't have git installed. try: - gclient_utils.CheckCall(["git", "rev-parse", "--is-inside-work-tree"]) + gclient_utils.CheckCall(["git", "rev-parse", "--is-inside-work-tree"], cwd) logging.info("Guessed VCS = Git") - return GIT(options) + return GIT(options, cwd) except gclient_utils.CheckCallError, e: if e.retcode != 2: # ENOENT -- they don't have git installed. raise @@ -398,6 +399,9 @@ def TryChange(argv, "patch created in a subdirectory") group.add_option("--patchlevel", type='int', metavar="LEVEL", help="Used as -pN parameter to patch") + group.add_option("--sub_rep", action="append", default=["."], + help="Subcheckout to use in addition. This is mainly " + "useful for gclient-style checkouts.") parser.add_option_group(group) group = optparse.OptionGroup(parser, "Access the try server by HTTP") @@ -440,6 +444,15 @@ def TryChange(argv, parser.error('Please specify an access method.') try: + # Process the VCS in any case at least to retrieve the email address. + checkouts = [] + for item in options.sub_rep: + checkout = GuessVCS(options, item) + if checkout.GetLocalRoot() in [c.GetLocalRoot() for c in checkouts]: + parser.error('Specified the root %s two times.' % + checkout.GetLocalRoot()) + checkouts.append(checkout) + # Convert options.diff into the content of the diff. if options.url: if options.files: @@ -449,26 +462,30 @@ def TryChange(argv, if options.files: parser.error('You cannot specify files and --diff at the same time.') options.diff = gclient_utils.FileRead(options.diff, 'rb') - # Process the VCS in any case at least to retrieve the email address. - try: - options.scm = GuessVCS(options) - except NoTryServerAccess, e: - # If we got the diff, we don't care. - if not options.diff: - # TODO(maruel): Raise what? - raise - - # Get try slaves from PRESUBMIT.py files if not specified. + else: + # Use this as the base. + root = checkouts[0].GetLocalRoot() + diffs = [] + for checkout in checkouts: + diff = checkout.GenerateDiff().splitlines(True) + # Munge it. + path_diff = gclient_utils.PathDifference(root, checkout.GetLocalRoot()) + for i in range(len(diff)): + if diff[i].startswith('--- ') or diff[i].startswith('+++ '): + diff[i] = diff[i][0:3] + path_diff + diff[i][4:] + diffs.extend(diff) + options.diff = ''.join(diffs) + if not options.bot: + # Get try slaves from PRESUBMIT.py files if not specified. # Even if the diff comes from options.url, use the local checkout for bot # selection. try: - # Get try slaves from PRESUBMIT.py files if not specified. import presubmit_support - root_presubmit = options.scm.GetBots() + root_presubmit = checkouts[0].GetBots() options.bot = presubmit_support.DoGetTrySlaves( - options.scm.GetFileNames(), - options.scm.GetLocalRoot(), + checkouts[0].GetFileNames(), + checkouts[0].GetLocalRoot(), root_presubmit, False, sys.stdout)