diff --git a/presubmit_support.py b/presubmit_support.py index 3fb03c61e..237dae58f 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -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( ' cannot be specified when --all-files is set.') + if options.diff_file and options.generate_diff: + parser.error( + ' cannot be specified when 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( diff --git a/tests/presubmit_support_test.py b/tests/presubmit_support_test.py index 11e8a94db..c2437b4a5 100755 --- a/tests/presubmit_support_test.py +++ b/tests/presubmit_support_test.py @@ -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) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 1af40ddab..c461e24ed 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -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( ' 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( + ' cannot be specified when 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(