[PATCH v3 3/3] commit-reach: die on contains walk errors
- From
Tamir Duberstein <tamird@gmail.com>
- Date
- Jun 12, 2026, 03:00 UTC
- Message-ID
- <20260611-ref-filter-memoized-contains-v3-3-b26af3dba285@gmail.com>
- In-Reply-To
- <20260611-ref-filter-memoized-contains-v3-0-b26af3dba285@gmail.com>
Without generation numbers, repo_is_descendant_of() can return -1 when it cannot read commit ancestry. commit_contains() exposes that result through a Boolean interface, so ref-filter treats it as true. This can include a ref for --contains or exclude it for --no-contains without failing the command.
Die when repo_is_descendant_of() reports an error. The memoized walk already dies when it cannot parse a commit, so callers of the non-memoized path no longer turn a failed walk into a match.
Reported-by: Jeff King <peff@peff.net> Link: https://lore.kernel.org/r/20260611072942.GG2191159@coredump.intra.peff.net Signed-off-by: Tamir Duberstein <tamird@gmail.com> --- commit-reach.c | 8 +++++++- t/t6301-for-each-ref-errors.sh | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/commit-reach.c b/commit-reach.c index 572d2d47ff..af5563d70f 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -820,10 +820,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate, int commit_contains(struct ref_filter *filter, struct commit *commit, struct commit_list *list, struct contains_cache *cache) { + int result; + if (filter->with_commit_tag_algo || generation_numbers_enabled(the_repository)) return contains_tag_algo(commit, list, cache) == CONTAINS_YES; - return repo_is_descendant_of(the_repository, commit, list); + + result = repo_is_descendant_of(the_repository, commit, list); + if (result < 0) + die(_("failed to check reachability")); + return result; } int can_all_from_reach_with_flag(struct object_array *from, diff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh index e06feb06e9..72b27c8be3 100755 --- a/t/t6301-for-each-ref-errors.sh +++ b/t/t6301-for-each-ref-errors.sh @@ -52,6 +52,28 @@ test_expect_success 'Missing objects are reported correctly' ' test_must_be_empty brief-err ' +test_expect_success 'missing ancestors are reported by contains filters' ' + test_when_finished "git update-ref -d refs/heads/missing-parent" && + { + echo "tree $(git rev-parse HEAD^{tree})" && + echo "parent $MISSING" && + git cat-file commit HEAD | + sed -n -e "/^author /p" -e "/^committer /p" && + echo && + echo "missing parent" + } >commit && + broken=$(git hash-object -t commit -w commit) && + git update-ref refs/heads/missing-parent "$broken" && + for option in --contains --no-contains + do + test_must_fail git for-each-ref "$option=HEAD" \ + refs/heads/missing-parent >out 2>err && + test_must_be_empty out && + test_grep "parse commit $MISSING" err || + return 1 + done +' + test_expect_success 'ahead-behind requires an argument' ' test_must_fail git for-each-ref \ --format="%(ahead-behind)" 2>err &&
-- 2.54.0.548.gbe7bb2469c