Re: [PATCH 2/2] commit-reach: guard !FIND_ALL early exit with generation ordering check
From: Junio C Hamano <hidden>
Date: 2026-07-08 17:24:35
"Kristofer Karlsson via GitGitGadget" [off-list ref] writes:
From: Kristofer Karlsson <redacted> When paint_down_to_common() falls back to commit-date ordering (for v1 commit graphs without corrected commit dates), the !FIND_ALL early exit incorrectly fires. The exit assumes the queue is generation- ordered, so the first RESULT commit found must be the shallowest. With date ordering this is not guaranteed: a closer merge base with a lower committer date (clock skew) may still be in the queue behind deeper commits.
Excellent description of a good observation.
Add a gen_ordered flag that is cleared when the date fallback fires, and require it for the early exit.
The solution is simple and straight-forward. The flag is initialized to true but we drop it when generation order is not in effect, and the early exit requires the flag to be still true.
Update the test from the previous commit to test_expect_success. Signed-off-by: Kristofer Karlsson <redacted> ---
Let's mark it for 'next'. Thanks.
quoted hunk ↗ jump to hunk
commit-reach.c | 10 +++++++--- t/t6600-test-reach.sh | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-)diff --git a/commit-reach.c b/commit-reach.c index 5df471a313..708798a39b 100644 --- a/commit-reach.c +++ b/commit-reach.c@@ -108,11 +108,14 @@ static int paint_down_to_common(struct repository *r, { compare_commits_by_gen_then_commit_date } }; int i; + int gen_ordered = 1; timestamp_t last_gen = GENERATION_NUMBER_INFINITY; struct commit_list **tail = result; - if (!min_generation && !corrected_commit_dates_enabled(r)) + if (!min_generation && !corrected_commit_dates_enabled(r)) { queue.pq.compare = compare_commits_by_commit_date; + gen_ordered = 0; + } one->object.flags |= PARENT1; if (!n) {@@ -147,11 +150,12 @@ static int paint_down_to_common(struct repository *r, commit->object.flags |= RESULT; tail = commit_list_append(commit, tail); /* - * The queue is generation-ordered; no - * remaining common ancestor can be a + * When the queue is generation-ordered, + * no remaining common ancestor can be a * descendant of this one. */ if (!(mb_flags & MERGE_BASE_FIND_ALL) && + gen_ordered && generation < GENERATION_NUMBER_INFINITY) break; }diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh index 1090104220..0ff41381ff 100755 --- a/t/t6600-test-reach.sh +++ b/t/t6600-test-reach.sh@@ -1003,7 +1003,7 @@ test_expect_success 'merge-base without --all is one of --all results' ' grep -F -f single all ' -test_expect_failure 'merge-base without --all, clock skew, v1 commit-graph' ' +test_expect_success 'merge-base without --all, clock skew, v1 commit-graph' ' git rev-parse skew-M2 >expect && merge_base_all_modes skew-P1 skew-P2 '