* [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-05 19:11 ` Torsten Bögershausen
2026-05-04 17:08 ` [PATCH v2 02/11] git-zlib: handle data streams larger than 4GB Johannes Schindelin via GitGitGadget
` (10 subsequent siblings)
11 siblings, 1 reply; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When unpacking objects from a packfile, the object size is decoded
from a variable-length encoding. On platforms where unsigned long is
32-bit (such as Windows, even in 64-bit builds), the shift operation
overflows when decoding sizes larger than 4GB. The result is a
truncated size value, causing the unpacked object to be corrupted or
rejected.
Fix this by changing the size variable to size_t, which is 64-bit on
64-bit platforms, and ensuring the shift arithmetic occurs in 64-bit
space.
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/index-pack.c | 9 +++++----
builtin/unpack-objects.c | 5 +++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index ca7784dc2c..cc660582e9 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -37,7 +37,7 @@ static const char index_pack_usage[] =
struct object_entry {
struct pack_idx_entry idx;
- unsigned long size;
+ size_t size;
unsigned char hdr_size;
signed char type;
signed char real_type;
@@ -469,7 +469,7 @@ static int is_delta_type(enum object_type type)
return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
}
-static void *unpack_entry_data(off_t offset, unsigned long size,
+static void *unpack_entry_data(off_t offset, size_t size,
enum object_type type, struct object_id *oid)
{
static char fixed_buf[8192];
@@ -524,7 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
struct object_id *oid)
{
unsigned char *p;
- unsigned long size, c;
+ size_t size;
+ unsigned long c;
off_t base_offset;
unsigned shift;
void *data;
@@ -542,7 +543,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
p = fill(1);
c = *p;
use(1);
- size += (c & 0x7f) << shift;
+ size += ((size_t)c & 0x7f) << shift;
shift += 7;
}
obj->size = size;
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index e01cf6e360..59a36c2481 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -533,7 +533,8 @@ static void unpack_one(unsigned nr)
{
unsigned shift;
unsigned char *pack;
- unsigned long size, c;
+ size_t size;
+ unsigned long c;
enum object_type type;
obj_list[nr].offset = consumed_bytes;
@@ -548,7 +549,7 @@ static void unpack_one(unsigned nr)
pack = fill(1);
c = *pack;
use(1);
- size += (c & 0x7f) << shift;
+ size += ((size_t)c & 0x7f) << shift;
shift += 7;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
2026-05-04 17:08 ` [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size Johannes Schindelin via GitGitGadget
@ 2026-05-05 19:11 ` Torsten Bögershausen
2026-05-08 7:36 ` Johannes Schindelin
0 siblings, 1 reply; 60+ messages in thread
From: Torsten Bögershausen @ 2026-05-05 19:11 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Derrick Stolee, Jeff King, Johannes Schindelin
On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When unpacking objects from a packfile, the object size is decoded
> from a variable-length encoding. On platforms where unsigned long is
> 32-bit (such as Windows, even in 64-bit builds), the shift operation
> overflows when decoding sizes larger than 4GB. The result is a
> truncated size value, causing the unpacked object to be corrupted or
> rejected.
>
> Fix this by changing the size variable to size_t, which is 64-bit on
> 64-bit platforms, and ensuring the shift arithmetic occurs in 64-bit
> space.
>
> This was originally authored by LordKiRon <https://github.com/LordKiRon>,
> who preferred not to reveal their real name and therefore agreed that I
> take over authorship.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> builtin/index-pack.c | 9 +++++----
> builtin/unpack-objects.c | 5 +++--
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index ca7784dc2c..cc660582e9 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -37,7 +37,7 @@ static const char index_pack_usage[] =
>
> struct object_entry {
> struct pack_idx_entry idx;
> - unsigned long size;
> + size_t size;
> unsigned char hdr_size;
> signed char type;
> signed char real_type;
> @@ -469,7 +469,7 @@ static int is_delta_type(enum object_type type)
> return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
> }
>
> -static void *unpack_entry_data(off_t offset, unsigned long size,
> +static void *unpack_entry_data(off_t offset, size_t size,
> enum object_type type, struct object_id *oid)
> {
> static char fixed_buf[8192];
> @@ -524,7 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
> struct object_id *oid)
> {
> unsigned char *p;
> - unsigned long size, c;
> + size_t size;
> + unsigned long c;
Does this look a little bit strange ?
p points to an unsigned char (better would be *uint8_t)
then it is dereferenced into an "unsigned long".
Then it is masked with 0x7f
In short: should "c" be declared as uint8_t ?
> off_t base_offset;
> unsigned shift;
> void *data;
> @@ -542,7 +543,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
> p = fill(1);
> c = *p;
> use(1);
> - size += (c & 0x7f) << shift;
> + size += ((size_t)c & 0x7f) << shift;
> shift += 7;
> }
> obj->size = size;
> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> index e01cf6e360..59a36c2481 100644
> --- a/builtin/unpack-objects.c
> +++ b/builtin/unpack-objects.c
> @@ -533,7 +533,8 @@ static void unpack_one(unsigned nr)
> {
> unsigned shift;
> unsigned char *pack;
> - unsigned long size, c;
> + size_t size;
> + unsigned long c;
> enum object_type type;
>
> obj_list[nr].offset = consumed_bytes;
> @@ -548,7 +549,7 @@ static void unpack_one(unsigned nr)
> pack = fill(1);
> c = *pack;
> use(1);
> - size += (c & 0x7f) << shift;
> + size += ((size_t)c & 0x7f) << shift;
> shift += 7;
> }
>
> --
> gitgitgadget
>
>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
2026-05-05 19:11 ` Torsten Bögershausen
@ 2026-05-08 7:36 ` Johannes Schindelin
2026-05-08 19:09 ` Torsten Bögershausen
0 siblings, 1 reply; 60+ messages in thread
From: Johannes Schindelin @ 2026-05-08 7:36 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Johannes Schindelin via GitGitGadget, git, Derrick Stolee,
Jeff King
[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]
Hi Torsten,
On Tue, 5 May 2026, Torsten Bögershausen wrote:
> On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > [...]
> > @@ -524,7 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
> > struct object_id *oid)
> > {
> > unsigned char *p;
> > - unsigned long size, c;
> > + size_t size;
> > + unsigned long c;
>
> Does this look a little bit strange ?
Good point.
> p points to an unsigned char (better would be *uint8_t)
> then it is dereferenced into an "unsigned long".
> Then it is masked with 0x7f
> In short: should "c" be declared as uint8_t ?
Almost. It should be a `size_t`, so that we don't have to cast it when
shifting it. I'll include a fix in the next iteration.
Thank you!
Johannes
>
> > off_t base_offset;
> > unsigned shift;
> > void *data;
> > @@ -542,7 +543,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
> > p = fill(1);
> > c = *p;
> > use(1);
> > - size += (c & 0x7f) << shift;
> > + size += ((size_t)c & 0x7f) << shift;
> > shift += 7;
> > }
> > obj->size = size;
> > diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> > index e01cf6e360..59a36c2481 100644
> > --- a/builtin/unpack-objects.c
> > +++ b/builtin/unpack-objects.c
> > @@ -533,7 +533,8 @@ static void unpack_one(unsigned nr)
> > {
> > unsigned shift;
> > unsigned char *pack;
> > - unsigned long size, c;
> > + size_t size;
> > + unsigned long c;
> > enum object_type type;
> >
> > obj_list[nr].offset = consumed_bytes;
> > @@ -548,7 +549,7 @@ static void unpack_one(unsigned nr)
> > pack = fill(1);
> > c = *pack;
> > use(1);
> > - size += (c & 0x7f) << shift;
> > + size += ((size_t)c & 0x7f) << shift;
> > shift += 7;
> > }
> >
> > --
> > gitgitgadget
> >
> >
>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
2026-05-08 7:36 ` Johannes Schindelin
@ 2026-05-08 19:09 ` Torsten Bögershausen
2026-05-10 2:41 ` Junio C Hamano
0 siblings, 1 reply; 60+ messages in thread
From: Torsten Bögershausen @ 2026-05-08 19:09 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Johannes Schindelin via GitGitGadget, git, Derrick Stolee,
Jeff King
On Fri, May 08, 2026 at 09:36:53AM +0200, Johannes Schindelin wrote:
> Hi Torsten,
>
> On Tue, 5 May 2026, Torsten Bögershausen wrote:
>
> > On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > >
> > > [...]
> > > @@ -524,7 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
> > > struct object_id *oid)
> > > {
> > > unsigned char *p;
> > > - unsigned long size, c;
> > > + size_t size;
> > > + unsigned long c;
> >
> > Does this look a little bit strange ?
>
> Good point.
>
> > p points to an unsigned char (better would be *uint8_t)
> > then it is dereferenced into an "unsigned long".
> > Then it is masked with 0x7f
> > In short: should "c" be declared as uint8_t ?
>
> Almost. It should be a `size_t`, so that we don't have to cast it when
> shifting it. I'll include a fix in the next iteration.
I think I was not very clear here.
De-referencing a long (or size_t) from any address is something
I would try to avoid:
Some processors do not like to read a 32 or 64 bit value from
an uneven address (and throw an exeption).
x86 processors just handle it, using more than one bus cycle.
In short: Please keep the up-cast and simply read an uint8_t.
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
2026-05-08 19:09 ` Torsten Bögershausen
@ 2026-05-10 2:41 ` Junio C Hamano
2026-05-10 9:14 ` Torsten Bögershausen
0 siblings, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2026-05-10 2:41 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Johannes Schindelin, Johannes Schindelin via GitGitGadget, git,
Derrick Stolee, Jeff King
Torsten Bögershausen <tboegi@web.de> writes:
> On Fri, May 08, 2026 at 09:36:53AM +0200, Johannes Schindelin wrote:
>> Hi Torsten,
>>
>> On Tue, 5 May 2026, Torsten Bögershausen wrote:
>>
>> > On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote:
>> > > From: Johannes Schindelin <johannes.schindelin@gmx.de>
>> > >
>> > > [...]
>> > > @@ -524,7 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
>> > > struct object_id *oid)
>> > > {
>> > > unsigned char *p;
>> > > - unsigned long size, c;
>> > > + size_t size;
>> > > + unsigned long c;
>> >
>> > Does this look a little bit strange ?
>>
>> Good point.
>>
>> > p points to an unsigned char (better would be *uint8_t)
>> > then it is dereferenced into an "unsigned long".
>> > Then it is masked with 0x7f
>> > In short: should "c" be declared as uint8_t ?
>>
>> Almost. It should be a `size_t`, so that we don't have to cast it when
>> shifting it. I'll include a fix in the next iteration.
>
> I think I was not very clear here.
> De-referencing a long (or size_t) from any address is something
> I would try to avoid:
> Some processors do not like to read a 32 or 64 bit value from
> an uneven address (and throw an exeption).
> x86 processors just handle it, using more than one bus cycle.
>
> In short: Please keep the up-cast and simply read an uint8_t.
Hmph, I do not think there is "up-cast" to keep. And we do not
dereference a random pointer that would be suitable for unsigned
char * as if it were "unsigned long *" or "size_t *" in this code.
This came from commit 48fb7deb5bbd87933e7d314b73d7c1b52667f80f
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed Jun 17 17:22:27 2009 -0700
Fix big left-shifts of unsigned char
Shifting 'unsigned char' or 'unsigned short' left can result in sign
extension errors, since the C integer promotion rules means that the
unsigned char/short will get implicitly promoted to a signed 'int' due to
the shift (or due to other operations).
This normally doesn't matter, but if you shift things up sufficiently, it
will now set the sign bit in 'int', and a subsequent cast to a bigger type
(eg 'long' or 'unsigned long') will now sign-extend the value despite the
original expression being unsigned.
One example of this would be something like
unsigned long size;
unsigned char c;
size += c << 24;
where despite all the variables being unsigned, 'c << 24' ends up being a
signed entity, and will get sign-extended when then doing the addition in
an 'unsigned long' type.
You could rewrite Linus's example to
unsigned char *cp;
unsigned long size;
unsigned char c;
c = *cp;
size += ((unsigned long)c) << 24;
While I am sympathetic to that position, I also would not mind
unsigned char *cp;
unsigned long size;
unsigned long c;
c = *cp;
size += c << 24;
all that much. In any case, such a "clean-up" has little to do with
the topic under discussion, and itshould be discussed separately on
its own merit, most likely when the dust settles after this topic
lands. Let's not contaminate the patches that is "a trivial rewrite
that is so obviously correct to fix the assumption that ulong and
size_t are of the same size everywhere" with unrelated clean-up.
Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
2026-05-10 2:41 ` Junio C Hamano
@ 2026-05-10 9:14 ` Torsten Bögershausen
0 siblings, 0 replies; 60+ messages in thread
From: Torsten Bögershausen @ 2026-05-10 9:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Johannes Schindelin via GitGitGadget, git,
Derrick Stolee, Jeff King
>
> Hmph, I do not think there is "up-cast" to keep. And we do not
> dereference a random pointer that would be suitable for unsigned
> char * as if it were "unsigned long *" or "size_t *" in this code.
>
> This came from commit 48fb7deb5bbd87933e7d314b73d7c1b52667f80f
>
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Wed Jun 17 17:22:27 2009 -0700
>
> Fix big left-shifts of unsigned char
>
> Shifting 'unsigned char' or 'unsigned short' left can result in sign
> extension errors, since the C integer promotion rules means that the
> unsigned char/short will get implicitly promoted to a signed 'int' due to
> the shift (or due to other operations).
>
> This normally doesn't matter, but if you shift things up sufficiently, it
> will now set the sign bit in 'int', and a subsequent cast to a bigger type
> (eg 'long' or 'unsigned long') will now sign-extend the value despite the
> original expression being unsigned.
>
> One example of this would be something like
>
> unsigned long size;
> unsigned char c;
>
> size += c << 24;
>
> where despite all the variables being unsigned, 'c << 24' ends up being a
> signed entity, and will get sign-extended when then doing the addition in
> an 'unsigned long' type.
>
>
> You could rewrite Linus's example to
>
> unsigned char *cp;
> unsigned long size;
> unsigned char c;
>
> c = *cp;
> size += ((unsigned long)c) << 24;
>
> While I am sympathetic to that position, I also would not mind
>
> unsigned char *cp;
> unsigned long size;
> unsigned long c;
>
> c = *cp;
> size += c << 24;
>
> all that much. In any case, such a "clean-up" has little to do with
> the topic under discussion, and itshould be discussed separately on
> its own merit, most likely when the dust settles after this topic
> lands. Let's not contaminate the patches that is "a trivial rewrite
> that is so obviously correct to fix the assumption that ulong and
> size_t are of the same size everywhere" with unrelated clean-up.
>
> Thanks.
>
Sorry for the confusion and noise.
My brain insisted to read
c = *cp; // fetch 8 bits from memory, upcast to unsigned long
as if we have written
c = *(long*)cp; // fetch 32/64 bits from memory
which is a completely different thing.
In short: all is good.
Thanks for digging and the patience.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 02/11] git-zlib: handle data streams larger than 4GB
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 03/11] odb, packfile: use size_t for streaming object sizes Johannes Schindelin via GitGitGadget
` (9 subsequent siblings)
11 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
On Windows, zlib's `uLong` type is 32-bit even on 64-bit systems. When
processing data streams larger than 4GB, the `total_in` and `total_out`
fields in zlib's `z_stream` structure wrap around, which caused the
sanity checks in `zlib_post_call()` to trigger `BUG()` assertions.
The git_zstream wrapper now tracks its own 64-bit totals rather than
copying them from zlib. The sanity checks compare only the low bits,
using `maximum_unsigned_value_of_type(uLong)` to mask appropriately for
the platform's `uLong` size.
This is based on work by LordKiRon in git-for-windows#6076.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
git-zlib.c | 25 +++++++++++++++++--------
git-zlib.h | 4 ++--
object-file.c | 2 +-
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/git-zlib.c b/git-zlib.c
index df9604910e..b91cb323ae 100644
--- a/git-zlib.c
+++ b/git-zlib.c
@@ -30,6 +30,9 @@ static const char *zerr_to_string(int status)
*/
/* #define ZLIB_BUF_MAX ((uInt)-1) */
#define ZLIB_BUF_MAX ((uInt) 1024 * 1024 * 1024) /* 1GB */
+
+/* uLong is 32-bit on Windows, even on 64-bit systems */
+#define ULONG_MAX_VALUE maximum_unsigned_value_of_type(uLong)
static inline uInt zlib_buf_cap(unsigned long len)
{
return (ZLIB_BUF_MAX < len) ? ZLIB_BUF_MAX : len;
@@ -39,31 +42,37 @@ static void zlib_pre_call(git_zstream *s)
{
s->z.next_in = s->next_in;
s->z.next_out = s->next_out;
- s->z.total_in = s->total_in;
- s->z.total_out = s->total_out;
+ s->z.total_in = (uLong)(s->total_in & ULONG_MAX_VALUE);
+ s->z.total_out = (uLong)(s->total_out & ULONG_MAX_VALUE);
s->z.avail_in = zlib_buf_cap(s->avail_in);
s->z.avail_out = zlib_buf_cap(s->avail_out);
}
static void zlib_post_call(git_zstream *s, int status)
{
- unsigned long bytes_consumed;
- unsigned long bytes_produced;
+ size_t bytes_consumed;
+ size_t bytes_produced;
bytes_consumed = s->z.next_in - s->next_in;
bytes_produced = s->z.next_out - s->next_out;
- if (s->z.total_out != s->total_out + bytes_produced)
+ /*
+ * zlib's total_out/total_in are uLong which may wrap for >4GB.
+ * We track our own totals and verify only the low bits match.
+ */
+ if ((s->z.total_out & ULONG_MAX_VALUE) !=
+ ((s->total_out + bytes_produced) & ULONG_MAX_VALUE))
BUG("total_out mismatch");
/*
* zlib does not update total_in when it returns Z_NEED_DICT,
* causing a mismatch here. Skip the sanity check in that case.
*/
if (status != Z_NEED_DICT &&
- s->z.total_in != s->total_in + bytes_consumed)
+ (s->z.total_in & ULONG_MAX_VALUE) !=
+ ((s->total_in + bytes_consumed) & ULONG_MAX_VALUE))
BUG("total_in mismatch");
- s->total_out = s->z.total_out;
- s->total_in = s->z.total_in;
+ s->total_out += bytes_produced;
+ s->total_in += bytes_consumed;
/* zlib-ng marks `next_in` as `const`, so we have to cast it away. */
s->next_in = (unsigned char *) s->z.next_in;
s->next_out = s->z.next_out;
diff --git a/git-zlib.h b/git-zlib.h
index 0e66fefa8c..44380e8ad3 100644
--- a/git-zlib.h
+++ b/git-zlib.h
@@ -7,8 +7,8 @@ typedef struct git_zstream {
struct z_stream_s z;
unsigned long avail_in;
unsigned long avail_out;
- unsigned long total_in;
- unsigned long total_out;
+ size_t total_in;
+ size_t total_out;
unsigned char *next_in;
unsigned char *next_out;
} git_zstream;
diff --git a/object-file.c b/object-file.c
index 2acc9522df..086b2b65ff 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1118,7 +1118,7 @@ int odb_source_loose_write_stream(struct odb_source *source,
} while (ret == Z_OK || ret == Z_BUF_ERROR);
if (stream.total_in != len + hdrlen)
- die(_("write stream object %ld != %"PRIuMAX), stream.total_in,
+ die(_("write stream object %"PRIuMAX" != %"PRIuMAX), (uintmax_t)stream.total_in,
(uintmax_t)len + hdrlen);
/*
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 03/11] odb, packfile: use size_t for streaming object sizes
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 02/11] git-zlib: handle data streams larger than 4GB Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-05 19:27 ` Torsten Bögershausen
2026-05-04 17:08 ` [PATCH v2 04/11] delta, packfile: use size_t for delta header sizes Johannes Schindelin via GitGitGadget
` (8 subsequent siblings)
11 siblings, 1 reply; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The odb_read_stream structure uses unsigned long for the size field,
which is 32-bit on Windows even in 64-bit builds. When streaming
objects larger than 4GB, the size would be truncated to zero or an
incorrect value, resulting in empty files being written to disk.
Change the size field in odb_read_stream to size_t and introduce
unpack_object_header_sz() to return sizes via size_t pointer. Since
object_info.sizep remains unsigned long for API compatibility, use
temporary variables where the types differ, with comments noting the
truncation limitation for code paths that still use unsigned long.
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/pack-objects.c | 23 ++++++++++++++++-------
object-file.c | 10 +++++++++-
odb/streaming.c | 13 ++++++++++++-
odb/streaming.h | 2 +-
| 2 +-
pack-bitmap.c | 2 +-
pack-check.c | 6 ++++--
packfile.c | 24 +++++++++++++++---------
packfile.h | 4 ++--
9 files changed, 61 insertions(+), 25 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index dd2480a73d..aa4b1cb9b8 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -629,14 +629,21 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,
struct packed_git *p = IN_PACK(entry);
struct pack_window *w_curs = NULL;
uint32_t pos;
- off_t offset;
+ off_t offset, cur;
enum object_type type = oe_type(entry);
+ enum object_type in_pack_type;
off_t datalen;
unsigned char header[MAX_PACK_OBJECT_HEADER],
dheader[MAX_PACK_OBJECT_HEADER];
unsigned hdrlen;
const unsigned hashsz = the_hash_algo->rawsz;
- unsigned long entry_size = SIZE(entry);
+ size_t entry_size;
+
+ cur = entry->in_pack_offset;
+ in_pack_type = unpack_object_header(p, &w_curs, &cur, &entry_size);
+ if (in_pack_type < 0)
+ die(_("write_reuse_object: unable to parse object header of %s"),
+ oid_to_hex(&entry->idx.oid));
if (DELTA(entry))
type = (allow_ofs_delta && DELTA(entry)->idx.offset) ?
@@ -1087,7 +1094,7 @@ static void write_reused_pack_one(struct packed_git *reuse_packfile,
{
off_t offset, next, cur;
enum object_type type;
- unsigned long size;
+ size_t size;
offset = pack_pos_to_offset(reuse_packfile, pos);
next = pack_pos_to_offset(reuse_packfile, pos + 1);
@@ -2243,7 +2250,7 @@ static void check_object(struct object_entry *entry, uint32_t object_index)
off_t ofs;
unsigned char *buf, c;
enum object_type type;
- unsigned long in_pack_size;
+ size_t in_pack_size;
buf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);
@@ -2734,16 +2741,18 @@ unsigned long oe_get_size_slow(struct packing_data *pack,
struct pack_window *w_curs;
unsigned char *buf;
enum object_type type;
- unsigned long used, avail, size;
+ unsigned long used, avail;
+ size_t size;
if (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {
+ unsigned long sz;
packing_data_lock(&to_pack);
if (odb_read_object_info(the_repository->objects,
- &e->idx.oid, &size) < 0)
+ &e->idx.oid, &sz) < 0)
die(_("unable to get size of %s"),
oid_to_hex(&e->idx.oid));
packing_data_unlock(&to_pack);
- return size;
+ return sz;
}
p = oe_in_pack(pack, e);
diff --git a/object-file.c b/object-file.c
index 086b2b65ff..0be2981c7a 100644
--- a/object-file.c
+++ b/object-file.c
@@ -2326,6 +2326,7 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
struct object_info oi = OBJECT_INFO_INIT;
struct odb_loose_read_stream *st;
unsigned long mapsize;
+ unsigned long size_ul;
void *mapped;
mapped = odb_source_loose_map_object(source, oid, &mapsize);
@@ -2349,11 +2350,18 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
goto error;
}
- oi.sizep = &st->base.size;
+ /*
+ * object_info.sizep is unsigned long* (32-bit on Windows), but
+ * st->base.size is size_t (64-bit). Use temporary variable.
+ * Note: loose objects >4GB would still truncate here, but such
+ * large loose objects are uncommon (they'd normally be packed).
+ */
+ oi.sizep = &size_ul;
oi.typep = &st->base.type;
if (parse_loose_header(st->hdr, &oi) < 0 || st->base.type < 0)
goto error;
+ st->base.size = size_ul;
st->mapped = mapped;
st->mapsize = mapsize;
diff --git a/odb/streaming.c b/odb/streaming.c
index 5927a12954..af2adf5ce7 100644
--- a/odb/streaming.c
+++ b/odb/streaming.c
@@ -157,15 +157,26 @@ static int open_istream_incore(struct odb_read_stream **out,
.base.read = read_istream_incore,
};
struct odb_incore_read_stream *st;
+ unsigned long size_ul;
int ret;
oi.typep = &stream.base.type;
- oi.sizep = &stream.base.size;
+ /*
+ * object_info.sizep is unsigned long* (32-bit on Windows), but
+ * stream.base.size is size_t (64-bit). We use a temporary variable
+ * because the types are incompatible. Note: this path still truncates
+ * for >4GB objects, but large objects should use pack streaming
+ * (packfile_store_read_object_stream) which handles size_t properly.
+ * This incore fallback is only used for small objects or when pack
+ * streaming is unavailable.
+ */
+ oi.sizep = &size_ul;
oi.contentp = (void **)&stream.buf;
ret = odb_read_object_info_extended(odb, oid, &oi,
OBJECT_INFO_DIE_IF_CORRUPT);
if (ret)
return ret;
+ stream.base.size = size_ul;
CALLOC_ARRAY(st, 1);
*st = stream;
diff --git a/odb/streaming.h b/odb/streaming.h
index c7861f7e13..517e2ea2d3 100644
--- a/odb/streaming.h
+++ b/odb/streaming.h
@@ -21,7 +21,7 @@ struct odb_read_stream {
odb_read_stream_close_fn close;
odb_read_stream_read_fn read;
enum object_type type;
- unsigned long size; /* inflated size of full object */
+ size_t size; /* inflated size of full object */
};
/*
--git a/oss-fuzz/fuzz-pack-headers.c b/oss-fuzz/fuzz-pack-headers.c
index 150c0f5fa2..ef61ab577c 100644
--- a/oss-fuzz/fuzz-pack-headers.c
+++ b/oss-fuzz/fuzz-pack-headers.c
@@ -6,7 +6,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
{
enum object_type type;
- unsigned long len;
+ size_t len;
unpack_object_header_buffer((const unsigned char *)data,
(unsigned long)size, &type, &len);
diff --git a/pack-bitmap.c b/pack-bitmap.c
index f6ec18d83a..f9af8a96bd 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2270,7 +2270,7 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
{
off_t delta_obj_offset;
enum object_type type;
- unsigned long size;
+ size_t size;
if (pack_pos >= pack->p->num_objects)
return -1; /* not actually in the pack */
diff --git a/pack-check.c b/pack-check.c
index 79992bb509..2792f34d25 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -110,7 +110,7 @@ static int verify_packfile(struct repository *r,
void *data;
struct object_id oid;
enum object_type type;
- unsigned long size;
+ size_t size;
off_t curpos;
int data_valid;
@@ -143,7 +143,9 @@ static int verify_packfile(struct repository *r,
data = NULL;
data_valid = 0;
} else {
- data = unpack_entry(r, p, entries[i].offset, &type, &size);
+ unsigned long sz;
+ data = unpack_entry(r, p, entries[i].offset, &type, &sz);
+ size = sz;
data_valid = 1;
}
diff --git a/packfile.c b/packfile.c
index b012d648ad..fdae91dd11 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1133,7 +1133,7 @@ out:
}
unsigned long unpack_object_header_buffer(const unsigned char *buf,
- unsigned long len, enum object_type *type, unsigned long *sizep)
+ unsigned long len, enum object_type *type, size_t *sizep)
{
unsigned shift;
size_t size, c;
@@ -1144,7 +1144,11 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
size = c & 15;
shift = 4;
while (c & 0x80) {
- if (len <= used || (bitsizeof(long) - 7) < shift) {
+ /*
+ * Each continuation byte adds 7 bits. Ensure shift won't
+ * overflow size_t (use size_t not long for 64-bit on Windows).
+ */
+ if (len <= used || (bitsizeof(size_t) - 7) < shift) {
error("bad object header");
size = used = 0;
break;
@@ -1153,7 +1157,7 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
size = st_add(size, st_left_shift(c & 0x7f, shift));
shift += 7;
}
- *sizep = cast_size_t_to_ulong(size);
+ *sizep = size;
return used;
}
@@ -1215,7 +1219,7 @@ unsigned long get_size_from_delta(struct packed_git *p,
int unpack_object_header(struct packed_git *p,
struct pack_window **w_curs,
off_t *curpos,
- unsigned long *sizep)
+ size_t *sizep)
{
unsigned char *base;
unsigned long left;
@@ -1367,7 +1371,7 @@ static enum object_type packed_to_object_type(struct repository *r,
while (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
off_t base_offset;
- unsigned long size;
+ size_t size;
/* Push the object we're going to leave behind */
if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) {
poi_stack_alloc = alloc_nr(poi_stack_nr);
@@ -1586,7 +1590,7 @@ static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_off
uint32_t *maybe_index_pos, struct object_info *oi)
{
struct pack_window *w_curs = NULL;
- unsigned long size;
+ size_t size;
off_t curpos = obj_offset;
enum object_type type = OBJ_NONE;
uint32_t pack_pos;
@@ -1778,7 +1782,7 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
struct pack_window *w_curs = NULL;
off_t curpos = obj_offset;
void *data = NULL;
- unsigned long size;
+ size_t size;
enum object_type type;
struct unpack_entry_stack_ent small_delta_stack[UNPACK_ENTRY_STACK_PREALLOC];
struct unpack_entry_stack_ent *delta_stack = small_delta_stack;
@@ -1943,8 +1947,10 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
(uintmax_t)curpos, p->pack_name);
data = NULL;
} else {
+ unsigned long sz;
data = patch_delta(base, base_size, delta_data,
- delta_size, &size);
+ delta_size, &sz);
+ size = sz;
/*
* We could not apply the delta; warn the user, but
@@ -2929,7 +2935,7 @@ int packfile_read_object_stream(struct odb_read_stream **out,
struct odb_packed_read_stream *stream;
struct pack_window *window = NULL;
enum object_type in_pack_type;
- unsigned long size;
+ size_t size;
in_pack_type = unpack_object_header(pack, &window, &offset, &size);
unuse_pack(&window);
diff --git a/packfile.h b/packfile.h
index 9b647da7dd..49d6bdecf6 100644
--- a/packfile.h
+++ b/packfile.h
@@ -456,9 +456,9 @@ off_t find_pack_entry_one(const struct object_id *oid, struct packed_git *);
int is_pack_valid(struct packed_git *);
void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);
-unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
+unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, size_t *sizep);
unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
-int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
+int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, size_t *);
off_t get_delta_base(struct packed_git *p, struct pack_window **w_curs,
off_t *curpos, enum object_type type,
off_t delta_obj_offset);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 03/11] odb, packfile: use size_t for streaming object sizes
2026-05-04 17:08 ` [PATCH v2 03/11] odb, packfile: use size_t for streaming object sizes Johannes Schindelin via GitGitGadget
@ 2026-05-05 19:27 ` Torsten Bögershausen
2026-05-08 7:38 ` Johannes Schindelin
0 siblings, 1 reply; 60+ messages in thread
From: Torsten Bögershausen @ 2026-05-05 19:27 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Derrick Stolee, Jeff King, Johannes Schindelin
On Mon, May 04, 2026 at 05:08:20PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The odb_read_stream structure uses unsigned long for the size field,
> which is 32-bit on Windows even in 64-bit builds. When streaming
> objects larger than 4GB, the size would be truncated to zero or an
> incorrect value, resulting in empty files being written to disk.
>
> Change the size field in odb_read_stream to size_t and introduce
> unpack_object_header_sz() to return sizes via size_t pointer. Since
> object_info.sizep remains unsigned long for API compatibility, use
> temporary variables where the types differ, with comments noting the
> truncation limitation for code paths that still use unsigned long.
>
> This was originally authored by LordKiRon <https://github.com/LordKiRon>,
> who preferred not to reveal their real name and therefore agreed that I
> take over authorship.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> builtin/pack-objects.c | 23 ++++++++++++++++-------
> object-file.c | 10 +++++++++-
> odb/streaming.c | 13 ++++++++++++-
> odb/streaming.h | 2 +-
> oss-fuzz/fuzz-pack-headers.c | 2 +-
> pack-bitmap.c | 2 +-
> pack-check.c | 6 ++++--
> packfile.c | 24 +++++++++++++++---------
> packfile.h | 4 ++--
> 9 files changed, 61 insertions(+), 25 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index dd2480a73d..aa4b1cb9b8 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
I haven't been able to follow all changes, so this may be false alarm.
Do we need a cast_size_t_to_ulong() somewhere ?
> @@ -629,14 +629,21 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,
> struct packed_git *p = IN_PACK(entry);
> struct pack_window *w_curs = NULL;
> uint32_t pos;
> - off_t offset;
> + off_t offset, cur;
> enum object_type type = oe_type(entry);
> + enum object_type in_pack_type;
> off_t datalen;
> unsigned char header[MAX_PACK_OBJECT_HEADER],
> dheader[MAX_PACK_OBJECT_HEADER];
> unsigned hdrlen;
> const unsigned hashsz = the_hash_algo->rawsz;
> - unsigned long entry_size = SIZE(entry);
> + size_t entry_size;
> +
> + cur = entry->in_pack_offset;
> + in_pack_type = unpack_object_header(p, &w_curs, &cur, &entry_size);
> + if (in_pack_type < 0)
> + die(_("write_reuse_object: unable to parse object header of %s"),
> + oid_to_hex(&entry->idx.oid));
>
> if (DELTA(entry))
> type = (allow_ofs_delta && DELTA(entry)->idx.offset) ?
> @@ -1087,7 +1094,7 @@ static void write_reused_pack_one(struct packed_git *reuse_packfile,
> {
> off_t offset, next, cur;
> enum object_type type;
> - unsigned long size;
> + size_t size;
>
> offset = pack_pos_to_offset(reuse_packfile, pos);
> next = pack_pos_to_offset(reuse_packfile, pos + 1);
> @@ -2243,7 +2250,7 @@ static void check_object(struct object_entry *entry, uint32_t object_index)
> off_t ofs;
> unsigned char *buf, c;
> enum object_type type;
> - unsigned long in_pack_size;
> + size_t in_pack_size;
>
> buf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);
>
> @@ -2734,16 +2741,18 @@ unsigned long oe_get_size_slow(struct packing_data *pack,
> struct pack_window *w_curs;
> unsigned char *buf;
> enum object_type type;
> - unsigned long used, avail, size;
> + unsigned long used, avail;
> + size_t size;
>
> if (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {
> + unsigned long sz;
> packing_data_lock(&to_pack);
> if (odb_read_object_info(the_repository->objects,
> - &e->idx.oid, &size) < 0)
> + &e->idx.oid, &sz) < 0)
> die(_("unable to get size of %s"),
> oid_to_hex(&e->idx.oid));
> packing_data_unlock(&to_pack);
> - return size;
> + return sz;
> }
>
> p = oe_in_pack(pack, e);
> diff --git a/object-file.c b/object-file.c
> index 086b2b65ff..0be2981c7a 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -2326,6 +2326,7 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
> struct object_info oi = OBJECT_INFO_INIT;
> struct odb_loose_read_stream *st;
> unsigned long mapsize;
> + unsigned long size_ul;
> void *mapped;
>
> mapped = odb_source_loose_map_object(source, oid, &mapsize);
> @@ -2349,11 +2350,18 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
> goto error;
> }
>
> - oi.sizep = &st->base.size;
> + /*
> + * object_info.sizep is unsigned long* (32-bit on Windows), but
> + * st->base.size is size_t (64-bit). Use temporary variable.
> + * Note: loose objects >4GB would still truncate here, but such
> + * large loose objects are uncommon (they'd normally be packed).
> + */
> + oi.sizep = &size_ul;
> oi.typep = &st->base.type;
>
> if (parse_loose_header(st->hdr, &oi) < 0 || st->base.type < 0)
> goto error;
> + st->base.size = size_ul;
>
> st->mapped = mapped;
> st->mapsize = mapsize;
> diff --git a/odb/streaming.c b/odb/streaming.c
> index 5927a12954..af2adf5ce7 100644
> --- a/odb/streaming.c
> +++ b/odb/streaming.c
> @@ -157,15 +157,26 @@ static int open_istream_incore(struct odb_read_stream **out,
> .base.read = read_istream_incore,
> };
> struct odb_incore_read_stream *st;
> + unsigned long size_ul;
> int ret;
>
> oi.typep = &stream.base.type;
> - oi.sizep = &stream.base.size;
> + /*
> + * object_info.sizep is unsigned long* (32-bit on Windows), but
> + * stream.base.size is size_t (64-bit). We use a temporary variable
> + * because the types are incompatible. Note: this path still truncates
> + * for >4GB objects, but large objects should use pack streaming
> + * (packfile_store_read_object_stream) which handles size_t properly.
> + * This incore fallback is only used for small objects or when pack
> + * streaming is unavailable.
> + */
> + oi.sizep = &size_ul;
> oi.contentp = (void **)&stream.buf;
> ret = odb_read_object_info_extended(odb, oid, &oi,
> OBJECT_INFO_DIE_IF_CORRUPT);
> if (ret)
> return ret;
> + stream.base.size = size_ul;
>
> CALLOC_ARRAY(st, 1);
> *st = stream;
> diff --git a/odb/streaming.h b/odb/streaming.h
> index c7861f7e13..517e2ea2d3 100644
> --- a/odb/streaming.h
> +++ b/odb/streaming.h
> @@ -21,7 +21,7 @@ struct odb_read_stream {
> odb_read_stream_close_fn close;
> odb_read_stream_read_fn read;
> enum object_type type;
> - unsigned long size; /* inflated size of full object */
> + size_t size; /* inflated size of full object */
> };
>
> /*
> diff --git a/oss-fuzz/fuzz-pack-headers.c b/oss-fuzz/fuzz-pack-headers.c
> index 150c0f5fa2..ef61ab577c 100644
> --- a/oss-fuzz/fuzz-pack-headers.c
> +++ b/oss-fuzz/fuzz-pack-headers.c
> @@ -6,7 +6,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
> int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> {
> enum object_type type;
> - unsigned long len;
> + size_t len;
>
> unpack_object_header_buffer((const unsigned char *)data,
> (unsigned long)size, &type, &len);
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index f6ec18d83a..f9af8a96bd 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -2270,7 +2270,7 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
> {
> off_t delta_obj_offset;
> enum object_type type;
> - unsigned long size;
> + size_t size;
>
> if (pack_pos >= pack->p->num_objects)
> return -1; /* not actually in the pack */
> diff --git a/pack-check.c b/pack-check.c
> index 79992bb509..2792f34d25 100644
> --- a/pack-check.c
> +++ b/pack-check.c
> @@ -110,7 +110,7 @@ static int verify_packfile(struct repository *r,
> void *data;
> struct object_id oid;
> enum object_type type;
> - unsigned long size;
> + size_t size;
> off_t curpos;
> int data_valid;
>
> @@ -143,7 +143,9 @@ static int verify_packfile(struct repository *r,
> data = NULL;
> data_valid = 0;
> } else {
> - data = unpack_entry(r, p, entries[i].offset, &type, &size);
> + unsigned long sz;
> + data = unpack_entry(r, p, entries[i].offset, &type, &sz);
> + size = sz;
> data_valid = 1;
> }
>
> diff --git a/packfile.c b/packfile.c
> index b012d648ad..fdae91dd11 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1133,7 +1133,7 @@ out:
> }
>
> unsigned long unpack_object_header_buffer(const unsigned char *buf,
> - unsigned long len, enum object_type *type, unsigned long *sizep)
> + unsigned long len, enum object_type *type, size_t *sizep)
> {
> unsigned shift;
> size_t size, c;
> @@ -1144,7 +1144,11 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
> size = c & 15;
> shift = 4;
> while (c & 0x80) {
> - if (len <= used || (bitsizeof(long) - 7) < shift) {
> + /*
> + * Each continuation byte adds 7 bits. Ensure shift won't
> + * overflow size_t (use size_t not long for 64-bit on Windows).
> + */
> + if (len <= used || (bitsizeof(size_t) - 7) < shift) {
> error("bad object header");
> size = used = 0;
> break;
> @@ -1153,7 +1157,7 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
> size = st_add(size, st_left_shift(c & 0x7f, shift));
> shift += 7;
> }
> - *sizep = cast_size_t_to_ulong(size);
> + *sizep = size;
> return used;
> }
>
> @@ -1215,7 +1219,7 @@ unsigned long get_size_from_delta(struct packed_git *p,
> int unpack_object_header(struct packed_git *p,
> struct pack_window **w_curs,
> off_t *curpos,
> - unsigned long *sizep)
> + size_t *sizep)
> {
> unsigned char *base;
> unsigned long left;
> @@ -1367,7 +1371,7 @@ static enum object_type packed_to_object_type(struct repository *r,
>
> while (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
> off_t base_offset;
> - unsigned long size;
> + size_t size;
> /* Push the object we're going to leave behind */
> if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) {
> poi_stack_alloc = alloc_nr(poi_stack_nr);
> @@ -1586,7 +1590,7 @@ static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_off
> uint32_t *maybe_index_pos, struct object_info *oi)
> {
> struct pack_window *w_curs = NULL;
> - unsigned long size;
> + size_t size;
> off_t curpos = obj_offset;
> enum object_type type = OBJ_NONE;
> uint32_t pack_pos;
> @@ -1778,7 +1782,7 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
> struct pack_window *w_curs = NULL;
> off_t curpos = obj_offset;
> void *data = NULL;
> - unsigned long size;
> + size_t size;
> enum object_type type;
> struct unpack_entry_stack_ent small_delta_stack[UNPACK_ENTRY_STACK_PREALLOC];
> struct unpack_entry_stack_ent *delta_stack = small_delta_stack;
> @@ -1943,8 +1947,10 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
> (uintmax_t)curpos, p->pack_name);
> data = NULL;
> } else {
> + unsigned long sz;
> data = patch_delta(base, base_size, delta_data,
> - delta_size, &size);
> + delta_size, &sz);
> + size = sz;
>
> /*
> * We could not apply the delta; warn the user, but
> @@ -2929,7 +2935,7 @@ int packfile_read_object_stream(struct odb_read_stream **out,
> struct odb_packed_read_stream *stream;
> struct pack_window *window = NULL;
> enum object_type in_pack_type;
> - unsigned long size;
> + size_t size;
>
> in_pack_type = unpack_object_header(pack, &window, &offset, &size);
> unuse_pack(&window);
> diff --git a/packfile.h b/packfile.h
> index 9b647da7dd..49d6bdecf6 100644
> --- a/packfile.h
> +++ b/packfile.h
> @@ -456,9 +456,9 @@ off_t find_pack_entry_one(const struct object_id *oid, struct packed_git *);
>
> int is_pack_valid(struct packed_git *);
> void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);
> -unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
> +unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, size_t *sizep);
> unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
> -int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
> +int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, size_t *);
> off_t get_delta_base(struct packed_git *p, struct pack_window **w_curs,
> off_t *curpos, enum object_type type,
> off_t delta_obj_offset);
> --
> gitgitgadget
>
>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 03/11] odb, packfile: use size_t for streaming object sizes
2026-05-05 19:27 ` Torsten Bögershausen
@ 2026-05-08 7:38 ` Johannes Schindelin
0 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin @ 2026-05-08 7:38 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Johannes Schindelin via GitGitGadget, git, Derrick Stolee,
Jeff King
[-- Attachment #1: Type: text/plain, Size: 14133 bytes --]
Hi Torsten,
On Tue, 5 May 2026, Torsten Bögershausen wrote:
> On Mon, May 04, 2026 at 05:08:20PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The odb_read_stream structure uses unsigned long for the size field,
> > which is 32-bit on Windows even in 64-bit builds. When streaming
> > objects larger than 4GB, the size would be truncated to zero or an
> > incorrect value, resulting in empty files being written to disk.
> >
> > Change the size field in odb_read_stream to size_t and introduce
> > unpack_object_header_sz() to return sizes via size_t pointer. Since
> > object_info.sizep remains unsigned long for API compatibility, use
> > temporary variables where the types differ, with comments noting the
> > truncation limitation for code paths that still use unsigned long.
> >
> > This was originally authored by LordKiRon <https://github.com/LordKiRon>,
> > who preferred not to reveal their real name and therefore agreed that I
> > take over authorship.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > builtin/pack-objects.c | 23 ++++++++++++++++-------
> > object-file.c | 10 +++++++++-
> > odb/streaming.c | 13 ++++++++++++-
> > odb/streaming.h | 2 +-
> > oss-fuzz/fuzz-pack-headers.c | 2 +-
> > pack-bitmap.c | 2 +-
> > pack-check.c | 6 ++++--
> > packfile.c | 24 +++++++++++++++---------
> > packfile.h | 4 ++--
> > 9 files changed, 61 insertions(+), 25 deletions(-)
> >
>
> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index dd2480a73d..aa4b1cb9b8 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
>
> I haven't been able to follow all changes, so this may be false alarm.
> Do we need a cast_size_t_to_ulong() somewhere ?
Oh, that's a really good catch, I forgot to add those. There are a couple
of callers, indeed, and they still have an implicit, _silent_ truncation
to `unsigned long` that I'd rather have explicit, with a check. I'll add
those callers in the next iteration.
Thank you!
Johannes
>
> > @@ -629,14 +629,21 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,
> > struct packed_git *p = IN_PACK(entry);
> > struct pack_window *w_curs = NULL;
> > uint32_t pos;
> > - off_t offset;
> > + off_t offset, cur;
> > enum object_type type = oe_type(entry);
> > + enum object_type in_pack_type;
> > off_t datalen;
> > unsigned char header[MAX_PACK_OBJECT_HEADER],
> > dheader[MAX_PACK_OBJECT_HEADER];
> > unsigned hdrlen;
> > const unsigned hashsz = the_hash_algo->rawsz;
> > - unsigned long entry_size = SIZE(entry);
> > + size_t entry_size;
> > +
> > + cur = entry->in_pack_offset;
> > + in_pack_type = unpack_object_header(p, &w_curs, &cur, &entry_size);
> > + if (in_pack_type < 0)
> > + die(_("write_reuse_object: unable to parse object header of %s"),
> > + oid_to_hex(&entry->idx.oid));
> >
> > if (DELTA(entry))
> > type = (allow_ofs_delta && DELTA(entry)->idx.offset) ?
> > @@ -1087,7 +1094,7 @@ static void write_reused_pack_one(struct packed_git *reuse_packfile,
> > {
> > off_t offset, next, cur;
> > enum object_type type;
> > - unsigned long size;
> > + size_t size;
> >
> > offset = pack_pos_to_offset(reuse_packfile, pos);
> > next = pack_pos_to_offset(reuse_packfile, pos + 1);
> > @@ -2243,7 +2250,7 @@ static void check_object(struct object_entry *entry, uint32_t object_index)
> > off_t ofs;
> > unsigned char *buf, c;
> > enum object_type type;
> > - unsigned long in_pack_size;
> > + size_t in_pack_size;
> >
> > buf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);
> >
> > @@ -2734,16 +2741,18 @@ unsigned long oe_get_size_slow(struct packing_data *pack,
> > struct pack_window *w_curs;
> > unsigned char *buf;
> > enum object_type type;
> > - unsigned long used, avail, size;
> > + unsigned long used, avail;
> > + size_t size;
> >
> > if (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {
> > + unsigned long sz;
> > packing_data_lock(&to_pack);
> > if (odb_read_object_info(the_repository->objects,
> > - &e->idx.oid, &size) < 0)
> > + &e->idx.oid, &sz) < 0)
> > die(_("unable to get size of %s"),
> > oid_to_hex(&e->idx.oid));
> > packing_data_unlock(&to_pack);
> > - return size;
> > + return sz;
> > }
> >
> > p = oe_in_pack(pack, e);
> > diff --git a/object-file.c b/object-file.c
> > index 086b2b65ff..0be2981c7a 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -2326,6 +2326,7 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
> > struct object_info oi = OBJECT_INFO_INIT;
> > struct odb_loose_read_stream *st;
> > unsigned long mapsize;
> > + unsigned long size_ul;
> > void *mapped;
> >
> > mapped = odb_source_loose_map_object(source, oid, &mapsize);
> > @@ -2349,11 +2350,18 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
> > goto error;
> > }
> >
> > - oi.sizep = &st->base.size;
> > + /*
> > + * object_info.sizep is unsigned long* (32-bit on Windows), but
> > + * st->base.size is size_t (64-bit). Use temporary variable.
> > + * Note: loose objects >4GB would still truncate here, but such
> > + * large loose objects are uncommon (they'd normally be packed).
> > + */
> > + oi.sizep = &size_ul;
> > oi.typep = &st->base.type;
> >
> > if (parse_loose_header(st->hdr, &oi) < 0 || st->base.type < 0)
> > goto error;
> > + st->base.size = size_ul;
> >
> > st->mapped = mapped;
> > st->mapsize = mapsize;
> > diff --git a/odb/streaming.c b/odb/streaming.c
> > index 5927a12954..af2adf5ce7 100644
> > --- a/odb/streaming.c
> > +++ b/odb/streaming.c
> > @@ -157,15 +157,26 @@ static int open_istream_incore(struct odb_read_stream **out,
> > .base.read = read_istream_incore,
> > };
> > struct odb_incore_read_stream *st;
> > + unsigned long size_ul;
> > int ret;
> >
> > oi.typep = &stream.base.type;
> > - oi.sizep = &stream.base.size;
> > + /*
> > + * object_info.sizep is unsigned long* (32-bit on Windows), but
> > + * stream.base.size is size_t (64-bit). We use a temporary variable
> > + * because the types are incompatible. Note: this path still truncates
> > + * for >4GB objects, but large objects should use pack streaming
> > + * (packfile_store_read_object_stream) which handles size_t properly.
> > + * This incore fallback is only used for small objects or when pack
> > + * streaming is unavailable.
> > + */
> > + oi.sizep = &size_ul;
> > oi.contentp = (void **)&stream.buf;
> > ret = odb_read_object_info_extended(odb, oid, &oi,
> > OBJECT_INFO_DIE_IF_CORRUPT);
> > if (ret)
> > return ret;
> > + stream.base.size = size_ul;
> >
> > CALLOC_ARRAY(st, 1);
> > *st = stream;
> > diff --git a/odb/streaming.h b/odb/streaming.h
> > index c7861f7e13..517e2ea2d3 100644
> > --- a/odb/streaming.h
> > +++ b/odb/streaming.h
> > @@ -21,7 +21,7 @@ struct odb_read_stream {
> > odb_read_stream_close_fn close;
> > odb_read_stream_read_fn read;
> > enum object_type type;
> > - unsigned long size; /* inflated size of full object */
> > + size_t size; /* inflated size of full object */
> > };
> >
> > /*
> > diff --git a/oss-fuzz/fuzz-pack-headers.c b/oss-fuzz/fuzz-pack-headers.c
> > index 150c0f5fa2..ef61ab577c 100644
> > --- a/oss-fuzz/fuzz-pack-headers.c
> > +++ b/oss-fuzz/fuzz-pack-headers.c
> > @@ -6,7 +6,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
> > int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> > {
> > enum object_type type;
> > - unsigned long len;
> > + size_t len;
> >
> > unpack_object_header_buffer((const unsigned char *)data,
> > (unsigned long)size, &type, &len);
> > diff --git a/pack-bitmap.c b/pack-bitmap.c
> > index f6ec18d83a..f9af8a96bd 100644
> > --- a/pack-bitmap.c
> > +++ b/pack-bitmap.c
> > @@ -2270,7 +2270,7 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
> > {
> > off_t delta_obj_offset;
> > enum object_type type;
> > - unsigned long size;
> > + size_t size;
> >
> > if (pack_pos >= pack->p->num_objects)
> > return -1; /* not actually in the pack */
> > diff --git a/pack-check.c b/pack-check.c
> > index 79992bb509..2792f34d25 100644
> > --- a/pack-check.c
> > +++ b/pack-check.c
> > @@ -110,7 +110,7 @@ static int verify_packfile(struct repository *r,
> > void *data;
> > struct object_id oid;
> > enum object_type type;
> > - unsigned long size;
> > + size_t size;
> > off_t curpos;
> > int data_valid;
> >
> > @@ -143,7 +143,9 @@ static int verify_packfile(struct repository *r,
> > data = NULL;
> > data_valid = 0;
> > } else {
> > - data = unpack_entry(r, p, entries[i].offset, &type, &size);
> > + unsigned long sz;
> > + data = unpack_entry(r, p, entries[i].offset, &type, &sz);
> > + size = sz;
> > data_valid = 1;
> > }
> >
> > diff --git a/packfile.c b/packfile.c
> > index b012d648ad..fdae91dd11 100644
> > --- a/packfile.c
> > +++ b/packfile.c
> > @@ -1133,7 +1133,7 @@ out:
> > }
> >
> > unsigned long unpack_object_header_buffer(const unsigned char *buf,
> > - unsigned long len, enum object_type *type, unsigned long *sizep)
> > + unsigned long len, enum object_type *type, size_t *sizep)
> > {
> > unsigned shift;
> > size_t size, c;
> > @@ -1144,7 +1144,11 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
> > size = c & 15;
> > shift = 4;
> > while (c & 0x80) {
> > - if (len <= used || (bitsizeof(long) - 7) < shift) {
> > + /*
> > + * Each continuation byte adds 7 bits. Ensure shift won't
> > + * overflow size_t (use size_t not long for 64-bit on Windows).
> > + */
> > + if (len <= used || (bitsizeof(size_t) - 7) < shift) {
> > error("bad object header");
> > size = used = 0;
> > break;
> > @@ -1153,7 +1157,7 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
> > size = st_add(size, st_left_shift(c & 0x7f, shift));
> > shift += 7;
> > }
> > - *sizep = cast_size_t_to_ulong(size);
> > + *sizep = size;
> > return used;
> > }
> >
> > @@ -1215,7 +1219,7 @@ unsigned long get_size_from_delta(struct packed_git *p,
> > int unpack_object_header(struct packed_git *p,
> > struct pack_window **w_curs,
> > off_t *curpos,
> > - unsigned long *sizep)
> > + size_t *sizep)
> > {
> > unsigned char *base;
> > unsigned long left;
> > @@ -1367,7 +1371,7 @@ static enum object_type packed_to_object_type(struct repository *r,
> >
> > while (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
> > off_t base_offset;
> > - unsigned long size;
> > + size_t size;
> > /* Push the object we're going to leave behind */
> > if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) {
> > poi_stack_alloc = alloc_nr(poi_stack_nr);
> > @@ -1586,7 +1590,7 @@ static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_off
> > uint32_t *maybe_index_pos, struct object_info *oi)
> > {
> > struct pack_window *w_curs = NULL;
> > - unsigned long size;
> > + size_t size;
> > off_t curpos = obj_offset;
> > enum object_type type = OBJ_NONE;
> > uint32_t pack_pos;
> > @@ -1778,7 +1782,7 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
> > struct pack_window *w_curs = NULL;
> > off_t curpos = obj_offset;
> > void *data = NULL;
> > - unsigned long size;
> > + size_t size;
> > enum object_type type;
> > struct unpack_entry_stack_ent small_delta_stack[UNPACK_ENTRY_STACK_PREALLOC];
> > struct unpack_entry_stack_ent *delta_stack = small_delta_stack;
> > @@ -1943,8 +1947,10 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
> > (uintmax_t)curpos, p->pack_name);
> > data = NULL;
> > } else {
> > + unsigned long sz;
> > data = patch_delta(base, base_size, delta_data,
> > - delta_size, &size);
> > + delta_size, &sz);
> > + size = sz;
> >
> > /*
> > * We could not apply the delta; warn the user, but
> > @@ -2929,7 +2935,7 @@ int packfile_read_object_stream(struct odb_read_stream **out,
> > struct odb_packed_read_stream *stream;
> > struct pack_window *window = NULL;
> > enum object_type in_pack_type;
> > - unsigned long size;
> > + size_t size;
> >
> > in_pack_type = unpack_object_header(pack, &window, &offset, &size);
> > unuse_pack(&window);
> > diff --git a/packfile.h b/packfile.h
> > index 9b647da7dd..49d6bdecf6 100644
> > --- a/packfile.h
> > +++ b/packfile.h
> > @@ -456,9 +456,9 @@ off_t find_pack_entry_one(const struct object_id *oid, struct packed_git *);
> >
> > int is_pack_valid(struct packed_git *);
> > void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);
> > -unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
> > +unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, size_t *sizep);
> > unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
> > -int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
> > +int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, size_t *);
> > off_t get_delta_base(struct packed_git *p, struct pack_window **w_curs,
> > off_t *curpos, enum object_type type,
> > off_t delta_obj_offset);
> > --
> > gitgitgadget
> >
> >
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 04/11] delta, packfile: use size_t for delta header sizes
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2026-05-04 17:08 ` [PATCH v2 03/11] odb, packfile: use size_t for streaming object sizes Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 05/11] test-tool: add a helper to synthesize large packfiles Johannes Schindelin via GitGitGadget
` (7 subsequent siblings)
11 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The delta header decoding functions return unsigned long, which
truncates on Windows for objects larger than 4GB. Introduce size_t
variants get_delta_hdr_size_sz() and get_size_from_delta_sz() that
preserve the full 64-bit size, and use them in packed_object_info()
where the size is needed for streaming decisions.
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
delta.h | 14 ++++++++++++--
packfile.c | 33 ++++++++++++++++++++++++---------
2 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/delta.h b/delta.h
index 8a56ec0799..fad68cfc45 100644
--- a/delta.h
+++ b/delta.h
@@ -86,8 +86,11 @@ void *patch_delta(const void *src_buf, unsigned long src_size,
* This must be called twice on the delta data buffer, first to get the
* expected source buffer size, and again to get the target buffer size.
*/
-static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
- const unsigned char *top)
+/*
+ * Size_t variant that doesn't truncate - use for >4GB objects on Windows.
+ */
+static inline size_t get_delta_hdr_size_sz(const unsigned char **datap,
+ const unsigned char *top)
{
const unsigned char *data = *datap;
size_t cmd, size = 0;
@@ -98,6 +101,13 @@ static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
i += 7;
} while (cmd & 0x80 && data < top);
*datap = data;
+ return size;
+}
+
+static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
+ const unsigned char *top)
+{
+ size_t size = get_delta_hdr_size_sz(datap, top);
return cast_size_t_to_ulong(size);
}
diff --git a/packfile.c b/packfile.c
index fdae91dd11..4208f53046 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1161,9 +1161,12 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
return used;
}
-unsigned long get_size_from_delta(struct packed_git *p,
- struct pack_window **w_curs,
- off_t curpos)
+/*
+ * Size_t variant for >4GB delta results on Windows.
+ */
+static size_t get_size_from_delta_sz(struct packed_git *p,
+ struct pack_window **w_curs,
+ off_t curpos)
{
const unsigned char *data;
unsigned char delta_head[20], *in;
@@ -1210,10 +1213,18 @@ unsigned long get_size_from_delta(struct packed_git *p,
data = delta_head;
/* ignore base size */
- get_delta_hdr_size(&data, delta_head+sizeof(delta_head));
+ get_delta_hdr_size_sz(&data, delta_head+sizeof(delta_head));
/* Read the result size */
- return get_delta_hdr_size(&data, delta_head+sizeof(delta_head));
+ return get_delta_hdr_size_sz(&data, delta_head+sizeof(delta_head));
+}
+
+unsigned long get_size_from_delta(struct packed_git *p,
+ struct pack_window **w_curs,
+ off_t curpos)
+{
+ size_t size = get_size_from_delta_sz(p, w_curs, curpos);
+ return cast_size_t_to_ulong(size);
}
int unpack_object_header(struct packed_git *p,
@@ -1618,14 +1629,18 @@ static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_off
ret = -1;
goto out;
}
- *oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);
- if (*oi->sizep == 0) {
+ /*
+ * Use size_t variant to avoid die() on >4GB deltas.
+ * oi->sizep is unsigned long, so truncation may occur,
+ * but streaming code uses its own size_t tracking.
+ */
+ size = get_size_from_delta_sz(p, &w_curs, tmp_pos);
+ if (size == 0) {
ret = -1;
goto out;
}
- } else {
- *oi->sizep = size;
}
+ *oi->sizep = (unsigned long)size;
}
if (oi->disk_sizep || (oi->mtimep && p->is_cruft)) {
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 05/11] test-tool: add a helper to synthesize large packfiles
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
` (3 preceding siblings ...)
2026-05-04 17:08 ` [PATCH v2 04/11] delta, packfile: use size_t for delta header sizes Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 06/11] t5608: add regression test for >4GB object clone Johannes Schindelin via GitGitGadget
` (6 subsequent siblings)
11 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
To test Git's behavior with very large pack files, we need a way to
generate such files quickly.
A naive approach using only readily-available Git commands would take
over 10 hours for a 4GB pack file, which is prohibitive.
Side-stepping Git's machinery and actual zlib compression by writing
uncompressed content with the appropriate zlib header makes things
much faster. The fastest method using this approach generates many
small, unreachable blob objects and takes about 1.5 minutes for 4GB.
However, this cannot be used because we need to test git clone, which
requires a reachable commit history.
Generating many reachable commits with small, uncompressed blobs takes
about 4 minutes for 4GB. But this approach 1) does not reproduce the
issues we want to fix (which require individual objects larger than
4GB) and 2) is comparatively slow because of the many SHA-1
calculations.
The approach taken here generates a single large blob (filled with NUL
bytes), along with the trees and commits needed to make it reachable.
This takes about 2.5 minutes for 4.5GB, which is the fastest option
that produces a valid, clonable repository with an object large enough
to trigger the bugs we want to test.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 1 +
compat/zlib-compat.h | 2 +
t/helper/meson.build | 1 +
t/helper/test-synthesize.c | 250 +++++++++++++++++++++++++++++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
6 files changed, 256 insertions(+)
create mode 100644 t/helper/test-synthesize.c
diff --git a/Makefile b/Makefile
index cedc234173..85405cb5b8 100644
--- a/Makefile
+++ b/Makefile
@@ -872,6 +872,7 @@ TEST_BUILTINS_OBJS += test-submodule-config.o
TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o
TEST_BUILTINS_OBJS += test-submodule.o
TEST_BUILTINS_OBJS += test-subprocess.o
+TEST_BUILTINS_OBJS += test-synthesize.o
TEST_BUILTINS_OBJS += test-trace2.o
TEST_BUILTINS_OBJS += test-truncate.o
TEST_BUILTINS_OBJS += test-userdiff.o
diff --git a/compat/zlib-compat.h b/compat/zlib-compat.h
index ac08276622..5078c5ef6c 100644
--- a/compat/zlib-compat.h
+++ b/compat/zlib-compat.h
@@ -7,6 +7,8 @@
# define z_stream_s zng_stream_s
# define gz_header_s zng_gz_header_s
+# define adler32(adler, buf, len) zng_adler32(adler, buf, len)
+
# define crc32(crc, buf, len) zng_crc32(crc, buf, len)
# define inflate(strm, bits) zng_inflate(strm, bits)
diff --git a/t/helper/meson.build b/t/helper/meson.build
index 675e64c010..3235f10ab8 100644
--- a/t/helper/meson.build
+++ b/t/helper/meson.build
@@ -69,6 +69,7 @@ test_tool_sources = [
'test-submodule-nested-repo-config.c',
'test-submodule.c',
'test-subprocess.c',
+ 'test-synthesize.c',
'test-tool.c',
'test-trace2.c',
'test-truncate.c',
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
new file mode 100644
index 0000000000..3ce7078078
--- /dev/null
+++ b/t/helper/test-synthesize.c
@@ -0,0 +1,250 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
+#include "test-tool.h"
+#include "git-compat-util.h"
+#include "git-zlib.h"
+#include "hash.h"
+#include "hex.h"
+#include "object-file.h"
+#include "object.h"
+#include "pack.h"
+#include "parse-options.h"
+#include "parse.h"
+#include "repository.h"
+#include "setup.h"
+#include "strbuf.h"
+#include "write-or-die.h"
+
+#define BLOCK_SIZE 0xffff
+static const unsigned char zeros[BLOCK_SIZE];
+
+/*
+ * Write data as an uncompressed zlib stream.
+ * For data larger than 64KB, writes multiple uncompressed blocks.
+ * If data is NULL, writes zeros.
+ * Updates the pack checksum context.
+ */
+static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,
+ const void *data, size_t len,
+ const struct git_hash_algo *algo)
+{
+ unsigned char zlib_header[2] = { 0x78, 0x01 }; /* CMF, FLG */
+ unsigned char block_header[5];
+ const unsigned char *p = data;
+ size_t remaining = len;
+ uint32_t adler = 1L; /* adler32 initial value */
+ unsigned char adler_buf[4];
+
+ /* Write zlib header */
+ fwrite_or_die(f, zlib_header, sizeof(zlib_header));
+ algo->update_fn(pack_ctx, zlib_header, 2);
+
+ /* Write uncompressed blocks (max 64KB each) */
+ do {
+ size_t block_len = remaining > BLOCK_SIZE ? BLOCK_SIZE : remaining;
+ int is_final = (block_len == remaining);
+ const unsigned char *block_data = data ? p : zeros;
+
+ block_header[0] = is_final ? 0x01 : 0x00;
+ block_header[1] = block_len & 0xff;
+ block_header[2] = (block_len >> 8) & 0xff;
+ block_header[3] = block_header[1] ^ 0xff;
+ block_header[4] = block_header[2] ^ 0xff;
+
+ fwrite_or_die(f, block_header, sizeof(block_header));
+ algo->update_fn(pack_ctx, block_header, 5);
+
+ if (block_len) {
+ fwrite_or_die(f, block_data, block_len);
+ algo->update_fn(pack_ctx, block_data, block_len);
+ adler = adler32(adler, block_data, block_len);
+ }
+
+ if (data)
+ p += block_len;
+ remaining -= block_len;
+ } while (remaining > 0);
+
+ /* Write adler32 checksum */
+ put_be32(adler_buf, adler);
+ fwrite_or_die(f, adler_buf, sizeof(adler_buf));
+ algo->update_fn(pack_ctx, adler_buf, 4);
+}
+
+/*
+ * Write an uncompressed object to the pack file.
+ * If `data == NULL`, it is treated like a buffer to NUL bytes.
+ * Updates the pack checksum context.
+ */
+static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,
+ enum object_type type,
+ const void *data, size_t len,
+ struct object_id *oid,
+ const struct git_hash_algo *algo)
+{
+ unsigned char pack_header[MAX_PACK_OBJECT_HEADER];
+ char object_header[32];
+ int pack_header_len, object_header_len;
+ struct git_hash_ctx ctx;
+
+ /* Write pack object header */
+ pack_header_len = encode_in_pack_object_header(pack_header,
+ sizeof(pack_header),
+ type, len);
+ fwrite_or_die(f, pack_header, pack_header_len);
+ algo->update_fn(pack_ctx, pack_header, pack_header_len);
+
+ /* Write the data as uncompressed zlib */
+ write_uncompressed_zlib(f, pack_ctx, data, len, algo);
+
+ algo->init_fn(&ctx);
+ object_header_len = format_object_header(object_header,
+ sizeof(object_header),
+ type, len);
+ algo->update_fn(&ctx, object_header, object_header_len);
+ if (data)
+ algo->update_fn(&ctx, data, len);
+ else {
+ for (size_t i = len / BLOCK_SIZE; i; i--)
+ algo->update_fn(&ctx, zeros, BLOCK_SIZE);
+ algo->update_fn(&ctx, zeros, len % BLOCK_SIZE);
+ }
+ algo->final_oid_fn(oid, &ctx);
+}
+
+/*
+ * Generate a pack file with a single large (>4GB) reachable object.
+ *
+ * Creates:
+ * 1. A large blob (all NUL bytes)
+ * 2. A tree containing that blob as "file"
+ * 3. A commit using that tree
+ * 4. The empty tree
+ * 5. A child commit using the empty tree
+ *
+ * This is useful for testing that Git can handle objects larger than 4GB.
+ */
+static int generate_pack_with_large_object(const char *path, size_t blob_size,
+ const struct git_hash_algo *algo)
+{
+ FILE *f = xfopen(path, "wb");
+ struct git_hash_ctx pack_ctx;
+ unsigned char pack_hash[GIT_MAX_RAWSZ];
+ struct object_id blob_oid, tree_oid, commit_oid, empty_tree_oid, final_commit_oid;
+ struct strbuf buf = STRBUF_INIT;
+ const uint32_t object_count = 5;
+ struct pack_header pack_header = {
+ .hdr_signature = htonl(PACK_SIGNATURE),
+ .hdr_version = htonl(PACK_VERSION),
+ .hdr_entries = htonl(object_count),
+ };
+
+ algo->init_fn(&pack_ctx);
+
+ /* Write pack header */
+ fwrite_or_die(f, &pack_header, sizeof(pack_header));
+ algo->update_fn(&pack_ctx, &pack_header, sizeof(pack_header));
+
+ /* 1. Write the large blob */
+ write_pack_object(f, &pack_ctx, OBJ_BLOB, NULL, blob_size, &blob_oid, algo);
+
+ /* 2. Write tree containing the blob as "file" */
+ strbuf_addf(&buf, "100644 file%c", '\0');
+ strbuf_add(&buf, blob_oid.hash, algo->rawsz);
+ write_pack_object(f, &pack_ctx, OBJ_TREE, buf.buf, buf.len, &tree_oid, algo);
+
+ /* 3. Write commit using that tree */
+ strbuf_reset(&buf);
+ strbuf_addf(&buf,
+ "tree %s\n"
+ "author A U Thor <author@example.com> 1234567890 +0000\n"
+ "committer C O Mitter <committer@example.com> 1234567890 +0000\n"
+ "\n"
+ "Large blob commit\n",
+ oid_to_hex(&tree_oid));
+ write_pack_object(f, &pack_ctx, OBJ_COMMIT, buf.buf, buf.len, &commit_oid, algo);
+
+ /* 4. Write the empty tree */
+ write_pack_object(f, &pack_ctx, OBJ_TREE, "", 0, &empty_tree_oid, algo);
+
+ /* 5. Write final commit using empty tree, with previous commit as parent */
+ strbuf_reset(&buf);
+ strbuf_addf(&buf,
+ "tree %s\n"
+ "parent %s\n"
+ "author A U Thor <author@example.com> 1234567890 +0000\n"
+ "committer C O Mitter <committer@example.com> 1234567890 +0000\n"
+ "\n"
+ "Empty tree commit\n",
+ oid_to_hex(&empty_tree_oid),
+ oid_to_hex(&commit_oid));
+ write_pack_object(f, &pack_ctx, OBJ_COMMIT, buf.buf, buf.len, &final_commit_oid, algo);
+
+ /* Write pack trailer (checksum) */
+ algo->final_fn(pack_hash, &pack_ctx);
+ fwrite_or_die(f, pack_hash, algo->rawsz);
+ if (fclose(f))
+ die_errno(_("could not close '%s'"), path);
+
+ strbuf_release(&buf);
+
+ /* Print the final commit OID so caller can set up refs */
+ printf("%s\n", oid_to_hex(&final_commit_oid));
+
+ return 0;
+}
+
+static int cmd__synthesize__pack(int argc, const char **argv,
+ const char *prefix UNUSED,
+ struct repository *repo)
+{
+ int non_git;
+ int reachable_large = 0;
+ const struct git_hash_algo *algo;
+ size_t blob_size;
+ uintmax_t blob_size_u;
+ const char *path;
+ const char * const usage[] = {
+ "test-tool synthesize pack "
+ "--reachable-large <blob-size> <filename>",
+ NULL
+ };
+ struct option options[] = {
+ OPT_BOOL(0, "reachable-large", &reachable_large,
+ N_("write a pack with a single reachable large blob")),
+ OPT_END()
+ };
+
+ setup_git_directory_gently(&non_git);
+ repo = the_repository;
+ algo = repo->hash_algo;
+
+ argc = parse_options(argc, argv, NULL, options, usage,
+ PARSE_OPT_KEEP_ARGV0);
+ if (argc != 3 || !reachable_large)
+ usage_with_options(usage, options);
+
+ if (!git_parse_unsigned(argv[1], &blob_size_u,
+ maximum_unsigned_value_of_type(size_t)))
+ die(_("'%s' is not a valid blob size"), argv[1]);
+ blob_size = blob_size_u;
+ path = argv[2];
+
+ return !!generate_pack_with_large_object(path, blob_size, algo);
+}
+
+int cmd__synthesize(int argc, const char **argv)
+{
+ const char *prefix = NULL;
+ char const * const synthesize_usage[] = {
+ "test-tool synthesize pack <options>",
+ NULL,
+ };
+ parse_opt_subcommand_fn *fn = NULL;
+ struct option options[] = {
+ OPT_SUBCOMMAND("pack", &fn, cmd__synthesize__pack),
+ OPT_END()
+ };
+ argc = parse_options(argc, argv, prefix, options, synthesize_usage, 0);
+ return !!fn(argc, argv, prefix, NULL);
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index a7abc618b3..b71a22b43b 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -82,6 +82,7 @@ static struct test_cmd cmds[] = {
{ "submodule-config", cmd__submodule_config },
{ "submodule-nested-repo-config", cmd__submodule_nested_repo_config },
{ "subprocess", cmd__subprocess },
+ { "synthesize", cmd__synthesize },
{ "trace2", cmd__trace2 },
{ "truncate", cmd__truncate },
{ "userdiff", cmd__userdiff },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 7f150fa1eb..f2885b33d5 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -75,6 +75,7 @@ int cmd__submodule(int argc, const char **argv);
int cmd__submodule_config(int argc, const char **argv);
int cmd__submodule_nested_repo_config(int argc, const char **argv);
int cmd__subprocess(int argc, const char **argv);
+int cmd__synthesize(int argc, const char **argv);
int cmd__trace2(int argc, const char **argv);
int cmd__truncate(int argc, const char **argv);
int cmd__userdiff(int argc, const char **argv);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 06/11] t5608: add regression test for >4GB object clone
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
` (4 preceding siblings ...)
2026-05-04 17:08 ` [PATCH v2 05/11] test-tool: add a helper to synthesize large packfiles Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 07/11] test-tool synthesize: use the unsafe hash for speed Johannes Schindelin via GitGitGadget
` (5 subsequent siblings)
11 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The shift overflow bug in index-pack and unpack-objects caused incorrect
object size calculation when the encoded size required more than 32 bits
of shift. This would result in corrupted or failed unpacking of objects
larger than 4GB.
Add a test that creates a pack file containing a 4GB+ blob using the
new 'test-tool synthesize pack --reachable-large' command, then clones
the repository to verify the fix works correctly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t5608-clone-2gb.sh | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
index 87a8cd9f98..af93302dde 100755
--- a/t/t5608-clone-2gb.sh
+++ b/t/t5608-clone-2gb.sh
@@ -49,4 +49,41 @@ test_expect_success 'clone - with worktree, file:// protocol' '
'
+test_expect_success SIZE_T_IS_64BIT 'set up repo with >4GB object' '
+ large_blob_size=$((4*1024*1024*1024+1)) &&
+ git init --bare 4gb-repo &&
+ head_oid=$(test-tool synthesize pack \
+ --reachable-large "$large_blob_size" \
+ 4gb-repo/objects/pack/test.pack) &&
+ git -C 4gb-repo index-pack objects/pack/test.pack &&
+ git -C 4gb-repo update-ref refs/heads/main $head_oid &&
+ git -C 4gb-repo symbolic-ref HEAD refs/heads/main
+'
+
+test_expect_success SIZE_T_IS_64BIT 'clone >4GB object via unpack-objects' '
+ # The synthesized pack has five objects, so a large unpack limit keeps
+ # fetch-pack on the unpack-objects path.
+ git -c fetch.unpackLimit=100 clone --bare \
+ "file://$(pwd)/4gb-repo" 4gb-clone-unpack &&
+
+ # Verify the large blob survived the clone by comparing its OID
+ # between source and clone. We cannot use "cat-file -s" because
+ # object_info.sizep is still unsigned long, which truncates >4GB
+ # sizes on Windows. OID equality proves content integrity since
+ # the clone already verified checksums via index-pack/unpack-objects.
+ source_blob=$(git -C 4gb-repo rev-parse main^:file) &&
+ clone_blob=$(git -C 4gb-clone-unpack rev-parse main^:file) &&
+ test "$source_blob" = "$clone_blob"
+'
+
+test_expect_success SIZE_T_IS_64BIT 'clone with >4GB object via index-pack' '
+ # Force fetch-pack to hand the pack to index-pack instead.
+ git -c fetch.unpackLimit=1 clone --bare \
+ "file://$(pwd)/4gb-repo" 4gb-clone-index &&
+
+ source_blob=$(git -C 4gb-repo rev-parse main^:file) &&
+ clone_blob=$(git -C 4gb-clone-index rev-parse main^:file) &&
+ test "$source_blob" = "$clone_blob"
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 07/11] test-tool synthesize: use the unsafe hash for speed
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
` (5 preceding siblings ...)
2026-05-04 17:08 ` [PATCH v2 06/11] t5608: add regression test for >4GB object clone Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 08/11] test-tool synthesize: precompute pack for 4 GiB + 1 Johannes Schindelin via GitGitGadget
` (4 subsequent siblings)
11 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Jeff King pointed out on the mailing list [1] that t5608's new >4GB
test cases dominate the entire test suite runtime: 160 seconds on his
laptop when the rest of the suite finishes in under 90 seconds, and
305-850 seconds across CI jobs. The bottleneck is that the synthesize
helper hashes roughly 8 GB of data through SHA-1 (4 GB for the pack
checksum plus 4 GB for the blob OID) for a 4 GB+1 blob.
Since the helper generates known test data, collision detection is
unnecessary. Switch from repo->hash_algo to unsafe_hash_algo(), which
uses hardware-accelerated SHA-1 (via OpenSSL or Apple CommonCrypto)
when available.
Benchmarks on an x86_64 machine generating a 4 GB+1 pack (2 runs
each, interleaved):
SHA-1 backend Run 1 Run 2
SHA1DC (safe) 75s 80s
OpenSSL (unsafe) 21s 19s
The effect scales linearly. At 64 MB with 10 randomized interleaved
runs, the OpenSSL unsafe backend shows a 5.4x improvement (median
0.202s vs 1.088s) with tight variance (stdev 0.028s vs 0.095s).
The speedup is only realized when the build has a fast unsafe backend
compiled in. The CI's linux-TEST-vars job already sets
OPENSSL_SHA1_UNSAFE=YesPlease; macOS benefits from Apple CommonCrypto
when configured. On builds without a separate unsafe backend (such as
the default Windows builds), unsafe_hash_algo() returns the regular
collision-detecting implementation and the change is a no-op.
[1] https://lore.kernel.org/git/20260501063805.GA2038915@coredump.intra.peff.net/
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-synthesize.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
index 3ce7078078..e2faaad7b4 100644
--- a/t/helper/test-synthesize.c
+++ b/t/helper/test-synthesize.c
@@ -217,7 +217,7 @@ static int cmd__synthesize__pack(int argc, const char **argv,
setup_git_directory_gently(&non_git);
repo = the_repository;
- algo = repo->hash_algo;
+ algo = unsafe_hash_algo(repo->hash_algo);
argc = parse_options(argc, argv, NULL, options, usage,
PARSE_OPT_KEEP_ARGV0);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 08/11] test-tool synthesize: precompute pack for 4 GiB + 1
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
` (6 preceding siblings ...)
2026-05-04 17:08 ` [PATCH v2 07/11] test-tool synthesize: use the unsafe hash for speed Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-04 18:27 ` Derrick Stolee
2026-05-04 17:08 ` [PATCH v2 09/11] test-tool synthesize: add precomputed SHA-256 " Johannes Schindelin via GitGitGadget
` (3 subsequent siblings)
11 siblings, 1 reply; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The synthesize helper hashes roughly 8 GiB of data through SHA-1 to
produce a 4 GiB + 1 pack (4 GiB for the pack checksum, 4 GiB for
the blob OID). Since the blob content is all NUL bytes, every byte
in the resulting pack file is deterministic for a given blob size and
hash algorithm.
Add a fast path that writes the pack from precomputed constants:
a 25-byte prefix (pack header, object header, zlib header, first
block header), the zero-filled bulk with periodic 5-byte deflate
block headers, and a 513-byte suffix (tree, two commits, empty tree,
pack SHA-1 checksum). This eliminates all SHA-1 and adler32
computation, making the helper purely I/O-bound.
The precomputed constants are stored in a struct fast_pack array
keyed by hash algorithm format_id, so that adding SHA-256 support
later requires only adding another array entry with its suffix.
The constants were generated by running the generic path and
extracting the non-zero bytes from the resulting pack file.
Benchmarks generating a 4 GiB + 1 pack (3 runs each, SHA1DC on
x86_64):
generic path: 88s / 81s / 140s
fast path: 14s / 13s / 15s
On CI, where t5608 currently takes 200-850 seconds depending on the
job, the fast path cuts the pack-generation phase from minutes to
seconds, leaving only the clone operations themselves.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-synthesize.c | 202 ++++++++++++++++++++++++++++++++++++-
1 file changed, 201 insertions(+), 1 deletion(-)
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
index e2faaad7b4..83c40ee02a 100644
--- a/t/helper/test-synthesize.c
+++ b/t/helper/test-synthesize.c
@@ -112,6 +112,201 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,
algo->final_oid_fn(oid, &ctx);
}
+/*
+ * Fast path: precomputed pack data for a 4 GiB + 1 all-NUL blob.
+ *
+ * The generated pack is almost entirely zeros with a small constant
+ * prefix, periodic deflate block headers, and a constant suffix
+ * containing the tree, two commits, and the pack checksum. Because
+ * every byte is deterministic for a given blob size and hash algorithm,
+ * we can write the pack without computing any hashes at all, reducing
+ * runtime from minutes of hash computation to seconds of pure I/O.
+ *
+ * The blob is stored as an uncompressed deflate stream: a two-byte
+ * zlib header, then 65538 blocks of up to 0xffff bytes each, followed
+ * by an adler32 checksum. The pack header and deflate framing are
+ * shared across hash algorithms; only the suffix (which contains OIDs
+ * and the pack checksum) differs.
+ *
+ * Constants were generated by running the generic path and extracting
+ * the non-zero bytes from the resulting pack file.
+ */
+
+#define FAST_PACK_4G1_BLOB_SIZE ((size_t)4 * 1024 * 1024 * 1024 + 1)
+#define FAST_PACK_4G1_N_FULL_BLOCKS 65537
+
+/*
+ * Per-hash-algorithm constants for the fast path. The prefix and
+ * deflate block structure are identical across algorithms; only the
+ * suffix (tree, commits, pack checksum) and the commit OID differ.
+ */
+struct fast_pack {
+ uint32_t format_id;
+ const unsigned char *suffix;
+ size_t suffix_len;
+ const char *commit_oid;
+};
+
+/* Pack header + pack object header + zlib header + first block header */
+static const unsigned char fast_pack_prefix[] = {
+ /* PACK header: signature, version 2, 5 objects */
+ 0x50, 0x41, 0x43, 0x4b, 0x00, 0x00, 0x00, 0x02,
+ 0x00, 0x00, 0x00, 0x05,
+ /* pack object header: blob, size = 4294967297 */
+ 0xb1, 0x80, 0x80, 0x80, 0x80, 0x01,
+ /* zlib header: CMF=0x78, FLG=0x01 */
+ 0x78, 0x01,
+ /* first non-final block header: BFINAL=0, LEN=0xffff, NLEN=0x0000 */
+ 0x00, 0xff, 0xff, 0x00, 0x00
+};
+
+/* Every non-final deflate block header is identical */
+static const unsigned char fast_pack_block_header[] = {
+ 0x00, 0xff, 0xff, 0x00, 0x00
+};
+
+/* Final block (2 data bytes) + adler32 of 4294967297 NUL bytes */
+static const unsigned char fast_pack_final_block[] = {
+ /* BFINAL=1, LEN=2, NLEN=0xfffd */
+ 0x01, 0x02, 0x00, 0xfd, 0xff,
+ /* 2 NUL data bytes */
+ 0x00, 0x00,
+ /* adler32 */
+ 0x00, 0xe2, 0x00, 0x01
+};
+
+/*
+ * SHA-1 suffix: tree, commit, empty tree, final commit, pack checksum.
+ */
+static const unsigned char fast_pack_sha1_suffix[] = {
+ 0xa0, 0x02, 0x78, 0x01, 0x01, 0x20, 0x00, 0xdf,
+ 0xff, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20,
+ 0x66, 0x69, 0x6c, 0x65, 0x00, 0x3e, 0xb7, 0xfe,
+ 0xb1, 0x41, 0x3c, 0x75, 0x7f, 0x0d, 0x81, 0x81,
+ 0xde, 0xb2, 0x8d, 0x1d, 0xab, 0x03, 0xd6, 0x48,
+ 0x46, 0xb4, 0xb4, 0x0c, 0x60, 0x95, 0x0b, 0x78,
+ 0x01, 0x01, 0xb5, 0x00, 0x4a, 0xff, 0x74, 0x72,
+ 0x65, 0x65, 0x20, 0x63, 0x36, 0x38, 0x33, 0x66,
+ 0x63, 0x63, 0x37, 0x64, 0x31, 0x64, 0x38, 0x33,
+ 0x65, 0x66, 0x32, 0x66, 0x65, 0x31, 0x61, 0x66,
+ 0x35, 0x35, 0x32, 0x31, 0x35, 0x64, 0x30, 0x31,
+ 0x36, 0x38, 0x64, 0x62, 0x35, 0x32, 0x61, 0x33,
+ 0x61, 0x33, 0x62, 0x0a, 0x61, 0x75, 0x74, 0x68,
+ 0x6f, 0x72, 0x20, 0x41, 0x20, 0x55, 0x20, 0x54,
+ 0x68, 0x6f, 0x72, 0x20, 0x3c, 0x61, 0x75, 0x74,
+ 0x68, 0x6f, 0x72, 0x40, 0x65, 0x78, 0x61, 0x6d,
+ 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0x3e,
+ 0x20, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+ 0x38, 0x39, 0x30, 0x20, 0x2b, 0x30, 0x30, 0x30,
+ 0x30, 0x0a, 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74,
+ 0x74, 0x65, 0x72, 0x20, 0x43, 0x20, 0x4f, 0x20,
+ 0x4d, 0x69, 0x74, 0x74, 0x65, 0x72, 0x20, 0x3c,
+ 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x74, 0x65,
+ 0x72, 0x40, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c,
+ 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0x3e, 0x20, 0x31,
+ 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39,
+ 0x30, 0x20, 0x2b, 0x30, 0x30, 0x30, 0x30, 0x0a,
+ 0x0a, 0x4c, 0x61, 0x72, 0x67, 0x65, 0x20, 0x62,
+ 0x6c, 0x6f, 0x62, 0x20, 0x63, 0x6f, 0x6d, 0x6d,
+ 0x69, 0x74, 0x0a, 0xc6, 0x55, 0x37, 0x6b, 0x20,
+ 0x78, 0x01, 0x01, 0x00, 0x00, 0xff, 0xff, 0x00,
+ 0x00, 0x00, 0x01, 0x95, 0x0e, 0x78, 0x01, 0x01,
+ 0xe5, 0x00, 0x1a, 0xff, 0x74, 0x72, 0x65, 0x65,
+ 0x20, 0x34, 0x62, 0x38, 0x32, 0x35, 0x64, 0x63,
+ 0x36, 0x34, 0x32, 0x63, 0x62, 0x36, 0x65, 0x62,
+ 0x39, 0x61, 0x30, 0x36, 0x30, 0x65, 0x35, 0x34,
+ 0x62, 0x66, 0x38, 0x64, 0x36, 0x39, 0x32, 0x38,
+ 0x38, 0x66, 0x62, 0x65, 0x65, 0x34, 0x39, 0x30,
+ 0x34, 0x0a, 0x70, 0x61, 0x72, 0x65, 0x6e, 0x74,
+ 0x20, 0x63, 0x35, 0x62, 0x32, 0x31, 0x63, 0x36,
+ 0x31, 0x31, 0x61, 0x61, 0x35, 0x39, 0x34, 0x65,
+ 0x63, 0x39, 0x66, 0x64, 0x37, 0x65, 0x39, 0x32,
+ 0x63, 0x66, 0x39, 0x36, 0x34, 0x38, 0x39, 0x31,
+ 0x34, 0x63, 0x61, 0x34, 0x63, 0x32, 0x34, 0x31,
+ 0x32, 0x0a, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72,
+ 0x20, 0x41, 0x20, 0x55, 0x20, 0x54, 0x68, 0x6f,
+ 0x72, 0x20, 0x3c, 0x61, 0x75, 0x74, 0x68, 0x6f,
+ 0x72, 0x40, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c,
+ 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0x3e, 0x20, 0x31,
+ 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39,
+ 0x30, 0x20, 0x2b, 0x30, 0x30, 0x30, 0x30, 0x0a,
+ 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x74, 0x65,
+ 0x72, 0x20, 0x43, 0x20, 0x4f, 0x20, 0x4d, 0x69,
+ 0x74, 0x74, 0x65, 0x72, 0x20, 0x3c, 0x63, 0x6f,
+ 0x6d, 0x6d, 0x69, 0x74, 0x74, 0x65, 0x72, 0x40,
+ 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e,
+ 0x63, 0x6f, 0x6d, 0x3e, 0x20, 0x31, 0x32, 0x33,
+ 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x20,
+ 0x2b, 0x30, 0x30, 0x30, 0x30, 0x0a, 0x0a, 0x45,
+ 0x6d, 0x70, 0x74, 0x79, 0x20, 0x74, 0x72, 0x65,
+ 0x65, 0x20, 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74,
+ 0x0a, 0xaa, 0xb8, 0x45, 0x01, 0x8e, 0xfc, 0xf0,
+ 0x2f, 0x9c, 0xc5, 0xcc, 0x4f, 0x6a, 0x1a, 0xc9,
+ 0x2b, 0x23, 0xa9, 0xff, 0x91, 0x06, 0xc2, 0x70,
+ 0xe3
+};
+
+static const struct fast_pack fast_packs[] = {
+ {
+ .format_id = GIT_SHA1_FORMAT_ID,
+ .suffix = fast_pack_sha1_suffix,
+ .suffix_len = sizeof(fast_pack_sha1_suffix),
+ .commit_oid = "aac43daf40d0377af31aa9c798a4ae8a31b55c1d",
+ },
+};
+
+/*
+ * Try the fast path for known blob sizes. Returns 1 if the pack was
+ * written from precomputed constants, 0 if the caller should fall
+ * through to the generic path.
+ */
+static int generate_fast_pack(const char *path, size_t blob_size,
+ const struct git_hash_algo *algo)
+{
+ const struct fast_pack *fp = NULL;
+ FILE *f;
+ size_t i;
+
+ if (blob_size != FAST_PACK_4G1_BLOB_SIZE)
+ return 0;
+
+ for (i = 0; i < ARRAY_SIZE(fast_packs); i++) {
+ if (fast_packs[i].format_id == algo->format_id) {
+ fp = &fast_packs[i];
+ break;
+ }
+ }
+ if (!fp)
+ return 0;
+
+ f = xfopen(path, "wb");
+
+ fwrite_or_die(f, fast_pack_prefix, sizeof(fast_pack_prefix));
+
+ /* First full block: 0xffff zero bytes (header already in prefix) */
+ fwrite_or_die(f, zeros, BLOCK_SIZE);
+
+ /* Remaining non-final full blocks */
+ for (i = 1; i < FAST_PACK_4G1_N_FULL_BLOCKS; i++) {
+ fwrite_or_die(f, fast_pack_block_header,
+ sizeof(fast_pack_block_header));
+ fwrite_or_die(f, zeros, BLOCK_SIZE);
+ }
+
+ /* Final block (2 data bytes) + adler32 */
+ fwrite_or_die(f, fast_pack_final_block,
+ sizeof(fast_pack_final_block));
+
+ /* Tree, commits, and pack checksum */
+ fwrite_or_die(f, fp->suffix, fp->suffix_len);
+
+ if (fclose(f))
+ die_errno(_("could not close '%s'"), path);
+
+ printf("%s\n", fp->commit_oid);
+ return 1;
+}
+
/*
* Generate a pack file with a single large (>4GB) reachable object.
*
@@ -127,7 +322,7 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,
static int generate_pack_with_large_object(const char *path, size_t blob_size,
const struct git_hash_algo *algo)
{
- FILE *f = xfopen(path, "wb");
+ FILE *f;
struct git_hash_ctx pack_ctx;
unsigned char pack_hash[GIT_MAX_RAWSZ];
struct object_id blob_oid, tree_oid, commit_oid, empty_tree_oid, final_commit_oid;
@@ -139,6 +334,11 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,
.hdr_entries = htonl(object_count),
};
+ if (generate_fast_pack(path, blob_size, algo))
+ return 0;
+
+ f = xfopen(path, "wb");
+
algo->init_fn(&pack_ctx);
/* Write pack header */
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 08/11] test-tool synthesize: precompute pack for 4 GiB + 1
2026-05-04 17:08 ` [PATCH v2 08/11] test-tool synthesize: precompute pack for 4 GiB + 1 Johannes Schindelin via GitGitGadget
@ 2026-05-04 18:27 ` Derrick Stolee
2026-05-05 20:54 ` Johannes Schindelin
0 siblings, 1 reply; 60+ messages in thread
From: Derrick Stolee @ 2026-05-04 18:27 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget, git
Cc: Torsten Bögershausen, Jeff King, Johannes Schindelin
On 5/4/2026 1:08 PM, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> Benchmarks generating a 4 GiB + 1 pack (3 runs each, SHA1DC on
> x86_64):
>
> generic path: 88s / 81s / 140s
> fast path: 14s / 13s / 15s
>
> On CI, where t5608 currently takes 200-850 seconds depending on the
> job, the fast path cuts the pack-generation phase from minutes to
> seconds, leaving only the clone operations themselves.
Are these numbers accurate for the patch position in the series?
The previous change replaced SHA1DC with the unsafe version, which
gained similar performance improvements. I'd be interested to see
the numbers for both enabled at the same time.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 08/11] test-tool synthesize: precompute pack for 4 GiB + 1
2026-05-04 18:27 ` Derrick Stolee
@ 2026-05-05 20:54 ` Johannes Schindelin
0 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin @ 2026-05-05 20:54 UTC (permalink / raw)
To: Derrick Stolee
Cc: Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King
Hi Stolee,
On Tue, 5 May 2026, Derrick Stolee wrote:
> On 5/4/2026 1:08 PM, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> > Benchmarks generating a 4 GiB + 1 pack (3 runs each, SHA1DC on
> > x86_64):
> >
> > generic path: 88s / 81s / 140s
> > fast path: 14s / 13s / 15s
> >
> > On CI, where t5608 currently takes 200-850 seconds depending on the
> > job, the fast path cuts the pack-generation phase from minutes to
> > seconds, leaving only the clone operations themselves.
>
> Are these numbers accurate for the patch position in the series?
Unfortunately, yes.
> The previous change replaced SHA1DC with the unsafe version, which
> gained similar performance improvements. I'd be interested to see
> the numbers for both enabled at the same time.
The problem is that in many (most?) cases, the "unsafe" version is the
_same_ as the safe version, i.e. SHA1DC. Only the `linux-TEST-vars` job on
CI (and most notably, _not_ in the `win-*` jobs) has a fast "unsafe"
version by default. So if I build the revision as per the previous patch
on Windows, I get no performance benefit whatsoever. That's what my lament
about not being able to link OpenSSL was all about.
Ciao,
Johannes
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 09/11] test-tool synthesize: add precomputed SHA-256 pack for 4 GiB + 1
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
` (7 preceding siblings ...)
2026-05-04 17:08 ` [PATCH v2 08/11] test-tool synthesize: precompute pack for 4 GiB + 1 Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 10/11] t5608: mark >4GB tests as EXPENSIVE Johannes Schindelin via GitGitGadget
` (2 subsequent siblings)
11 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Add a SHA-256 entry to the fast_packs[] table. The pack prefix and
deflate block structure are identical to SHA-1 (the pack format does
not encode the hash algorithm in its header). Only the suffix differs:
SHA-256 OIDs are 32 bytes instead of 20, giving a 609-byte suffix
compared to 513 for SHA-1, and a different pack checksum.
The constants were generated by running the generic path inside a
repository initialized with --object-format=sha256.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-synthesize.c | 91 ++++++++++++++++++++++++++++++++++++++
1 file changed, 91 insertions(+)
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
index 83c40ee02a..1f28ecf0f2 100644
--- a/t/helper/test-synthesize.c
+++ b/t/helper/test-synthesize.c
@@ -246,6 +246,90 @@ static const unsigned char fast_pack_sha1_suffix[] = {
0xe3
};
+/*
+ * SHA-256 suffix: same structure, but with 32-byte OIDs and SHA-256
+ * pack checksum (609 bytes vs 513 for SHA-1).
+ */
+static const unsigned char fast_pack_sha256_suffix[] = {
+ 0xac, 0x02, 0x78, 0x01, 0x01, 0x2c, 0x00, 0xd3,
+ 0xff, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20,
+ 0x66, 0x69, 0x6c, 0x65, 0x00, 0x42, 0x53, 0xc1,
+ 0x8a, 0x9f, 0x5e, 0xc3, 0xbb, 0x47, 0xb0, 0x83,
+ 0x8a, 0x19, 0xdb, 0x31, 0xbb, 0x7b, 0x0f, 0x3b,
+ 0x80, 0xa4, 0xbc, 0x2f, 0xaf, 0x72, 0x6b, 0xdb,
+ 0x62, 0xaa, 0xba, 0xdd, 0xde, 0x77, 0xc6, 0x13,
+ 0xeb, 0x9d, 0x0c, 0x78, 0x01, 0x01, 0xcd, 0x00,
+ 0x32, 0xff, 0x74, 0x72, 0x65, 0x65, 0x20, 0x62,
+ 0x36, 0x30, 0x39, 0x37, 0x37, 0x64, 0x37, 0x63,
+ 0x34, 0x63, 0x32, 0x64, 0x31, 0x65, 0x63, 0x63,
+ 0x33, 0x66, 0x62, 0x61, 0x31, 0x64, 0x39, 0x38,
+ 0x65, 0x65, 0x31, 0x32, 0x30, 0x61, 0x64, 0x63,
+ 0x32, 0x34, 0x38, 0x33, 0x34, 0x39, 0x35, 0x30,
+ 0x62, 0x65, 0x34, 0x31, 0x32, 0x64, 0x39, 0x34,
+ 0x63, 0x38, 0x30, 0x39, 0x34, 0x38, 0x30, 0x66,
+ 0x35, 0x38, 0x62, 0x61, 0x39, 0x64, 0x61, 0x0a,
+ 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x20, 0x41,
+ 0x20, 0x55, 0x20, 0x54, 0x68, 0x6f, 0x72, 0x20,
+ 0x3c, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x40,
+ 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e,
+ 0x63, 0x6f, 0x6d, 0x3e, 0x20, 0x31, 0x32, 0x33,
+ 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x20,
+ 0x2b, 0x30, 0x30, 0x30, 0x30, 0x0a, 0x63, 0x6f,
+ 0x6d, 0x6d, 0x69, 0x74, 0x74, 0x65, 0x72, 0x20,
+ 0x43, 0x20, 0x4f, 0x20, 0x4d, 0x69, 0x74, 0x74,
+ 0x65, 0x72, 0x20, 0x3c, 0x63, 0x6f, 0x6d, 0x6d,
+ 0x69, 0x74, 0x74, 0x65, 0x72, 0x40, 0x65, 0x78,
+ 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f,
+ 0x6d, 0x3e, 0x20, 0x31, 0x32, 0x33, 0x34, 0x35,
+ 0x36, 0x37, 0x38, 0x39, 0x30, 0x20, 0x2b, 0x30,
+ 0x30, 0x30, 0x30, 0x0a, 0x0a, 0x4c, 0x61, 0x72,
+ 0x67, 0x65, 0x20, 0x62, 0x6c, 0x6f, 0x62, 0x20,
+ 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x0a, 0xb7,
+ 0x80, 0x3d, 0xd7, 0x20, 0x78, 0x01, 0x01, 0x00,
+ 0x00, 0xff, 0xff, 0x00, 0x00, 0x00, 0x01, 0x95,
+ 0x11, 0x78, 0x01, 0x01, 0x15, 0x01, 0xea, 0xfe,
+ 0x74, 0x72, 0x65, 0x65, 0x20, 0x36, 0x65, 0x66,
+ 0x31, 0x39, 0x62, 0x34, 0x31, 0x32, 0x32, 0x35,
+ 0x63, 0x35, 0x33, 0x36, 0x39, 0x66, 0x31, 0x63,
+ 0x31, 0x30, 0x34, 0x64, 0x34, 0x35, 0x64, 0x38,
+ 0x64, 0x38, 0x35, 0x65, 0x66, 0x61, 0x39, 0x62,
+ 0x30, 0x35, 0x37, 0x62, 0x35, 0x33, 0x62, 0x31,
+ 0x34, 0x62, 0x34, 0x62, 0x39, 0x62, 0x39, 0x33,
+ 0x39, 0x64, 0x64, 0x37, 0x34, 0x64, 0x65, 0x63,
+ 0x63, 0x35, 0x33, 0x32, 0x31, 0x0a, 0x70, 0x61,
+ 0x72, 0x65, 0x6e, 0x74, 0x20, 0x37, 0x35, 0x62,
+ 0x66, 0x30, 0x63, 0x34, 0x37, 0x61, 0x65, 0x34,
+ 0x62, 0x62, 0x33, 0x30, 0x38, 0x65, 0x37, 0x63,
+ 0x63, 0x32, 0x34, 0x38, 0x32, 0x65, 0x32, 0x32,
+ 0x65, 0x66, 0x61, 0x65, 0x33, 0x37, 0x38, 0x37,
+ 0x61, 0x39, 0x36, 0x38, 0x34, 0x38, 0x62, 0x64,
+ 0x31, 0x37, 0x34, 0x39, 0x35, 0x36, 0x37, 0x31,
+ 0x34, 0x37, 0x31, 0x35, 0x32, 0x34, 0x36, 0x64,
+ 0x64, 0x62, 0x64, 0x35, 0x34, 0x0a, 0x61, 0x75,
+ 0x74, 0x68, 0x6f, 0x72, 0x20, 0x41, 0x20, 0x55,
+ 0x20, 0x54, 0x68, 0x6f, 0x72, 0x20, 0x3c, 0x61,
+ 0x75, 0x74, 0x68, 0x6f, 0x72, 0x40, 0x65, 0x78,
+ 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f,
+ 0x6d, 0x3e, 0x20, 0x31, 0x32, 0x33, 0x34, 0x35,
+ 0x36, 0x37, 0x38, 0x39, 0x30, 0x20, 0x2b, 0x30,
+ 0x30, 0x30, 0x30, 0x0a, 0x63, 0x6f, 0x6d, 0x6d,
+ 0x69, 0x74, 0x74, 0x65, 0x72, 0x20, 0x43, 0x20,
+ 0x4f, 0x20, 0x4d, 0x69, 0x74, 0x74, 0x65, 0x72,
+ 0x20, 0x3c, 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74,
+ 0x74, 0x65, 0x72, 0x40, 0x65, 0x78, 0x61, 0x6d,
+ 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0x3e,
+ 0x20, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+ 0x38, 0x39, 0x30, 0x20, 0x2b, 0x30, 0x30, 0x30,
+ 0x30, 0x0a, 0x0a, 0x45, 0x6d, 0x70, 0x74, 0x79,
+ 0x20, 0x74, 0x72, 0x65, 0x65, 0x20, 0x63, 0x6f,
+ 0x6d, 0x6d, 0x69, 0x74, 0x0a, 0x6d, 0x6d, 0x51,
+ 0x9a, 0xc9, 0x11, 0x76, 0x61, 0xa3, 0x89, 0x49,
+ 0xb7, 0xa1, 0x58, 0xc6, 0x1d, 0x8c, 0x33, 0x75,
+ 0x8d, 0x7e, 0x4d, 0x8e, 0x58, 0x91, 0xf8, 0x5c,
+ 0x57, 0xd9, 0x89, 0x9e, 0xb8, 0xd2, 0x9a, 0xd8,
+ 0xc9
+};
+
static const struct fast_pack fast_packs[] = {
{
.format_id = GIT_SHA1_FORMAT_ID,
@@ -253,6 +337,13 @@ static const struct fast_pack fast_packs[] = {
.suffix_len = sizeof(fast_pack_sha1_suffix),
.commit_oid = "aac43daf40d0377af31aa9c798a4ae8a31b55c1d",
},
+ {
+ .format_id = GIT_SHA256_FORMAT_ID,
+ .suffix = fast_pack_sha256_suffix,
+ .suffix_len = sizeof(fast_pack_sha256_suffix),
+ .commit_oid = "63c46ca51267b1d45be69a044bb84b4bf0559f09"
+ "d727f861d2ae94ddebdddbc9",
+ },
};
/*
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 10/11] t5608: mark >4GB tests as EXPENSIVE
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
` (8 preceding siblings ...)
2026-05-04 17:08 ` [PATCH v2 09/11] test-tool synthesize: add precomputed SHA-256 " Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-04 17:08 ` [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
11 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Even with precomputed pack constants that reduced the helper's
runtime from minutes to seconds, the >4GB clone tests still take
200-850 seconds across CI jobs. The bottleneck is no longer the
pack generation but the clone operations themselves: transporting,
unpacking, and indexing 4 GiB of data through unpack-objects and
index-pack is inherently expensive.
As Jeff King pointed out [1], t5608 alone takes 160 seconds on his
laptop while the rest of the entire test suite finishes in under 90
seconds, and the test's disk footprint (4+ GiB source repo, then
two clones) is problematic for developers who use RAM disks for
their trash directories.
Gate the >4GB tests on the EXPENSIVE prereq (which requires
GIT_TEST_LONG to be set) in addition to SIZE_T_IS_64BIT, keeping
them out of normal local test runs.
[1] https://lore.kernel.org/git/20260501063805.GA2038915@coredump.intra.peff.net/
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t5608-clone-2gb.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
index af93302dde..4f8a95ddda 100755
--- a/t/t5608-clone-2gb.sh
+++ b/t/t5608-clone-2gb.sh
@@ -49,7 +49,7 @@ test_expect_success 'clone - with worktree, file:// protocol' '
'
-test_expect_success SIZE_T_IS_64BIT 'set up repo with >4GB object' '
+test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'set up repo with >4GB object' '
large_blob_size=$((4*1024*1024*1024+1)) &&
git init --bare 4gb-repo &&
head_oid=$(test-tool synthesize pack \
@@ -60,7 +60,7 @@ test_expect_success SIZE_T_IS_64BIT 'set up repo with >4GB object' '
git -C 4gb-repo symbolic-ref HEAD refs/heads/main
'
-test_expect_success SIZE_T_IS_64BIT 'clone >4GB object via unpack-objects' '
+test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'clone >4GB object via unpack-objects' '
# The synthesized pack has five objects, so a large unpack limit keeps
# fetch-pack on the unpack-objects path.
git -c fetch.unpackLimit=100 clone --bare \
@@ -76,7 +76,7 @@ test_expect_success SIZE_T_IS_64BIT 'clone >4GB object via unpack-objects' '
test "$source_blob" = "$clone_blob"
'
-test_expect_success SIZE_T_IS_64BIT 'clone with >4GB object via index-pack' '
+test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'clone with >4GB object via index-pack' '
# Force fetch-pack to hand the pack to index-pack instead.
git -c fetch.unpackLimit=1 clone --bare \
"file://$(pwd)/4gb-repo" 4gb-clone-index &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
` (9 preceding siblings ...)
2026-05-04 17:08 ` [PATCH v2 10/11] t5608: mark >4GB tests as EXPENSIVE Johannes Schindelin via GitGitGadget
@ 2026-05-04 17:08 ` Johannes Schindelin via GitGitGadget
2026-05-04 18:35 ` Derrick Stolee
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
11 siblings, 1 reply; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-04 17:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Derrick Stolee suggested [1] that expensive tests should be run at a
regular cadence rather than on every PR iteration. Gate GIT_TEST_LONG
on push builds to the integration branches (next, master, main, maint)
so that the EXPENSIVE prereq is satisfied there but not during PR
validation, where the extra minutes of wall-clock time do not justify
themselves.
[1] https://lore.kernel.org/git/e1e8837f-7374-4079-ba87-ab95dd156e33@gmail.com/
Helped-by: Derrick Stolee <derrickstolee@github.com>
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
ci/lib.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/ci/lib.sh b/ci/lib.sh
index 42a2b6a318..a671994bdf 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -314,6 +314,15 @@ export DEFAULT_TEST_TARGET=prove
export GIT_TEST_CLONE_2GB=true
export SKIP_DASHED_BUILT_INS=YesPlease
+# Enable expensive tests on push builds to integration branches, but
+# not on PR builds where the extra time is not justified for every
+# iteration.
+case "$GITHUB_EVENT_NAME,$CI_BRANCH" in
+push,*next*|push,*master*|push,*main*|push,*maint*)
+ export GIT_TEST_LONG=YesPlease
+ ;;
+esac
+
case "$distro" in
ubuntu-*)
# Python 2 is end of life, and Ubuntu 23.04 and newer don't actually
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
2026-05-04 17:08 ` [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches Johannes Schindelin via GitGitGadget
@ 2026-05-04 18:35 ` Derrick Stolee
2026-05-05 12:56 ` Junio C Hamano
0 siblings, 1 reply; 60+ messages in thread
From: Derrick Stolee @ 2026-05-04 18:35 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget, git
Cc: Torsten Bögershausen, Jeff King, Johannes Schindelin
On 5/4/2026 1:08 PM, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Derrick Stolee suggested [1] that expensive tests should be run at a
> regular cadence rather than on every PR iteration. Gate GIT_TEST_LONG
> on push builds to the integration branches (next, master, main, maint)
> so that the EXPENSIVE prereq is satisfied there but not during PR
> validation, where the extra minutes of wall-clock time do not justify
> themselves.
I like that this will be run as part of regular updates to the
important branches. The important bit after that is whether or
not a human pays attention to the signal of these builds.
Junio: Do you pay attention to CI breaks when you push to
'master'?
One way to help this procedure could be to have GitHub CI
failures trigger new issues, which could then be more easily
viewed and noticed by the community watching the repo. This
is of course out-of-scope for this patch series, but could be
considered in the future.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
2026-05-04 18:35 ` Derrick Stolee
@ 2026-05-05 12:56 ` Junio C Hamano
2026-05-05 23:07 ` Junio C Hamano
2026-05-08 2:50 ` Junio C Hamano
0 siblings, 2 replies; 60+ messages in thread
From: Junio C Hamano @ 2026-05-05 12:56 UTC (permalink / raw)
To: Derrick Stolee
Cc: Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King, Johannes Schindelin
Derrick Stolee <stolee@gmail.com> writes:
> On 5/4/2026 1:08 PM, Johannes Schindelin via GitGitGadget wrote:
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> Derrick Stolee suggested [1] that expensive tests should be run at a
>> regular cadence rather than on every PR iteration. Gate GIT_TEST_LONG
>> on push builds to the integration branches (next, master, main, maint)
>> so that the EXPENSIVE prereq is satisfied there but not during PR
>> validation, where the extra minutes of wall-clock time do not justify
>> themselves.
> I like that this will be run as part of regular updates to the
> important branches. The important bit after that is whether or
> not a human pays attention to the signal of these builds.
>
> Junio: Do you pay attention to CI breaks when you push to
> 'master'?
Well, it is way too late to notice breakage when the faulty update
hits 'master'. CI failures should be noticed before breakage hits
'next'.
I often notice and complain when I see failures on 'seen', and
sometimes I help original submitter by bisecting, but I do not
necessarily have enough time and bandwidth to help everybody.
Quite honestly, the best place to give widest test coverage is much
closer to the source of the problems than in my tree and mixed with
other topics, i.e., at individual contributor's CI. That way, I
presume that GitGitGadget can also help submitters avoid sending a
faulty series, reducing the load on the list and the maintainer.
Ideally the CI tests by the integrator should only be catching any
mismerges and unexpected inter-topic interactions, as they cannot be
caught by contributor's standalone tests, so I do not mind widening
coverage of CI tests when I push the integration results out. But
so far, the majority of what I have seen and reported back to the
list have been something that the authors should be equipped to spot
in their topic without getting mixed with other topics into any
integration branches.
> One way to help this procedure could be to have GitHub CI
> failures trigger new issues, which could then be more easily
> viewed and noticed by the community watching the repo. This
> is of course out-of-scope for this patch series, but could be
> considered in the future.
I think a better way to help would be to arrange the workflow so
that we do not even have to trigger an issue, and stop before the
patches leave the original authors' hand. They can of course ask
for help saying "here is my topic in my fork of the repository and
failing in this way for macOS that I do not have access to. Could
anybody help me figuring out what macOS peculiarity my changes are
tickling?", or something like that.
It would be best to find problems early, and make it easier for
individual contributors to help each other by having a concrete CI
failure reports in their forks that they can point at when they ask
for help. And CI run when I push 'seen' or 'master' out would not
help as much as CI run when they publish their forked branches would.
By the way, please expect slow responses as I am (officially) still
mostly offline for the rest of the week.
Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
2026-05-05 12:56 ` Junio C Hamano
@ 2026-05-05 23:07 ` Junio C Hamano
2026-05-06 8:33 ` Johannes Schindelin
2026-05-08 2:50 ` Junio C Hamano
1 sibling, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2026-05-05 23:07 UTC (permalink / raw)
To: Derrick Stolee
Cc: Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King, Johannes Schindelin
(in GMail web interface, excuse typos)
https://github.com/git/git/actions/runs/25366120610/job/74377320625
We seem to be hitting the same _Generic error in various (but not all) jobs
/usr/include/x86_64-linux-gnu/sys/cdefs.h:838:3: note: expanded from
macro '__glibc_const_generic'
838 | _Generic (0 ? (PTR) : (void *) 1, \
| ^
Error: list-objects-filter-options.c:222:10: '_Generic' is a C11
extension [-Werror,-Wc11-extensions]
I thought we updated the codebase to avoid stripping away constness
with strchr() and friends, but the error seems to be more like one
hand in the system passing -Wc11-extensions to stick to older version
of C and the other hand in the system that uses _Generic to implement
the const/non-const variants of strchr() in the system header not
knowing that the other tells C11 const-preserving strchr() should not
be used?
2026年5月5日(火) 21:56 Junio C Hamano <gitster@pobox.com>:
>
> Derrick Stolee <stolee@gmail.com> writes:
>
> > On 5/4/2026 1:08 PM, Johannes Schindelin via GitGitGadget wrote:
> >> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >>
> >> Derrick Stolee suggested [1] that expensive tests should be run at a
> >> regular cadence rather than on every PR iteration. Gate GIT_TEST_LONG
> >> on push builds to the integration branches (next, master, main, maint)
> >> so that the EXPENSIVE prereq is satisfied there but not during PR
> >> validation, where the extra minutes of wall-clock time do not justify
> >> themselves.
> > I like that this will be run as part of regular updates to the
> > important branches. The important bit after that is whether or
> > not a human pays attention to the signal of these builds.
> >
> > Junio: Do you pay attention to CI breaks when you push to
> > 'master'?
>
> Well, it is way too late to notice breakage when the faulty update
> hits 'master'. CI failures should be noticed before breakage hits
> 'next'.
>
> I often notice and complain when I see failures on 'seen', and
> sometimes I help original submitter by bisecting, but I do not
> necessarily have enough time and bandwidth to help everybody.
>
> Quite honestly, the best place to give widest test coverage is much
> closer to the source of the problems than in my tree and mixed with
> other topics, i.e., at individual contributor's CI. That way, I
> presume that GitGitGadget can also help submitters avoid sending a
> faulty series, reducing the load on the list and the maintainer.
>
> Ideally the CI tests by the integrator should only be catching any
> mismerges and unexpected inter-topic interactions, as they cannot be
> caught by contributor's standalone tests, so I do not mind widening
> coverage of CI tests when I push the integration results out. But
> so far, the majority of what I have seen and reported back to the
> list have been something that the authors should be equipped to spot
> in their topic without getting mixed with other topics into any
> integration branches.
>
> > One way to help this procedure could be to have GitHub CI
> > failures trigger new issues, which could then be more easily
> > viewed and noticed by the community watching the repo. This
> > is of course out-of-scope for this patch series, but could be
> > considered in the future.
>
> I think a better way to help would be to arrange the workflow so
> that we do not even have to trigger an issue, and stop before the
> patches leave the original authors' hand. They can of course ask
> for help saying "here is my topic in my fork of the repository and
> failing in this way for macOS that I do not have access to. Could
> anybody help me figuring out what macOS peculiarity my changes are
> tickling?", or something like that.
>
> It would be best to find problems early, and make it easier for
> individual contributors to help each other by having a concrete CI
> failure reports in their forks that they can point at when they ask
> for help. And CI run when I push 'seen' or 'master' out would not
> help as much as CI run when they publish their forked branches would.
>
> By the way, please expect slow responses as I am (officially) still
> mostly offline for the rest of the week.
>
> Thanks.
>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
2026-05-05 23:07 ` Junio C Hamano
@ 2026-05-06 8:33 ` Johannes Schindelin
2026-05-07 9:18 ` Junio C Hamano
0 siblings, 1 reply; 60+ messages in thread
From: Johannes Schindelin @ 2026-05-06 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Derrick Stolee, Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King, Patrick Steinhardt
Hi Junio,
On Wed, 6 May 2026, Junio C Hamano wrote:
> https://github.com/git/git/actions/runs/25366120610/job/74377320625
>
> We seem to be hitting the same _Generic error in various (but not all) jobs
>
> /usr/include/x86_64-linux-gnu/sys/cdefs.h:838:3: note: expanded from
> macro '__glibc_const_generic'
> 838 | _Generic (0 ? (PTR) : (void *) 1, \
> | ^
> Error: list-objects-filter-options.c:222:10: '_Generic' is a C11
> extension [-Werror,-Wc11-extensions]
>
> I thought we updated the codebase to avoid stripping away constness
> with strchr() and friends, but the error seems to be more like one
> hand in the system passing -Wc11-extensions to stick to older version
> of C and the other hand in the system that uses _Generic to implement
> the const/non-const variants of strchr() in the system header not
> knowing that the other tells C11 const-preserving strchr() should not
> be used?
This was diagnosed (with a proposed fix) by Patrick over in
https://lore.kernel.org/git/20260505-b4-pks-ci-tolerate-glibc-generic-v1-1-5786386fe512@pks.im/.
tl;dr It's not about `const`-ness at all, but about glibc using a C11
construct which clang's strict c99 checker now refuses, thanks to the
upgrade to Ubuntu 26.04 in the `ubuntu:rolling` runners.
Ciao,
Johannes
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
2026-05-06 8:33 ` Johannes Schindelin
@ 2026-05-07 9:18 ` Junio C Hamano
2026-05-07 10:24 ` Patrick Steinhardt
0 siblings, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2026-05-07 9:18 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Derrick Stolee, Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King, Patrick Steinhardt
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> I thought we updated the codebase to avoid stripping away constness
>> with strchr() and friends, but the error seems to be more like one
>> hand in the system passing -Wc11-extensions to stick to older version
>> of C and the other hand in the system that uses _Generic to implement
>> the const/non-const variants of strchr() in the system header not
>> knowing that the other tells C11 const-preserving strchr() should not
>> be used?
>
> This was diagnosed (with a proposed fix) by Patrick over in
> https://lore.kernel.org/git/20260505-b4-pks-ci-tolerate-glibc-generic-v1-1-5786386fe512@pks.im/.
Indeed.
> tl;dr It's not about `const`-ness at all, but about glibc using a C11
> construct which clang's strict c99 checker now refuses, thanks to the
> upgrade to Ubuntu 26.04 in the `ubuntu:rolling` runners.
Yes, that is exactly what I meant by one hand knowing that it was
told not to use c11 extensions while the other hand ignoring and
always using c11 extensions in the header. I recall that in the
past gnu library headers were a bit more careful to make the life
more pleasant when we use (or decline to use) various features by
using conditional compilation, but apparently not this case.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
2026-05-07 9:18 ` Junio C Hamano
@ 2026-05-07 10:24 ` Patrick Steinhardt
0 siblings, 0 replies; 60+ messages in thread
From: Patrick Steinhardt @ 2026-05-07 10:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Derrick Stolee,
Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King
On Thu, May 07, 2026 at 06:18:34PM +0900, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> I thought we updated the codebase to avoid stripping away constness
> >> with strchr() and friends, but the error seems to be more like one
> >> hand in the system passing -Wc11-extensions to stick to older version
> >> of C and the other hand in the system that uses _Generic to implement
> >> the const/non-const variants of strchr() in the system header not
> >> knowing that the other tells C11 const-preserving strchr() should not
> >> be used?
> >
> > This was diagnosed (with a proposed fix) by Patrick over in
> > https://lore.kernel.org/git/20260505-b4-pks-ci-tolerate-glibc-generic-v1-1-5786386fe512@pks.im/.
>
> Indeed.
>
> > tl;dr It's not about `const`-ness at all, but about glibc using a C11
> > construct which clang's strict c99 checker now refuses, thanks to the
> > upgrade to Ubuntu 26.04 in the `ubuntu:rolling` runners.
>
> Yes, that is exactly what I meant by one hand knowing that it was
> told not to use c11 extensions while the other hand ignoring and
> always using c11 extensions in the header. I recall that in the
> past gnu library headers were a bit more careful to make the life
> more pleasant when we use (or decline to use) various features by
> using conditional compilation, but apparently not this case.
Yeah, it's a bit unfortunate indeed. I'd claim that this is a plain bug
though -- as mentioned in the commit message, I think what glibc should
have used is `_has_feature()` instead of `_has_extension()`, and if so,
I think the issue wouldn't exist.
But oh, well.
Patrick
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
2026-05-05 12:56 ` Junio C Hamano
2026-05-05 23:07 ` Junio C Hamano
@ 2026-05-08 2:50 ` Junio C Hamano
1 sibling, 0 replies; 60+ messages in thread
From: Junio C Hamano @ 2026-05-08 2:50 UTC (permalink / raw)
To: Derrick Stolee
Cc: Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King, Johannes Schindelin
Junio C Hamano <gitster@pobox.com> writes:
> Derrick Stolee <stolee@gmail.com> writes:
>
>> On 5/4/2026 1:08 PM, Johannes Schindelin via GitGitGadget wrote:
>>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>>
>>> Derrick Stolee suggested [1] that expensive tests should be run at a
>>> regular cadence rather than on every PR iteration. Gate GIT_TEST_LONG
>>> on push builds to the integration branches (next, master, main, maint)
>>> so that the EXPENSIVE prereq is satisfied there but not during PR
>>> validation, where the extra minutes of wall-clock time do not justify
>>> themselves.
>> I like that this will be run as part of regular updates to the
>> important branches. The important bit after that is whether or
>> not a human pays attention to the signal of these builds.
>>
>> Junio: Do you pay attention to CI breaks when you push to
>> 'master'?
>
> Well, it is way too late to notice breakage when the faulty update
> hits 'master'. CI failures should be noticed before breakage hits
> 'next'.
>
> I often notice and complain when I see failures on 'seen', and
> sometimes I help original submitter by bisecting, but I do not
> necessarily have enough time and bandwidth to help everybody.
To more directly answer your question, yes I do pay attention, but
not only when I push to 'master', but when I push to any of the
integration branches. I pay most attention to breakage in 'seen',
so that I can notify authors of new topics early. Sometimes you may
see many pushes only to 'seen' at github.com/git/git while 'seen' on
the other hosting sites are not updated with these commits, and if
you notice them, you caught me bisecting the breakage on it. This
is so that I can eject offending topics and notify the author.
But this does not scale, and I shouldn't have to do it myself.
Making sure the topics come in a shape that they pass the tests
before they hit my tree is one way to reduce the need for the
maintainer bottleneck.
> It would be best to find problems early, and make it easier for
> individual contributors to help each other by having a concrete CI
> failure reports in their forks that they can point at when they ask
> for help. And CI run when I push 'seen' or 'master' out would not
> help as much as CI run when they publish their forked branches would.
>
> By the way, please expect slow responses as I am (officially) still
> mostly offline for the rest of the week.
>
> Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows
2026-05-04 17:08 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
` (10 preceding siblings ...)
2026-05-04 17:08 ` [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 01/11] index-pack, unpack-objects: use size_t for object size Johannes Schindelin via GitGitGadget
` (10 more replies)
11 siblings, 11 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin
On Windows, unsigned long is 32-bit even on 64-bit systems. This causes
multiple problems when Git handles objects larger than 4GB. This patch
series is a very targeted fix for a very early part of the problem: it
addresses the most fundamental truncation points that prevent a >4GB object
from surviving a clone at all.
Specifically, this fixes:
* zlib's uLong wrapping and triggering BUG() assertions in the git_zstream
wrapper
* Object sizes being truncated in pack streaming, delta headers, and
index-pack/unpack-objects
* pack-objects re-encoding reused pack entries with a truncated size,
producing corrupt packs on the wire
Many other code paths still use unsigned long for object sizes (e.g.,
cat-file -s, object_info.sizep, the delta machinery) and will need their own
conversions. This series does not attempt to fix those.
Based on work by @LordKiRon in git-for-windows/git#6076.
For testing, add a test helper that synthesizes a pack with a >4GB blob and
regression tests that clone it via both the unpack-objects and index-pack
code paths using file:// transport. Since these test cases are quite slow
(even after optimizing the pack generation part, the git clone test has no
chance but to hash 2x4GB of data), they are marked as EXPENSIVE. To ensure
that they are passing well in advance of any release, the CI is changed to
run them in the CI builds of relatively infrequent integration branch
updates.
Changes since v2:
* Now uses the proper data type for the varint decoding value (thanks,
Torsten!)
* The callers that now would silently narrow size_t to unsigned long
properly check and error out instead (thanks, Torsten!)
Changes since v1:
* dramatically accelerated the test helper that generates 4GB pack files,
via two separate strategies:
1. using the "unsafe" SHA-1 for the blob OID computation.
2. using pre-computed "Lego blocks" to construct the 4GB packs needed in
the test cases, where the size (and therefore the involved OIDs) are
well-known in advance.
* even with these improvements, the actual git clone is still slow (of
course, because it cannot use any of those shortcuts), therefore the
tests are marked as EXPENSIVE.
* to exercise those tests nevertheless, the last patch lets all EXPENSIVE
test cases be run for the integration branches other than seen.
Johannes Schindelin (11):
index-pack, unpack-objects: use size_t for object size
git-zlib: handle data streams larger than 4GB
odb, packfile: use size_t for streaming object sizes
delta, packfile: use size_t for delta header sizes
test-tool: add a helper to synthesize large packfiles
t5608: add regression test for >4GB object clone
test-tool synthesize: use the unsafe hash for speed
test-tool synthesize: precompute pack for 4 GiB + 1
test-tool synthesize: add precomputed SHA-256 pack for 4 GiB + 1
t5608: mark >4GB tests as EXPENSIVE
ci: run expensive tests on push builds to integration branches
Makefile | 1 +
builtin/index-pack.c | 8 +-
builtin/pack-objects.c | 34 ++-
builtin/unpack-objects.c | 4 +-
ci/lib.sh | 9 +
compat/zlib-compat.h | 2 +
delta.h | 14 +-
git-zlib.c | 25 +-
git-zlib.h | 4 +-
object-file.c | 12 +-
odb/streaming.c | 13 +-
odb/streaming.h | 2 +-
oss-fuzz/fuzz-pack-headers.c | 2 +-
pack-bitmap.c | 2 +-
pack-check.c | 6 +-
packfile.c | 57 ++--
packfile.h | 4 +-
t/helper/meson.build | 1 +
t/helper/test-synthesize.c | 541 +++++++++++++++++++++++++++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t5608-clone-2gb.sh | 37 +++
22 files changed, 724 insertions(+), 56 deletions(-)
create mode 100644 t/helper/test-synthesize.c
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2102%2Fdscho%2Ffix-large-clones-on-windows-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2102/dscho/fix-large-clones-on-windows-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2102
Range-diff vs v2:
1: dc660106ea ! 1: 311cdc601d index-pack, unpack-objects: use size_t for object size
@@ Commit message
64-bit platforms, and ensuring the shift arithmetic occurs in 64-bit
space.
+ Declare the per-byte continuation variable `c` as size_t as well,
+ matching the canonical varint decoder unpack_object_header_buffer()
+ in packfile.c. With c as size_t the expression (c & 0x7f) << shift
+ is naturally size_t-typed, so the explicit cast that an earlier
+ iteration carried at the use site is no longer needed.
+
+ While at it, add the same overflow guard that
+ unpack_object_header_buffer() carries: if the cumulative shift would
+ exceed bitsizeof(size_t) - 7, refuse the input rather than invoking
+ undefined behavior. Unlike unpack_object_header_buffer(), which
+ labels this case "bad object header", report it as the platform
+ limit it actually is: a header may be perfectly well-formed and
+ still encode a size we cannot represent locally (notably on a
+ 32-bit build consuming a packfile produced on a 64-bit host).
+
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
+ Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
## builtin/index-pack.c ##
@@ builtin/index-pack.c: static void *unpack_raw_entry(struct object_entry *obj,
{
unsigned char *p;
- unsigned long size, c;
-+ size_t size;
-+ unsigned long c;
++ size_t size, c;
off_t base_offset;
unsigned shift;
void *data;
@@ builtin/index-pack.c: static void *unpack_raw_entry(struct object_entry *obj,
+ size = (c & 15);
+ shift = 4;
+ while (c & 0x80) {
++ if ((bitsizeof(size_t) - 7) < shift)
++ die(_("object size too large for this platform"));
p = fill(1);
c = *p;
use(1);
-- size += (c & 0x7f) << shift;
-+ size += ((size_t)c & 0x7f) << shift;
- shift += 7;
- }
- obj->size = size;
## builtin/unpack-objects.c ##
@@ builtin/unpack-objects.c: static void unpack_one(unsigned nr)
@@ builtin/unpack-objects.c: static void unpack_one(unsigned nr)
unsigned shift;
unsigned char *pack;
- unsigned long size, c;
-+ size_t size;
-+ unsigned long c;
++ size_t size, c;
enum object_type type;
obj_list[nr].offset = consumed_bytes;
@@ builtin/unpack-objects.c: static void unpack_one(unsigned nr)
+ size = (c & 15);
+ shift = 4;
+ while (c & 0x80) {
++ if ((bitsizeof(size_t) - 7) < shift)
++ die(_("object size too large for this platform"));
pack = fill(1);
c = *pack;
use(1);
-- size += (c & 0x7f) << shift;
-+ size += ((size_t)c & 0x7f) << shift;
- shift += 7;
- }
-
2: 92f4327b1f = 2: c611913194 git-zlib: handle data streams larger than 4GB
3: 3a539061c5 ! 3: b789f57de9 odb, packfile: use size_t for streaming object sizes
@@ Commit message
temporary variables where the types differ, with comments noting the
truncation limitation for code paths that still use unsigned long.
+ Widening the producers to size_t in this way introduces a handful of
+ silent size_t -> unsigned long narrowings on Windows, all in
+ builtin/pack-objects.c, where the consumers are still typed
+ unsigned long. Make those narrowings explicit with
+ cast_size_t_to_ulong() so they assert loudly the moment an object
+ actually exceeds ULONG_MAX bytes:
+
+ - oe_get_size_slow() returns unsigned long but holds a size_t
+ locally; cast at the return.
+ - write_reuse_object() passes a size_t into check_pack_inflate(),
+ whose expect parameter is unsigned long; cast at the call.
+ - check_object() routes a size_t through SET_SIZE() and
+ SET_DELTA_SIZE(), both of which take unsigned long via
+ oe_set_size() / oe_set_delta_size(); cast at the three call
+ sites in the OBJ_OFS_DELTA / OBJ_REF_DELTA branches and in the
+ non-delta default arm.
+
+ The cast-only treatment is deliberately a stop-gap. Properly
+ widening oe_set_size, oe_get_size_slow's return type,
+ check_pack_inflate's expect parameter, object_info.sizep,
+ patch_delta, and the OE_SIZE_BITS bit-fields cascades into a series
+ that is too large to be reviewable, so the proper widening is
+ deferred to a follow-up topic. Until then,
+ cast_size_t_to_ulong() at least makes the truncation explicit at
+ the source: it documents the boundary, and on a 64-bit non-Windows
+ platform it is a no-op.
+
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
+ Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
## builtin/pack-objects.c ##
@@ builtin/pack-objects.c: static off_t write_reuse_object(struct hashfile *f, stru
if (DELTA(entry))
type = (allow_ofs_delta && DELTA(entry)->idx.offset) ?
+@@ builtin/pack-objects.c: static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,
+ datalen -= entry->in_pack_header_size;
+
+ if (!pack_to_stdout && p->index_version == 1 &&
+- check_pack_inflate(p, &w_curs, offset, datalen, entry_size)) {
++ check_pack_inflate(p, &w_curs, offset, datalen,
++ cast_size_t_to_ulong(entry_size))) {
+ error(_("corrupt packed object for %s"),
+ oid_to_hex(&entry->idx.oid));
+ unuse_pack(&w_curs);
@@ builtin/pack-objects.c: static void write_reused_pack_one(struct packed_git *reuse_packfile,
{
off_t offset, next, cur;
@@ builtin/pack-objects.c: static void check_object(struct object_entry *entry, uin
buf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);
+@@ builtin/pack-objects.c: static void check_object(struct object_entry *entry, uint32_t object_index)
+ default:
+ /* Not a delta hence we've already got all we need. */
+ oe_set_type(entry, entry->in_pack_type);
+- SET_SIZE(entry, in_pack_size);
++ SET_SIZE(entry, cast_size_t_to_ulong(in_pack_size));
+ entry->in_pack_header_size = used;
+ if (oe_type(entry) < OBJ_COMMIT || oe_type(entry) > OBJ_BLOB)
+ goto give_up;
+@@ builtin/pack-objects.c: static void check_object(struct object_entry *entry, uint32_t object_index)
+ if (have_base &&
+ can_reuse_delta(&base_ref, entry, &base_entry)) {
+ oe_set_type(entry, entry->in_pack_type);
+- SET_SIZE(entry, in_pack_size); /* delta size */
+- SET_DELTA_SIZE(entry, in_pack_size);
++ SET_SIZE(entry, cast_size_t_to_ulong(in_pack_size)); /* delta size */
++ SET_DELTA_SIZE(entry, cast_size_t_to_ulong(in_pack_size));
+
+ if (base_entry) {
+ SET_DELTA(entry, base_entry);
@@ builtin/pack-objects.c: unsigned long oe_get_size_slow(struct packing_data *pack,
struct pack_window *w_curs;
unsigned char *buf;
@@ builtin/pack-objects.c: unsigned long oe_get_size_slow(struct packing_data *pack
}
p = oe_in_pack(pack, e);
+@@ builtin/pack-objects.c: unsigned long oe_get_size_slow(struct packing_data *pack,
+
+ unuse_pack(&w_curs);
+ packing_data_unlock(&to_pack);
+- return size;
++ return cast_size_t_to_ulong(size);
+ }
+
+ static int try_delta(struct unpacked *trg, struct unpacked *src,
## object-file.c ##
@@ object-file.c: int odb_source_loose_read_object_stream(struct odb_read_stream **out,
4: 3274cba862 = 4: 8e87a4e71f delta, packfile: use size_t for delta header sizes
5: afa74a3a2b = 5: 34fec4a32d test-tool: add a helper to synthesize large packfiles
6: a3019888d8 = 6: 88f992903f t5608: add regression test for >4GB object clone
7: 859e93e7a9 = 7: 4f207c8a47 test-tool synthesize: use the unsafe hash for speed
8: 29b9a74e91 = 8: 2751c21c6e test-tool synthesize: precompute pack for 4 GiB + 1
9: 8e6e720804 = 9: 3a006d96c3 test-tool synthesize: add precomputed SHA-256 pack for 4 GiB + 1
10: 5b44410b2f = 10: 86c09af4f5 t5608: mark >4GB tests as EXPENSIVE
11: 1eaaa7fad7 = 11: 2159f6a271 ci: run expensive tests on push builds to integration branches
--
gitgitgadget
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH v3 01/11] index-pack, unpack-objects: use size_t for object size
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 02/11] git-zlib: handle data streams larger than 4GB Johannes Schindelin via GitGitGadget
` (9 subsequent siblings)
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When unpacking objects from a packfile, the object size is decoded
from a variable-length encoding. On platforms where unsigned long is
32-bit (such as Windows, even in 64-bit builds), the shift operation
overflows when decoding sizes larger than 4GB. The result is a
truncated size value, causing the unpacked object to be corrupted or
rejected.
Fix this by changing the size variable to size_t, which is 64-bit on
64-bit platforms, and ensuring the shift arithmetic occurs in 64-bit
space.
Declare the per-byte continuation variable `c` as size_t as well,
matching the canonical varint decoder unpack_object_header_buffer()
in packfile.c. With c as size_t the expression (c & 0x7f) << shift
is naturally size_t-typed, so the explicit cast that an earlier
iteration carried at the use site is no longer needed.
While at it, add the same overflow guard that
unpack_object_header_buffer() carries: if the cumulative shift would
exceed bitsizeof(size_t) - 7, refuse the input rather than invoking
undefined behavior. Unlike unpack_object_header_buffer(), which
labels this case "bad object header", report it as the platform
limit it actually is: a header may be perfectly well-formed and
still encode a size we cannot represent locally (notably on a
32-bit build consuming a packfile produced on a 64-bit host).
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/index-pack.c | 8 +++++---
builtin/unpack-objects.c | 4 +++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index ca7784dc2c..2e4b42fa12 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -37,7 +37,7 @@ static const char index_pack_usage[] =
struct object_entry {
struct pack_idx_entry idx;
- unsigned long size;
+ size_t size;
unsigned char hdr_size;
signed char type;
signed char real_type;
@@ -469,7 +469,7 @@ static int is_delta_type(enum object_type type)
return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
}
-static void *unpack_entry_data(off_t offset, unsigned long size,
+static void *unpack_entry_data(off_t offset, size_t size,
enum object_type type, struct object_id *oid)
{
static char fixed_buf[8192];
@@ -524,7 +524,7 @@ static void *unpack_raw_entry(struct object_entry *obj,
struct object_id *oid)
{
unsigned char *p;
- unsigned long size, c;
+ size_t size, c;
off_t base_offset;
unsigned shift;
void *data;
@@ -539,6 +539,8 @@ static void *unpack_raw_entry(struct object_entry *obj,
size = (c & 15);
shift = 4;
while (c & 0x80) {
+ if ((bitsizeof(size_t) - 7) < shift)
+ die(_("object size too large for this platform"));
p = fill(1);
c = *p;
use(1);
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index e01cf6e360..76b3d0dee3 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -533,7 +533,7 @@ static void unpack_one(unsigned nr)
{
unsigned shift;
unsigned char *pack;
- unsigned long size, c;
+ size_t size, c;
enum object_type type;
obj_list[nr].offset = consumed_bytes;
@@ -545,6 +545,8 @@ static void unpack_one(unsigned nr)
size = (c & 15);
shift = 4;
while (c & 0x80) {
+ if ((bitsizeof(size_t) - 7) < shift)
+ die(_("object size too large for this platform"));
pack = fill(1);
c = *pack;
use(1);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 02/11] git-zlib: handle data streams larger than 4GB
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 01/11] index-pack, unpack-objects: use size_t for object size Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 03/11] odb, packfile: use size_t for streaming object sizes Johannes Schindelin via GitGitGadget
` (8 subsequent siblings)
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
On Windows, zlib's `uLong` type is 32-bit even on 64-bit systems. When
processing data streams larger than 4GB, the `total_in` and `total_out`
fields in zlib's `z_stream` structure wrap around, which caused the
sanity checks in `zlib_post_call()` to trigger `BUG()` assertions.
The git_zstream wrapper now tracks its own 64-bit totals rather than
copying them from zlib. The sanity checks compare only the low bits,
using `maximum_unsigned_value_of_type(uLong)` to mask appropriately for
the platform's `uLong` size.
This is based on work by LordKiRon in git-for-windows#6076.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
git-zlib.c | 25 +++++++++++++++++--------
git-zlib.h | 4 ++--
object-file.c | 2 +-
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/git-zlib.c b/git-zlib.c
index df9604910e..b91cb323ae 100644
--- a/git-zlib.c
+++ b/git-zlib.c
@@ -30,6 +30,9 @@ static const char *zerr_to_string(int status)
*/
/* #define ZLIB_BUF_MAX ((uInt)-1) */
#define ZLIB_BUF_MAX ((uInt) 1024 * 1024 * 1024) /* 1GB */
+
+/* uLong is 32-bit on Windows, even on 64-bit systems */
+#define ULONG_MAX_VALUE maximum_unsigned_value_of_type(uLong)
static inline uInt zlib_buf_cap(unsigned long len)
{
return (ZLIB_BUF_MAX < len) ? ZLIB_BUF_MAX : len;
@@ -39,31 +42,37 @@ static void zlib_pre_call(git_zstream *s)
{
s->z.next_in = s->next_in;
s->z.next_out = s->next_out;
- s->z.total_in = s->total_in;
- s->z.total_out = s->total_out;
+ s->z.total_in = (uLong)(s->total_in & ULONG_MAX_VALUE);
+ s->z.total_out = (uLong)(s->total_out & ULONG_MAX_VALUE);
s->z.avail_in = zlib_buf_cap(s->avail_in);
s->z.avail_out = zlib_buf_cap(s->avail_out);
}
static void zlib_post_call(git_zstream *s, int status)
{
- unsigned long bytes_consumed;
- unsigned long bytes_produced;
+ size_t bytes_consumed;
+ size_t bytes_produced;
bytes_consumed = s->z.next_in - s->next_in;
bytes_produced = s->z.next_out - s->next_out;
- if (s->z.total_out != s->total_out + bytes_produced)
+ /*
+ * zlib's total_out/total_in are uLong which may wrap for >4GB.
+ * We track our own totals and verify only the low bits match.
+ */
+ if ((s->z.total_out & ULONG_MAX_VALUE) !=
+ ((s->total_out + bytes_produced) & ULONG_MAX_VALUE))
BUG("total_out mismatch");
/*
* zlib does not update total_in when it returns Z_NEED_DICT,
* causing a mismatch here. Skip the sanity check in that case.
*/
if (status != Z_NEED_DICT &&
- s->z.total_in != s->total_in + bytes_consumed)
+ (s->z.total_in & ULONG_MAX_VALUE) !=
+ ((s->total_in + bytes_consumed) & ULONG_MAX_VALUE))
BUG("total_in mismatch");
- s->total_out = s->z.total_out;
- s->total_in = s->z.total_in;
+ s->total_out += bytes_produced;
+ s->total_in += bytes_consumed;
/* zlib-ng marks `next_in` as `const`, so we have to cast it away. */
s->next_in = (unsigned char *) s->z.next_in;
s->next_out = s->z.next_out;
diff --git a/git-zlib.h b/git-zlib.h
index 0e66fefa8c..44380e8ad3 100644
--- a/git-zlib.h
+++ b/git-zlib.h
@@ -7,8 +7,8 @@ typedef struct git_zstream {
struct z_stream_s z;
unsigned long avail_in;
unsigned long avail_out;
- unsigned long total_in;
- unsigned long total_out;
+ size_t total_in;
+ size_t total_out;
unsigned char *next_in;
unsigned char *next_out;
} git_zstream;
diff --git a/object-file.c b/object-file.c
index 2acc9522df..086b2b65ff 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1118,7 +1118,7 @@ int odb_source_loose_write_stream(struct odb_source *source,
} while (ret == Z_OK || ret == Z_BUF_ERROR);
if (stream.total_in != len + hdrlen)
- die(_("write stream object %ld != %"PRIuMAX), stream.total_in,
+ die(_("write stream object %"PRIuMAX" != %"PRIuMAX), (uintmax_t)stream.total_in,
(uintmax_t)len + hdrlen);
/*
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 03/11] odb, packfile: use size_t for streaming object sizes
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 01/11] index-pack, unpack-objects: use size_t for object size Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 02/11] git-zlib: handle data streams larger than 4GB Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 04/11] delta, packfile: use size_t for delta header sizes Johannes Schindelin via GitGitGadget
` (7 subsequent siblings)
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The odb_read_stream structure uses unsigned long for the size field,
which is 32-bit on Windows even in 64-bit builds. When streaming
objects larger than 4GB, the size would be truncated to zero or an
incorrect value, resulting in empty files being written to disk.
Change the size field in odb_read_stream to size_t and introduce
unpack_object_header_sz() to return sizes via size_t pointer. Since
object_info.sizep remains unsigned long for API compatibility, use
temporary variables where the types differ, with comments noting the
truncation limitation for code paths that still use unsigned long.
Widening the producers to size_t in this way introduces a handful of
silent size_t -> unsigned long narrowings on Windows, all in
builtin/pack-objects.c, where the consumers are still typed
unsigned long. Make those narrowings explicit with
cast_size_t_to_ulong() so they assert loudly the moment an object
actually exceeds ULONG_MAX bytes:
- oe_get_size_slow() returns unsigned long but holds a size_t
locally; cast at the return.
- write_reuse_object() passes a size_t into check_pack_inflate(),
whose expect parameter is unsigned long; cast at the call.
- check_object() routes a size_t through SET_SIZE() and
SET_DELTA_SIZE(), both of which take unsigned long via
oe_set_size() / oe_set_delta_size(); cast at the three call
sites in the OBJ_OFS_DELTA / OBJ_REF_DELTA branches and in the
non-delta default arm.
The cast-only treatment is deliberately a stop-gap. Properly
widening oe_set_size, oe_get_size_slow's return type,
check_pack_inflate's expect parameter, object_info.sizep,
patch_delta, and the OE_SIZE_BITS bit-fields cascades into a series
that is too large to be reviewable, so the proper widening is
deferred to a follow-up topic. Until then,
cast_size_t_to_ulong() at least makes the truncation explicit at
the source: it documents the boundary, and on a 64-bit non-Windows
platform it is a no-op.
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/pack-objects.c | 34 ++++++++++++++++++++++------------
object-file.c | 10 +++++++++-
odb/streaming.c | 13 ++++++++++++-
odb/streaming.h | 2 +-
| 2 +-
pack-bitmap.c | 2 +-
pack-check.c | 6 ++++--
packfile.c | 24 +++++++++++++++---------
packfile.h | 4 ++--
9 files changed, 67 insertions(+), 30 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index dd2480a73d..480cc0bd8c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -629,14 +629,21 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,
struct packed_git *p = IN_PACK(entry);
struct pack_window *w_curs = NULL;
uint32_t pos;
- off_t offset;
+ off_t offset, cur;
enum object_type type = oe_type(entry);
+ enum object_type in_pack_type;
off_t datalen;
unsigned char header[MAX_PACK_OBJECT_HEADER],
dheader[MAX_PACK_OBJECT_HEADER];
unsigned hdrlen;
const unsigned hashsz = the_hash_algo->rawsz;
- unsigned long entry_size = SIZE(entry);
+ size_t entry_size;
+
+ cur = entry->in_pack_offset;
+ in_pack_type = unpack_object_header(p, &w_curs, &cur, &entry_size);
+ if (in_pack_type < 0)
+ die(_("write_reuse_object: unable to parse object header of %s"),
+ oid_to_hex(&entry->idx.oid));
if (DELTA(entry))
type = (allow_ofs_delta && DELTA(entry)->idx.offset) ?
@@ -664,7 +671,8 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,
datalen -= entry->in_pack_header_size;
if (!pack_to_stdout && p->index_version == 1 &&
- check_pack_inflate(p, &w_curs, offset, datalen, entry_size)) {
+ check_pack_inflate(p, &w_curs, offset, datalen,
+ cast_size_t_to_ulong(entry_size))) {
error(_("corrupt packed object for %s"),
oid_to_hex(&entry->idx.oid));
unuse_pack(&w_curs);
@@ -1087,7 +1095,7 @@ static void write_reused_pack_one(struct packed_git *reuse_packfile,
{
off_t offset, next, cur;
enum object_type type;
- unsigned long size;
+ size_t size;
offset = pack_pos_to_offset(reuse_packfile, pos);
next = pack_pos_to_offset(reuse_packfile, pos + 1);
@@ -2243,7 +2251,7 @@ static void check_object(struct object_entry *entry, uint32_t object_index)
off_t ofs;
unsigned char *buf, c;
enum object_type type;
- unsigned long in_pack_size;
+ size_t in_pack_size;
buf = use_pack(p, &w_curs, entry->in_pack_offset, &avail);
@@ -2270,7 +2278,7 @@ static void check_object(struct object_entry *entry, uint32_t object_index)
default:
/* Not a delta hence we've already got all we need. */
oe_set_type(entry, entry->in_pack_type);
- SET_SIZE(entry, in_pack_size);
+ SET_SIZE(entry, cast_size_t_to_ulong(in_pack_size));
entry->in_pack_header_size = used;
if (oe_type(entry) < OBJ_COMMIT || oe_type(entry) > OBJ_BLOB)
goto give_up;
@@ -2324,8 +2332,8 @@ static void check_object(struct object_entry *entry, uint32_t object_index)
if (have_base &&
can_reuse_delta(&base_ref, entry, &base_entry)) {
oe_set_type(entry, entry->in_pack_type);
- SET_SIZE(entry, in_pack_size); /* delta size */
- SET_DELTA_SIZE(entry, in_pack_size);
+ SET_SIZE(entry, cast_size_t_to_ulong(in_pack_size)); /* delta size */
+ SET_DELTA_SIZE(entry, cast_size_t_to_ulong(in_pack_size));
if (base_entry) {
SET_DELTA(entry, base_entry);
@@ -2734,16 +2742,18 @@ unsigned long oe_get_size_slow(struct packing_data *pack,
struct pack_window *w_curs;
unsigned char *buf;
enum object_type type;
- unsigned long used, avail, size;
+ unsigned long used, avail;
+ size_t size;
if (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {
+ unsigned long sz;
packing_data_lock(&to_pack);
if (odb_read_object_info(the_repository->objects,
- &e->idx.oid, &size) < 0)
+ &e->idx.oid, &sz) < 0)
die(_("unable to get size of %s"),
oid_to_hex(&e->idx.oid));
packing_data_unlock(&to_pack);
- return size;
+ return sz;
}
p = oe_in_pack(pack, e);
@@ -2760,7 +2770,7 @@ unsigned long oe_get_size_slow(struct packing_data *pack,
unuse_pack(&w_curs);
packing_data_unlock(&to_pack);
- return size;
+ return cast_size_t_to_ulong(size);
}
static int try_delta(struct unpacked *trg, struct unpacked *src,
diff --git a/object-file.c b/object-file.c
index 086b2b65ff..0be2981c7a 100644
--- a/object-file.c
+++ b/object-file.c
@@ -2326,6 +2326,7 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
struct object_info oi = OBJECT_INFO_INIT;
struct odb_loose_read_stream *st;
unsigned long mapsize;
+ unsigned long size_ul;
void *mapped;
mapped = odb_source_loose_map_object(source, oid, &mapsize);
@@ -2349,11 +2350,18 @@ int odb_source_loose_read_object_stream(struct odb_read_stream **out,
goto error;
}
- oi.sizep = &st->base.size;
+ /*
+ * object_info.sizep is unsigned long* (32-bit on Windows), but
+ * st->base.size is size_t (64-bit). Use temporary variable.
+ * Note: loose objects >4GB would still truncate here, but such
+ * large loose objects are uncommon (they'd normally be packed).
+ */
+ oi.sizep = &size_ul;
oi.typep = &st->base.type;
if (parse_loose_header(st->hdr, &oi) < 0 || st->base.type < 0)
goto error;
+ st->base.size = size_ul;
st->mapped = mapped;
st->mapsize = mapsize;
diff --git a/odb/streaming.c b/odb/streaming.c
index 5927a12954..af2adf5ce7 100644
--- a/odb/streaming.c
+++ b/odb/streaming.c
@@ -157,15 +157,26 @@ static int open_istream_incore(struct odb_read_stream **out,
.base.read = read_istream_incore,
};
struct odb_incore_read_stream *st;
+ unsigned long size_ul;
int ret;
oi.typep = &stream.base.type;
- oi.sizep = &stream.base.size;
+ /*
+ * object_info.sizep is unsigned long* (32-bit on Windows), but
+ * stream.base.size is size_t (64-bit). We use a temporary variable
+ * because the types are incompatible. Note: this path still truncates
+ * for >4GB objects, but large objects should use pack streaming
+ * (packfile_store_read_object_stream) which handles size_t properly.
+ * This incore fallback is only used for small objects or when pack
+ * streaming is unavailable.
+ */
+ oi.sizep = &size_ul;
oi.contentp = (void **)&stream.buf;
ret = odb_read_object_info_extended(odb, oid, &oi,
OBJECT_INFO_DIE_IF_CORRUPT);
if (ret)
return ret;
+ stream.base.size = size_ul;
CALLOC_ARRAY(st, 1);
*st = stream;
diff --git a/odb/streaming.h b/odb/streaming.h
index c7861f7e13..517e2ea2d3 100644
--- a/odb/streaming.h
+++ b/odb/streaming.h
@@ -21,7 +21,7 @@ struct odb_read_stream {
odb_read_stream_close_fn close;
odb_read_stream_read_fn read;
enum object_type type;
- unsigned long size; /* inflated size of full object */
+ size_t size; /* inflated size of full object */
};
/*
--git a/oss-fuzz/fuzz-pack-headers.c b/oss-fuzz/fuzz-pack-headers.c
index 150c0f5fa2..ef61ab577c 100644
--- a/oss-fuzz/fuzz-pack-headers.c
+++ b/oss-fuzz/fuzz-pack-headers.c
@@ -6,7 +6,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
{
enum object_type type;
- unsigned long len;
+ size_t len;
unpack_object_header_buffer((const unsigned char *)data,
(unsigned long)size, &type, &len);
diff --git a/pack-bitmap.c b/pack-bitmap.c
index f6ec18d83a..f9af8a96bd 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2270,7 +2270,7 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
{
off_t delta_obj_offset;
enum object_type type;
- unsigned long size;
+ size_t size;
if (pack_pos >= pack->p->num_objects)
return -1; /* not actually in the pack */
diff --git a/pack-check.c b/pack-check.c
index 79992bb509..2792f34d25 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -110,7 +110,7 @@ static int verify_packfile(struct repository *r,
void *data;
struct object_id oid;
enum object_type type;
- unsigned long size;
+ size_t size;
off_t curpos;
int data_valid;
@@ -143,7 +143,9 @@ static int verify_packfile(struct repository *r,
data = NULL;
data_valid = 0;
} else {
- data = unpack_entry(r, p, entries[i].offset, &type, &size);
+ unsigned long sz;
+ data = unpack_entry(r, p, entries[i].offset, &type, &sz);
+ size = sz;
data_valid = 1;
}
diff --git a/packfile.c b/packfile.c
index b012d648ad..fdae91dd11 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1133,7 +1133,7 @@ out:
}
unsigned long unpack_object_header_buffer(const unsigned char *buf,
- unsigned long len, enum object_type *type, unsigned long *sizep)
+ unsigned long len, enum object_type *type, size_t *sizep)
{
unsigned shift;
size_t size, c;
@@ -1144,7 +1144,11 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
size = c & 15;
shift = 4;
while (c & 0x80) {
- if (len <= used || (bitsizeof(long) - 7) < shift) {
+ /*
+ * Each continuation byte adds 7 bits. Ensure shift won't
+ * overflow size_t (use size_t not long for 64-bit on Windows).
+ */
+ if (len <= used || (bitsizeof(size_t) - 7) < shift) {
error("bad object header");
size = used = 0;
break;
@@ -1153,7 +1157,7 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
size = st_add(size, st_left_shift(c & 0x7f, shift));
shift += 7;
}
- *sizep = cast_size_t_to_ulong(size);
+ *sizep = size;
return used;
}
@@ -1215,7 +1219,7 @@ unsigned long get_size_from_delta(struct packed_git *p,
int unpack_object_header(struct packed_git *p,
struct pack_window **w_curs,
off_t *curpos,
- unsigned long *sizep)
+ size_t *sizep)
{
unsigned char *base;
unsigned long left;
@@ -1367,7 +1371,7 @@ static enum object_type packed_to_object_type(struct repository *r,
while (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
off_t base_offset;
- unsigned long size;
+ size_t size;
/* Push the object we're going to leave behind */
if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) {
poi_stack_alloc = alloc_nr(poi_stack_nr);
@@ -1586,7 +1590,7 @@ static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_off
uint32_t *maybe_index_pos, struct object_info *oi)
{
struct pack_window *w_curs = NULL;
- unsigned long size;
+ size_t size;
off_t curpos = obj_offset;
enum object_type type = OBJ_NONE;
uint32_t pack_pos;
@@ -1778,7 +1782,7 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
struct pack_window *w_curs = NULL;
off_t curpos = obj_offset;
void *data = NULL;
- unsigned long size;
+ size_t size;
enum object_type type;
struct unpack_entry_stack_ent small_delta_stack[UNPACK_ENTRY_STACK_PREALLOC];
struct unpack_entry_stack_ent *delta_stack = small_delta_stack;
@@ -1943,8 +1947,10 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
(uintmax_t)curpos, p->pack_name);
data = NULL;
} else {
+ unsigned long sz;
data = patch_delta(base, base_size, delta_data,
- delta_size, &size);
+ delta_size, &sz);
+ size = sz;
/*
* We could not apply the delta; warn the user, but
@@ -2929,7 +2935,7 @@ int packfile_read_object_stream(struct odb_read_stream **out,
struct odb_packed_read_stream *stream;
struct pack_window *window = NULL;
enum object_type in_pack_type;
- unsigned long size;
+ size_t size;
in_pack_type = unpack_object_header(pack, &window, &offset, &size);
unuse_pack(&window);
diff --git a/packfile.h b/packfile.h
index 9b647da7dd..49d6bdecf6 100644
--- a/packfile.h
+++ b/packfile.h
@@ -456,9 +456,9 @@ off_t find_pack_entry_one(const struct object_id *oid, struct packed_git *);
int is_pack_valid(struct packed_git *);
void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);
-unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
+unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, size_t *sizep);
unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
-int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
+int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, size_t *);
off_t get_delta_base(struct packed_git *p, struct pack_window **w_curs,
off_t *curpos, enum object_type type,
off_t delta_obj_offset);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 04/11] delta, packfile: use size_t for delta header sizes
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2026-05-08 8:16 ` [PATCH v3 03/11] odb, packfile: use size_t for streaming object sizes Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 05/11] test-tool: add a helper to synthesize large packfiles Johannes Schindelin via GitGitGadget
` (6 subsequent siblings)
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The delta header decoding functions return unsigned long, which
truncates on Windows for objects larger than 4GB. Introduce size_t
variants get_delta_hdr_size_sz() and get_size_from_delta_sz() that
preserve the full 64-bit size, and use them in packed_object_info()
where the size is needed for streaming decisions.
This was originally authored by LordKiRon <https://github.com/LordKiRon>,
who preferred not to reveal their real name and therefore agreed that I
take over authorship.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
delta.h | 14 ++++++++++++--
packfile.c | 33 ++++++++++++++++++++++++---------
2 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/delta.h b/delta.h
index 8a56ec0799..fad68cfc45 100644
--- a/delta.h
+++ b/delta.h
@@ -86,8 +86,11 @@ void *patch_delta(const void *src_buf, unsigned long src_size,
* This must be called twice on the delta data buffer, first to get the
* expected source buffer size, and again to get the target buffer size.
*/
-static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
- const unsigned char *top)
+/*
+ * Size_t variant that doesn't truncate - use for >4GB objects on Windows.
+ */
+static inline size_t get_delta_hdr_size_sz(const unsigned char **datap,
+ const unsigned char *top)
{
const unsigned char *data = *datap;
size_t cmd, size = 0;
@@ -98,6 +101,13 @@ static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
i += 7;
} while (cmd & 0x80 && data < top);
*datap = data;
+ return size;
+}
+
+static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
+ const unsigned char *top)
+{
+ size_t size = get_delta_hdr_size_sz(datap, top);
return cast_size_t_to_ulong(size);
}
diff --git a/packfile.c b/packfile.c
index fdae91dd11..4208f53046 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1161,9 +1161,12 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
return used;
}
-unsigned long get_size_from_delta(struct packed_git *p,
- struct pack_window **w_curs,
- off_t curpos)
+/*
+ * Size_t variant for >4GB delta results on Windows.
+ */
+static size_t get_size_from_delta_sz(struct packed_git *p,
+ struct pack_window **w_curs,
+ off_t curpos)
{
const unsigned char *data;
unsigned char delta_head[20], *in;
@@ -1210,10 +1213,18 @@ unsigned long get_size_from_delta(struct packed_git *p,
data = delta_head;
/* ignore base size */
- get_delta_hdr_size(&data, delta_head+sizeof(delta_head));
+ get_delta_hdr_size_sz(&data, delta_head+sizeof(delta_head));
/* Read the result size */
- return get_delta_hdr_size(&data, delta_head+sizeof(delta_head));
+ return get_delta_hdr_size_sz(&data, delta_head+sizeof(delta_head));
+}
+
+unsigned long get_size_from_delta(struct packed_git *p,
+ struct pack_window **w_curs,
+ off_t curpos)
+{
+ size_t size = get_size_from_delta_sz(p, w_curs, curpos);
+ return cast_size_t_to_ulong(size);
}
int unpack_object_header(struct packed_git *p,
@@ -1618,14 +1629,18 @@ static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_off
ret = -1;
goto out;
}
- *oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);
- if (*oi->sizep == 0) {
+ /*
+ * Use size_t variant to avoid die() on >4GB deltas.
+ * oi->sizep is unsigned long, so truncation may occur,
+ * but streaming code uses its own size_t tracking.
+ */
+ size = get_size_from_delta_sz(p, &w_curs, tmp_pos);
+ if (size == 0) {
ret = -1;
goto out;
}
- } else {
- *oi->sizep = size;
}
+ *oi->sizep = (unsigned long)size;
}
if (oi->disk_sizep || (oi->mtimep && p->is_cruft)) {
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 05/11] test-tool: add a helper to synthesize large packfiles
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
` (3 preceding siblings ...)
2026-05-08 8:16 ` [PATCH v3 04/11] delta, packfile: use size_t for delta header sizes Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 06/11] t5608: add regression test for >4GB object clone Johannes Schindelin via GitGitGadget
` (5 subsequent siblings)
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
To test Git's behavior with very large pack files, we need a way to
generate such files quickly.
A naive approach using only readily-available Git commands would take
over 10 hours for a 4GB pack file, which is prohibitive.
Side-stepping Git's machinery and actual zlib compression by writing
uncompressed content with the appropriate zlib header makes things
much faster. The fastest method using this approach generates many
small, unreachable blob objects and takes about 1.5 minutes for 4GB.
However, this cannot be used because we need to test git clone, which
requires a reachable commit history.
Generating many reachable commits with small, uncompressed blobs takes
about 4 minutes for 4GB. But this approach 1) does not reproduce the
issues we want to fix (which require individual objects larger than
4GB) and 2) is comparatively slow because of the many SHA-1
calculations.
The approach taken here generates a single large blob (filled with NUL
bytes), along with the trees and commits needed to make it reachable.
This takes about 2.5 minutes for 4.5GB, which is the fastest option
that produces a valid, clonable repository with an object large enough
to trigger the bugs we want to test.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 1 +
compat/zlib-compat.h | 2 +
t/helper/meson.build | 1 +
t/helper/test-synthesize.c | 250 +++++++++++++++++++++++++++++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
6 files changed, 256 insertions(+)
create mode 100644 t/helper/test-synthesize.c
diff --git a/Makefile b/Makefile
index cedc234173..85405cb5b8 100644
--- a/Makefile
+++ b/Makefile
@@ -872,6 +872,7 @@ TEST_BUILTINS_OBJS += test-submodule-config.o
TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o
TEST_BUILTINS_OBJS += test-submodule.o
TEST_BUILTINS_OBJS += test-subprocess.o
+TEST_BUILTINS_OBJS += test-synthesize.o
TEST_BUILTINS_OBJS += test-trace2.o
TEST_BUILTINS_OBJS += test-truncate.o
TEST_BUILTINS_OBJS += test-userdiff.o
diff --git a/compat/zlib-compat.h b/compat/zlib-compat.h
index ac08276622..5078c5ef6c 100644
--- a/compat/zlib-compat.h
+++ b/compat/zlib-compat.h
@@ -7,6 +7,8 @@
# define z_stream_s zng_stream_s
# define gz_header_s zng_gz_header_s
+# define adler32(adler, buf, len) zng_adler32(adler, buf, len)
+
# define crc32(crc, buf, len) zng_crc32(crc, buf, len)
# define inflate(strm, bits) zng_inflate(strm, bits)
diff --git a/t/helper/meson.build b/t/helper/meson.build
index 675e64c010..3235f10ab8 100644
--- a/t/helper/meson.build
+++ b/t/helper/meson.build
@@ -69,6 +69,7 @@ test_tool_sources = [
'test-submodule-nested-repo-config.c',
'test-submodule.c',
'test-subprocess.c',
+ 'test-synthesize.c',
'test-tool.c',
'test-trace2.c',
'test-truncate.c',
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
new file mode 100644
index 0000000000..3ce7078078
--- /dev/null
+++ b/t/helper/test-synthesize.c
@@ -0,0 +1,250 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
+#include "test-tool.h"
+#include "git-compat-util.h"
+#include "git-zlib.h"
+#include "hash.h"
+#include "hex.h"
+#include "object-file.h"
+#include "object.h"
+#include "pack.h"
+#include "parse-options.h"
+#include "parse.h"
+#include "repository.h"
+#include "setup.h"
+#include "strbuf.h"
+#include "write-or-die.h"
+
+#define BLOCK_SIZE 0xffff
+static const unsigned char zeros[BLOCK_SIZE];
+
+/*
+ * Write data as an uncompressed zlib stream.
+ * For data larger than 64KB, writes multiple uncompressed blocks.
+ * If data is NULL, writes zeros.
+ * Updates the pack checksum context.
+ */
+static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,
+ const void *data, size_t len,
+ const struct git_hash_algo *algo)
+{
+ unsigned char zlib_header[2] = { 0x78, 0x01 }; /* CMF, FLG */
+ unsigned char block_header[5];
+ const unsigned char *p = data;
+ size_t remaining = len;
+ uint32_t adler = 1L; /* adler32 initial value */
+ unsigned char adler_buf[4];
+
+ /* Write zlib header */
+ fwrite_or_die(f, zlib_header, sizeof(zlib_header));
+ algo->update_fn(pack_ctx, zlib_header, 2);
+
+ /* Write uncompressed blocks (max 64KB each) */
+ do {
+ size_t block_len = remaining > BLOCK_SIZE ? BLOCK_SIZE : remaining;
+ int is_final = (block_len == remaining);
+ const unsigned char *block_data = data ? p : zeros;
+
+ block_header[0] = is_final ? 0x01 : 0x00;
+ block_header[1] = block_len & 0xff;
+ block_header[2] = (block_len >> 8) & 0xff;
+ block_header[3] = block_header[1] ^ 0xff;
+ block_header[4] = block_header[2] ^ 0xff;
+
+ fwrite_or_die(f, block_header, sizeof(block_header));
+ algo->update_fn(pack_ctx, block_header, 5);
+
+ if (block_len) {
+ fwrite_or_die(f, block_data, block_len);
+ algo->update_fn(pack_ctx, block_data, block_len);
+ adler = adler32(adler, block_data, block_len);
+ }
+
+ if (data)
+ p += block_len;
+ remaining -= block_len;
+ } while (remaining > 0);
+
+ /* Write adler32 checksum */
+ put_be32(adler_buf, adler);
+ fwrite_or_die(f, adler_buf, sizeof(adler_buf));
+ algo->update_fn(pack_ctx, adler_buf, 4);
+}
+
+/*
+ * Write an uncompressed object to the pack file.
+ * If `data == NULL`, it is treated like a buffer to NUL bytes.
+ * Updates the pack checksum context.
+ */
+static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,
+ enum object_type type,
+ const void *data, size_t len,
+ struct object_id *oid,
+ const struct git_hash_algo *algo)
+{
+ unsigned char pack_header[MAX_PACK_OBJECT_HEADER];
+ char object_header[32];
+ int pack_header_len, object_header_len;
+ struct git_hash_ctx ctx;
+
+ /* Write pack object header */
+ pack_header_len = encode_in_pack_object_header(pack_header,
+ sizeof(pack_header),
+ type, len);
+ fwrite_or_die(f, pack_header, pack_header_len);
+ algo->update_fn(pack_ctx, pack_header, pack_header_len);
+
+ /* Write the data as uncompressed zlib */
+ write_uncompressed_zlib(f, pack_ctx, data, len, algo);
+
+ algo->init_fn(&ctx);
+ object_header_len = format_object_header(object_header,
+ sizeof(object_header),
+ type, len);
+ algo->update_fn(&ctx, object_header, object_header_len);
+ if (data)
+ algo->update_fn(&ctx, data, len);
+ else {
+ for (size_t i = len / BLOCK_SIZE; i; i--)
+ algo->update_fn(&ctx, zeros, BLOCK_SIZE);
+ algo->update_fn(&ctx, zeros, len % BLOCK_SIZE);
+ }
+ algo->final_oid_fn(oid, &ctx);
+}
+
+/*
+ * Generate a pack file with a single large (>4GB) reachable object.
+ *
+ * Creates:
+ * 1. A large blob (all NUL bytes)
+ * 2. A tree containing that blob as "file"
+ * 3. A commit using that tree
+ * 4. The empty tree
+ * 5. A child commit using the empty tree
+ *
+ * This is useful for testing that Git can handle objects larger than 4GB.
+ */
+static int generate_pack_with_large_object(const char *path, size_t blob_size,
+ const struct git_hash_algo *algo)
+{
+ FILE *f = xfopen(path, "wb");
+ struct git_hash_ctx pack_ctx;
+ unsigned char pack_hash[GIT_MAX_RAWSZ];
+ struct object_id blob_oid, tree_oid, commit_oid, empty_tree_oid, final_commit_oid;
+ struct strbuf buf = STRBUF_INIT;
+ const uint32_t object_count = 5;
+ struct pack_header pack_header = {
+ .hdr_signature = htonl(PACK_SIGNATURE),
+ .hdr_version = htonl(PACK_VERSION),
+ .hdr_entries = htonl(object_count),
+ };
+
+ algo->init_fn(&pack_ctx);
+
+ /* Write pack header */
+ fwrite_or_die(f, &pack_header, sizeof(pack_header));
+ algo->update_fn(&pack_ctx, &pack_header, sizeof(pack_header));
+
+ /* 1. Write the large blob */
+ write_pack_object(f, &pack_ctx, OBJ_BLOB, NULL, blob_size, &blob_oid, algo);
+
+ /* 2. Write tree containing the blob as "file" */
+ strbuf_addf(&buf, "100644 file%c", '\0');
+ strbuf_add(&buf, blob_oid.hash, algo->rawsz);
+ write_pack_object(f, &pack_ctx, OBJ_TREE, buf.buf, buf.len, &tree_oid, algo);
+
+ /* 3. Write commit using that tree */
+ strbuf_reset(&buf);
+ strbuf_addf(&buf,
+ "tree %s\n"
+ "author A U Thor <author@example.com> 1234567890 +0000\n"
+ "committer C O Mitter <committer@example.com> 1234567890 +0000\n"
+ "\n"
+ "Large blob commit\n",
+ oid_to_hex(&tree_oid));
+ write_pack_object(f, &pack_ctx, OBJ_COMMIT, buf.buf, buf.len, &commit_oid, algo);
+
+ /* 4. Write the empty tree */
+ write_pack_object(f, &pack_ctx, OBJ_TREE, "", 0, &empty_tree_oid, algo);
+
+ /* 5. Write final commit using empty tree, with previous commit as parent */
+ strbuf_reset(&buf);
+ strbuf_addf(&buf,
+ "tree %s\n"
+ "parent %s\n"
+ "author A U Thor <author@example.com> 1234567890 +0000\n"
+ "committer C O Mitter <committer@example.com> 1234567890 +0000\n"
+ "\n"
+ "Empty tree commit\n",
+ oid_to_hex(&empty_tree_oid),
+ oid_to_hex(&commit_oid));
+ write_pack_object(f, &pack_ctx, OBJ_COMMIT, buf.buf, buf.len, &final_commit_oid, algo);
+
+ /* Write pack trailer (checksum) */
+ algo->final_fn(pack_hash, &pack_ctx);
+ fwrite_or_die(f, pack_hash, algo->rawsz);
+ if (fclose(f))
+ die_errno(_("could not close '%s'"), path);
+
+ strbuf_release(&buf);
+
+ /* Print the final commit OID so caller can set up refs */
+ printf("%s\n", oid_to_hex(&final_commit_oid));
+
+ return 0;
+}
+
+static int cmd__synthesize__pack(int argc, const char **argv,
+ const char *prefix UNUSED,
+ struct repository *repo)
+{
+ int non_git;
+ int reachable_large = 0;
+ const struct git_hash_algo *algo;
+ size_t blob_size;
+ uintmax_t blob_size_u;
+ const char *path;
+ const char * const usage[] = {
+ "test-tool synthesize pack "
+ "--reachable-large <blob-size> <filename>",
+ NULL
+ };
+ struct option options[] = {
+ OPT_BOOL(0, "reachable-large", &reachable_large,
+ N_("write a pack with a single reachable large blob")),
+ OPT_END()
+ };
+
+ setup_git_directory_gently(&non_git);
+ repo = the_repository;
+ algo = repo->hash_algo;
+
+ argc = parse_options(argc, argv, NULL, options, usage,
+ PARSE_OPT_KEEP_ARGV0);
+ if (argc != 3 || !reachable_large)
+ usage_with_options(usage, options);
+
+ if (!git_parse_unsigned(argv[1], &blob_size_u,
+ maximum_unsigned_value_of_type(size_t)))
+ die(_("'%s' is not a valid blob size"), argv[1]);
+ blob_size = blob_size_u;
+ path = argv[2];
+
+ return !!generate_pack_with_large_object(path, blob_size, algo);
+}
+
+int cmd__synthesize(int argc, const char **argv)
+{
+ const char *prefix = NULL;
+ char const * const synthesize_usage[] = {
+ "test-tool synthesize pack <options>",
+ NULL,
+ };
+ parse_opt_subcommand_fn *fn = NULL;
+ struct option options[] = {
+ OPT_SUBCOMMAND("pack", &fn, cmd__synthesize__pack),
+ OPT_END()
+ };
+ argc = parse_options(argc, argv, prefix, options, synthesize_usage, 0);
+ return !!fn(argc, argv, prefix, NULL);
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index a7abc618b3..b71a22b43b 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -82,6 +82,7 @@ static struct test_cmd cmds[] = {
{ "submodule-config", cmd__submodule_config },
{ "submodule-nested-repo-config", cmd__submodule_nested_repo_config },
{ "subprocess", cmd__subprocess },
+ { "synthesize", cmd__synthesize },
{ "trace2", cmd__trace2 },
{ "truncate", cmd__truncate },
{ "userdiff", cmd__userdiff },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 7f150fa1eb..f2885b33d5 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -75,6 +75,7 @@ int cmd__submodule(int argc, const char **argv);
int cmd__submodule_config(int argc, const char **argv);
int cmd__submodule_nested_repo_config(int argc, const char **argv);
int cmd__subprocess(int argc, const char **argv);
+int cmd__synthesize(int argc, const char **argv);
int cmd__trace2(int argc, const char **argv);
int cmd__truncate(int argc, const char **argv);
int cmd__userdiff(int argc, const char **argv);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 06/11] t5608: add regression test for >4GB object clone
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
` (4 preceding siblings ...)
2026-05-08 8:16 ` [PATCH v3 05/11] test-tool: add a helper to synthesize large packfiles Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 07/11] test-tool synthesize: use the unsafe hash for speed Johannes Schindelin via GitGitGadget
` (4 subsequent siblings)
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The shift overflow bug in index-pack and unpack-objects caused incorrect
object size calculation when the encoded size required more than 32 bits
of shift. This would result in corrupted or failed unpacking of objects
larger than 4GB.
Add a test that creates a pack file containing a 4GB+ blob using the
new 'test-tool synthesize pack --reachable-large' command, then clones
the repository to verify the fix works correctly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t5608-clone-2gb.sh | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
index 87a8cd9f98..af93302dde 100755
--- a/t/t5608-clone-2gb.sh
+++ b/t/t5608-clone-2gb.sh
@@ -49,4 +49,41 @@ test_expect_success 'clone - with worktree, file:// protocol' '
'
+test_expect_success SIZE_T_IS_64BIT 'set up repo with >4GB object' '
+ large_blob_size=$((4*1024*1024*1024+1)) &&
+ git init --bare 4gb-repo &&
+ head_oid=$(test-tool synthesize pack \
+ --reachable-large "$large_blob_size" \
+ 4gb-repo/objects/pack/test.pack) &&
+ git -C 4gb-repo index-pack objects/pack/test.pack &&
+ git -C 4gb-repo update-ref refs/heads/main $head_oid &&
+ git -C 4gb-repo symbolic-ref HEAD refs/heads/main
+'
+
+test_expect_success SIZE_T_IS_64BIT 'clone >4GB object via unpack-objects' '
+ # The synthesized pack has five objects, so a large unpack limit keeps
+ # fetch-pack on the unpack-objects path.
+ git -c fetch.unpackLimit=100 clone --bare \
+ "file://$(pwd)/4gb-repo" 4gb-clone-unpack &&
+
+ # Verify the large blob survived the clone by comparing its OID
+ # between source and clone. We cannot use "cat-file -s" because
+ # object_info.sizep is still unsigned long, which truncates >4GB
+ # sizes on Windows. OID equality proves content integrity since
+ # the clone already verified checksums via index-pack/unpack-objects.
+ source_blob=$(git -C 4gb-repo rev-parse main^:file) &&
+ clone_blob=$(git -C 4gb-clone-unpack rev-parse main^:file) &&
+ test "$source_blob" = "$clone_blob"
+'
+
+test_expect_success SIZE_T_IS_64BIT 'clone with >4GB object via index-pack' '
+ # Force fetch-pack to hand the pack to index-pack instead.
+ git -c fetch.unpackLimit=1 clone --bare \
+ "file://$(pwd)/4gb-repo" 4gb-clone-index &&
+
+ source_blob=$(git -C 4gb-repo rev-parse main^:file) &&
+ clone_blob=$(git -C 4gb-clone-index rev-parse main^:file) &&
+ test "$source_blob" = "$clone_blob"
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 07/11] test-tool synthesize: use the unsafe hash for speed
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
` (5 preceding siblings ...)
2026-05-08 8:16 ` [PATCH v3 06/11] t5608: add regression test for >4GB object clone Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 08/11] test-tool synthesize: precompute pack for 4 GiB + 1 Johannes Schindelin via GitGitGadget
` (3 subsequent siblings)
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Jeff King pointed out on the mailing list [1] that t5608's new >4GB
test cases dominate the entire test suite runtime: 160 seconds on his
laptop when the rest of the suite finishes in under 90 seconds, and
305-850 seconds across CI jobs. The bottleneck is that the synthesize
helper hashes roughly 8 GB of data through SHA-1 (4 GB for the pack
checksum plus 4 GB for the blob OID) for a 4 GB+1 blob.
Since the helper generates known test data, collision detection is
unnecessary. Switch from repo->hash_algo to unsafe_hash_algo(), which
uses hardware-accelerated SHA-1 (via OpenSSL or Apple CommonCrypto)
when available.
Benchmarks on an x86_64 machine generating a 4 GB+1 pack (2 runs
each, interleaved):
SHA-1 backend Run 1 Run 2
SHA1DC (safe) 75s 80s
OpenSSL (unsafe) 21s 19s
The effect scales linearly. At 64 MB with 10 randomized interleaved
runs, the OpenSSL unsafe backend shows a 5.4x improvement (median
0.202s vs 1.088s) with tight variance (stdev 0.028s vs 0.095s).
The speedup is only realized when the build has a fast unsafe backend
compiled in. The CI's linux-TEST-vars job already sets
OPENSSL_SHA1_UNSAFE=YesPlease; macOS benefits from Apple CommonCrypto
when configured. On builds without a separate unsafe backend (such as
the default Windows builds), unsafe_hash_algo() returns the regular
collision-detecting implementation and the change is a no-op.
[1] https://lore.kernel.org/git/20260501063805.GA2038915@coredump.intra.peff.net/
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-synthesize.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
index 3ce7078078..e2faaad7b4 100644
--- a/t/helper/test-synthesize.c
+++ b/t/helper/test-synthesize.c
@@ -217,7 +217,7 @@ static int cmd__synthesize__pack(int argc, const char **argv,
setup_git_directory_gently(&non_git);
repo = the_repository;
- algo = repo->hash_algo;
+ algo = unsafe_hash_algo(repo->hash_algo);
argc = parse_options(argc, argv, NULL, options, usage,
PARSE_OPT_KEEP_ARGV0);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 08/11] test-tool synthesize: precompute pack for 4 GiB + 1
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
` (6 preceding siblings ...)
2026-05-08 8:16 ` [PATCH v3 07/11] test-tool synthesize: use the unsafe hash for speed Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 09/11] test-tool synthesize: add precomputed SHA-256 " Johannes Schindelin via GitGitGadget
` (2 subsequent siblings)
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The synthesize helper hashes roughly 8 GiB of data through SHA-1 to
produce a 4 GiB + 1 pack (4 GiB for the pack checksum, 4 GiB for
the blob OID). Since the blob content is all NUL bytes, every byte
in the resulting pack file is deterministic for a given blob size and
hash algorithm.
Add a fast path that writes the pack from precomputed constants:
a 25-byte prefix (pack header, object header, zlib header, first
block header), the zero-filled bulk with periodic 5-byte deflate
block headers, and a 513-byte suffix (tree, two commits, empty tree,
pack SHA-1 checksum). This eliminates all SHA-1 and adler32
computation, making the helper purely I/O-bound.
The precomputed constants are stored in a struct fast_pack array
keyed by hash algorithm format_id, so that adding SHA-256 support
later requires only adding another array entry with its suffix.
The constants were generated by running the generic path and
extracting the non-zero bytes from the resulting pack file.
Benchmarks generating a 4 GiB + 1 pack (3 runs each, SHA1DC on
x86_64):
generic path: 88s / 81s / 140s
fast path: 14s / 13s / 15s
On CI, where t5608 currently takes 200-850 seconds depending on the
job, the fast path cuts the pack-generation phase from minutes to
seconds, leaving only the clone operations themselves.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-synthesize.c | 202 ++++++++++++++++++++++++++++++++++++-
1 file changed, 201 insertions(+), 1 deletion(-)
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
index e2faaad7b4..83c40ee02a 100644
--- a/t/helper/test-synthesize.c
+++ b/t/helper/test-synthesize.c
@@ -112,6 +112,201 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,
algo->final_oid_fn(oid, &ctx);
}
+/*
+ * Fast path: precomputed pack data for a 4 GiB + 1 all-NUL blob.
+ *
+ * The generated pack is almost entirely zeros with a small constant
+ * prefix, periodic deflate block headers, and a constant suffix
+ * containing the tree, two commits, and the pack checksum. Because
+ * every byte is deterministic for a given blob size and hash algorithm,
+ * we can write the pack without computing any hashes at all, reducing
+ * runtime from minutes of hash computation to seconds of pure I/O.
+ *
+ * The blob is stored as an uncompressed deflate stream: a two-byte
+ * zlib header, then 65538 blocks of up to 0xffff bytes each, followed
+ * by an adler32 checksum. The pack header and deflate framing are
+ * shared across hash algorithms; only the suffix (which contains OIDs
+ * and the pack checksum) differs.
+ *
+ * Constants were generated by running the generic path and extracting
+ * the non-zero bytes from the resulting pack file.
+ */
+
+#define FAST_PACK_4G1_BLOB_SIZE ((size_t)4 * 1024 * 1024 * 1024 + 1)
+#define FAST_PACK_4G1_N_FULL_BLOCKS 65537
+
+/*
+ * Per-hash-algorithm constants for the fast path. The prefix and
+ * deflate block structure are identical across algorithms; only the
+ * suffix (tree, commits, pack checksum) and the commit OID differ.
+ */
+struct fast_pack {
+ uint32_t format_id;
+ const unsigned char *suffix;
+ size_t suffix_len;
+ const char *commit_oid;
+};
+
+/* Pack header + pack object header + zlib header + first block header */
+static const unsigned char fast_pack_prefix[] = {
+ /* PACK header: signature, version 2, 5 objects */
+ 0x50, 0x41, 0x43, 0x4b, 0x00, 0x00, 0x00, 0x02,
+ 0x00, 0x00, 0x00, 0x05,
+ /* pack object header: blob, size = 4294967297 */
+ 0xb1, 0x80, 0x80, 0x80, 0x80, 0x01,
+ /* zlib header: CMF=0x78, FLG=0x01 */
+ 0x78, 0x01,
+ /* first non-final block header: BFINAL=0, LEN=0xffff, NLEN=0x0000 */
+ 0x00, 0xff, 0xff, 0x00, 0x00
+};
+
+/* Every non-final deflate block header is identical */
+static const unsigned char fast_pack_block_header[] = {
+ 0x00, 0xff, 0xff, 0x00, 0x00
+};
+
+/* Final block (2 data bytes) + adler32 of 4294967297 NUL bytes */
+static const unsigned char fast_pack_final_block[] = {
+ /* BFINAL=1, LEN=2, NLEN=0xfffd */
+ 0x01, 0x02, 0x00, 0xfd, 0xff,
+ /* 2 NUL data bytes */
+ 0x00, 0x00,
+ /* adler32 */
+ 0x00, 0xe2, 0x00, 0x01
+};
+
+/*
+ * SHA-1 suffix: tree, commit, empty tree, final commit, pack checksum.
+ */
+static const unsigned char fast_pack_sha1_suffix[] = {
+ 0xa0, 0x02, 0x78, 0x01, 0x01, 0x20, 0x00, 0xdf,
+ 0xff, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20,
+ 0x66, 0x69, 0x6c, 0x65, 0x00, 0x3e, 0xb7, 0xfe,
+ 0xb1, 0x41, 0x3c, 0x75, 0x7f, 0x0d, 0x81, 0x81,
+ 0xde, 0xb2, 0x8d, 0x1d, 0xab, 0x03, 0xd6, 0x48,
+ 0x46, 0xb4, 0xb4, 0x0c, 0x60, 0x95, 0x0b, 0x78,
+ 0x01, 0x01, 0xb5, 0x00, 0x4a, 0xff, 0x74, 0x72,
+ 0x65, 0x65, 0x20, 0x63, 0x36, 0x38, 0x33, 0x66,
+ 0x63, 0x63, 0x37, 0x64, 0x31, 0x64, 0x38, 0x33,
+ 0x65, 0x66, 0x32, 0x66, 0x65, 0x31, 0x61, 0x66,
+ 0x35, 0x35, 0x32, 0x31, 0x35, 0x64, 0x30, 0x31,
+ 0x36, 0x38, 0x64, 0x62, 0x35, 0x32, 0x61, 0x33,
+ 0x61, 0x33, 0x62, 0x0a, 0x61, 0x75, 0x74, 0x68,
+ 0x6f, 0x72, 0x20, 0x41, 0x20, 0x55, 0x20, 0x54,
+ 0x68, 0x6f, 0x72, 0x20, 0x3c, 0x61, 0x75, 0x74,
+ 0x68, 0x6f, 0x72, 0x40, 0x65, 0x78, 0x61, 0x6d,
+ 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0x3e,
+ 0x20, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+ 0x38, 0x39, 0x30, 0x20, 0x2b, 0x30, 0x30, 0x30,
+ 0x30, 0x0a, 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74,
+ 0x74, 0x65, 0x72, 0x20, 0x43, 0x20, 0x4f, 0x20,
+ 0x4d, 0x69, 0x74, 0x74, 0x65, 0x72, 0x20, 0x3c,
+ 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x74, 0x65,
+ 0x72, 0x40, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c,
+ 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0x3e, 0x20, 0x31,
+ 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39,
+ 0x30, 0x20, 0x2b, 0x30, 0x30, 0x30, 0x30, 0x0a,
+ 0x0a, 0x4c, 0x61, 0x72, 0x67, 0x65, 0x20, 0x62,
+ 0x6c, 0x6f, 0x62, 0x20, 0x63, 0x6f, 0x6d, 0x6d,
+ 0x69, 0x74, 0x0a, 0xc6, 0x55, 0x37, 0x6b, 0x20,
+ 0x78, 0x01, 0x01, 0x00, 0x00, 0xff, 0xff, 0x00,
+ 0x00, 0x00, 0x01, 0x95, 0x0e, 0x78, 0x01, 0x01,
+ 0xe5, 0x00, 0x1a, 0xff, 0x74, 0x72, 0x65, 0x65,
+ 0x20, 0x34, 0x62, 0x38, 0x32, 0x35, 0x64, 0x63,
+ 0x36, 0x34, 0x32, 0x63, 0x62, 0x36, 0x65, 0x62,
+ 0x39, 0x61, 0x30, 0x36, 0x30, 0x65, 0x35, 0x34,
+ 0x62, 0x66, 0x38, 0x64, 0x36, 0x39, 0x32, 0x38,
+ 0x38, 0x66, 0x62, 0x65, 0x65, 0x34, 0x39, 0x30,
+ 0x34, 0x0a, 0x70, 0x61, 0x72, 0x65, 0x6e, 0x74,
+ 0x20, 0x63, 0x35, 0x62, 0x32, 0x31, 0x63, 0x36,
+ 0x31, 0x31, 0x61, 0x61, 0x35, 0x39, 0x34, 0x65,
+ 0x63, 0x39, 0x66, 0x64, 0x37, 0x65, 0x39, 0x32,
+ 0x63, 0x66, 0x39, 0x36, 0x34, 0x38, 0x39, 0x31,
+ 0x34, 0x63, 0x61, 0x34, 0x63, 0x32, 0x34, 0x31,
+ 0x32, 0x0a, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72,
+ 0x20, 0x41, 0x20, 0x55, 0x20, 0x54, 0x68, 0x6f,
+ 0x72, 0x20, 0x3c, 0x61, 0x75, 0x74, 0x68, 0x6f,
+ 0x72, 0x40, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c,
+ 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0x3e, 0x20, 0x31,
+ 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39,
+ 0x30, 0x20, 0x2b, 0x30, 0x30, 0x30, 0x30, 0x0a,
+ 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x74, 0x65,
+ 0x72, 0x20, 0x43, 0x20, 0x4f, 0x20, 0x4d, 0x69,
+ 0x74, 0x74, 0x65, 0x72, 0x20, 0x3c, 0x63, 0x6f,
+ 0x6d, 0x6d, 0x69, 0x74, 0x74, 0x65, 0x72, 0x40,
+ 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e,
+ 0x63, 0x6f, 0x6d, 0x3e, 0x20, 0x31, 0x32, 0x33,
+ 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x20,
+ 0x2b, 0x30, 0x30, 0x30, 0x30, 0x0a, 0x0a, 0x45,
+ 0x6d, 0x70, 0x74, 0x79, 0x20, 0x74, 0x72, 0x65,
+ 0x65, 0x20, 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74,
+ 0x0a, 0xaa, 0xb8, 0x45, 0x01, 0x8e, 0xfc, 0xf0,
+ 0x2f, 0x9c, 0xc5, 0xcc, 0x4f, 0x6a, 0x1a, 0xc9,
+ 0x2b, 0x23, 0xa9, 0xff, 0x91, 0x06, 0xc2, 0x70,
+ 0xe3
+};
+
+static const struct fast_pack fast_packs[] = {
+ {
+ .format_id = GIT_SHA1_FORMAT_ID,
+ .suffix = fast_pack_sha1_suffix,
+ .suffix_len = sizeof(fast_pack_sha1_suffix),
+ .commit_oid = "aac43daf40d0377af31aa9c798a4ae8a31b55c1d",
+ },
+};
+
+/*
+ * Try the fast path for known blob sizes. Returns 1 if the pack was
+ * written from precomputed constants, 0 if the caller should fall
+ * through to the generic path.
+ */
+static int generate_fast_pack(const char *path, size_t blob_size,
+ const struct git_hash_algo *algo)
+{
+ const struct fast_pack *fp = NULL;
+ FILE *f;
+ size_t i;
+
+ if (blob_size != FAST_PACK_4G1_BLOB_SIZE)
+ return 0;
+
+ for (i = 0; i < ARRAY_SIZE(fast_packs); i++) {
+ if (fast_packs[i].format_id == algo->format_id) {
+ fp = &fast_packs[i];
+ break;
+ }
+ }
+ if (!fp)
+ return 0;
+
+ f = xfopen(path, "wb");
+
+ fwrite_or_die(f, fast_pack_prefix, sizeof(fast_pack_prefix));
+
+ /* First full block: 0xffff zero bytes (header already in prefix) */
+ fwrite_or_die(f, zeros, BLOCK_SIZE);
+
+ /* Remaining non-final full blocks */
+ for (i = 1; i < FAST_PACK_4G1_N_FULL_BLOCKS; i++) {
+ fwrite_or_die(f, fast_pack_block_header,
+ sizeof(fast_pack_block_header));
+ fwrite_or_die(f, zeros, BLOCK_SIZE);
+ }
+
+ /* Final block (2 data bytes) + adler32 */
+ fwrite_or_die(f, fast_pack_final_block,
+ sizeof(fast_pack_final_block));
+
+ /* Tree, commits, and pack checksum */
+ fwrite_or_die(f, fp->suffix, fp->suffix_len);
+
+ if (fclose(f))
+ die_errno(_("could not close '%s'"), path);
+
+ printf("%s\n", fp->commit_oid);
+ return 1;
+}
+
/*
* Generate a pack file with a single large (>4GB) reachable object.
*
@@ -127,7 +322,7 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,
static int generate_pack_with_large_object(const char *path, size_t blob_size,
const struct git_hash_algo *algo)
{
- FILE *f = xfopen(path, "wb");
+ FILE *f;
struct git_hash_ctx pack_ctx;
unsigned char pack_hash[GIT_MAX_RAWSZ];
struct object_id blob_oid, tree_oid, commit_oid, empty_tree_oid, final_commit_oid;
@@ -139,6 +334,11 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,
.hdr_entries = htonl(object_count),
};
+ if (generate_fast_pack(path, blob_size, algo))
+ return 0;
+
+ f = xfopen(path, "wb");
+
algo->init_fn(&pack_ctx);
/* Write pack header */
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 09/11] test-tool synthesize: add precomputed SHA-256 pack for 4 GiB + 1
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
` (7 preceding siblings ...)
2026-05-08 8:16 ` [PATCH v3 08/11] test-tool synthesize: precompute pack for 4 GiB + 1 Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 10/11] t5608: mark >4GB tests as EXPENSIVE Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 11/11] ci: run expensive tests on push builds to integration branches Johannes Schindelin via GitGitGadget
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Add a SHA-256 entry to the fast_packs[] table. The pack prefix and
deflate block structure are identical to SHA-1 (the pack format does
not encode the hash algorithm in its header). Only the suffix differs:
SHA-256 OIDs are 32 bytes instead of 20, giving a 609-byte suffix
compared to 513 for SHA-1, and a different pack checksum.
The constants were generated by running the generic path inside a
repository initialized with --object-format=sha256.
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-synthesize.c | 91 ++++++++++++++++++++++++++++++++++++++
1 file changed, 91 insertions(+)
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
index 83c40ee02a..1f28ecf0f2 100644
--- a/t/helper/test-synthesize.c
+++ b/t/helper/test-synthesize.c
@@ -246,6 +246,90 @@ static const unsigned char fast_pack_sha1_suffix[] = {
0xe3
};
+/*
+ * SHA-256 suffix: same structure, but with 32-byte OIDs and SHA-256
+ * pack checksum (609 bytes vs 513 for SHA-1).
+ */
+static const unsigned char fast_pack_sha256_suffix[] = {
+ 0xac, 0x02, 0x78, 0x01, 0x01, 0x2c, 0x00, 0xd3,
+ 0xff, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20,
+ 0x66, 0x69, 0x6c, 0x65, 0x00, 0x42, 0x53, 0xc1,
+ 0x8a, 0x9f, 0x5e, 0xc3, 0xbb, 0x47, 0xb0, 0x83,
+ 0x8a, 0x19, 0xdb, 0x31, 0xbb, 0x7b, 0x0f, 0x3b,
+ 0x80, 0xa4, 0xbc, 0x2f, 0xaf, 0x72, 0x6b, 0xdb,
+ 0x62, 0xaa, 0xba, 0xdd, 0xde, 0x77, 0xc6, 0x13,
+ 0xeb, 0x9d, 0x0c, 0x78, 0x01, 0x01, 0xcd, 0x00,
+ 0x32, 0xff, 0x74, 0x72, 0x65, 0x65, 0x20, 0x62,
+ 0x36, 0x30, 0x39, 0x37, 0x37, 0x64, 0x37, 0x63,
+ 0x34, 0x63, 0x32, 0x64, 0x31, 0x65, 0x63, 0x63,
+ 0x33, 0x66, 0x62, 0x61, 0x31, 0x64, 0x39, 0x38,
+ 0x65, 0x65, 0x31, 0x32, 0x30, 0x61, 0x64, 0x63,
+ 0x32, 0x34, 0x38, 0x33, 0x34, 0x39, 0x35, 0x30,
+ 0x62, 0x65, 0x34, 0x31, 0x32, 0x64, 0x39, 0x34,
+ 0x63, 0x38, 0x30, 0x39, 0x34, 0x38, 0x30, 0x66,
+ 0x35, 0x38, 0x62, 0x61, 0x39, 0x64, 0x61, 0x0a,
+ 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x20, 0x41,
+ 0x20, 0x55, 0x20, 0x54, 0x68, 0x6f, 0x72, 0x20,
+ 0x3c, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x40,
+ 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e,
+ 0x63, 0x6f, 0x6d, 0x3e, 0x20, 0x31, 0x32, 0x33,
+ 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x20,
+ 0x2b, 0x30, 0x30, 0x30, 0x30, 0x0a, 0x63, 0x6f,
+ 0x6d, 0x6d, 0x69, 0x74, 0x74, 0x65, 0x72, 0x20,
+ 0x43, 0x20, 0x4f, 0x20, 0x4d, 0x69, 0x74, 0x74,
+ 0x65, 0x72, 0x20, 0x3c, 0x63, 0x6f, 0x6d, 0x6d,
+ 0x69, 0x74, 0x74, 0x65, 0x72, 0x40, 0x65, 0x78,
+ 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f,
+ 0x6d, 0x3e, 0x20, 0x31, 0x32, 0x33, 0x34, 0x35,
+ 0x36, 0x37, 0x38, 0x39, 0x30, 0x20, 0x2b, 0x30,
+ 0x30, 0x30, 0x30, 0x0a, 0x0a, 0x4c, 0x61, 0x72,
+ 0x67, 0x65, 0x20, 0x62, 0x6c, 0x6f, 0x62, 0x20,
+ 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74, 0x0a, 0xb7,
+ 0x80, 0x3d, 0xd7, 0x20, 0x78, 0x01, 0x01, 0x00,
+ 0x00, 0xff, 0xff, 0x00, 0x00, 0x00, 0x01, 0x95,
+ 0x11, 0x78, 0x01, 0x01, 0x15, 0x01, 0xea, 0xfe,
+ 0x74, 0x72, 0x65, 0x65, 0x20, 0x36, 0x65, 0x66,
+ 0x31, 0x39, 0x62, 0x34, 0x31, 0x32, 0x32, 0x35,
+ 0x63, 0x35, 0x33, 0x36, 0x39, 0x66, 0x31, 0x63,
+ 0x31, 0x30, 0x34, 0x64, 0x34, 0x35, 0x64, 0x38,
+ 0x64, 0x38, 0x35, 0x65, 0x66, 0x61, 0x39, 0x62,
+ 0x30, 0x35, 0x37, 0x62, 0x35, 0x33, 0x62, 0x31,
+ 0x34, 0x62, 0x34, 0x62, 0x39, 0x62, 0x39, 0x33,
+ 0x39, 0x64, 0x64, 0x37, 0x34, 0x64, 0x65, 0x63,
+ 0x63, 0x35, 0x33, 0x32, 0x31, 0x0a, 0x70, 0x61,
+ 0x72, 0x65, 0x6e, 0x74, 0x20, 0x37, 0x35, 0x62,
+ 0x66, 0x30, 0x63, 0x34, 0x37, 0x61, 0x65, 0x34,
+ 0x62, 0x62, 0x33, 0x30, 0x38, 0x65, 0x37, 0x63,
+ 0x63, 0x32, 0x34, 0x38, 0x32, 0x65, 0x32, 0x32,
+ 0x65, 0x66, 0x61, 0x65, 0x33, 0x37, 0x38, 0x37,
+ 0x61, 0x39, 0x36, 0x38, 0x34, 0x38, 0x62, 0x64,
+ 0x31, 0x37, 0x34, 0x39, 0x35, 0x36, 0x37, 0x31,
+ 0x34, 0x37, 0x31, 0x35, 0x32, 0x34, 0x36, 0x64,
+ 0x64, 0x62, 0x64, 0x35, 0x34, 0x0a, 0x61, 0x75,
+ 0x74, 0x68, 0x6f, 0x72, 0x20, 0x41, 0x20, 0x55,
+ 0x20, 0x54, 0x68, 0x6f, 0x72, 0x20, 0x3c, 0x61,
+ 0x75, 0x74, 0x68, 0x6f, 0x72, 0x40, 0x65, 0x78,
+ 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f,
+ 0x6d, 0x3e, 0x20, 0x31, 0x32, 0x33, 0x34, 0x35,
+ 0x36, 0x37, 0x38, 0x39, 0x30, 0x20, 0x2b, 0x30,
+ 0x30, 0x30, 0x30, 0x0a, 0x63, 0x6f, 0x6d, 0x6d,
+ 0x69, 0x74, 0x74, 0x65, 0x72, 0x20, 0x43, 0x20,
+ 0x4f, 0x20, 0x4d, 0x69, 0x74, 0x74, 0x65, 0x72,
+ 0x20, 0x3c, 0x63, 0x6f, 0x6d, 0x6d, 0x69, 0x74,
+ 0x74, 0x65, 0x72, 0x40, 0x65, 0x78, 0x61, 0x6d,
+ 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0x3e,
+ 0x20, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+ 0x38, 0x39, 0x30, 0x20, 0x2b, 0x30, 0x30, 0x30,
+ 0x30, 0x0a, 0x0a, 0x45, 0x6d, 0x70, 0x74, 0x79,
+ 0x20, 0x74, 0x72, 0x65, 0x65, 0x20, 0x63, 0x6f,
+ 0x6d, 0x6d, 0x69, 0x74, 0x0a, 0x6d, 0x6d, 0x51,
+ 0x9a, 0xc9, 0x11, 0x76, 0x61, 0xa3, 0x89, 0x49,
+ 0xb7, 0xa1, 0x58, 0xc6, 0x1d, 0x8c, 0x33, 0x75,
+ 0x8d, 0x7e, 0x4d, 0x8e, 0x58, 0x91, 0xf8, 0x5c,
+ 0x57, 0xd9, 0x89, 0x9e, 0xb8, 0xd2, 0x9a, 0xd8,
+ 0xc9
+};
+
static const struct fast_pack fast_packs[] = {
{
.format_id = GIT_SHA1_FORMAT_ID,
@@ -253,6 +337,13 @@ static const struct fast_pack fast_packs[] = {
.suffix_len = sizeof(fast_pack_sha1_suffix),
.commit_oid = "aac43daf40d0377af31aa9c798a4ae8a31b55c1d",
},
+ {
+ .format_id = GIT_SHA256_FORMAT_ID,
+ .suffix = fast_pack_sha256_suffix,
+ .suffix_len = sizeof(fast_pack_sha256_suffix),
+ .commit_oid = "63c46ca51267b1d45be69a044bb84b4bf0559f09"
+ "d727f861d2ae94ddebdddbc9",
+ },
};
/*
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 10/11] t5608: mark >4GB tests as EXPENSIVE
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
` (8 preceding siblings ...)
2026-05-08 8:16 ` [PATCH v3 09/11] test-tool synthesize: add precomputed SHA-256 " Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-08 8:16 ` [PATCH v3 11/11] ci: run expensive tests on push builds to integration branches Johannes Schindelin via GitGitGadget
10 siblings, 0 replies; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Even with precomputed pack constants that reduced the helper's
runtime from minutes to seconds, the >4GB clone tests still take
200-850 seconds across CI jobs. The bottleneck is no longer the
pack generation but the clone operations themselves: transporting,
unpacking, and indexing 4 GiB of data through unpack-objects and
index-pack is inherently expensive.
As Jeff King pointed out [1], t5608 alone takes 160 seconds on his
laptop while the rest of the entire test suite finishes in under 90
seconds, and the test's disk footprint (4+ GiB source repo, then
two clones) is problematic for developers who use RAM disks for
their trash directories.
Gate the >4GB tests on the EXPENSIVE prereq (which requires
GIT_TEST_LONG to be set) in addition to SIZE_T_IS_64BIT, keeping
them out of normal local test runs.
[1] https://lore.kernel.org/git/20260501063805.GA2038915@coredump.intra.peff.net/
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t5608-clone-2gb.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
index af93302dde..4f8a95ddda 100755
--- a/t/t5608-clone-2gb.sh
+++ b/t/t5608-clone-2gb.sh
@@ -49,7 +49,7 @@ test_expect_success 'clone - with worktree, file:// protocol' '
'
-test_expect_success SIZE_T_IS_64BIT 'set up repo with >4GB object' '
+test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'set up repo with >4GB object' '
large_blob_size=$((4*1024*1024*1024+1)) &&
git init --bare 4gb-repo &&
head_oid=$(test-tool synthesize pack \
@@ -60,7 +60,7 @@ test_expect_success SIZE_T_IS_64BIT 'set up repo with >4GB object' '
git -C 4gb-repo symbolic-ref HEAD refs/heads/main
'
-test_expect_success SIZE_T_IS_64BIT 'clone >4GB object via unpack-objects' '
+test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'clone >4GB object via unpack-objects' '
# The synthesized pack has five objects, so a large unpack limit keeps
# fetch-pack on the unpack-objects path.
git -c fetch.unpackLimit=100 clone --bare \
@@ -76,7 +76,7 @@ test_expect_success SIZE_T_IS_64BIT 'clone >4GB object via unpack-objects' '
test "$source_blob" = "$clone_blob"
'
-test_expect_success SIZE_T_IS_64BIT 'clone with >4GB object via index-pack' '
+test_expect_success SIZE_T_IS_64BIT,EXPENSIVE 'clone with >4GB object via index-pack' '
# Force fetch-pack to hand the pack to index-pack instead.
git -c fetch.unpackLimit=1 clone --bare \
"file://$(pwd)/4gb-repo" 4gb-clone-index &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v3 11/11] ci: run expensive tests on push builds to integration branches
2026-05-08 8:16 ` [PATCH v3 00/11] Handle cloning of objects larger than 4GB on Windows Johannes Schindelin via GitGitGadget
` (9 preceding siblings ...)
2026-05-08 8:16 ` [PATCH v3 10/11] t5608: mark >4GB tests as EXPENSIVE Johannes Schindelin via GitGitGadget
@ 2026-05-08 8:16 ` Johannes Schindelin via GitGitGadget
2026-05-10 23:51 ` [PATCH] ci: enable EXPENSIVE for contributor builds Junio C Hamano
10 siblings, 1 reply; 60+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2026-05-08 8:16 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Derrick Stolee suggested [1] that expensive tests should be run at a
regular cadence rather than on every PR iteration. Gate GIT_TEST_LONG
on push builds to the integration branches (next, master, main, maint)
so that the EXPENSIVE prereq is satisfied there but not during PR
validation, where the extra minutes of wall-clock time do not justify
themselves.
[1] https://lore.kernel.org/git/e1e8837f-7374-4079-ba87-ab95dd156e33@gmail.com/
Helped-by: Derrick Stolee <derrickstolee@github.com>
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
ci/lib.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/ci/lib.sh b/ci/lib.sh
index 42a2b6a318..a671994bdf 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -314,6 +314,15 @@ export DEFAULT_TEST_TARGET=prove
export GIT_TEST_CLONE_2GB=true
export SKIP_DASHED_BUILT_INS=YesPlease
+# Enable expensive tests on push builds to integration branches, but
+# not on PR builds where the extra time is not justified for every
+# iteration.
+case "$GITHUB_EVENT_NAME,$CI_BRANCH" in
+push,*next*|push,*master*|push,*main*|push,*maint*)
+ export GIT_TEST_LONG=YesPlease
+ ;;
+esac
+
case "$distro" in
ubuntu-*)
# Python 2 is end of life, and Ubuntu 23.04 and newer don't actually
--
gitgitgadget
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH] ci: enable EXPENSIVE for contributor builds
2026-05-08 8:16 ` [PATCH v3 11/11] ci: run expensive tests on push builds to integration branches Johannes Schindelin via GitGitGadget
@ 2026-05-10 23:51 ` Junio C Hamano
2026-05-11 7:05 ` Patrick Steinhardt
0 siblings, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2026-05-10 23:51 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Derrick Stolee, Torsten Bögershausen, Jeff King,
Patrick Steinhardt, Johannes Schindelin
Earlier, we enabled EXPENSIVE tests for pushes to integration
branches. As we didn't have any CI jobs that run these tests, this
was a step in the right direction.
It however is an ineffective and inefficient use of the maintainer
time, which does not scale, to allow contributors to send changes
that are less tested at the list, only to force the maintainer
notice breakages caused by their changes but only after these
changes are mixed with changes from other contributors. The
problematic topic needs to be isolated by bisecting, and it
historically has been done by the maintainer alone.
It is far better to let the problem identified early, preferably
before the problematic code leaves the hands of the original
developer. In order for it to happen, the test coverage of the
contributor tests must be at least as wide as the coverage of the
integration tests.
Enable expensive tests for CI jobs triggered by pull requests. This
will make each contributor take care of their own, which scales much
better.
Keep the expensive tests also enabled for the pushes of integration
branches, as that is the only place we can notice problems stemming
from mismerges and inter-topic interactions, even if the topics from
the contributors in isolation all passes these tests.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This is to be applied on top of "ci: run expensive tests on push
builds to integration branches", currently sitting at the tip of
the js/objects-larger-than-4gb-on-windows topic.
---
ci/lib.sh | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index a671994bdf..4ca3ecef2c 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -314,11 +314,13 @@ export DEFAULT_TEST_TARGET=prove
export GIT_TEST_CLONE_2GB=true
export SKIP_DASHED_BUILT_INS=YesPlease
-# Enable expensive tests on push builds to integration branches, but
-# not on PR builds where the extra time is not justified for every
-# iteration.
+# In order to give maximum test coverage to contributor builds,
+# preferrably even before the changes consume public review bandwidth,
+# enable "expensive" tests for PR events.
+# In order to catch bugs introduced at integration time by mismerges,
+# enable the long tests for pushes to the integration branches as well.
case "$GITHUB_EVENT_NAME,$CI_BRANCH" in
-push,*next*|push,*master*|push,*main*|push,*maint*)
+pull_request,*|push,*next*|push,*master*|push,*main*|push,*maint*)
export GIT_TEST_LONG=YesPlease
;;
esac
--
2.54.0-162-gf1ca62098f
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH] ci: enable EXPENSIVE for contributor builds
2026-05-10 23:51 ` [PATCH] ci: enable EXPENSIVE for contributor builds Junio C Hamano
@ 2026-05-11 7:05 ` Patrick Steinhardt
2026-05-11 8:29 ` Junio C Hamano
0 siblings, 1 reply; 60+ messages in thread
From: Patrick Steinhardt @ 2026-05-11 7:05 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin via GitGitGadget, git, Derrick Stolee,
Torsten Bögershausen, Jeff King, Johannes Schindelin
On Mon, May 11, 2026 at 08:51:15AM +0900, Junio C Hamano wrote:
> diff --git a/ci/lib.sh b/ci/lib.sh
> index a671994bdf..4ca3ecef2c 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -314,11 +314,13 @@ export DEFAULT_TEST_TARGET=prove
> export GIT_TEST_CLONE_2GB=true
> export SKIP_DASHED_BUILT_INS=YesPlease
>
> -# Enable expensive tests on push builds to integration branches, but
> -# not on PR builds where the extra time is not justified for every
> -# iteration.
> +# In order to give maximum test coverage to contributor builds,
> +# preferrably even before the changes consume public review bandwidth,
> +# enable "expensive" tests for PR events.
> +# In order to catch bugs introduced at integration time by mismerges,
> +# enable the long tests for pushes to the integration branches as well.
> case "$GITHUB_EVENT_NAME,$CI_BRANCH" in
> -push,*next*|push,*master*|push,*main*|push,*maint*)
> +pull_request,*|push,*next*|push,*master*|push,*main*|push,*maint*)
> export GIT_TEST_LONG=YesPlease
> ;;
> esac
So with this change we now run the tests for all "official" branches,
and on pull requests. Which raises the question: are there any events
that happen regularly that are excluded by this? Because if not I think
it might be sensible to just enable this unconditionally, also because
that would make jobs on GitLab CI run expensive tests, as well.
Patrick
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] ci: enable EXPENSIVE for contributor builds
2026-05-11 7:05 ` Patrick Steinhardt
@ 2026-05-11 8:29 ` Junio C Hamano
2026-05-11 10:02 ` Patrick Steinhardt
0 siblings, 1 reply; 60+ messages in thread
From: Junio C Hamano @ 2026-05-11 8:29 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Johannes Schindelin via GitGitGadget, git, Derrick Stolee,
Torsten Bögershausen, Jeff King, Johannes Schindelin
Patrick Steinhardt <ps@pks.im> writes:
> So with this change we now run the tests for all "official" branches,
> and on pull requests. Which raises the question: are there any events
> that happen regularly that are excluded by this? Because if not I think
> it might be sensible to just enable this unconditionally, also because
> that would make jobs on GitLab CI run expensive tests, as well.
The simplicity certainly is tempting.
We could instead do the "let's enable only on linux-test-vars" kind
of "optimization", which is on the other side of the extreme, but
that is only valid if the kind of bugs that can be revealed only by
EXPENSIVE tests, which may not be caught by others, is expected to
be pretty much platform or configuration agnostic. I somehow doubt
that it is the case.
In any case, I think spending on more machine cycles is certainly
cheaper than human resources for things like this.
Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH] ci: enable EXPENSIVE for contributor builds
2026-05-11 8:29 ` Junio C Hamano
@ 2026-05-11 10:02 ` Patrick Steinhardt
0 siblings, 0 replies; 60+ messages in thread
From: Patrick Steinhardt @ 2026-05-11 10:02 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin via GitGitGadget, git, Derrick Stolee,
Torsten Bögershausen, Jeff King, Johannes Schindelin
On Mon, May 11, 2026 at 05:29:01PM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > So with this change we now run the tests for all "official" branches,
> > and on pull requests. Which raises the question: are there any events
> > that happen regularly that are excluded by this? Because if not I think
> > it might be sensible to just enable this unconditionally, also because
> > that would make jobs on GitLab CI run expensive tests, as well.
>
> The simplicity certainly is tempting.
>
> We could instead do the "let's enable only on linux-test-vars" kind
> of "optimization", which is on the other side of the extreme, but
> that is only valid if the kind of bugs that can be revealed only by
> EXPENSIVE tests, which may not be caught by others, is expected to
> be pretty much platform or configuration agnostic. I somehow doubt
> that it is the case.
Yeah, it's probably not.
> In any case, I think spending on more machine cycles is certainly
> cheaper than human resources for things like this.
Agreed.
Patrick
^ permalink raw reply [flat|nested] 60+ messages in thread