All of lore.kernel.org
 help / color / mirror / Atom feed
* [BUG] 2.56.0: filtered --unshallow of a moved ref aborts in pack-objects (should_include_obj) via inherited GIT_SHALLOW_FILE
@ 2026-10-24 17:59 Daniel Zoghalchali
  2026-10-26 13:04 ` Patrick Steinhardt
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Zoghalchali @ 2026-10-24 17:59 UTC (permalink / raw)
  To: git; +Cc: ps

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

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.

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.

Thanks,
Daniel Zoghalchali

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [BUG] 2.56.0: filtered --unshallow of a moved ref aborts in pack-objects (should_include_obj) via inherited GIT_SHALLOW_FILE
  2026-10-24 17:59 [BUG] 2.56.0: filtered --unshallow of a moved ref aborts in pack-objects (should_include_obj) via inherited GIT_SHALLOW_FILE Daniel Zoghalchali
@ 2026-10-26 13:04 ` Patrick Steinhardt
  2026-10-27  7:03   ` Patrick Steinhardt
  0 siblings, 1 reply; 3+ messages in thread
From: Patrick Steinhardt @ 2026-10-26 13:04 UTC (permalink / raw)
  To: Daniel Zoghalchali; +Cc: git

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [BUG] 2.56.0: filtered --unshallow of a moved ref aborts in pack-objects (should_include_obj) via inherited GIT_SHALLOW_FILE
  2026-10-26 13:04 ` Patrick Steinhardt
@ 2026-10-27  7:03   ` Patrick Steinhardt
  0 siblings, 0 replies; 3+ messages in thread
From: Patrick Steinhardt @ 2026-10-27  7:03 UTC (permalink / raw)
  To: Daniel Zoghalchali; +Cc: git

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-27  7:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-24 17:59 [BUG] 2.56.0: filtered --unshallow of a moved ref aborts in pack-objects (should_include_obj) via inherited GIT_SHALLOW_FILE Daniel Zoghalchali
2026-10-26 13:04 ` Patrick Steinhardt
2026-10-27  7:03   ` Patrick Steinhardt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.