From e3ab5ed4c1c946abf976fd4c74cce90c3a996128 Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Fri, 12 Mar 2021 11:08:28 -0500 Subject: [PATCH] Don't use --fork-point to determine merge base (#1001) * Don't use --fork-point to determine merge base It turns out that the --fork-point option is subtle and error-prone. Its intent is not to show the nearest common base commit, but rather the commit on a branch that the HEAD (in this case) was originally forked off of, _whether it is currently part of the history of the specified branch or not_ (this can happen if the branch is rewritten). The option also relies on the presence of the fork point in the reflog for the branch, which can be discarded in the course of a "git gc". It is fairly easy to construct a case where the use of --fork-point causes an error and outputs nothing. In fact, I discovered the problem as a result of this occuring spontaneously on one of my own branches (likely related to a rebase). Since the fork-point is empty, we end up diffing against the index instead of the common commit. This may have been a factor in some of the unrelated reformatting that we've seen in past PRs. Change this to a simple "merge-base origin/master HEAD", which outputs the commit id of the most recent common base revision. This change also quotes the forkPoint variable, which likely would have resulted in an error in this case instead of silently producing the wrong output. --- java-format/google-java-format-git-diff.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-format/google-java-format-git-diff.sh b/java-format/google-java-format-git-diff.sh index 6ff0dc584..20601ca37 100755 --- a/java-format/google-java-format-git-diff.sh +++ b/java-format/google-java-format-git-diff.sh @@ -55,7 +55,7 @@ function showNoncompliantFiles() { function callGoogleJavaFormatDiff() { local forkPoint - forkPoint=$(git merge-base --fork-point origin/master) + forkPoint=$(git merge-base origin/master HEAD) local callResult case "$1" in