git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Duy Nguyen <pclouds@gmail.com>
Date
Jun 22, 2016, 18:32 UTC
Message-ID
<CACsJy8Acb+Hx1R66hcHQ7gNQ6TmKoUzC7Ar2PpSPkQeKM1EY8w@mail.gmail.com>
In-Reply-To
<xmqqlh1xm7c5.fsf@gitster.mtv.corp.google.com>
On Wed, Jun 22, 2016 at 8:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 33 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes:
>
>>> 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.

Heh.. I did guess that. Since neither solution is complete, I'm in favor of Charles's and assume that i-t-a forces to ignore CE_SKIP and CE_SKIP_WORKTREE. I could wait for people to come back complaining, then we know there are real users in very obscure cases and will fix it then.

-- 
Duy
Previous: Junio C HamanoNext: Junio C Hamano
Message 6 of 9 in “Fix duplicated test name”
  1. 1/2 Fix duplicated test nameCharles Bailey, Jun 21, 2016
  2. 2/2 grep: fix grepping for "intent to add" filesCharles Bailey, Jun 21, 2016
  3. Junio C HamanoJun 21, 2016
  4. Duy NguyenJun 22, 2016
  5. Junio C HamanoJun 22, 2016
  6. Duy NguyenJun 22, 2016
  7. Junio C HamanoJun 22, 2016
  8. Eric SunshineJun 22, 2016
  9. Duy NguyenJun 22, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.