Jeff King <peff@xxxxxxxx> writes:
> 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.
> +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.