From ea84ef18281d0a2ce588b7ae9bfcdbb8823cc8b6 Mon Sep 17 00:00:00 2001 From: "agable@chromium.org" Date: Wed, 30 Apr 2014 19:55:12 +0000 Subject: [PATCH] Get presubmit_support to compute diffs against a given upstream. R=hinoka@chromium.org BUG=368720 Review URL: https://codereview.chromium.org/269483002 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@267309 0039d316-1c4b-4281-b951-d872f2087c98 --- git_cl.py | 3 ++- presubmit_support.py | 33 +++++++++++++++----------- tests/presubmit_unittest.py | 46 ++++++++++++++++++++++--------------- 3 files changed, 48 insertions(+), 34 deletions(-) diff --git a/git_cl.py b/git_cl.py index e8cadf270..a47da6d5f 100755 --- a/git_cl.py +++ b/git_cl.py @@ -795,7 +795,8 @@ or verify this branch is set up to track another (via the --track argument to files, issue, patchset, - author) + author, + upstream=upstream_branch) def RunHook(self, committing, may_prompt, verbose, change): """Calls sys.exit() if the hook fails; returns a HookResults otherwise.""" diff --git a/presubmit_support.py b/presubmit_support.py index 0b9e75b4c..62a39be3c 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -500,6 +500,9 @@ class InputApi(object): class _DiffCache(object): """Caches diffs retrieved from a particular SCM.""" + def __init__(self, upstream=None): + """Stores the upstream revision against which all diffs will be computed.""" + self._upstream = upstream def GetDiff(self, path, local_root): """Get the diff for a particular path.""" @@ -508,8 +511,8 @@ class _DiffCache(object): class _SvnDiffCache(_DiffCache): """DiffCache implementation for subversion.""" - def __init__(self): - super(_SvnDiffCache, self).__init__() + def __init__(self, *args, **kwargs): + super(_SvnDiffCache, self).__init__(*args, **kwargs) self._diffs_by_file = {} def GetDiff(self, path, local_root): @@ -521,8 +524,8 @@ class _SvnDiffCache(_DiffCache): class _GitDiffCache(_DiffCache): """DiffCache implementation for git; gets all file diffs at once.""" - def __init__(self): - super(_GitDiffCache, self).__init__() + def __init__(self, upstream): + super(_GitDiffCache, self).__init__(upstream=upstream) self._diffs_by_file = None def GetDiff(self, path, local_root): @@ -533,7 +536,8 @@ class _GitDiffCache(_DiffCache): # Don't specify any filenames below, because there are command line length # limits on some platforms and GenerateDiff would fail. - unified_diff = scm.GIT.GenerateDiff(local_root, files=[], full_move=True) + unified_diff = scm.GIT.GenerateDiff(local_root, files=[], full_move=True, + branch=self._upstream) # This regex matches the path twice, separated by a space. Note that # filename itself may contain spaces. @@ -567,7 +571,7 @@ class AffectedFile(object): # Method could be a function # pylint: disable=R0201 - def __init__(self, path, action, repository_root, diff_cache=None): + def __init__(self, path, action, repository_root, diff_cache): self._path = path self._action = action self._local_root = repository_root @@ -575,10 +579,7 @@ class AffectedFile(object): self._properties = {} self._cached_changed_contents = None self._cached_new_contents = None - if diff_cache: - self._diff_cache = diff_cache - else: - self._diff_cache = self.DIFF_CACHE() + self._diff_cache = diff_cache logging.debug('%s(%s)' % (self.__class__.__name__, self._path)) def ServerPath(self): @@ -792,12 +793,14 @@ class Change(object): scm = '' def __init__( - self, name, description, local_root, files, issue, patchset, author): + self, name, description, local_root, files, issue, patchset, author, + upstream=None): if files is None: files = [] self._name = name # Convert root into an absolute path. self._local_root = os.path.abspath(local_root) + self._upstream = upstream self.issue = issue self.patchset = patchset self.author_email = author @@ -810,7 +813,7 @@ class Change(object): assert all( (isinstance(f, (list, tuple)) and len(f) == 2) for f in files), files - diff_cache = self._AFFECTED_FILES.DIFF_CACHE() + diff_cache = self._AFFECTED_FILES.DIFF_CACHE(self._upstream) self._affected_files = [ self._AFFECTED_FILES(path, action.strip(), self._local_root, diff_cache) for action, path in files @@ -1612,7 +1615,8 @@ def Main(argv): files, options.issue, options.patchset, - options.author) + options.author, + upstream=options.upstream) trybots = DoGetTrySlaves( change, change.LocalPaths(), @@ -1631,7 +1635,8 @@ def Main(argv): files, options.issue, options.patchset, - options.author), + options.author, + upstream=options.upstream), options.commit, options.verbose, sys.stdout, diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 474d6fa00..2f973bdd0 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -506,7 +506,8 @@ class PresubmitUnittest(PresubmitTestsBase): os.path.exists(full_path).AndReturn(False) os.path.isfile(full_path).AndReturn(True) - presubmit.scm.GIT.GenerateDiff(self.fake_root_dir, files=[], full_move=True + presubmit.scm.GIT.GenerateDiff( + self.fake_root_dir, files=[], full_move=True, branch=None ).AndReturn('\n'.join(unified_diff)) self.mox.ReplayAll() @@ -518,7 +519,8 @@ class PresubmitUnittest(PresubmitTestsBase): files, 0, 0, - None) + None, + upstream=None) self.failUnless(change.Name() == 'mychange') self.failUnless(change.DescriptionText() == 'Hello there\nthis is a change\nand some more regular text') @@ -1342,7 +1344,7 @@ class InputApiUnittest(PresubmitTestsBase): def testDefaultWhiteListBlackListFilters(self): def f(x): - return presubmit.AffectedFile(x, 'M', self.fake_root_dir) + return presubmit.AffectedFile(x, 'M', self.fake_root_dir, None) files = [ ( [ @@ -1672,11 +1674,11 @@ class AffectedFileUnittest(PresubmitTestsBase): ] # If this test fails, you should add the relevant test. self.compareMembers( - presubmit.AffectedFile('a', 'b', self.fake_root_dir), members) + presubmit.AffectedFile('a', 'b', self.fake_root_dir, None), members) self.compareMembers( - presubmit.SvnAffectedFile('a', 'b', self.fake_root_dir), members) + presubmit.SvnAffectedFile('a', 'b', self.fake_root_dir, None), members) self.compareMembers( - presubmit.GitAffectedFile('a', 'b', self.fake_root_dir), members) + presubmit.GitAffectedFile('a', 'b', self.fake_root_dir, None), members) def testAffectedFile(self): path = presubmit.os.path.join('foo', 'blat.cc') @@ -1687,7 +1689,7 @@ class AffectedFileUnittest(PresubmitTestsBase): presubmit.scm.SVN._CaptureInfo([path], self.fake_root_dir).AndReturn( {'URL': 'svn:/foo/foo/blat.cc'}) self.mox.ReplayAll() - af = presubmit.SvnAffectedFile('foo/blat.cc', 'M', self.fake_root_dir) + af = presubmit.SvnAffectedFile('foo/blat.cc', 'M', self.fake_root_dir, None) self.assertEquals('svn:/foo/foo/blat.cc', af.ServerPath()) self.assertEquals(presubmit.normpath('foo/blat.cc'), af.LocalPath()) self.assertEquals('M', af.Action()) @@ -1699,7 +1701,7 @@ class AffectedFileUnittest(PresubmitTestsBase): presubmit.os.path.exists(f_notfound).AndReturn(False) presubmit.gclient_utils.FileRead(f_notfound, 'rU').AndRaise(IOError) self.mox.ReplayAll() - af = presubmit.AffectedFile(notfound, 'A', self.fake_root_dir) + af = presubmit.AffectedFile(notfound, 'A', self.fake_root_dir, None) self.assertEquals('', af.ServerPath()) self.assertEquals([], af.NewContents()) @@ -1708,7 +1710,8 @@ class AffectedFileUnittest(PresubmitTestsBase): 'foo.cc', 'svn:secret-property', self.fake_root_dir ).AndReturn('secret-property-value') self.mox.ReplayAll() - affected_file = presubmit.SvnAffectedFile('foo.cc', 'A', self.fake_root_dir) + affected_file = presubmit.SvnAffectedFile('foo.cc', 'A', self.fake_root_dir, + None) # Verify cache coherency. self.assertEquals('secret-property-value', affected_file.Property('svn:secret-property')) @@ -1721,7 +1724,8 @@ class AffectedFileUnittest(PresubmitTestsBase): presubmit.os.path.exists(f_filename).AndReturn(False) presubmit.scm.SVN._CaptureInfo([filename], self.fake_root_dir).AndReturn({}) self.mox.ReplayAll() - affected_file = presubmit.SvnAffectedFile(filename, 'A', self.fake_root_dir) + affected_file = presubmit.SvnAffectedFile(filename, 'A', self.fake_root_dir, + None) # Verify cache coherency. self.failIf(affected_file.IsDirectory()) self.failIf(affected_file.IsDirectory()) @@ -1732,16 +1736,20 @@ class AffectedFileUnittest(PresubmitTestsBase): presubmit.os.path.exists(f_filename).AndReturn(True) presubmit.os.path.isdir(f_filename).AndReturn(True) self.mox.ReplayAll() - affected_file = presubmit.SvnAffectedFile(filename, 'A', self.fake_root_dir) + affected_file = presubmit.SvnAffectedFile(filename, 'A', self.fake_root_dir, + None) # Verify cache coherency. self.failUnless(affected_file.IsDirectory()) self.failUnless(affected_file.IsDirectory()) def testIsTextFile(self): files = [ - presubmit.SvnAffectedFile('foo/blat.txt', 'M', self.fake_root_dir), - presubmit.SvnAffectedFile('foo/binary.blob', 'M', self.fake_root_dir), - presubmit.SvnAffectedFile('blat/flop.txt', 'D', self.fake_root_dir) + presubmit.SvnAffectedFile('foo/blat.txt', 'M', self.fake_root_dir, + None), + presubmit.SvnAffectedFile('foo/binary.blob', 'M', self.fake_root_dir, + None), + presubmit.SvnAffectedFile('blat/flop.txt', 'D', self.fake_root_dir, + None) ] blat = presubmit.os.path.join('foo', 'blat.txt') blob = presubmit.os.path.join('foo', 'binary.blob') @@ -1984,8 +1992,8 @@ class CannedChecksUnittest(PresubmitTestsBase): 'mychange', '', self.fake_root_dir, [], 0, 0, None) input_api1 = self.MockInputApi(change1, committing) files1 = [ - presubmit.SvnAffectedFile('foo/bar.cc', 'A', self.fake_root_dir), - presubmit.SvnAffectedFile('foo.cc', 'M', self.fake_root_dir), + presubmit.SvnAffectedFile('foo/bar.cc', 'A', self.fake_root_dir, None), + presubmit.SvnAffectedFile('foo.cc', 'M', self.fake_root_dir, None), ] if use_source_file: input_api1.AffectedSourceFiles(None).AndReturn(files1) @@ -2001,8 +2009,8 @@ class CannedChecksUnittest(PresubmitTestsBase): 'mychange', '', self.fake_root_dir, [], 0, 0, None) input_api2 = self.MockInputApi(change2, committing) files2 = [ - presubmit.SvnAffectedFile('foo/bar.cc', 'A', self.fake_root_dir), - presubmit.SvnAffectedFile('foo.cc', 'M', self.fake_root_dir), + presubmit.SvnAffectedFile('foo/bar.cc', 'A', self.fake_root_dir, None), + presubmit.SvnAffectedFile('foo.cc', 'M', self.fake_root_dir, None), ] if use_source_file: input_api2.AffectedSourceFiles(None).AndReturn(files2) @@ -2368,7 +2376,7 @@ class CannedChecksUnittest(PresubmitTestsBase): self.mox.StubOutWithMock(presubmit_canned_checks, 'CheckSvnProperty') input_api = self.MockInputApi(None, False) output_api = presubmit.OutputApi(False) - A = lambda x: presubmit.AffectedFile(x, 'M', self.fake_root_dir) + A = lambda x: presubmit.AffectedFile(x, 'M', self.fake_root_dir, None) files = [ A('a.pdf'), A('b.bmp'), A('c.gif'), A('d.png'), A('e.jpg'), A('f.jpe'), A('random'), A('g.jpeg'), A('h.ico'),