Re: [PATCH] blame: default to ignoring revisions in .git-blame-ignore-revs

> "Junio C Hamano" <gitster@pobox.com> writes:
>
>> While wanting consistency is reasonable, the description above does
>> not quite match that goal.  If an untracked '.git-blame-ignore-revs'
>> file exists at the root of the working tree, or if a tracked one has
>> local changes relative to HEAD, the local repository behaves
>> differently from hosting sites that operate on the
>> 'HEAD:.git-blame-ignore-revs' blob.  It may make more sense to say:
>> "If the 'HEAD:.git-blame-ignore-revs' blob exists, it is added as
>> the initial element in the list of ignore-revs files.  Other files
>> listed in the configuration are also used, but an empty element
>> makes all elements that appeared before in the list forgotten."
>> This rule should apply whether the repository is bare or not.
>
> Thank you very much for the detailed feedback, Junio! Reading the
> committed blob from HEAD instead of the working tree totally makes
> sense.
>
>> Somebody has to audit the parser for these files (one unabbreviated
>> object name per line, ignoring whitespace and lines starting with
>> '#') and ensure that the implementation is truly secure.
>
> I looked through the parser in oidset.c (which we can share for
> both the HEAD blob and configured files) and peel_to_commit_oid in
> builtin/blame.c. Mostly looks good, IMHO, but there may be two edge
> cases we can tighten up:
>
> 1. Rejecting lines with embedded NUL bytes via memchr in oidset.c
>    (where strchr and the check after parse_oid_hex_algop currently
>    stop at the first NUL byte and ignore trailing bytes on the
>    line).
>
> 2. Passing OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK in
>    peel_to_commit_oid and peeling tags step by step so missing OIDs
>    or tag targets do not trigger lazy promisor fetches in partial
>    clones.
>
> Does this plan sound good to you for v2?