Re: [PATCH v6 00/11] nd/pack-objects-pack-struct updates

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 21, 2018, 16:31 UTC
Message-ID: <87tvt9xuel.fsf@evledraar.gmail.com>
In-Reply-To: <20180321082441.GB25537@sigill.intra.peff.net>

On Wed, Mar 21 2018, Jeff King wrote:

Show 10 quoted lines

> On Sun, Mar 18, 2018 at 03:25:15PM +0100, Nguyễn Thái Ngọc Duy wrote:
>
>> v6 fixes the one optimization that I just couldn't get right, fixes
>> two off-by-one error messages and a couple commit message update
>> (biggest change is in 11/11 to record some numbers from AEvar)
>
> [...]Yes, having that many packs is insane, but that's going to be
> small consolation to somebody whose automated maintenance program now
> craps out at 16k packs, when it previously would have just worked to
> fix the situation[...]

That's going to be super rare (and probably nonexisting) edge case, but (untested) I wonder if something like this on top would alleviate your concerns, i.e. instead of dying we just take the first N packs up to our limit:

    diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
    index 4406af640f..49d467ab2a 100644
    --- a/builtin/pack-objects.c
    +++ b/builtin/pack-objects.c
    @@ -1065,8 +1065,9 @@ static int want_object_in_pack(const struct object_id *oid,

            want = 1;
     done:
    -       if (want && *found_pack && !(*found_pack)->index)
    -               oe_add_pack(&to_pack, *found_pack);
    +       if (want && *found_pack && !(*found_pack)->index) {
    +               if (oe_add_pack(&to_pack, *found_pack) == -1)
    +                       return 0;

            return want;
     }
    diff --git a/pack-objects.h b/pack-objects.h
    index 9f8e450e19..50ed2028fb 100644
    --- a/pack-objects.h
    +++ b/pack-objects.h
    @@ -171,15 +171,17 @@ static inline void oe_set_in_pack_pos(const struct packing_data *pack,
            pack->in_pack_pos[e - pack->objects] = pos;
     }

    -static inline unsigned int oe_add_pack(struct packing_data *pack,
    +static inline int oe_add_pack(struct packing_data *pack,
                                           struct packed_git *p)
     {
    -       if (pack->in_pack_count >= (1 << OE_IN_PACK_BITS))
    -               die(_("too many packs to handle in one go. "
    -                     "Please add .keep files to exclude\n"
    -                     "some pack files and keep the number "
    -                     "of non-kept files below %d."),
    +       if (pack->in_pack_count >= (1 << OE_IN_PACK_BITS)) {
    +               warning(_("Too many packs to handle in one go. "
    +                         "Ran into the limit of %d.\n"
    +                         "Limping along by pretending packs beyond that"
    +                         "number have *.keep!"),
                        1 << OE_IN_PACK_BITS);
    +               return -1;
    +       }
            if (p) {
                    if (p->index > 0)
                            die("BUG: this packed is already indexed");