Jeff King <peff@xxxxxxxx> writes:
> On Tue, May 09, 2017 at 11:14:18AM +0900, Junio C Hamano wrote:
>
>> Jeff King <peff@xxxxxxxx> writes:
>>
>> >> Ah, OK, and now I understand why you called this a "bug" (which is
>> >> older and do not need to be addressed as part of 2.13) in the
>> >> original message. The new tests check requests that ought to
>> >> produce an empty packfile as the result actually do, but with the
>> >> current code, the reuse code does not work with --local and friends
>> >> and ends up including what was requested to be excluded.
>> >
>> > Right. Did you want me to try re-wording the commit message, or does it
>> > make sense now?
>>
>> It does make sense to me now, but I do not speak for all future
>> readers of "git log", so...
>
> I guess what I was asking was: do you still think it was unclear, or do
> you think you were just being dense?
>
> I don't feel like I gave any information in the follow-on explanation
> that wasn't in the commit message, so I wasn't clear if I worded it
> better or if it just sunk in better.
At least, "the current code is buggy when --local and friend are
given and includes needless objects in the result" was something I
learned only during the discussion, and would never have guessed by
reading the log message. The second paragraph does talk about "This
bug has been present since...", but the first paragraph does not say
anything about the current output being broken.
So, I am not sure if this was a case where I was dense. I think the
first paragraph needs a bit more clarity.
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.
Perhaps "... should be filtered out." can be followed by "However,
the current code fails to do so, and we end up including these
unwanted objects in the output.", and then "This is totally..." can
instead begin with "Besides, having to do per-object filtering is
totally...". I wouldn't have been confused if it were like so.