On Mon, Mar 10, 2025 at 01:50:43AM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@xxxxxxxxx>
>
> This will be helpful in a future change, which will reuse this logic.
>
> Signed-off-by: Derrick Stolee <stolee@xxxxxxxxx>
> ---
> builtin/pack-objects.c | 53 +++++++++++++++++++++++-------------------
> 1 file changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 58a9b161262..1d0992a8dac 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -3196,6 +3196,33 @@ static int add_ref_tag(const char *tag UNUSED, const char *referent UNUSED, cons
> return 0;
> }
>
> +static int should_attempt_deltas(struct object_entry *entry)
> +{
> + if (DELTA(entry))
> + return 0;
> +
> + if (!entry->type_valid ||
> + oe_size_less_than(&to_pack, entry, 50))
> + return 0;
> +
> + if (entry->no_try_delta)
> + return 0;
> +
> + if (!entry->preferred_base) {
> + if (oe_type(entry) < 0)
> + die(_("unable to get type of object %s"),
> + oid_to_hex(&entry->idx.oid));
> + } else if (oe_type(entry) < 0) {
> + /*
> + * This object is not found, but we
> + * don't have to include it anyway.
> + */
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> static void prepare_pack(int window, int depth)
> {
> struct object_entry **delta_list;
> @@ -3226,33 +3253,11 @@ static void prepare_pack(int window, int depth)
> for (i = 0; i < to_pack.nr_objects; i++) {
> struct object_entry *entry = to_pack.objects + i;
>
> - if (DELTA(entry))
> - /* This happens if we decided to reuse existing
> - * delta from a pack. "reuse_delta &&" is implied.
> - */
It looks like this comment went away when this part of prepare_pack()
was extracted into should_attempt_deltas().
> - continue;
> -
> - if (!entry->type_valid ||
> - oe_size_less_than(&to_pack, entry, 50))
> + if (!should_attempt_deltas(entry))
> continue;
>
> - if (entry->no_try_delta)
> - continue;
> -
> - if (!entry->preferred_base) {
> + if (!entry->preferred_base)
> nr_deltas++;
Makes sense; should_attempt_deltas() doesn't itself change nr_deltas, so
we want to do it ourselves here. Looking good!
Thanks,
Taylor
