InTheForest/chromium_depot_tools
1
0
Fork 0
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools.git synced 2026-01-11 18:51:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

git_cl: update the clang, java, and yapf formatters to be git-free

Browse Source 
This makes the formatter runners to be git-free if --input_diff_file
is given. They still run `git diff`, if --diff is given. However,
for the purpose of making the presubmit check for format verification
git-free, it's ok as --diff is not used by the presubmit check.


Bug: 386840832
Change-Id: If5ab68fa4e2fec1aafa22e15ddeabb744993342b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6931775
Commit-Queue: Scott Lee <ddoman@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
This commit is contained in:
Scott Lee
2025-09-11 10:28:39 -07:00
committed by LUCI CQ
parent f1e3fbdf09
commit 363e910475
2 changed files with 126 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

95
git_cl.py
View File

@@ -642,16 +642,18 @@ def _print_tryjobs(options, builds):
print('Total: %d tryjobs' % total) print('Total: %d tryjobs' % total)
def _ComputeFormatDiffLineRanges(files, upstream_commit, expand=0): def _ComputeFormatDiffLineRanges(files, diffs, expand=0):
"""Gets the changed line ranges for each file since upstream_commit. """Gets the changed line ranges for each file.
Parses a git diff on provided files and returns a dict that maps a file name Parses a git diff on provided files and returns a dict that maps a file name
to an ordered list of range tuples in the form (start_line, count). to an ordered list of range tuples in the form (start_line, count).
Ranges are in the same format as a git diff. Ranges are in the same format as a git diff.
Args: Args:
files: List of paths to diff. files: List of files to parse the diff of and return the ranges for.
upstream_commit: Commit to diff against to find changed lines. diffs: a dict of diffs, where key is the file without the prefix,
and the value is the diff generated for the corresponding file.
Note that the hunks in the diffs contain prefxies, such as a/.., b/..
expand: Expand diff ranges by this many lines before & after. expand: Expand diff ranges by this many lines before & after.
Returns: Returns:
@@ -661,13 +663,7 @@ def _ComputeFormatDiffLineRanges(files, upstream_commit, expand=0):
if len(files) == 0: if len(files) == 0:
return {} return {}
# Take the git diff and find the line ranges where there are changes. pattern = r'^@@.*\+(.*) @@'
diff_output = RunGitDiffCmd(['-U0'],
upstream_commit,
files,
allow_prefix=True)
pattern = r'(?:^diff --git a/(?:.*) b/(.*))|(?:^@@.*\+(.*) @@)'
# 2 capture groups # 2 capture groups
# 0 == fname of diff file # 0 == fname of diff file
# 1 == 'diff_start,diff_count' or 'diff_start' # 1 == 'diff_start,diff_count' or 'diff_start'
@@ -678,38 +674,39 @@ def _ComputeFormatDiffLineRanges(files, upstream_commit, expand=0):
# running re.findall on the above string with pattern will give # running re.findall on the above string with pattern will give
# [('foo.py', ''), ('', '14,3'), ('', '17')] # [('foo.py', ''), ('', '14,3'), ('', '17')]
curr_file = None line_diffs = collections.defaultdict(list)
line_diffs = {} for file in files:
for match in re.findall(pattern, diff_output, flags=re.MULTILINE): diff = diffs.get(file, "")
if match[0] != '': if not diff:
# Will match the second filename in diff --git a/a.py b/b.py. continue
curr_file = match[0]
line_diffs[curr_file] = [] prev_end = 1
prev_end = 1 for match in re.findall(pattern, diff, flags=re.MULTILINE):
else: if not match:
continue
# Matches +14,3 # Matches +14,3
if ',' in match[1]: if ',' in match:
diff_start, diff_count = match[1].split(',') diff_start, diff_count = match.split(',')
else: else:
# Single line changes are of the form +12 instead of +12,1. # Matches +12
diff_start = match[1] diff_start, diff_count = match, 1
diff_count = 1
# if the original lines were removed without replacements, # if the original lines were removed without replacements,
# the diff count is 0. Then, no formatting is necessary. # the diff count is 0. Then, no formatting is necessary.
if diff_count == 0: if diff_count == 0:
continue continue
diff_start = int(diff_start)
diff_count = int(diff_count)
# diff_count contains the diff_start line, and the line numbers # diff_count contains the diff_start line, and the line numbers
# given to formatter args are inclusive. For example, in # given to formatter args are inclusive. For example, in
# google-java-format "--lines 5:10" includes 5th-10th lines. # google-java-format "--lines 5:10" includes 5th-10th lines.
diff_start = int(diff_start)
diff_count = int(diff_count)
diff_end = diff_start + diff_count - 1 + expand diff_end = diff_start + diff_count - 1 + expand
diff_start = max(prev_end + 1, diff_start - expand) diff_start = max(prev_end + 1, diff_start - expand)
if diff_start <= diff_end: if diff_start <= diff_end:
prev_end = diff_end prev_end = diff_end
line_diffs[curr_file].append((diff_start, diff_end)) line_diffs[file].append((diff_start, diff_end))
return line_diffs return line_diffs
@@ -6718,7 +6715,7 @@ def RunGitDiffCmd(diff_type,
return output return output
def _RunClangFormatDiff(opts, paths, top_dir, upstream_commit): def _RunClangFormatDiff(opts, paths, top_dir, diffs):
"""Runs clang-format-diff and sets a return value if necessary.""" """Runs clang-format-diff and sets a return value if necessary."""
# Set to 2 to signal to CheckPatchFormatted() that this patch isn't # Set to 2 to signal to CheckPatchFormatted() that this patch isn't
# formatted. This is used to block during the presubmit. # formatted. This is used to block during the presubmit.
@@ -6731,7 +6728,7 @@ def _RunClangFormatDiff(opts, paths, top_dir, upstream_commit):
DieWithError(e) DieWithError(e)
# Full formatting # Full formatting
if opts.full: if not diffs:
cmd = [clang_format_tool] cmd = [clang_format_tool]
if not opts.dry_run and not opts.diff: if not opts.dry_run and not opts.diff:
cmd.append('-i') cmd.append('-i')
@@ -6759,20 +6756,20 @@ def _RunClangFormatDiff(opts, paths, top_dir, upstream_commit):
except clang_format.NotFoundError as e: except clang_format.NotFoundError as e:
DieWithError(e) DieWithError(e)
cmd = ['vpython3', script, '-p0'] cmd = ['vpython3', script, '-p1']
if not opts.dry_run and not opts.diff: if not opts.dry_run and not opts.diff:
cmd.append('-i') cmd.append('-i')
diff_output = RunGitDiffCmd(['-U0'], upstream_commit, paths)
env = os.environ.copy() env = os.environ.copy()
env['PATH'] = (str(os.path.dirname(clang_format_tool)) + os.pathsep + env['PATH'] = (str(os.path.dirname(clang_format_tool)) + os.pathsep +
env['PATH']) env['PATH'])
# If `clang-format-diff.py` is run without `-i` and the diff is # If `clang-format-diff.py` is run without `-i` and the diff is
# non-empty, it returns an error code of 1. This will cause `RunCommand` # non-empty, it returns an error code of 1. This will cause `RunCommand`
# to die with an error if `error_ok` is not set. # to die with an error if `error_ok` is not set.
input_diff = "\n".join(diffs.get(p, "") for p in paths)
stdout = RunCommand(cmd, stdout = RunCommand(cmd,
error_ok=True, error_ok=True,
stdin=diff_output.encode(), stdin=input_diff.encode(),
cwd=top_dir, cwd=top_dir,
env=env, env=env,
shell=sys.platform.startswith('win32')) shell=sys.platform.startswith('win32'))
@@ -6784,7 +6781,7 @@ def _RunClangFormatDiff(opts, paths, top_dir, upstream_commit):
return return_value return return_value
def _RunGoogleJavaFormat(opts, paths, top_dir, upstream_commit): def _RunGoogleJavaFormat(opts, paths, top_dir, diffs):
"""Runs google-java-format and sets a return value if necessary.""" """Runs google-java-format and sets a return value if necessary."""
tool = google_java_format.FindGoogleJavaFormat() tool = google_java_format.FindGoogleJavaFormat()
if tool is None: if tool is None:
@@ -6800,13 +6797,11 @@ def _RunGoogleJavaFormat(opts, paths, top_dir, upstream_commit):
else: else:
base_cmd += ['--replace'] base_cmd += ['--replace']
changed_lines_only = not opts.full changed_lines_only = bool(diffs)
if changed_lines_only: if changed_lines_only:
# Format two lines around each changed line so that the correct amount # Format two lines around each changed line so that the correct amount
# of blank lines will be added between symbols. # of blank lines will be added between symbols.
line_diffs = _ComputeFormatDiffLineRanges(paths, line_diffs = _ComputeFormatDiffLineRanges(paths, diffs, expand=2)
upstream_commit,
expand=2)
def RunFormat(cmd, path, range_args, **kwds): def RunFormat(cmd, path, range_args, **kwds):
stdout = RunCommand(cmd + range_args + [path], **kwds) stdout = RunCommand(cmd + range_args + [path], **kwds)
@@ -6861,7 +6856,7 @@ def _RunGoogleJavaFormat(opts, paths, top_dir, upstream_commit):
return return_value return return_value
def _RunRustFmt(opts, paths, top_dir, upstream_commit): def _RunRustFmt(opts, paths, top_dir, diffs):
"""Runs rustfmt. Just like _RunClangFormatDiff returns 2 to indicate that """Runs rustfmt. Just like _RunClangFormatDiff returns 2 to indicate that
presubmit checks have failed (and returns 0 otherwise).""" presubmit checks have failed (and returns 0 otherwise)."""
# Locate the rustfmt binary. # Locate the rustfmt binary.
@@ -6887,7 +6882,7 @@ def _RunRustFmt(opts, paths, top_dir, upstream_commit):
return 0 return 0
def _RunSwiftFormat(opts, paths, top_dir, upstream_commit): def _RunSwiftFormat(opts, paths, top_dir, diffs):
"""Runs swift-format. Just like _RunClangFormatDiff returns 2 to indicate """Runs swift-format. Just like _RunClangFormatDiff returns 2 to indicate
that presubmit checks have failed (and returns 0 otherwise).""" that presubmit checks have failed (and returns 0 otherwise)."""
if sys.platform != 'darwin': if sys.platform != 'darwin':
@@ -6912,7 +6907,7 @@ def _RunSwiftFormat(opts, paths, top_dir, upstream_commit):
return 0 return 0
def _RunYapf(opts, paths, top_dir, upstream_commit): def _RunYapf(opts, paths, top_dir, diffs):
depot_tools_path = os.path.dirname(os.path.abspath(__file__)) depot_tools_path = os.path.dirname(os.path.abspath(__file__))
yapf_tool = os.path.join(depot_tools_path, 'yapf') yapf_tool = os.path.join(depot_tools_path, 'yapf')
@@ -6935,8 +6930,8 @@ def _RunYapf(opts, paths, top_dir, upstream_commit):
# Note: yapf still seems to fix indentation of the entire file # Note: yapf still seems to fix indentation of the entire file
# even if line ranges are specified. # even if line ranges are specified.
# See https://github.com/google/yapf/issues/499 # See https://github.com/google/yapf/issues/499
if not opts.full and paths: if paths and diffs:
line_diffs = _ComputeFormatDiffLineRanges(paths, upstream_commit) line_diffs = _ComputeFormatDiffLineRanges(paths, diffs)
yapfignore_patterns = _GetYapfIgnorePatterns(top_dir) yapfignore_patterns = _GetYapfIgnorePatterns(top_dir)
paths = _FilterYapfIgnoredFiles(paths, yapfignore_patterns) paths = _FilterYapfIgnoredFiles(paths, yapfignore_patterns)
@@ -6976,7 +6971,7 @@ def _RunYapf(opts, paths, top_dir, upstream_commit):
return return_value return return_value
def _RunGnFormat(opts, paths, top_dir, upstream_commit): def _RunGnFormat(opts, paths, top_dir, diffs):
cmd = [sys.executable, os.path.join(DEPOT_TOOLS, 'gn.py'), 'format'] cmd = [sys.executable, os.path.join(DEPOT_TOOLS, 'gn.py'), 'format']
if opts.dry_run or opts.diff: if opts.dry_run or opts.diff:
cmd.append('--dry-run') cmd.append('--dry-run')
@@ -6999,7 +6994,7 @@ def _RunGnFormat(opts, paths, top_dir, upstream_commit):
return return_value return return_value
def _RunMojomFormat(opts, paths, top_dir, upstream_commit): def _RunMojomFormat(opts, paths, top_dir, diffs):
primary_solution_path = gclient_paths.GetPrimarySolutionPath() primary_solution_path = gclient_paths.GetPrimarySolutionPath()
if not primary_solution_path: if not primary_solution_path:
DieWithError('Could not find the primary solution path (e.g. ' DieWithError('Could not find the primary solution path (e.g. '
@@ -7022,7 +7017,7 @@ def _RunMojomFormat(opts, paths, top_dir, upstream_commit):
return ret return ret
def _RunMetricsXMLFormat(opts, paths, top_dir, upstream_commit): def _RunMetricsXMLFormat(opts, paths, top_dir, diffs):
# Skip the metrics formatting from the global presubmit hook. These files # Skip the metrics formatting from the global presubmit hook. These files
# have a separate presubmit hook that issues an error if the files need # have a separate presubmit hook that issues an error if the files need
# formatting, whereas the top-level presubmit script merely issues a # formatting, whereas the top-level presubmit script merely issues a
@@ -7061,7 +7056,7 @@ def _RunMetricsXMLFormat(opts, paths, top_dir, upstream_commit):
return return_value return return_value
def _RunLUCICfgFormat(opts, paths, top_dir, upstream_commit): def _RunLUCICfgFormat(opts, paths, top_dir, diffs):
depot_tools_path = os.path.dirname(os.path.abspath(__file__)) depot_tools_path = os.path.dirname(os.path.abspath(__file__))
lucicfg = os.path.join(depot_tools_path, 'lucicfg') lucicfg = os.path.join(depot_tools_path, 'lucicfg')
if sys.platform == 'win32': if sys.platform == 'win32':
@@ -7266,8 +7261,9 @@ def CMDformat(parser: optparse.OptionParser, args: list[str]):
# Normalize files against the current path, so paths relative to the # Normalize files against the current path, so paths relative to the
# current directory are still resolved as expected. # current directory are still resolved as expected.
top_dir = os.getcwd() if opts.presubmit else settings.GetRoot()
files = [os.path.join(os.getcwd(), file) for file in files] files = [os.path.join(os.getcwd(), file) for file in files]
diff_files, _ = _FindFilesToFormat(opts, files, upstream_commit) diff_files, diffs = _FindFilesToFormat(opts, files, upstream_commit)
if opts.js: if opts.js:
clang_exts.extend(['.js', '.ts']) clang_exts.extend(['.js', '.ts'])
@@ -7291,13 +7287,12 @@ def CMDformat(parser: optparse.OptionParser, args: list[str]):
if opts.lucicfg: if opts.lucicfg:
formatters.append((['.star'], _RunLUCICfgFormat)) formatters.append((['.star'], _RunLUCICfgFormat))
top_dir = os.getcwd() if opts.presubmit else settings.GetRoot()
return_value = 0 return_value = 0
for file_types, format_func in formatters: for file_types, format_func in formatters:
paths = [p for p in diff_files if p.lower().endswith(tuple(file_types))] paths = [p for p in diff_files if p.lower().endswith(tuple(file_types))]
if not paths: if not paths:
continue continue
ret = format_func(opts, paths, top_dir, upstream_commit) ret = format_func(opts, paths, top_dir, diffs)
return_value = return_value or ret return_value = return_value or ret
return return_value return return_value

110
tests/git_cl_test.py
View File

@@ -4926,6 +4926,18 @@ diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
// A blob that we referenced during construction is broken, or a browser-side // 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 // builder tries to build a blob with a blob reference that isn't finished
// constructing. // constructing.
diff --git a/testing/xvfb_unittest.py b/testing/xvfb_unittest.py
--- a/testing/xvfb_unittest.py
+++ b/testing/xvfb_unittest.py
@@ -18,7 +18,7 @@
# pylint: disable=super-with-arguments
TEST_FILE = __file__.replace('.pyc', '.py')
-XVFB = TEST_FILE.replace('_unittest', '')
+XVFB = TEST_FILE.replace('_unittest', '')
XVFB_TEST_SCRIPT = TEST_FILE.replace('_unittest', '_test_script')
""") """)
test_format_input_diff_windows = "\r\n".join([ test_format_input_diff_windows = "\r\n".join([
@@ -4954,6 +4966,10 @@ class CMDFormatTestCase(unittest.TestCase):
super(CMDFormatTestCase, self).tearDown() super(CMDFormatTestCase, self).tearDown()
def _make_temp_file(self, fname, contents): def _make_temp_file(self, fname, contents):
dir_path = os.path.dirname(fname)
if dir_path:
os.makedirs(os.path.join(self._top_dir, dir_path), exist_ok=True)
gclient_utils.FileWrite(os.path.join(self._top_dir, fname), gclient_utils.FileWrite(os.path.join(self._top_dir, fname),
('\n'.join(contents))) ('\n'.join(contents)))
@@ -4974,41 +4990,51 @@ class CMDFormatTestCase(unittest.TestCase):
return f return f
def testClangFormatDiffFull(self): @mock.patch('git_cl.Settings.GetRoot')
self._make_temp_file('test.cc', ['// test']) def testClangFormatDryRun(self, GetRoot):
git_cl.settings.GetFormatFullByDefault.return_value = False diffs = git_cl._SplitDiffsByFile(test_format_input_diff)
diff_file = [os.path.join(self._top_dir, 'test.cc')] files = [f for f in diffs if f.endswith('.h')]
mock_opts = mock.Mock(full=True, dry_run=True, diff=False) mock_opts = mock.Mock(full=True, dry_run=True, diff=False)
for f in files:
self._make_temp_file(f, ['// test'])
# Diff try:
git_cl.RunCommand.side_effect = self._run_command_mock(' // test') previous_cwd = os.getcwd()
return_value = git_cl._RunClangFormatDiff(mock_opts, diff_file, os.chdir(self._top_dir)
self._top_dir, 'HEAD') # If the clang-format-diff returns 0, if the input and output codes
self.assertEqual(2, return_value) # are the same.
git_cl.RunCommand.side_effect = self._run_command_mock('// test')
return_value = git_cl._RunClangFormatDiff(mock_opts, files,
self._top_dir, None)
self.assertEqual(0, return_value)
# No diff # Returns 2, otherwise.
git_cl.RunCommand.side_effect = self._run_command_mock('// test') git_cl.RunCommand.side_effect = self._run_command_mock(' // test')
return_value = git_cl._RunClangFormatDiff(mock_opts, diff_file, return_value = git_cl._RunClangFormatDiff(mock_opts, files,
self._top_dir, 'HEAD') self._top_dir, None)
self.assertEqual(0, return_value) self.assertEqual(2, return_value)
finally:
os.chdir(previous_cwd)
def testClangFormatDiff(self): def testClangFormatDiff(self):
git_cl.settings.GetFormatFullByDefault.return_value = False diffs = git_cl._SplitDiffsByFile(test_format_input_diff)
# A valid file is required, so use this test. files = [f for f in diffs if f.endswith('.h')]
clang_format.FindClangFormatToolInChromiumTree.return_value = __file__ mock_opts = mock.Mock(full=False, dry_run=False, diff=False)
mock_opts = mock.Mock(full=False, dry_run=True, diff=False)
# Diff # Diff
git_cl.RunCommand.side_effect = self._run_command_mock('error') git_cl.RunCommand.retrun_value = 0
return_value = git_cl._RunClangFormatDiff(mock_opts, ['.'], return_value = git_cl._RunClangFormatDiff(mock_opts, files,
self._top_dir, 'HEAD') self._top_dir, diffs)
self.assertEqual(2, return_value)
# No diff
git_cl.RunCommand.side_effect = self._run_command_mock('')
return_value = git_cl._RunClangFormatDiff(mock_opts, ['.'],
self._top_dir, 'HEAD')
self.assertEqual(0, return_value) self.assertEqual(0, return_value)
git_cl.RunCommand.assert_called_with(
mock.ANY, # command
error_ok=True,
# it should stream the patches for the selected files only.
stdin="\n".join(diffs.get(f, "") for f in files).encode(),
cwd=self._top_dir,
env=mock.ANY,
shell=mock.ANY,
)
def testYapfignoreExplicit(self): def testYapfignoreExplicit(self):
self._make_yapfignore(['foo/bar.py', 'foo/bar/baz.py']) self._make_yapfignore(['foo/bar.py', 'foo/bar/baz.py'])
@@ -5268,14 +5294,35 @@ class CMDFormatTestCase(unittest.TestCase):
input_diff.write(test_format_input_diff) input_diff.write(test_format_input_diff)
try: try:
ret = git_cl.main(['format', "--input_diff_file", input_diff.name]) previous_cwd = os.getcwd()
os.chdir(self._top_dir)
ret = git_cl.main([
'format',
"--input_diff_file",
input_diff.name,
"--presubmit",
"--dry-run",
"--python",
])
self.assertEqual(0, ret) self.assertEqual(0, ret)
clang_formatter.assert_called_with( clang_formatter.assert_called_with(
mock.ANY, mock.ANY,
['net/base/net_error_details.h', 'net/base/net_error_list.h'], ['net/base/net_error_details.h', 'net/base/net_error_list.h'],
mock.ANY, mock.ANY) mock.ANY, mock.ANY)
git_cl.RunCommand.assert_called_with(
[
'vpython3', mock.ANY, '--style', mock.ANY,
'testing/xvfb_unittest.py', '-l', '18-24', '--diff'
],
cwd=self._top_dir,
error_ok=True,
shell=mock.ANY,
stderr=-1,
)
finally: finally:
os.remove(input_diff.name) os.remove(input_diff.name)
os.chdir(previous_cwd)
@mock.patch('git_cl._RunClangFormatDiff', return_value=0) @mock.patch('git_cl._RunClangFormatDiff', return_value=0)
def testInputDiffFileWithWindowsPatch(self, clang_formatter): def testInputDiffFileWithWindowsPatch(self, clang_formatter):
@@ -5285,7 +5332,12 @@ class CMDFormatTestCase(unittest.TestCase):
input_diff.write(test_format_input_diff_windows) input_diff.write(test_format_input_diff_windows)
try: try:
ret = git_cl.main(['format', "--input_diff_file", input_diff.name]) ret = git_cl.main([
'format',
"--input_diff_file",
input_diff.name,
"--presubmit",
])
self.assertEqual(0, ret) self.assertEqual(0, ret)
clang_formatter.assert_called_with( clang_formatter.assert_called_with(
mock.ANY, ['C:\\\\path\\\\to\\\\dst\\\\file.cc'], mock.ANY, mock.ANY, ['C:\\\\path\\\\to\\\\dst\\\\file.cc'], mock.ANY,