Re: [PATCH] pack-objects: disable pack reuse for object-selection options
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 8, 2017, 04:56 UTC
- Message-ID
- <xmqq4lww9cas.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170502084326.65eisqmr4th5cbf7@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 7 quoted lines
> If certain options like --honor-pack-keep, --local, or > --incremental are used with pack-objects, then we need to > feed each potential object to want_object_in_pack() to see > if it should be filtered out. This is totally contrary to > the purpose of the pack-reuse optimization, which tries hard > to avoid doing any per-object work. Therefore we need to > disable this optimization when these options are in use.
I read this five times, as I wanted to understand what you are saying, but I am not sure if I got it right. One of the reasons why I was confused was that I originally thought this "reuse" was about delta reuse, but it is not. It is "sending a slice of the original packfile straight to the output". But even after getting myself out of that confusion, I still do not see why we "need to disable". Surely, even if we need to exclude some objects from an existing packfile due to these selection options, we should be able to reuse the non-excluded part, no? The end result may involve having to pick and reuse more and smaller slices from existing packfiles, which may be much less efficient, but it is no immediately obvious to me if it leads to "need to disable". I would understand it if it were "it becomes much less efficient and we are better off not using the bitmap code at all", though.
Is the real reason why we want to disable the "reuse" because it is not easy to update the reuse_partial_packfile_from_bitmap() logic to take these selection options into account? If so, I would also understand why this is a good change.
Puzzled.
> This bug has been present since the inception of the > pack-reuse code, but was unlikely to come up in practice.
Show 5 quoted lines
> +test_expect_success 'pack reuse respects --honor-pack-keep' '
> + test_when_finished "rm -f .git/objects/pack/*.keep" &&
> + for i in .git/objects/pack/*.pack; do
> + >${i%.pack}.keep
> + done &&Micronit: style.