Skip to content

Commit 33d1e97

Browse files
committed
Merge branch 'kk/commit-reach-find-all-fix'
Fix a commit-graph v1 regression in v2.55.0: The regression was introduced by git/git@93e5b1680e (merged as git/git@6390da42c7) and first shipped in v2.55.0, which is why the base here is vfs-2.55.0. That commit added a fast-path early exit in paint_down_to_common() predicated on the priority queue being generation-ordered. The predicate holds under the generation-then-date ordering, but the same function falls back to plain commit-date ordering when !min_generation && !corrected_commit_dates_enabled(r), which is true whenever the loaded commit-graph lacks a GDAT chunk (i.e. a v1 graph). Under that fallback the early exit is unsound: with clock skew, a closer merge base can sit behind deeper commits in the queue, and git merge-base (without --all) can silently return a non-best answer. The tip commit git/git@ae68032a8d0 threads a gen_ordered flag through and gates the early exit on it. The reason microsoft/git in particular needs to carry this promptly, rather than wait the usual weeks for upstream graduation, is Scalar. set_recommended_config() pins commitGraph.generationVersion=1 (scalar.c:191), and that pin is applied on every scalar clone, scalar register, and scalar reconfigure. Stolee introduced it in 18580f0 as a deliberate but temporary compatibility measure for existing enlistments with v1 graphs on disk, with the accompanying Documentation/scalar.adoc note calling out that v2 is preferred and the pin is expected to change once the upgrade story solidifies. Every Scalar-managed enlistment on microsoft/git 2.55.x therefore lands on exactly the broken code path, and Scalar's target audience (Office monorepos and external adopters) is precisely the large-monorepo, many-active-branches cohort Kristofer's original optimization was aimed at in the first place. Sadly, even though this regression first appeared in v2.55.0, the topic branch is unfortunately based on an older commit where a context line changed, causing merge conflicts. Resolve them in the same way as 0444c74 (Merge branch 'kk/commit-reach-find-all-fix' into next, 2026-07-10). Signed-off-by: Johannes Schindelin <johasc@microsoft.com>
2 parents 0359381 + ae68032 commit 33d1e97

2 files changed

Lines changed: 48 additions & 3 deletions

File tree

commit-reach.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,14 @@ static int paint_down_to_common(struct repository *r,
108108
{ compare_commits_by_gen_then_commit_date }
109109
};
110110
int i;
111+
int gen_ordered = 1;
111112
timestamp_t last_gen = GENERATION_NUMBER_INFINITY;
112113
struct commit_list **tail = result;
113114

114-
if (!min_generation && !corrected_commit_dates_enabled(r))
115+
if (!min_generation && !corrected_commit_dates_enabled(r)) {
115116
queue.pq.compare = compare_commits_by_commit_date;
117+
gen_ordered = 0;
118+
}
116119

117120
one->object.flags |= PARENT1;
118121
if (!n) {
@@ -147,11 +150,12 @@ static int paint_down_to_common(struct repository *r,
147150
commit->object.flags |= RESULT;
148151
tail = commit_list_append(commit, tail);
149152
/*
150-
* The queue is generation-ordered; no
151-
* remaining common ancestor can be a
153+
* When the queue is generation-ordered,
154+
* no remaining common ancestor can be a
152155
* descendant of this one.
153156
*/
154157
if (!(mb_flags & MERGE_BASE_FIND_ALL) &&
158+
gen_ordered &&
155159
generation < GENERATION_NUMBER_INFINITY)
156160
break;
157161
}

t/t6600-test-reach.sh

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,42 @@ test_expect_success 'setup' '
4949
git tag -a -m "$x-$i" tag-$x-$i commit-$x-$i || return 1
5050
done
5151
done &&
52+
# Build a topology with clock skew to test the !FIND_ALL early
53+
# exit in paint_down_to_common(). M2 is the correct merge base
54+
# of P1 and P2, but its ancestor M1 has a higher committer date
55+
# due to clock skew. With date-only ordering (v1 commit graph
56+
# without corrected commit dates), M1 pops from the queue first,
57+
# gets both paint sides, and the early exit fires before M2 is
58+
# ever visited.
59+
#
60+
# P1 P2 @7000
61+
# | / \
62+
# A B D @6000
63+
# / \ | |
64+
# | M2--+ | @2000 (correct merge base)
65+
# \ | |
66+
# M1--------+ @5000 (clock skew: date > M2)
67+
# |
68+
# root @1000
69+
#
70+
git checkout --orphan skew-orphan &&
71+
skew_tree=$(git mktree </dev/null) &&
72+
skew_commit () {
73+
GIT_COMMITTER_DATE="@$1 +0000" GIT_AUTHOR_DATE="@$1 +0000" \
74+
git commit-tree -m "$2" "$skew_tree" $3 $4 $5 $6
75+
} &&
76+
skew_root=$(skew_commit 1000 root) &&
77+
skew_M1=$(skew_commit 5000 M1 -p "$skew_root") &&
78+
skew_M2=$(skew_commit 2000 M2 -p "$skew_M1") &&
79+
skew_A=$(skew_commit 6000 A -p "$skew_M1" -p "$skew_M2") &&
80+
skew_B=$(skew_commit 6000 B -p "$skew_M2") &&
81+
skew_D=$(skew_commit 6000 D -p "$skew_M1") &&
82+
skew_P1=$(skew_commit 7000 P1 -p "$skew_A") &&
83+
skew_P2=$(skew_commit 7000 P2 -p "$skew_B" -p "$skew_D") &&
84+
git branch -f skew-P1 "$skew_P1" &&
85+
git branch -f skew-P2 "$skew_P2" &&
86+
git tag skew-M2 "$skew_M2" &&
87+
5288
git commit-graph write --reachable &&
5389
mv .git/objects/info/commit-graph commit-graph-full &&
5490
chmod u+w commit-graph-full &&
@@ -967,4 +1003,9 @@ test_expect_success 'merge-base without --all is one of --all results' '
9671003
grep -F -f single all
9681004
'
9691005

1006+
test_expect_success 'merge-base without --all, clock skew, v1 commit-graph' '
1007+
git rev-parse skew-M2 >expect &&
1008+
merge_base_all_modes skew-P1 skew-P2
1009+
'
1010+
9701011
test_done

0 commit comments

Comments
 (0)