Re: [PATCH v2 2/2] grep: fix grepping for "intent to add" files

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 22, 2016, 18:00 UTC
Message-ID: <xmqqlh1xm7c5.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CACsJy8C9Dh_Owr3UFJnCtvXserG4V-e1ws8ZY52ME1yr+fefOw@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 21 quoted lines

>> So I wonder if a better change would be more like
>>
>>         for (...) {
>>                 if (!S_ISREG(ce->ce_mode))
>>                         continue; /* not a regular file */
>>                 if (!ce_path_match(ce, pathspec, NULL)
>>                         continue; /* uninteresting */
>> +               if (cached && ce_intent_to_add(ce))
>> +                       continue; /* path not yet in the index */
>>
>>                 if (cached || ...)
>>                         UNCHANGED FROM THE ORIGINAL
>>
>> perhaps?
>
> I did wonder a bit about these cases. But, can i-t-a really be
> combined with CE_VALID or CE_SKIP_WORKTREE? CE_SKIP_... is
> automatically set and should not cover i-t-a entries imo (I didn't
> check the implementation). CE_VALID is about real entries, yes you
> could do "git update-index --assume-unchanged <ita-path>" but it does
> not feel right to me.

Yeah but we know people are stupid^W^Wdo unexpected things ;-)

> If cached is false and ce_ita() is true and either CE_VALID or
> CE_SKIP_WORKTREE is set, we would continue to grep an _empty_ SHA-1.
> But I think we should grep_file() instead, at least for CE_VALID.

Yes, that is the breakage I noticed in the patch under discussion and that I wanted to fix in the "I wonder if a better change would be..." version.