Re: [BUG] 2.56.0: filtered --unshallow of a moved ref aborts in pack-objects (should_include_obj) via inherited GIT_SHALLOW_FILE

[Patrick Steinhardt <ps@pks.im>]

Hi,

On Sat, Oct 24, 2026 at 10:59:55AM -0700, Daniel Zoghalchali wrote:
> Hi,
> 
> Since v2.56.0, a fetch that combines a tree filter with --unshallow
> (or --deepen) aborts in a non-partial shallow clone when the fetched
> ref has moved past the local tip:
> 
>   BUG: builtin/pack-objects.c:5098: should_include_obj should only be
> called on existing objects
>   error: pack-objects died of signal 6
>   fatal: could not finish pack-objects to repack local links
>   fatal: index-pack failed
> 
> v2.55.0 completes the same fetch. We hit it in CI, where a job checks
> out a single commit at depth 1 and then unshallows its branch, which
> by then has moved on.
> 
> Reproduction (offline, any Linux):
> 
>   set -e
>   W=$(mktemp -d); cd "$W"
>   git init -q -b dev seed
>   for i in 1 2 3 4; do
>     mkdir -p seed/src; echo "$i" > seed/src/f.txt
>     git -C seed add -A
>     git -C seed -c user.name=t -c user.email=t@t \
>       commit -q -m "c$i"
>   done
>   git init -q --bare o.git
>   git -C seed push -q "$W/o.git" dev
>   git -C o.git config uploadpack.allowFilter true
>   git -C o.git config uploadpack.allowAnySHA1InWant true
>   git init -q co
>   git -C co remote add origin "file://$W/o.git"
>   TIP1=$(git -C seed rev-parse dev~1)
>   git -C co fetch -q --no-tags --depth=1 origin \
>     "+$TIP1:refs/remotes/origin/dev"
>   git -C co checkout -q -B dev refs/remotes/origin/dev
>   git -C co fetch --no-tags --filter=tree:0 --unshallow origin \
>     +refs/heads/dev:refs/remotes/origin/dev

Thanks, this reproducer works for me.

> The last command dies as above with 2.56.0, both Ubuntu 24.04's
> git-core PPA build (1:2.56.0-0ppa1~ubuntu24.04.2) and v2.56.0 built
> from source. It succeeds with v2.55.0 built from source. With 2.56.0
> it also succeeds when dev has not moved past the checkout, with
> --filter=blob:none, or without a filter. --deepen=N with the same
> filter aborts the same way.
> 
> What happens:
> - fetch-pack runs "git --shallow-file '' index-pack --promisor ...".
> - The pack names the local tip as the parent of the new dev commit, so
>   index-pack's repack_local_links() starts "git pack-objects
>   --exclude-promisor-objects-best-effort".
> - Since d43b9acb05 (setup: unify setup of shallow file), --shallow-file
>   is applied by exporting GIT_SHALLOW_FILE, so that grandchild inherits
>   the empty shallow file.
> - The grandchild no longer treats the local tip as a shallow boundary.
>   It walks to the tip's parent, a promisor commit fetched without trees,
>   and is_not_in_promisor_pack_obj() hits the BUG on that parent's
>   missing root tree.
> - Before d43b9acb05, the grandchild read $GIT_DIR/shallow and stopped at
>   the tip.
> 
> To confirm this, we wrapped only the repack child. Unsetting
> GIT_SHALLOW_FILE for it makes 2.56.0 pass, and setting
> GIT_SHALLOW_FILE= for it makes 2.55.0 fail the same way, at its
> builtin/pack-objects.c:5004.

Makes sense. Even though I think that the bug isn't actually that we
started propagating GIT_SHALLOW_FILE, as we now basically behave like we
do it with all other global variables, too. And if we were to execute a
non-local command like git-receive-pack(1) we already know to unset that
value again via `local_repo_env`.

What's fishy is "builtin/pack-objects.c:is_not_in_promisor_pack_obj()",
which explicitly checks whether a given object is in a promisor or not.
But in the case where we assume that an object may be a promisor object
we know that we're running in a repostitory that may not even have all
of its objects in the first place -- that's the whole ponit of a partial
clone after all. But when `odb_read_object_info_ext()` returns an error
we simply `BUG()` right away, which does not make any sense to me.

> Separately, the BUG() is reachable without the environment change. On
> 2.55.0, a filtered --unshallow of the local tip, followed by a
> filtered fetch of a moved ref, also dies there, because the walk from
> the local tip reaches promisor commits whose trees were never fetched.

Yeah, this bug has the same root cause and I was able to reproduce it,
too. And indeed it confirms my above take as it already exists before
the change to how we propagate GIT_SHALLOW_FILE. So overall, I think the
fix is as simple as this:

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ca3a891dfbc..e634173fb1f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -5085,7 +5085,7 @@ static int is_not_in_promisor_pack_obj(struct object *obj, void *data UNUSED)
        };

        if (odb_read_object_info_extended(the_repository->objects, &obj->oid, &info, 0))
-               BUG("should_include_obj should only be called on existing objects");
+               return 0;
        return source_info.source->type != ODB_SOURCE_PACKED || !source_info.u.packed.pack->pack_promisor;
 }

I'll think a bit more about this and send a patch tomorrow. Thanks!

Patrick