diff --git a/git_cl.py b/git_cl.py index 1a83ca21b1..05f703ddcb 100755 --- a/git_cl.py +++ b/git_cl.py @@ -7082,17 +7082,67 @@ def _RunLUCICfgFormat(opts, paths, top_dir, upstream_commit): FormatterFunction = Callable[[Any, list[str], str, str], int] +def _SplitDiffsByFile(diff_string: str) -> Dict[str, str]: + """Split a given diff string into per-file patches. + + This function expects that the diff_string was generated with a prefix. + In other words, if it was generated by git-diff, don't add `--no-prefix`. + `diff` always generates one with prefixes. + + Deleted files will be skipped. + + Args: + diff_string: Unified diff + Returns: + a dict of file_path -> patch + """ + if not diff_string: + return {} + + file_diffs = re.split(r'^(?=diff --git)', diff_string, flags=re.MULTILINE) + file_diffs = [d for d in file_diffs if d.strip()] + ret = {} + + for file_diff in file_diffs: + # Look for the line with `+++ ${prefix}/path/file.cc` + # For file deletion, the line would be omitted or with `+++ /dev/null`, + # which wouldn't match the regex. + match = re.search(r'^\+\+\+ [^\s/]+/(.+)', + file_diff, + flags=re.MULTILINE) + if match: + ret[match.group(1)] = file_diff + + return ret + + +def _FindFilesToFormat( + opts: optparse.Values, files: list[str] | None, + upstream_commit: str | None) -> Tuple[List[str], Dict[str, str] | None]: + """Returns a list of files to format and the diffs. + + If opts.full, returns None for the diffs. + Deleted files are always excluded in the return. + """ + if opts.input_diff_file: + with open(opts.input_diff_file) as f: + diffs = _SplitDiffsByFile(f.read()) + return diffs.keys(), diffs + + if opts.full: + files = RunGitDiffCmd(['--name-only', '--diff-filter=d'], + upstream_commit, files).splitlines() + return files.keys(), None + + diffs = _SplitDiffsByFile( + RunGitDiffCmd(['-U0'], upstream_commit, files, allow_prefix=True)) + return diffs.keys(), diffs + + @subcommand.usage('[files or directories to diff]') @metrics.collector.collect_metrics('git cl format') def CMDformat(parser: optparse.OptionParser, args: list[str]): """Runs auto-formatting tools (clang-format etc.) on the diff.""" - if gclient_utils.IsEnvCog(): - print( - 'format command is not supported in non-git environment. Please ' - 'use the "Format Modified Lines in All Files (git cl format)" ' - 'functionality in the command palette in the editor instead.', - file=sys.stderr) - return 1 clang_exts = ['.cc', '.cpp', '.h', '.m', '.mm', '.proto'] GN_EXTS = ['.gn', '.gni', '.typemap'] parser.add_option('--full', @@ -7154,53 +7204,70 @@ def CMDformat(parser: optparse.OptionParser, args: list[str]): dest='use_swift_format', action='store_false', help='Disables formatting of Swift file types using swift-format.') - + parser.add_option('--input_diff_file', + help='File to read input change diff from. ' + 'If given, the added blocks of the files in the diff ' + 'will be formatted.') parser.add_option('--mojom', action='store_true', help='Enables formatting of .mojom files.') - parser.add_option('--no-java', action='store_true', help='Disable auto-formatting of .java') - parser.add_option('--lucicfg', action='store_true', help='Enables formatting of .star files.') opts, files = parser.parse_args(args) - opts.full = opts.full or settings.GetFormatFullByDefault() - - # Normalize files against the current path, so paths relative to the - # current directory are still resolved as expected. - files = [os.path.join(os.getcwd(), file) for file in files] - - # git diff generates paths against the root of the repository. Change - # to that directory so clang-format can find files even within subdirs. - rel_base_path = settings.GetRelativeRoot() - if rel_base_path: - os.chdir(rel_base_path) - - # Grab the merge-base commit, i.e. the upstream commit of the current - # branch when it was created or the last time it was rebased. This is - # to cover the case where the user may have called "git fetch origin", - # moving the origin branch to a newer commit, but hasn't rebased yet. upstream_commit: str | None = None upstream_branch: str | None = opts.upstream - if not upstream_branch: - cl = Changelist() - upstream_branch = cl.GetUpstreamBranch() - if upstream_branch: - upstream_commit = RunGit(['merge-base', 'HEAD', - upstream_branch]).strip() + top_dir: str | None = None - if not upstream_commit: - DieWithError('Could not find base commit for this branch. ' - 'Are you in detached state?') + if opts.input_diff_file: + if opts.full: + print('--full and --input_diff_file cannot be used together.', + file=sys.stderr) + return 1 + if files: + print( + 'No file paths to format are allowed ' + 'if --input_diff_file is given.', + file=sys.stderr) + return 1 + elif gclient_utils.IsEnvCog(): + print( + 'Use --input_diff_file to run the format command in non-git ' + 'environments. In CiderG, please use the ' + '"Format Modified Lines in All Files (git cl format)" ' + 'functionality in the command palette instead.', + file=sys.stderr) + return 1 + else: + opts.full = opts.full or settings.GetFormatFullByDefault() + # git diff generates paths against the root of the repository. Change + # to that directory so clang-format can find files even within subdirs. + rel_base_path = settings.GetRelativeRoot() + if rel_base_path: + os.chdir(rel_base_path) + + # Grab the merge-base commit, i.e. the upstream commit of the current + # branch when it was created or the last time it was rebased. This is + # to cover the case where the user may have called "git fetch origin", + # moving the origin branch to a newer commit, but hasn't rebased yet. + if not upstream_branch: + cl = Changelist() + upstream_branch = cl.GetUpstreamBranch() + if upstream_branch: + upstream_commit = RunGit(['merge-base', 'HEAD', + upstream_branch]).strip() + if not upstream_commit: + DieWithError('Could not find base commit for this branch. ' + 'Are you in detached state?') - # Filter out deleted files - diff_output = RunGitDiffCmd(['--name-only', '--diff-filter=d'], - upstream_commit, files) - diff_files = diff_output.splitlines() + # Normalize files against the current path, so paths relative to the + # current directory are still resolved as expected. + files = [os.path.join(os.getcwd(), file) for file in files] + diff_files, _ = _FindFilesToFormat(opts, files, upstream_commit) if opts.js: clang_exts.extend(['.js', '.ts']) @@ -7224,7 +7291,7 @@ def CMDformat(parser: optparse.OptionParser, args: list[str]): if opts.lucicfg: formatters.append((['.star'], _RunLUCICfgFormat)) - top_dir = settings.GetRoot() + top_dir = os.getcwd() if opts.presubmit else settings.GetRoot() return_value = 0 for file_types, format_func in formatters: paths = [p for p in diff_files if p.lower().endswith(tuple(file_types))] diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index b5fbb1c231..a64ea1bd27 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -4880,6 +4880,65 @@ class MakeRequestsHelperTestCase(unittest.TestCase): {'category': 'my-special-category'}) +test_format_input_diff = (""" +diff --git a/README.md b/README.md +--- a/README.md ++++ b/README.md +@@ -6,11 +6,8 @@ + The project's web site is https://www.chromium.org. + + To check out the source code locally, don't use `git clone`! Instead, +-follow [the instructions on how to get the code](docs/get_the_code.md). ++follow [the instructionource is rooted in [docs/README.md](docs/README.md). + +-Documentation in the source is rooted in [docs/README.md](docs/README.md). +- +-Learn how to [Get Around the Chromium Source Code Directory + Structure](https://www.chromium.org/developers/how-tos/getting-around-the-chrome-source-code). + + For historical reasons, there are some small top level directories. Now the +diff --git a/net/base/net_error_details.h b/net/base/net_error_details.h +--- a/net/base/net_error_details.h ++++ b/net/base/net_error_details.h +@@ -13,13 +13,12 @@ + namespace net { + + // A record of net errors with granular error specification generated by +-// net stack. + struct NET_EXPORT NetErrorDetails { + NetErrorDetails() + : quic_broken(false), quic_connection_error(quic::QUIC_NO_ERROR) {} + + NetErrorDetails(bool quic_broken, quic::QuicErrorCode quic_connection_error) +- : quic_broken(quic_broken), ++ : quic_broken(quic_broken), + quic_connection_error(quic_connection_error) {} + + // True if all QUIC alternative services are marked broken for the origin. +diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h +--- a/net/base/net_error_list.h ++++ b/net/base/net_error_list.h +@@ -1063,7 +1063,6 @@ + // were no outstanding references (no one is waiting to read) to keep the + // blob alive. + NET_ERROR(BLOB_DEREFERENCED_WHILE_BUILDING, -904) +- + // A blob that we referenced during construction is broken, or a browser-side + // builder tries to build a blob with a blob reference that isn't finished + // constructing. +""") + +test_format_input_diff_windows = "\r\n".join([ + 'diff --git "a/C:\\\\path\\\\to\\\\src\\\\file.cc" "b/C:\\\\path\\\\to\\\\dst\\\\file.cc"', + 'index ce013625..dd7e1c6f 100644', + '--- "a/C:\\\\path\\\\to\\\\src\\\\file.cc', + '+++ "b/C:\\\\path\\\\to\\\\dst\\\\file.cc', + '@@ -1 +1 @@', + '-random', + '+content', +]) + + class CMDFormatTestCase(unittest.TestCase): def setUp(self): super(CMDFormatTestCase, self).setUp() @@ -5169,6 +5228,71 @@ class CMDFormatTestCase(unittest.TestCase): mock_call.return_value = 255 self.assertEqual(run(mock_opts, files, self._top_dir, 'HEAD'), 2) + @mock.patch('sys.stderr', io.StringIO()) + def testInputDiffFileIsMutallyExclusiveWithFull(self): + """Verifies that opts.input_diff_file cannot be set with opts.full.""" + ret = git_cl.main([ + 'format', + "--input_diff_file", + "/tmp/patch.diff", + "--full", + ]) + self.assertEqual(1, ret) + self.assertEqual( + sys.stderr.getvalue().strip(), + '--full and --input_diff_file cannot be used together.', + ) + + @mock.patch('sys.stderr', io.StringIO()) + def testInputDiffFileIsMutallyExclusiveWithPositionalFilePaths(self): + """Verifies that opts.input_diff_file cannot be set if file given.""" + ret = git_cl.main([ + 'format', + "--input_diff_file", + "/tmp/patch.diff", + "common.cc", + ]) + self.assertEqual(1, ret) + self.assertEqual( + sys.stderr.getvalue().strip(), + 'No file paths to format are allowed ' + 'if --input_diff_file is given.', + ) + + @mock.patch('git_cl._RunClangFormatDiff', return_value=0) + def testInputDiffFile(self, clang_formatter): + """Tests git cl format with --input_diff_file.""" + # Windows doesn't allow a file to be reopened while it's open by + # another handler. + with tempfile.NamedTemporaryFile(mode="w+", delete=False) as input_diff: + input_diff.write(test_format_input_diff) + + try: + ret = git_cl.main(['format', "--input_diff_file", input_diff.name]) + self.assertEqual(0, ret) + clang_formatter.assert_called_with( + mock.ANY, + ['net/base/net_error_details.h', 'net/base/net_error_list.h'], + mock.ANY, mock.ANY) + finally: + os.remove(input_diff.name) + + @mock.patch('git_cl._RunClangFormatDiff', return_value=0) + def testInputDiffFileWithWindowsPatch(self, clang_formatter): + # Windows doesn't allow a file to be reopened while it's open by + # another handler. + with tempfile.NamedTemporaryFile(mode="w+", delete=False) as input_diff: + input_diff.write(test_format_input_diff_windows) + + try: + ret = git_cl.main(['format', "--input_diff_file", input_diff.name]) + self.assertEqual(0, ret) + clang_formatter.assert_called_with( + mock.ANY, ['C:\\\\path\\\\to\\\\dst\\\\file.cc'], mock.ANY, + mock.ANY) + finally: + os.remove(input_diff.name) + @unittest.skipIf(gclient_utils.IsEnvCog(), 'not supported in non-git environment')