From: Karthik Nayak <karthik.188@gmail.com>
Date: Tue, 06 Jan 2026 16:30:16 GMT
Subject: Re: [PATCH v3 0/2] Some random object database related fixes
Message-ID: <CAOLa=ZT8_vij=2TU3GNZSST0N8Oj1CmaOd0ZzBcp32N8Aze0WQ@mail.gmail.com>
In-Reply-To: <20260106-odb-related-fixes-v3-0-7ac157207b20@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> this patch series fixes some small issues I've discovered while working
> on some other patch series. I've decided to split it out of these
> because I'm hitting the same issues in multiple series, and I don't want
> those to become dependent on one another.
>
> The patch series is built on top of f0ef5b6d9b with
> ps/object-source-management at ac65c70663 (odb: handle recreation of
> quarantine directories, 2025-11-19) merged into it.
>
> Changes in v3:
>   - Use `test_commit ()` so that we the same object at multiple tips.
>   - Slightly reword the commit message.
>   - Link to v2: https://lore.kernel.org/r/20251211-odb-related-fixes-v2-0-bdf875ce51fc@pks.im
>
> Changes in v2:
>   - Drop the first commit that regards geometric repacking with promisor
>     remotes. As it turns out my assertion was wrong: geometric repacks
>     do and have to consider promisors, but they will fail to handle
>     them. This is a bigger topic to fix though, so I'll rather want to
>     move this into a separate patch series.
>   - Tighten tests a bit for the commit-graph generation.
>   - Stop referring to a "subsequent" commit that doesn't exist.
>   - Link to v1: https://lore.kernel.org/r/20251205-odb-related-fixes-v1-0-ef4250abb584@pks.im
>
> Thanks!
>
> Patrick
>
> ---
> Patrick Steinhardt (2):
>       builtin/gc: fix condition for whether to write commit graphs
>       odb: properly close sources before freeing them
>
>  builtin/gc.c           |  8 +++++---
>  odb.c                  |  2 +-
>  t/t7900-maintenance.sh | 25 +++++++++++++++++++++++++
>  3 files changed, 31 insertions(+), 4 deletions(-)
>
> Range-diff versus v2:
>
> 1:  564b26fa6b ! 1:  3ef6ea3560 builtin/gc: fix condition for whether to write commit graphs
>     @@ Commit message
>          establish the proper border at which the auto-condition would be
>          fulfilled. As it turns out, there's another bug: if an object is at the
>          tip of any reference we don't mark it as seen. Consequently, if it is
>     -    reachable via any other reference, we'd count that object twice.
>     +    the tip of or reachable via another ref, we'd count that object multiple
>     +    times.
>
>          Fix both of these bugs so that we properly count objects without leaking
>          any memory.
>     @@ t/t7900-maintenance.sh: test_expect_success 'commit-graph auto condition' '
>      +	(
>      +		cd repo &&
>      +		git config set maintenance.auto false &&
>     -+		git commit --allow-empty -m initial &&
>     ++		test_commit initial &&
>      +		git switch --create feature &&
>     -+		git commit --allow-empty -m feature-1 &&
>     -+		git commit --allow-empty -m feature-2 &&
>     ++		test_commit feature-1 &&
>     ++		test_commit feature-2 &&
>      +		git switch - &&
>     -+		git commit --allow-empty -m main-1 &&
>     -+		git commit --allow-empty -m main-2 &&
>     ++		test_commit main-1 &&
>     ++		test_commit main-2 &&
>      +		git merge feature &&
>      +
>     -+		# We have 6 commit, none of which are covered by a commit
>     ++		# We have 6 commits, none of which are covered by a commit
>      +		# graph. So this must be the boundary at which we start to
>      +		# perform maintenance.
>      +		test_must_fail git -c maintenance.commit-graph.auto=7 \
> 2:  20bb4741eb = 2:  55cad3ea0f odb: properly close sources before freeing them
>
> ---
> base-commit: 2797238193944b52d12624a04a962f40b9bcad69
> change-id: 20251205-odb-related-fixes-5f48a0993ef7

The changes in this version looks good to me! :)