Skip to content

Commit

Permalink
[clang-format] Add "three dot" diff option to git-clang-format (#74230)
Browse files Browse the repository at this point in the history
This patch adds in the ability to do a "three dot" git-clang-format
between two commits. This looks at the diff between the second commit
and the common merge base rather than comparing at the point of the
specified commits. This is needed to improve the reliability of the LLVM
code formatting CI action which currently breaks in some cases where
files have been modified in the upstream tree and when the person
created their branch, leaving phantom formatting diffs that weren't
touched by the PR author.

Part of a fix for #73873
  • Loading branch information
boomanaiden154 committed Dec 6, 2023
1 parent 689db42 commit aeaae53
Showing 1 changed file with 20 additions and 9 deletions.
29 changes: 20 additions & 9 deletions clang/tools/clang-format/git-clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ def main():
help='passed to clang-format'),
p.add_argument('-v', '--verbose', action='count', default=0,
help='print extra information')
p.add_argument('--diff_from_common_commit', action='store_true',
help=('diff from the last common commit for commits in '
'separate branches rather than the exact point of the '
'commits'))
# We gather all the remaining positional arguments into 'args' since we need
# to use some heuristics to determine whether or not <commit> was present.
# However, to print pretty messages, we make use of metavar and help.
Expand All @@ -145,19 +149,23 @@ def main():
del opts.quiet

commits, files = interpret_args(opts.args, dash_dash, opts.commit)
if len(commits) > 1:
if len(commits) > 2:
die('at most two commits allowed; %d given' % len(commits))
if len(commits) == 2:
if opts.staged:
die('--staged is not allowed when two commits are given')
if not opts.diff:
die('--diff is required when two commits are given')
else:
if len(commits) > 2:
die('at most two commits allowed; %d given' % len(commits))
elif opts.diff_from_common_commit:
die('--diff_from_common_commit is only allowed when two commits are given')

if os.path.dirname(opts.binary):
opts.binary = os.path.abspath(opts.binary)

changed_lines = compute_diff_and_extract_lines(commits, files, opts.staged)
changed_lines = compute_diff_and_extract_lines(commits,
files,
opts.staged,
opts.diff_from_common_commit)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
Expand Down Expand Up @@ -305,9 +313,9 @@ def get_object_type(value):
return convert_string(stdout.strip())


def compute_diff_and_extract_lines(commits, files, staged):
def compute_diff_and_extract_lines(commits, files, staged, diff_common_commit):
"""Calls compute_diff() followed by extract_lines()."""
diff_process = compute_diff(commits, files, staged)
diff_process = compute_diff(commits, files, staged, diff_common_commit)
changed_lines = extract_lines(diff_process.stdout)
diff_process.stdout.close()
diff_process.wait()
Expand All @@ -317,7 +325,7 @@ def compute_diff_and_extract_lines(commits, files, staged):
return changed_lines


def compute_diff(commits, files, staged):
def compute_diff(commits, files, staged, diff_common_commit):
"""Return a subprocess object producing the diff from `commits`.
The return value's `stdin` file object will produce a patch with the
Expand All @@ -327,10 +335,13 @@ def compute_diff(commits, files, staged):
Zero context lines are used in the patch."""
git_tool = 'diff-index'
extra_args = []
if len(commits) > 1:
if len(commits) == 2:
git_tool = 'diff-tree'
if diff_common_commit:
commits = [f'{commits[0]}...{commits[1]}']
elif staged:
extra_args += ['--cached']

cmd = ['git', git_tool, '-p', '-U0'] + extra_args + commits + ['--']
cmd.extend(files)
p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
Expand Down

0 comments on commit aeaae53

Please sign in to comment.