mirror of
https://chromium.googlesource.com/chromium/tools/depot_tools.git
synced 2026-01-11 10:41:31 +00:00
Add --generate_diff flag to presubmit_support.py
This flag allows a diff to be generated based on an upstream commit for provided files and for checks to be run with on that diff. Bug: b/323243527 Change-Id: Ia3f4ec50b15eabb90d391b9bc795fc4b35a35a08 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5376402 Reviewed-by: Josip Sokcevic <sokcevic@chromium.org> Commit-Queue: Gavin Mak <gavinmak@google.com>
This commit is contained in:
@@ -18,7 +18,6 @@ import cpplint
|
||||
import fnmatch # Exposed through the API.
|
||||
import glob
|
||||
import inspect
|
||||
import itertools
|
||||
import json # Exposed through the API.
|
||||
import logging
|
||||
import mimetypes
|
||||
@@ -49,6 +48,7 @@ import gerrit_util
|
||||
import owners_client
|
||||
import owners_finder
|
||||
import presubmit_canned_checks
|
||||
import presubmit_diff
|
||||
import rdb_wrapper
|
||||
import scm
|
||||
import subprocess2 as subprocess # Exposed through the API.
|
||||
@@ -2070,6 +2070,10 @@ def _parse_change(parser, options):
|
||||
parser.error(
|
||||
'<diff_file> cannot be specified when --all-files is set.')
|
||||
|
||||
if options.diff_file and options.generate_diff:
|
||||
parser.error(
|
||||
'<diff_file> cannot be specified when <generate_diff> is set.')
|
||||
|
||||
# TODO(b/323243527): Consider adding a SCM for provided diff.
|
||||
change_scm = scm.determine_scm(options.root)
|
||||
if change_scm != 'git' and not options.files and not options.diff_file:
|
||||
@@ -2086,6 +2090,17 @@ def _parse_change(parser, options):
|
||||
if fnmatch.fnmatch(name, mask):
|
||||
change_files.append(('M', name))
|
||||
break
|
||||
elif options.generate_diff:
|
||||
gerrit_url = urlparse.urlparse(options.gerrit_url).netloc
|
||||
diffs = presubmit_diff.create_diffs(
|
||||
host=gerrit_url.split('-review')[0],
|
||||
repo=options.gerrit_project,
|
||||
ref=options.upstream,
|
||||
root=options.root,
|
||||
files=options.files,
|
||||
)
|
||||
diff = '\n'.join(diffs.values())
|
||||
change_files = _diffs_to_change_files(diffs)
|
||||
else:
|
||||
# Get the filtered set of files from a directory scan.
|
||||
change_files = _parse_files(options.files, options.recursive)
|
||||
@@ -2169,7 +2184,14 @@ def _parse_unified_diff(diff):
|
||||
|
||||
|
||||
def _process_diff_file(diff_file):
|
||||
"""Validates a git diff file and processes it into a list of change files.
|
||||
diff = gclient_utils.FileRead(diff_file)
|
||||
if not diff:
|
||||
raise PresubmitFailure('diff file is empty')
|
||||
return diff, _diffs_to_change_files(_parse_unified_diff(diff))
|
||||
|
||||
|
||||
def _diffs_to_change_files(diffs):
|
||||
"""Validates a dict of diffs and processes it into a list of change files.
|
||||
|
||||
Each change file is a tuple of (action, path) where action is one of:
|
||||
* A: newly added file
|
||||
@@ -2177,20 +2199,16 @@ def _process_diff_file(diff_file):
|
||||
* D: deleted file
|
||||
|
||||
Args:
|
||||
diff_file: Path to file containing unified git diff.
|
||||
diffs: Dict of (path, diff) tuples.
|
||||
|
||||
Returns:
|
||||
Contents of diff_file and a list of change file tuples from the diff.
|
||||
A list of change file tuples from the diffs.
|
||||
|
||||
Raises:
|
||||
PresubmitFailure: If the provided diff is empty or otherwise invalid.
|
||||
PresubmitFailure: If a diff is empty or otherwise invalid.
|
||||
"""
|
||||
diff = gclient_utils.FileRead(diff_file)
|
||||
if not diff:
|
||||
raise PresubmitFailure('diff file is empty')
|
||||
|
||||
change_files = []
|
||||
for file, file_diff in _parse_unified_diff(diff).items():
|
||||
for file, file_diff in diffs.items():
|
||||
header_line = file_diff.splitlines()[1]
|
||||
if not header_line:
|
||||
raise PresubmitFailure('diff header is empty')
|
||||
@@ -2201,7 +2219,7 @@ def _process_diff_file(diff_file):
|
||||
else:
|
||||
action = 'M'
|
||||
change_files.append((action, file))
|
||||
return diff, change_files
|
||||
return change_files
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
@@ -2276,6 +2294,9 @@ def main(argv=None):
|
||||
desc.add_argument('--description_file',
|
||||
help='File to read change description from.')
|
||||
parser.add_argument('--diff_file', help='File to read change diff from.')
|
||||
parser.add_argument('--generate_diff',
|
||||
action='store_true',
|
||||
help='Create a diff using upstream server as base.')
|
||||
parser.add_argument('--issue', type=int, default=0)
|
||||
parser.add_argument('--patchset', type=int, default=0)
|
||||
parser.add_argument('--root',
|
||||
@@ -2284,10 +2305,9 @@ def main(argv=None):
|
||||
'If inherit-review-settings-ok is present in this '
|
||||
'directory, parent directories up to the root file '
|
||||
'system directories will also be searched.')
|
||||
parser.add_argument(
|
||||
'--upstream',
|
||||
help='Git only: the base ref or upstream branch against '
|
||||
'which the diff should be computed.')
|
||||
parser.add_argument('--upstream',
|
||||
help='The base ref or upstream branch against '
|
||||
'which the diff should be computed.')
|
||||
parser.add_argument('--default_presubmit')
|
||||
parser.add_argument('--may_prompt', action='store_true', default=False)
|
||||
parser.add_argument(
|
||||
|
||||
@@ -66,6 +66,7 @@ class ProvidedDiffChangeTest(fake_repos.FakeReposTestBase):
|
||||
gclient_utils.FileWrite(tmp, diff)
|
||||
options = mock.Mock(root=self.repo,
|
||||
all_files=False,
|
||||
generate_diff=False,
|
||||
description='description',
|
||||
files=None,
|
||||
diff_file=tmp)
|
||||
|
||||
@@ -966,7 +966,9 @@ def CheckChangeOnCommit(input_api, output_api):
|
||||
def testParseChange_Files(self):
|
||||
presubmit._parse_files.return_value = [('M', 'random_file.txt')]
|
||||
scm.determine_scm.return_value = None
|
||||
options = mock.Mock(all_files=False, source_controlled_only=False)
|
||||
options = mock.Mock(all_files=False,
|
||||
generate_diff=False,
|
||||
source_controlled_only=False)
|
||||
|
||||
change = presubmit._parse_change(None, options)
|
||||
self.assertEqual(presubmit.Change.return_value, change)
|
||||
@@ -1009,11 +1011,26 @@ def CheckChangeOnCommit(input_api, output_api):
|
||||
parser.error.assert_called_once_with(
|
||||
'<diff_file> cannot be specified when --all-files is set.')
|
||||
|
||||
def testParseChange_DiffAndGenerateDiff(self):
|
||||
parser = mock.Mock()
|
||||
parser.error.side_effect = [SystemExit]
|
||||
options = mock.Mock(all_files=False,
|
||||
files=[],
|
||||
generate_diff=True,
|
||||
diff_file='foo.diff')
|
||||
|
||||
with self.assertRaises(SystemExit):
|
||||
presubmit._parse_change(parser, options)
|
||||
parser.error.assert_called_once_with(
|
||||
'<diff_file> cannot be specified when <generate_diff> is set.')
|
||||
|
||||
@mock.patch('presubmit_support.GitChange', mock.Mock())
|
||||
def testParseChange_FilesAndGit(self):
|
||||
scm.determine_scm.return_value = 'git'
|
||||
presubmit._parse_files.return_value = [('M', 'random_file.txt')]
|
||||
options = mock.Mock(all_files=False, source_controlled_only=False)
|
||||
options = mock.Mock(all_files=False,
|
||||
generate_diff=False,
|
||||
source_controlled_only=False)
|
||||
|
||||
change = presubmit._parse_change(None, options)
|
||||
self.assertEqual(presubmit.GitChange.return_value, change)
|
||||
@@ -1071,7 +1088,10 @@ def CheckChangeOnCommit(input_api, output_api):
|
||||
|
||||
def testParseChange_EmptyDiffFile(self):
|
||||
gclient_utils.FileRead.return_value = ''
|
||||
options = mock.Mock(all_files=False, files=[], diff_file='foo.diff')
|
||||
options = mock.Mock(all_files=False,
|
||||
files=[],
|
||||
generate_diff=False,
|
||||
diff_file='foo.diff')
|
||||
with self.assertRaises(presubmit.PresubmitFailure):
|
||||
presubmit._parse_change(None, options)
|
||||
|
||||
@@ -1100,7 +1120,10 @@ index d7ba659f..b7957f3 100644
|
||||
-baz
|
||||
+bat"""
|
||||
gclient_utils.FileRead.return_value = diff
|
||||
options = mock.Mock(all_files=False, files=[], diff_file='foo.diff')
|
||||
options = mock.Mock(all_files=False,
|
||||
files=[],
|
||||
generate_diff=False,
|
||||
diff_file='foo.diff')
|
||||
change = presubmit._parse_change(None, options)
|
||||
self.assertEqual(presubmit.ProvidedDiffChange.return_value, change)
|
||||
presubmit.ProvidedDiffChange.assert_called_once_with(
|
||||
|
||||
Reference in New Issue
Block a user