Skip to content

Commit d43a3eb

Browse files
jowang4105LUCI CQ
jowang4105
authored and
LUCI CQ
committed
Add end_commit for calculating GitChange diffs.
Bug: b/365162784 Change-Id: I97b69b1ae5183bbdaa7d4de0b1f23e62f4e1d8d0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5853908 Reviewed-by: Gavin Mak <[email protected]> Commit-Queue: Joanna Wang <[email protected]>
1 parent 20a0cda commit d43a3eb

File tree

4 files changed

+84
-52
lines changed

4 files changed

+84
-52
lines changed

‎git_cl.py

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1733,19 +1733,22 @@ def _GetCommonPresubmitArgs(self, verbose, upstream):
17331733
return args
17341734

17351735
def RunHook(self,
1736-
committing,
1737-
may_prompt,
1738-
verbose,
1739-
parallel,
1740-
upstream,
1741-
description,
1742-
all_files,
1743-
files=None,
1744-
resultdb=False,
1745-
realm=None):
1736+
committing: bool,
1737+
may_prompt: bool,
1738+
verbose: bool,
1739+
parallel: bool,
1740+
upstream: str,
1741+
description: str,
1742+
all_files: bool,
1743+
files: Optional[Sequence[str]] = None,
1744+
resultdb: Optional[bool] = None,
1745+
realm: Optional[str] = None,
1746+
end_commit: Optional[str] = None) -> Mapping[str, Any]:
17461747
"""Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
17471748
args = self._GetCommonPresubmitArgs(verbose, upstream)
17481749
args.append('--commit' if committing else '--upload')
1750+
if end_commit:
1751+
args.extend(['--end_commit', end_commit])
17491752
if may_prompt:
17501753
args.append('--may_prompt')
17511754
if parallel:
@@ -2068,7 +2071,8 @@ def _PrepareChange(
20682071
parallel=options.parallel,
20692072
upstream=parent,
20702073
description=change_desc.description,
2071-
all_files=False)
2074+
all_files=False,
2075+
end_commit=end_commit)
20722076
self.ExtendCC(hook_results['more_cc'])
20732077

20742078
# Update the change description and ensure we have a Change Id.

‎presubmit_support.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -927,10 +927,11 @@ def GetOldContents(self, path, local_root):
927927
class _GitDiffCache(_DiffCache):
928928
"""DiffCache implementation for git; gets all file diffs at once."""
929929

930-
def __init__(self, upstream):
930+
def __init__(self, upstream, end_commit):
931931
"""Stores the upstream revision against which all diffs are computed."""
932932
super(_GitDiffCache, self).__init__()
933933
self._upstream = upstream
934+
self._end_commit = end_commit
934935
self._diffs_by_file = None
935936

936937
def GetDiff(self, path, local_root):
@@ -942,7 +943,8 @@ def GetDiff(self, path, local_root):
942943
unified_diff = scm.GIT.GenerateDiff(local_root,
943944
files=[],
944945
full_move=True,
945-
branch=self._upstream)
946+
branch=self._upstream,
947+
branch_head=self._end_commit)
946948
# Compute a single diff for all files and parse the output; with git
947949
# this is much faster than computing one diff for each file.
948950
self._diffs_by_file = _parse_unified_diff(unified_diff)
@@ -1437,12 +1439,13 @@ class GitChange(Change):
14371439
_AFFECTED_FILES = GitAffectedFile
14381440
scm = 'git'
14391441

1440-
def __init__(self, *args, upstream, **kwargs):
1442+
def __init__(self, *args, upstream, end_commit, **kwargs):
14411443
self._upstream = upstream
1444+
self._end_commit = end_commit
14421445
super(GitChange, self).__init__(*args)
14431446

14441447
def _diff_cache(self):
1445-
return self._AFFECTED_FILES.DIFF_CACHE(self._upstream)
1448+
return self._AFFECTED_FILES.DIFF_CACHE(self._upstream, self._end_commit)
14461449

14471450
def UpstreamBranch(self):
14481451
"""Returns the upstream branch for the change."""
@@ -2129,10 +2132,13 @@ def _parse_change(parser, options):
21292132
options.name, options.description, options.root, change_files,
21302133
options.issue, options.patchset, options.author
21312134
]
2135+
21322136
if diff:
21332137
return ProvidedDiffChange(*change_args, diff=diff)
21342138
if change_scm == 'git':
2135-
return GitChange(*change_args, upstream=options.upstream)
2139+
return GitChange(*change_args,
2140+
upstream=options.upstream,
2141+
end_commit=options.end_commit)
21362142
return Change(*change_args)
21372143

21382144

@@ -2327,6 +2333,10 @@ def main(argv=None):
23272333
parser.add_argument('--upstream',
23282334
help='The base ref or upstream branch against '
23292335
'which the diff should be computed.')
2336+
parser.add_argument('--end_commit',
2337+
default='HEAD',
2338+
help='The commit to diff against upstream. '
2339+
'By default this is HEAD.')
23302340
parser.add_argument('--default_presubmit')
23312341
parser.add_argument('--may_prompt', action='store_true', default=False)
23322342
parser.add_argument(

‎tests/git_cl_test.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4183,7 +4183,8 @@ def testPrepareChange_new(self, mockEnsureCanUploadPatchset,
41834183
parallel=False,
41844184
upstream='420parent',
41854185
description=desc,
4186-
all_files=False)
4186+
all_files=False,
4187+
end_commit='420latest_tree')
41874188

41884189
@mock.patch('git_cl.Changelist.GetAffectedFiles', return_value=[])
41894190
@mock.patch('git_cl.Changelist.GetIssue', return_value='123')
@@ -4233,9 +4234,10 @@ def testPrepareChange_existing(self, mockEnsureCanUploadPatchset,
42334234
may_prompt=True,
42344235
verbose=False,
42354236
parallel=False,
4236-
upstream='420parent',
4237+
upstream=parent,
42374238
description=desc,
4238-
all_files=False)
4239+
all_files=False,
4240+
end_commit=latest_tree)
42394241
mockEnsureCanUploadPatchset.assert_called_once()
42404242

42414243
# Test preserve_tryjob

‎tests/presubmit_unittest.py

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,8 @@ def testGitChange(self):
385385
0,
386386
0,
387387
None,
388-
upstream='upstream')
388+
upstream='upstream',
389+
end_commit='end_commit')
389390
self.assertIsNotNone(change.Name() == 'mychange')
390391
self.assertIsNotNone(change.DescriptionText(
391392
) == 'Hello there\nthis is a change\nand some more regular text')
@@ -413,6 +414,12 @@ def testGitChange(self):
413414
actual_rhs_lines = []
414415
for f, linenum, line in change.RightHandSideLines():
415416
actual_rhs_lines.append((f.LocalPath(), linenum, line))
417+
scm.GIT.GenerateDiff.assert_called_once_with(self.fake_root_dir,
418+
files=[],
419+
full_move=True,
420+
branch='upstream',
421+
branch_head='end_commit')
422+
416423

417424
f_blat = os.path.normpath('boo/blat.cc')
418425
f_test_expectations = os.path.normpath('foo/TestExpectations')
@@ -450,7 +457,8 @@ def testInvalidChange(self):
450457
0,
451458
0,
452459
None,
453-
upstream='upstream')
460+
upstream='upstream',
461+
end_commit='HEAD')
454462

455463
def testExecPresubmitScript(self):
456464
description_lines = ('Hello there', 'this is a change', 'BUG=123')
@@ -1035,14 +1043,15 @@ def testParseChange_FilesAndGit(self):
10351043

10361044
change = presubmit._parse_change(None, options)
10371045
self.assertEqual(presubmit.GitChange.return_value, change)
1038-
presubmit.GitChange.assert_called_once_with(options.name,
1039-
options.description,
1040-
options.root,
1041-
[('M', 'random_file.txt')],
1042-
options.issue,
1043-
options.patchset,
1044-
options.author,
1045-
upstream=options.upstream)
1046+
presubmit.GitChange.assert_called_once_with(
1047+
options.name,
1048+
options.description,
1049+
options.root, [('M', 'random_file.txt')],
1050+
options.issue,
1051+
options.patchset,
1052+
options.author,
1053+
upstream=options.upstream,
1054+
end_commit=options.end_commit)
10461055
presubmit._parse_files.assert_called_once_with(options.files,
10471056
options.recursive)
10481057

@@ -1055,14 +1064,15 @@ def testParseChange_NoFilesAndGit(self):
10551064

10561065
change = presubmit._parse_change(None, options)
10571066
self.assertEqual(presubmit.GitChange.return_value, change)
1058-
presubmit.GitChange.assert_called_once_with(options.name,
1059-
options.description,
1060-
options.root,
1061-
[('A', 'added.txt')],
1062-
options.issue,
1063-
options.patchset,
1064-
options.author,
1065-
upstream=options.upstream)
1067+
presubmit.GitChange.assert_called_once_with(
1068+
options.name,
1069+
options.description,
1070+
options.root, [('A', 'added.txt')],
1071+
options.issue,
1072+
options.patchset,
1073+
options.author,
1074+
upstream=options.upstream,
1075+
end_commit=options.end_commit)
10661076
scm.GIT.CaptureStatus.assert_called_once_with(options.root,
10671077
options.upstream,
10681078
ignore_submodules=False)
@@ -1076,15 +1086,15 @@ def testParseChange_AllFilesAndGit(self):
10761086

10771087
change = presubmit._parse_change(None, options)
10781088
self.assertEqual(presubmit.GitChange.return_value, change)
1079-
presubmit.GitChange.assert_called_once_with(options.name,
1080-
options.description,
1081-
options.root,
1082-
[('M', 'foo.txt'),
1083-
('M', 'bar.txt')],
1084-
options.issue,
1085-
options.patchset,
1086-
options.author,
1087-
upstream=options.upstream)
1089+
presubmit.GitChange.assert_called_once_with(
1090+
options.name,
1091+
options.description,
1092+
options.root, [('M', 'foo.txt'), ('M', 'bar.txt')],
1093+
options.issue,
1094+
options.patchset,
1095+
options.author,
1096+
upstream=options.upstream,
1097+
end_commit=options.end_commit)
10881098
scm.GIT.GetAllFiles.assert_called_once_with(options.root)
10891099

10901100
def testParseChange_EmptyDiffFile(self):
@@ -1270,7 +1280,8 @@ def testInputApiPresubmitScriptFiltering(self):
12701280
0,
12711281
0,
12721282
None,
1273-
upstream='upstream')
1283+
upstream='upstream',
1284+
end_commit='HEAD')
12741285
input_api = presubmit.InputApi(
12751286
change, os.path.join(self.fake_root_dir, 'foo', 'PRESUBMIT.py'),
12761287
False, None, False)
@@ -1331,7 +1342,8 @@ def testInputApiFilterSourceFile(self):
13311342
0,
13321343
0,
13331344
None,
1334-
upstream='upstream')
1345+
upstream='upstream',
1346+
end_commit='HEAD')
13351347
input_api = presubmit.InputApi(
13361348
change, os.path.join(self.fake_root_dir, 'foo', 'PRESUBMIT.py'),
13371349
False, None, False)
@@ -1466,7 +1478,8 @@ def FilterSourceFile(affected_file):
14661478
0,
14671479
0,
14681480
None,
1469-
upstream='upstream')
1481+
upstream='upstream',
1482+
end_commit='HEAD')
14701483
input_api = presubmit.InputApi(
14711484
change, os.path.join(self.fake_root_dir, 'PRESUBMIT.py'), False,
14721485
None, False)
@@ -1493,7 +1506,8 @@ def testLambdaFilter(self):
14931506
0,
14941507
0,
14951508
None,
1496-
upstream='upstream')
1509+
upstream='upstream',
1510+
end_commit='HEAD')
14971511
input_api = presubmit.InputApi(
14981512
change, os.path.join(self.fake_root_dir, 'PRESUBMIT.py'), False,
14991513
None, False)
@@ -1799,7 +1813,8 @@ def testAffectedSubmodules(self, mockListSubmodules):
17991813
3,
18001814
5,
18011815
'',
1802-
upstream='upstream')
1816+
upstream='upstream',
1817+
end_commit='HEAD')
18031818
self.assertEqual(1, len(change.AffectedSubmodules()))
18041819
self.assertEqual('A', change.AffectedSubmodules()[0].Action())
18051820

@@ -1812,7 +1827,8 @@ def testAffectedSubmodulesCachesSubmodules(self, mockListSubmodules):
18121827
3,
18131828
5,
18141829
'',
1815-
upstream='upstream')
1830+
upstream='upstream',
1831+
end_commit='HEAD')
18161832
change.AffectedSubmodules()
18171833
mockListSubmodules.assert_called_once()
18181834
change.AffectedSubmodules()

0 commit comments

Comments
 (0)