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>]

On Mon, Oct 26, 2026 at 02:04:30PM +0100, Patrick Steinhardt wrote:
> 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!

And here it is: <20261027-b4-pks-pack-objects-repack-links-with-missing-objects-v1-1-f50f90651274@pks.im>

Patrick