Skip to content

Commit 7a4bd1b

Browse files
pks-tgitster
authored andcommitted
packfile: always declare object info to be OI_PACKED
When reading object info via a packfile we yield one of two types: - The object can either be OI_PACKED, which is what a caller would typically expect. - Or it can be OI_DBCACHED if it is stored in the delta base cache. The latter really is an implementation detail though, and callers typically don't care at all about the difference. Furthermore, the information whether or not it is part of the delta base cache can already be derived via the `is_delta` field, so the fact that we discern between OI_PACKED and OI_DBCACHED only further complicates the interface. There aren't all that many callers that care about the `whence` field in the first place. In fact, there's only three: - `packfile_store_read_object_info()` checks for `whence == OI_PACKED` and then populates the packfile information of the object info structure. We now start to do this also for deltified objects, which gives its callers strictly more information. - `repack_local_links()` wants to determine whether the object is part of a promisor pack and checks for `whence == OI_PACKED`. If so, it verifies that the packfile is a promisor pack. It's arguably wrong to declare that an object is not part of a promisor pack only because it is stored in the delta base cache. - `is_not_in_promisor_pack_obj()` does the same, but checks that a specific object is _not_ part of a promisor pack. The same reasoning as above applies. Drop the OI_DBCACHED enum completely. None of the callers seem to care about the distinction. Note that this also fixes a segfault introduced in 8c1b84b (streaming: move logic to read packed objects streams into backend, 2025-11-23), which refactors how we stream packed objects. The intent is to only read packed objects in case they are stored non-deltified as we'd otherwise have to deflate them first. But the check for whether or not the object is stored as a delta was unconditionally done via `oi.u.packed.is_delta`, which is only valid in case `oi.whence` is `OI_PACKED`. But under some circumstances we got `OI_DBCACHED` here, which means that none of the `oi.u.packed` fields were initialized at all. Consequently, we assumed the object was not stored as a delta, and then try to read the object from `oi.u.packed.pack`, which is a `NULL` pointer and thus causes a segfault. Add a test case for this issue so that this cannot regress in the future anymore. Reported-by: Matt Smiley <msmiley@gitlab.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 123e818 commit 7a4bd1b

3 files changed

Lines changed: 35 additions & 3 deletions

File tree

‎odb.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,6 @@ struct object_info {
330330
OI_CACHED,
331331
OI_LOOSE,
332332
OI_PACKED,
333-
OI_DBCACHED
334333
} whence;
335334
union {
336335
/*

‎packfile.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1656,8 +1656,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,
16561656
oidclr(oi->delta_base_oid, p->repo->hash_algo);
16571657
}
16581658

1659-
oi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :
1660-
OI_PACKED;
1659+
oi->whence = OI_PACKED;
16611660

16621661
out:
16631662
unuse_pack(&w_curs);

‎t/t5003-archive-zip.sh‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,40 @@ check_zip with_untracked2
239239
check_added with_untracked2 untracked one/untracked
240240
check_added with_untracked2 untracked two/untracked
241241

242+
test_expect_success 'git-archive --format=zip with bigFile delta chains' '
243+
test_when_finished rm -rf repo &&
244+
git init repo &&
245+
(
246+
cd repo &&
247+
test-tool genrandom foo 100000 >base &&
248+
{
249+
cat base &&
250+
echo "trailing data"
251+
} >delta-1 &&
252+
{
253+
cat delta-1 &&
254+
echo "trailing data"
255+
} >delta-2 &&
256+
git add . &&
257+
git commit -m "blobs" &&
258+
git repack -Ad &&
259+
git verify-pack -v .git/objects/pack/pack-*.idx >stats &&
260+
test_grep "chain length = 1: 1 object" stats &&
261+
test_grep "chain length = 2: 1 object" stats &&
262+
263+
git -c core.bigFileThreshold=1k archive --format=zip HEAD >archive.zip &&
264+
if test_have_prereq UNZIP
265+
then
266+
mkdir unpack &&
267+
cd unpack &&
268+
"$GIT_UNZIP" ../archive.zip &&
269+
test_cmp base ../base &&
270+
test_cmp delta-1 ../delta-1 &&
271+
test_cmp delta-2 ../delta-2
272+
fi
273+
)
274+
'
275+
242276
# Test remote archive over HTTP protocol.
243277
#
244278
# Note: this should be the last part of this test suite, because

0 commit comments

Comments
 (0)