Re: [PATCH 13/49] repack: move 'delta_base_offset' to 'struct pack_objects_args'

From: Jeff King <peff@peff.net>
Date: Oct 10, 2025, 05:54 UTC
Message-ID: <20251010055431.GE1965904@coredump.intra.peff.net>
In-Reply-To: <06347b6ab23d5ab1d9a7945da67473b5b9e02ad1.1759097191.git.me@ttaylorr.com>

On Sun, Sep 28, 2025 at 06:08:00PM -0400, Taylor Blau wrote:

Show 14 quoted lines

> The static variable 'delta_base_offset' determines whether or not we
> pass the "--delta-base-offset" command-line argument when spawning
> pack-objects as a child process. Its introduction dates back to when
> repack was rewritten in C, all the way back in a1bbc6c017 (repack:
> rewrite the shell script in C, 2013-09-15).
> 
> 'struct pack_objects_args' was introduced much later on in 4571324b99
> (builtin/repack.c: allow configuring cruft pack generation, 2022-05-20),
> but did not move the 'delta_base_offset' variable.
> 
> Since the 'delta_base_offset' is a property of an individual
> pack-objects command, re-introduce that variable as a member of 'struct
> pack_objects_args', which will enable further code movement in the
> subsequent commits.

Hmm, OK. It is true that it is a property of a pack-objects invocation, but would we ever want it to be different between the two? I'd think not, and from what I can see:

> +		cruft_po_args.delta_base_offset = po_args.delta_base_offset;

we'd always use the same value in both cases. It kind of makes me wonder if it should be in the repack_ctx variable instead, then, with both pack-objects invocations pulling from a common source. But I could also believe that makes life harder later on, when you want to pass just the pack_objects_args to prepare_pack_objects().

So I dunno. Mainly I am worried about somehow forgetting to set the flag in the right spot, but in practice there really is only place where it needs to get pulled over from po_args to cruft_po_args.

-Peff