[clang] [clang-format] Add "three dot" diff option to git-clang-format (PR #74230)
Aiden Grossman via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 5 17:42:23 PST 2023
https://github.com/boomanaiden154 updated https://github.com/llvm/llvm-project/pull/74230
>From ae4097b53b90e31802be0be5c8a81fb74c81efc9 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Sat, 2 Dec 2023 23:46:58 -0800
Subject: [PATCH 1/3] [clang-format] Add "three dot" diff option to
git-clang-format
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
---
clang/tools/clang-format/git-clang-format | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format b/clang/tools/clang-format/git-clang-format
index 6e827e17b4ee2..4fe4671482e3f 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -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.
@@ -154,7 +158,10 @@ def main():
if len(commits) > 2:
die('at most two commits allowed; %d given' % len(commits))
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(','))
@@ -302,9 +309,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()
@@ -314,7 +321,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
@@ -328,6 +335,10 @@ def compute_diff(commits, files, staged):
git_tool = 'diff-tree'
elif staged:
extra_args += ['--cached']
+
+ if len(commits) > 1 and diff_common_commit:
+ commits = [f'{commits[0]}...{commits[1]}']
+
cmd = ['git', git_tool, '-p', '-U0'] + extra_args + commits + ['--']
cmd.extend(files)
p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
>From 4584ebc58337f04290a5c753f585426c8c6f739f Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Sun, 3 Dec 2023 13:08:16 -0800
Subject: [PATCH 2/3] Add early exit when flag is specified with invalid
parameters
---
clang/tools/clang-format/git-clang-format | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/tools/clang-format/git-clang-format b/clang/tools/clang-format/git-clang-format
index 4fe4671482e3f..51ae8739bee0e 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -157,6 +157,8 @@ def main():
else:
if len(commits) > 2:
die('at most two commits allowed; %d given' % len(commits))
+ if len(commits) < 2 and opts.diff_from_common_commit:
+ die('--diff_from_common_commit is only allowed when two commits are given')
opts.binary=os.path.abspath(opts.binary)
changed_lines = compute_diff_and_extract_lines(commits,
files,
>From a8b385070845ba20f63f1e0c6835ff9bc609548f Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Tue, 5 Dec 2023 17:42:12 -0800
Subject: [PATCH 3/3] Address reviewer feedback
---
clang/tools/clang-format/git-clang-format | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format b/clang/tools/clang-format/git-clang-format
index 51ae8739bee0e..48d55d162a7c4 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -149,15 +149,14 @@ 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))
- if len(commits) < 2 and opts.diff_from_common_commit:
+ elif opts.diff_from_common_commit:
die('--diff_from_common_commit is only allowed when two commits are given')
opts.binary=os.path.abspath(opts.binary)
changed_lines = compute_diff_and_extract_lines(commits,
@@ -333,14 +332,13 @@ def compute_diff(commits, files, staged, diff_common_commit):
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']
- if len(commits) > 1 and diff_common_commit:
- commits = [f'{commits[0]}...{commits[1]}']
-
cmd = ['git', git_tool, '-p', '-U0'] + extra_args + commits + ['--']
cmd.extend(files)
p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
More information about the cfe-commits
mailing list