Re: [PATCH v2 2/2] packfile: fix corruption due to stale delta base cache entries

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 7, 2026, 05:29 UTC
Message-ID: <asXYtkkRs6kvEjBJ@pks.im>
In-Reply-To: <xmqqse2id2kq.fsf@gitster.g>

On Tue, Oct 06, 2026 at 12:57:41PM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Note that the added test reliably reproduces the above bug on my machine
> > that uses NixOS at c59305bab206 (cosmic-applets: add missing runtime
> > dependency (#566040), 2026-10-01) with glibc 2.44-25. But as we rely on
> > specific allocation behaviour of glibc it is very likely that the test
> > will not work on other platforms.
> 
> In other words, the test will not detect the bug, when the fix is
> reverted, unless the glibc allocator is used?

It is specific to memory allocation patterns and thus to the platform, yes. But I just confirmed that the test also fails on for example Alpine Linux, so it even reproduces with musl libc. I haven't tested any other platforms though.

Show 5 quoted lines

> Adding an unreliable reproducer for a bug that is already fixed may be
> of dubious value.  However, even if the test is unreliable (since
> other allocators might hide the bug when the fix is reverted), it may
> be OK as long as it catches the bug on widely used configurations and
> does not trigger false positives.

Yeah, it at least catches the bug on some systems. And I think even if it eventually didn't anymore, it exercises a part of our system (doing submodule merges across many submodules) that wasn't previously exercised, I think. So it would still have some value there.

> On the other hand, the earlier suggestion to write custom low-level
> code to simulate a colliding allocation address somehow smells like a
> maintenance burden to me.

Agreed. It simply is too much boilerplate for too specific a failure, if you ask me.

Patrick