Skip to content

Conversation

boomanaiden154
Copy link
Contributor

@boomanaiden154 boomanaiden154 commented Aug 25, 2025

From local testing, git diff-tree does not support three dot diffs correctly, instead expecting the --merge-base flag to be passed along with two commits. From my reading, the documentation (https://git-scm.com/docs/git-diff-tree) also confirms this. This patch updates the git-clang-format script to be correct.

I don't think we ever ran into this issue before because we never ended up using it. For the PR code format job I believe we would just explicitly pass the merge base, completely bypassing the problem.

From local testing, git diff-tree does not support three dot diffs
correctly, instead expecting the --merge-base flag to be passed along with
two commits. This patch updates the git-clang-format script to be
correct.

I don't think we ever ran into this issue before because we never ended
up using it. For the PR code format job I believe we would just
explicitly pass the merge base, completely bypassing the problem.
@llvmbot
Copy link
Member

llvmbot commented Aug 25, 2025

@llvm/pr-subscribers-clang-format

Author: Aiden Grossman (boomanaiden154)

Changes

From local testing, git diff-tree does not support three dot diffs correctly, instead expecting the --merge-base flag to be passed along with two commits. From my reading, the documentation (https://git-scm.com/docs/git-diff-tree) also confirms this. This patch updates the git-clang-format script to be correct.

I don't think we ever ran into this issue before because we never ended up using it. For the PR code format job I believe we would just explicitly pass the merge base, completely bypassing the problem.


Full diff: https://github.com/llvm/llvm-project/pull/155247.diff

1 Files Affected:

  • (modified) clang/tools/clang-format/git-clang-format (+1-1)
diff --git a/clang/tools/clang-format/git-clang-format b/clang/tools/clang-format/git-clang-format
index e709803d9a3f1..fe2dd283d403e 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -419,7 +419,7 @@ def compute_diff(commits, files, staged, diff_common_commit):
     if len(commits) == 2:
         git_tool = "diff-tree"
         if diff_common_commit:
-            commits = [f"{commits[0]}...{commits[1]}"]
+            extra_args += ["--merge-base"]
     elif staged:
         extra_args += ["--cached"]
 

@boomanaiden154 boomanaiden154 enabled auto-merge (squash) August 26, 2025 16:32
@boomanaiden154 boomanaiden154 merged commit f09986a into llvm:main Aug 26, 2025
9 checks passed
@boomanaiden154 boomanaiden154 deleted the clang-format-merge-base-common-commit branch August 26, 2025 18:15
@boomanaiden154 boomanaiden154 added this to the LLVM 21.x Release milestone Aug 26, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Aug 26, 2025
@boomanaiden154
Copy link
Contributor Author

/cherry-pick f09986a

@llvmbot
Copy link
Member

llvmbot commented Aug 26, 2025

/pull-request #155466

@llvmbot llvmbot moved this from Needs Triage to Done in LLVM Release Status Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants