Re: [PATCH 1/3] repack: modify behavior of -A option to leave unreferenced objects unpacked

From: Brandon Casey <drafnel@gmail.com>
Date: May 11, 2008, 04:16 UTC
Message-ID: <ee63ef30805102116m68e83fadr8ef9afb080d26cf0@mail.gmail.com>
In-Reply-To: <20080510060345.GC11556@sigill.intra.peff.net>

On Sat, May 10, 2008 at 1:03 AM, Jeff King <peff@peff.net> wrote:

Show 7 quoted lines

> On Fri, May 09, 2008 at 11:01:55PM -0500, drafnel@gmail.com wrote:
>
>> -             keep_unreachable=--keep-unreachable ;;
>> +             keep_unreachable=t ;;
>
> Can we call this something else (like unpack_unreachable) since it now
> has nothing to do with the --keep-unreachable flag?

Actually I initially changed it to unpack_unreachable, and then changed it back. The reason I did this is because I think keep_unreachable still describes what is being accomplished, that unreachables are being kept. When -A is supplied along with -d, unreachables are kept by being unpacked. When -d is not supplied, unreachables are kept in their original pack file. If Geert's proposal or something else is implemented, keep_unreachable may still be appropriate. hmm?

Show 17 quoted lines

> Also, should --keep-unreachable be deprecated / removed?
>
>> +                     *)
>> +                             rm -f "$e.idx" "$e.keep"
>> +                             if test -n "$keep_unreachable" &&
>> +                                test -f "$e.pack"
>> +                             then
>> +                                     git unpack-objects < "$e.pack" || {
>> +                                             echo >&2 "Failed unpacking unreachable objects from redundant pack file $e.pack"
>> +                                             exit 1
>> +                                     }
>> +                             fi
>
> I still like Geert's suggestion of unpacking them to a _different_
> place. That helps to avoid spurious "gc --auto" invocations caused by
> too many prunable objects. Though it certainly doesn't solve it, and
> maybe that just needs to be fixed separately.

That was my thinking.

Show 10 quoted lines

>
> Possibly the "gc --auto" test should be:
>
>  - count objects; if too few, exit
>  - count unreachable loose objects; if too few, exit
>  - run gc
>
> That means having a lot of unreachable objects will still incur some
> extra processing, but not as much as a full repack. And it won't bug the
> user with a "you need to repack" message.

I've got a thought. How about limiting how often auto repack repacks by looking at the timestamp of the most recent pack? Wouldn't the packs already be prepared in most cases i.e. prepare_packed_git()

-brandon