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

Re: [PATCH v2 03/14] dir.c: git-status --ignored: don't list empty ignored directories

From
Karsten Blees <karsten.blees@gmail.com>
Date
Apr 17, 2013, 00:31 UTC
Message-ID
<516DED4A.5050800@gmail.com>
In-Reply-To
<CALkWK0mG6bzWu7dqdP1mBe+AZfUDK=Mx4+ZkDOc2saXr3gBsUQ@mail.gmail.com>
Am 16.04.2013 19:48, schrieb Ramkumar Ramachandra:
Show 15 quoted lines
> Karsten Blees wrote:
>> 'git-status --ignored' lists ignored tracked directories without any
>> ignored files if a tracked file happens to match an exclude pattern.
> 
> Here, I have:
> 
>     quux/
>         bar
>         baz/
>             foo
> 
> So, quux is an ignored tracked directory.  bar is tracked, but matches
> an ignore pattern.  Currently, git status --ignored lists quux/.  I'm
> confused.
> 
The crucial part is 'without any ignored files'. So if you 'rm quux/baz/foo' or 'git-add -f quux/baz/foo', you get this:
1) git-status --ignored -uall: <empty>
2) git-status --ignored -unormal: quux/
In case 2), quux/ is listed as ignored even though there are no ignored files and the DIR_HIDE_EMPTY_DIRECTORIES flag should kick in.
Show 7 quoted lines
>> Always exclude tracked files.
> 
> "exclude" it from the 'git status --ignored' output, I presume?
> There's already an _exclude_ pattern in your previous sentence, so you
> can see why the reader might be confused about what you're talking
> about.
> 
I see, but "Always *ignore* tracked files" is also ambiguous. Suggestions?
Show 27 quoted lines
>> diff --git a/dir.c b/dir.c
>> index 7c9bc9c..fd1f088 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -1109,16 +1109,13 @@ static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude,
>>         struct path_exclude_check check;
>>         int exclude_file = 0;
>>
>> +       /* Always exclude indexed files */
>> +       if (index_name_exists(&the_index, path->buf, path->len, ignore_case))
>> +               return 1;
>> +
>>         if (exclude)
>>                 exclude_file = !(dir->flags & DIR_SHOW_IGNORED);
>>         else if (dir->flags & DIR_SHOW_IGNORED) {
>> -               /* Always exclude indexed files */
>> -               struct cache_entry *ce = index_name_exists(&the_index,
>> -                   path->buf, path->len, ignore_case);
>> -
>> -               if (ce)
>> -                       return 1;
>> -
> 
> Okay, so you just moved this segment outside the else if()
> conditional.  Can you explain what the old logic was doing, and what
> the rationale for it was?
>

Quoting Antoine's patch from January: + /* + * Optimization: + * Don't spend time on indexed files, they won't be + * added to the list anyway + */

In other words: don't do the (expensive) recursive is_path_excluded check if the file will be dropped later anyway (see the additional 'cache_name_is_other' checks in wt_status.c::wt_status_collect_untracked and builtin/ls-files.c::show_other_files).
In the DIR_SHOW_OTHER_DIRECTORIES case, we use read_directory_recursive to just *check* for ignored / untracked files, all within dir.c. So the results must be correct *here*, we cannot rely on some other modules to fix up incorrect files later.
Show 19 quoted lines
>> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
>> index 4ece129..28b7d95 100755
>> --- a/t/t7061-wtstatus-ignore.sh
>> +++ b/t/t7061-wtstatus-ignore.sh
>> @@ -122,10 +122,34 @@ cat >expected <<\EOF
>>  ?? .gitignore
>>  ?? actual
>>  ?? expected
>> +EOF
>> +
>> +test_expect_success 'status ignored tracked directory and ignored file with --ignore' '
>> +       echo "committed" >>.gitignore &&
>> +       git status --porcelain --ignored >actual &&
>> +       test_cmp expected actual
>> +'
> 
> Um, didn't really get this one.  You have three untracked files, and
> git status seems to be showing them fine.  What am I missing?
> 
We have this:
	tracked/
		committed

.gitignore: tracked committed

Both the directory and tracked file match an exclude pattern. There are no untracked ignored files, yet before this patch, git-status --ignored would list tracked/ as ignored directory.
Show 30 quoted lines
>> +cat >expected <<\EOF
>> +?? .gitignore
>> +?? actual
>> +?? expected
>> +EOF
>> +
>> +test_expect_success 'status ignored tracked directory and ignored file with --ignore -u' '
>> +       git status --porcelain --ignored -u >actual &&
>> +       test_cmp expected actual
>> +'
> 
> I didn't understand why you're invoking -u here (doesn't it imply
> "all", as opposed to "normal" when unspecified?).  There are really no
> directories, so I don't know what I'm expected to see.
> 
>> +cat >expected <<\EOF
>> +?? .gitignore
>> +?? actual
>> +?? expected
>>  !! tracked/
>>  EOF
>>
>>  test_expect_success 'status ignored tracked directory and uncommitted file with --ignore' '
>> +       echo "tracked" >.gitignore &&
>>         : >tracked/uncommitted &&
>>         git status --porcelain --ignored >actual &&
>>         test_cmp expected actual
> 
> Didn't we test this in the last patch?  Okay, I'm completely confused now.
> 
'echo "tracked" >.gitignore' reverts the 'echo "committed" >>.gitignore' from above.
Previous: Ramkumar RamachandraNext: Karsten Blees
Message 16 of 36 in “Improve git-status --ignored”
  1. 0/8 Improve git-status --ignoredKarsten Blees, Mar 18, 2013
  2. Junio C HamanoMar 19, 2013
  3. Duy NguyenMar 19, 2013
  4. Karsten BleesMar 19, 2013
  5. Junio C HamanoMar 19, 2013
  6. Duy NguyenMar 19, 2013
  7. 00/14 Improve git-status --ignoredKarsten Blees, Apr 15, 2013
  8. 01/14 dir.c: git-status --ignored: don't drop ignored directoriesKarsten Blees, Apr 15, 2013
  9. Ramkumar RamachandraApr 16, 2013
  10. Karsten BleesApr 17, 2013
  11. 02/14 dir.c: git-status --ignored: don't list files in ignored directoriesKarsten Blees, Apr 15, 2013
  12. read_revisions_from_stdin: make copies for handle_revision_argThomas Rast, Apr 16, 2013
  13. Junio C HamanoApr 16, 2013
  14. 03/14 dir.c: git-status --ignored: don't list empty ignored directoriesKarsten Blees, Apr 15, 2013
  15. Ramkumar RamachandraApr 16, 2013
  16. Karsten BleesApr 17, 2013
  17. 04/14 dir.c: git-ls-files --directories: don't hide empty directoriesKarsten Blees, Apr 15, 2013
  18. 05/14 dir.c: git-status --ignored: don't list empty directories as ignoredKarsten Blees, Apr 15, 2013
  19. 06/14 dir.c: make 'git-status --ignored' work within leading directoriesKarsten Blees, Apr 15, 2013
  20. 07/14 dir.c: git-clean -d -X: don't delete tracked directoriesKarsten Blees, Apr 15, 2013
  21. 08/14 dir.c: factor out parts of last_exclude_matching for later reuseKarsten Blees, Apr 15, 2013
  22. 09/14 dir.c: move prep_excludeKarsten Blees, Apr 15, 2013
  23. 10/14 dir.c: unify is_excluded and is_path_excluded APIsKarsten Blees, Apr 15, 2013
  24. Junio C HamanoApr 15, 2013
  25. 11/14 dir.c: replace is_path_excluded with now equivalent is_excluded APIKarsten Blees, Apr 15, 2013
  26. 12/14 dir.c: git-status: avoid is_excluded checks for tracked filesKarsten Blees, Apr 15, 2013
  27. 13/14 dir.c: git-status --ignored: don't scan the work tree three timesKarsten Blees, Apr 15, 2013
  28. 14/14 dir.c: git-status --ignored: don't scan the work tree twiceKarsten Blees, Apr 15, 2013
  29. Junio C HamanoApr 15, 2013
  30. Junio C HamanoApr 15, 2013
  31. Karsten BleesApr 15, 2013
  32. Junio C HamanoApr 15, 2013
  33. Karsten BleesApr 17, 2013
  34. Junio C HamanoApr 17, 2013
  35. 11/14 dir.c: replace is_path_excluded with now equivalent is_excluded APIKarsten Blees, Apr 17, 2013
  36. 14/14 dir.c: git-status --ignored: don't scan the work tree twiceKarsten Blees, Apr 17, 2013

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.