This patch series addresses several bugs and performance issues in .gitignore processing that came up in the inotify discussion.
Also available here:
https://github.com/kblees/git/tree/kb/improve-git-status-ignored
git pull git://github.com/kblees/git.git kb/improve-git-status-ignored
Patches 1 - 4 fix bugs in 'git-status --ignored' and add appropriate test cases.
Patches 5 - 7 eliminate the is_path_excluded API, in favor of a slightly improved and faster is_excluded. This speeds up 'git-ls-files --cached --ignored' by factor 5 - 6.
Patch 8 finally skips excluded checks for tracked files. With the bugs and is_path_excluded out of the way, it should be obvious that this can safely be done unconditionally without introducing regressions. Speeds up 'git-status [--ignored]' by factor 1.4 - 2.
I still believe that 'git-status --ignored' shouldn't list "ignored tracked" directories, to be consistent with the listing of untracked directories, and because "ignored tracked" contradicts the very definition of ignored content in gitignore(5).
Cheers,
Karsten
Karsten Blees (8):
dir.c: git-status --ignored: don't drop ignored directories
dir.c: git-status --ignored: don't list files in ignored directories
dir.c: git-status --ignored: don't list empty ignored directories
dir.c: git-status --ignored: don't list empty directories as ignored
dir.c: move prep_exclude and factor out parts of last_exclude_matching
dir.c: unify is_excluded and is_path_excluded APIs
dir.c: replace is_path_excluded with now equivalent is_excluded API
dir.c: git-status: avoid is_excluded checks for tracked files
builtin/add.c | 5 +-
builtin/check-ignore.c | 6 +-
builtin/ls-files.c | 15 +-
dir.c | 351 +++++++++++++++++----------------------------
dir.h | 22 +--
t/t7061-wtstatus-ignore.sh | 104 +++++++++++++-
unpack-trees.c | 10 +-
unpack-trees.h | 1 -
8 files changed, 243 insertions(+), 271 deletions(-)
--
This patch series addresses several bugs and performance issues in
.gitignore processing.
Patches #1 - #6 fix bugs and add appropriate test cases.
Patch #7 changes handling of "ignored tracked" directories, as I discovered
that with the current bahavior git-clean can delete tracked content.
Patches #8 - #14 are performance optimizations.
Also available here:
https://github.com/kblees/git/tree/kb/improve-git-status-ignored-v2
git pull git://github.com/kblees/git.git kb/improve-git-status-ignored-v2
Changes since v1 (old#->new#: description):
1->1: dir.c: git-status --ignored: don't drop ignored directories
2->2: dir.c: git-status --ignored: don't list files in ignored directories
3->3: dir.c: git-status --ignored: don't list empty ignored directories
*->4: dir.c: git-ls-files --directories: don't hide empty directories
- new bugfix
4->5: dir.c: git-status --ignored: don't list empty directories as ignored
- fixed typo in commit message thanks to Eric Sunshine
- patch is reduced to a one-liner (the DIR_HIDE_EMPTY_DIRECTORIES
flag has already been fixed in patch 4, renaming the variable and
tweaking DIR_SHOW_OTHER_DIRECTORIES is not strictly necessary)
*->6: dir.c: make 'git-status --ignored' work within leading directories
- new bugfix
*->7: dir.c: git-clean -d -X: don't delete tracked directories
- changes handling of "ignored tracked" directories
5->8: dir.c: factor out parts of last_exclude_matching for later reuse
5->9: dir.c: move prep_exclude
- split in two patches for cleaner diff
- removed unnecessary ';' after '}'
6->10: dir.c: unify is_excluded and is_path_excluded APIs
- fixed another typo in commit message
7->11: dir.c: replace is_path_excluded with now equivalent is_excluded API
8->12: dir.c: git-status: avoid is_excluded checks for tracked files
*->13: dir.c: git-status --ignored: don't scan the work tree three times
- new optimization
*->14: dir.c: git-status --ignored: don't scan the work tree twice
- new optimization
Karsten Blees (14):
dir.c: git-status --ignored: don't drop ignored directories
dir.c: git-status --ignored: don't list files in ignored directories
dir.c: git-status --ignored: don't list empty ignored directories
dir.c: git-ls-files --directories: don't hide empty directories
dir.c: git-status --ignored: don't list empty directories as ignored
dir.c: make 'git-status --ignored' work within leading directories
dir.c: git-clean -d -X: don't delete tracked directories
dir.c: factor out parts of last_exclude_matching for later reuse
dir.c: move prep_exclude
dir.c: unify is_excluded and is_path_excluded APIs
dir.c: replace is_path_excluded with now equivalent is_excluded API
dir.c: git-status: avoid is_excluded checks for tracked files
dir.c: git-status --ignored: don't scan the work tree three times
dir.c: git-status --ignored: don't scan the work tree twice
Documentation/technical/api-directory-listing.txt | 25 +-
builtin/add.c | 5 +-
builtin/check-ignore.c | 7 +-
builtin/ls-files.c | 15 +-
dir.c | 499 +++++++++-------------
dir.h | 25 +-
t/t3001-ls-files-others-exclude.sh | 49 +++
t/t7061-wtstatus-ignore.sh | 125 +++++-
t/t7300-clean.sh | 34 ++
unpack-trees.c | 10 +-
unpack-trees.h | 1 -
wt-status.c | 24 +-
12 files changed, 455 insertions(+), 364 deletions(-)
'git-status --ignored' drops ignored directories if they contain untracked
files in an untracked sub directory.
Fix it by getting exact (recursive) excluded status in treat_directory.
Signed-off-by: Karsten Blees <redacted>
---
dir.c | 9 +++++++++
t/t7061-wtstatus-ignore.sh | 27 +++++++++++++++++++++++++++
2 files changed, 36 insertions(+)
@@ -1060,6 +1060,15 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,/* This is the "show_other_directories" case */+/* might be a sub directory in an excluded directory */+if(!exclude){+structpath_exclude_checkcheck;+intdt=DT_DIR;+path_exclude_check_init(&check,dir);+exclude=is_path_excluded(&check,dirname,len,&dt);+path_exclude_check_clear(&check);+}+/**Wearelookingforignoredfilesandourdirectoryisnotignored,*checkifitcontainsonlyignoredfiles
'git-status --ignored' lists both the ignored directory and the ignored
files if the files are in a tracked sub directory.
When recursing into sub directories in read_directory_recursive, pass on
the check_only parameter so that we don't accidentally add the files.
Signed-off-by: Karsten Blees <redacted>
---
dir.c | 4 +---
t/t7061-wtstatus-ignore.sh | 27 +++++++++++++++++++++++++++
2 files changed, 28 insertions(+), 3 deletions(-)
@@ -170,4 +170,31 @@ test_expect_success 'status ignored tracked directory with uncommitted file in utest_cmpexpectedactual'+cat>expected<<\EOF+??.gitignore+??actual+??expected+!!tracked/+EOF++test_expect_success'status ignored tracked directory with uncommitted file in tracked subdir with --ignore''+:>tracked/ignored/committed&&+gitadd-ftracked/ignored/committed&&+gitcommit-m.&&+gitstatus--porcelain--ignored>actual&&+test_cmpexpectedactual+'++cat>expected<<\EOF+??.gitignore+??actual+??expected+!!tracked/ignored/uncommitted+EOF++test_expect_success'status ignored tracked directory with uncommitted file in tracked subdir with --ignore -u''+gitstatus--porcelain--ignored-u>actual&&+test_cmpexpectedactual+'+ test_done
'git-ls-files --ignored --directories' hides empty directories even though
--no-empty-directory was not specified.
Treat the DIR_HIDE_EMPTY_DIRECTORIES flag independently from
DIR_SHOW_IGNORED to make all git-ls-files options work as expected.
Signed-off-by: Karsten Blees <redacted>
---
dir.c | 6 ++----
t/t3001-ls-files-others-exclude.sh | 23 +++++++++++++++++++++++
wt-status.c | 2 +-
3 files changed, 26 insertions(+), 5 deletions(-)
'git-status --ignored' lists empty untracked directories as ignored, even
though they don't have any ignored files.
When checking if a directory is already listed as untracked (i.e. shouldn't
be listed as ignored as well), don't assume that the directory has only
ignored files if it doesn't have untracked files, as the directory may be
empty.
Signed-off-by: Karsten Blees <redacted>
---
dir.c | 5 +++--
t/t7061-wtstatus-ignore.sh | 26 ++++++++++++++++++++++++--
2 files changed, 27 insertions(+), 4 deletions(-)
'git-status --ignored path/' doesn't list ignored files and directories
within 'path' if some component of 'path' is classified as untracked.
Disable the DIR_SHOW_OTHER_DIRECTORIES flag while traversing leading
directories. This prevents treat_leading_path() with DIR_SHOW_IGNORED flag
from aborting at the top level untracked directory.
As a side effect, this also eliminates a recursive directory scan per
leading directory level, as treat_directory() can no longer call
read_directory_recursive() when called from treat_leading_path().
Signed-off-by: Karsten Blees <redacted>
---
dir.c | 3 +++
t/t7061-wtstatus-ignore.sh | 19 +++++++++++++++++++
2 files changed, 22 insertions(+)
The notion of "ignored tracked" directories introduced in 721ac4ed "dir.c:
Make git-status --ignored more consistent" has a few unwanted side effects:
- git-clean -d -X: deletes ignored tracked directories. git-clean should
never delete tracked content.
- git-ls-files --ignored --other --directory: lists ignored tracked
directories instead of "other" directories.
- git-status --ignored: lists ignored tracked directories while contained
files may be listed as modified. Paths listed by git-status should be
disjoint (except in long format where a path may be listed in both the
staged and unstaged section).
Additionally, the current behaviour violates documentation in gitignore(5)
("Specifies intentionally *untracked* files to ignore") and Documentation/
technical/api-directory-listing.txt ("DIR_SHOW_OTHER_DIRECTORIES: Include
a directory that is *not tracked*.").
In dir.c::treat_directory, remove the special handling of ignored tracked
directories, so that the DIR_SHOW_OTHER_DIRECTORIES flag only affects
"other" (i.e. untracked) directories. In dir.c::dir_add_name, check that
added paths are untracked even if DIR_SHOW_IGNORED is set.
Signed-off-by: Karsten Blees <redacted>
---
dir.c | 11 +++--------
t/t3001-ls-files-others-exclude.sh | 26 ++++++++++++++++++++++++++
t/t7061-wtstatus-ignore.sh | 6 +++---
t/t7300-clean.sh | 34 ++++++++++++++++++++++++++++++++++
4 files changed, 66 insertions(+), 11 deletions(-)
@@ -1037,9 +1035,6 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,/* The "len-1" is to strip the final '/' */switch(directory_exists_in_index(dirname,len-1)){caseindex_directory:-if((dir->flags&DIR_SHOW_OTHER_DIRECTORIES)&&exclude)-break;-returnrecurse_into_directory;caseindex_gitdir:
@@ -549,78 +549,6 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)die("cannot use %s as an exclude file",fname);}-/*-*Loadstheper-directoryexcludelistforthesubstringofbase-*whichhasacharlengthofbaselen.-*/-staticvoidprep_exclude(structdir_struct*dir,constchar*base,intbaselen)-{-structexclude_list_group*group;-structexclude_list*el;-structexclude_stack*stk=NULL;-intcurrent;--if((!dir->exclude_per_dir)||-(baselen+strlen(dir->exclude_per_dir)>=PATH_MAX))-return;/* too long a path -- ignore */--group=&dir->exclude_list_group[EXC_DIRS];--/* Pop the exclude lists from the EXCL_DIRS exclude_list_group-*whichoriginatefromdirectoriesnotintheprefixofthe-*pathbeingchecked.*/-while((stk=dir->exclude_stack)!=NULL){-if(stk->baselen<=baselen&&-!strncmp(dir->basebuf,base,stk->baselen))-break;-el=&group->el[dir->exclude_stack->exclude_ix];-dir->exclude_stack=stk->prev;-free((char*)el->src);/* see strdup() below */-clear_exclude_list(el);-free(stk);-group->nr--;-}--/* Read from the parent directories and push them down. */-current=stk?stk->baselen:-1;-while(current<baselen){-structexclude_stack*stk=xcalloc(1,sizeof(*stk));-constchar*cp;--if(current<0){-cp=base;-current=0;-}-else{-cp=strchr(base+current+1,'/');-if(!cp)-die("oops in prep_exclude");-cp++;-}-stk->prev=dir->exclude_stack;-stk->baselen=cp-base;-memcpy(dir->basebuf+current,base+current,-stk->baselen-current);-strcpy(dir->basebuf+stk->baselen,dir->exclude_per_dir);-/*-*dir->basebufgetsreusedbythetraversal,butwe-*needfnametoremainunchangedtoensurethesrc-*memberofeachstructexcludecorrectly-*back-referencesitssourcefile.Otherinvocations-*ofadd_exclude_listprovidestablestrings,sowe-*strdup()andfree()hereinthecaller.-*/-el=add_exclude_list(dir,EXC_DIRS,strdup(dir->basebuf));-stk->exclude_ix=group->nr-1;-add_excludes_from_file_to_list(dir->basebuf,-dir->basebuf,stk->baselen,-el,1);-dir->exclude_stack=stk;-current=stk->baselen;-}-dir->basebuf[baselen]='\0';-}-intmatch_basename(constchar*basename,intbasenamelen,constchar*pattern,intprefix,intpatternlen,intflags)
@@ -772,6 +700,78 @@ static struct exclude *last_exclude_matching_from_lists(struct dir_struct *dir,}/*+*Loadstheper-directoryexcludelistforthesubstringofbase+*whichhasacharlengthofbaselen.+*/+staticvoidprep_exclude(structdir_struct*dir,constchar*base,intbaselen)+{+structexclude_list_group*group;+structexclude_list*el;+structexclude_stack*stk=NULL;+intcurrent;++if((!dir->exclude_per_dir)||+(baselen+strlen(dir->exclude_per_dir)>=PATH_MAX))+return;/* too long a path -- ignore */++group=&dir->exclude_list_group[EXC_DIRS];++/* Pop the exclude lists from the EXCL_DIRS exclude_list_group+*whichoriginatefromdirectoriesnotintheprefixofthe+*pathbeingchecked.*/+while((stk=dir->exclude_stack)!=NULL){+if(stk->baselen<=baselen&&+!strncmp(dir->basebuf,base,stk->baselen))+break;+el=&group->el[dir->exclude_stack->exclude_ix];+dir->exclude_stack=stk->prev;+free((char*)el->src);/* see strdup() below */+clear_exclude_list(el);+free(stk);+group->nr--;+}++/* Read from the parent directories and push them down. */+current=stk?stk->baselen:-1;+while(current<baselen){+structexclude_stack*stk=xcalloc(1,sizeof(*stk));+constchar*cp;++if(current<0){+cp=base;+current=0;+}+else{+cp=strchr(base+current+1,'/');+if(!cp)+die("oops in prep_exclude");+cp++;+}+stk->prev=dir->exclude_stack;+stk->baselen=cp-base;+memcpy(dir->basebuf+current,base+current,+stk->baselen-current);+strcpy(dir->basebuf+stk->baselen,dir->exclude_per_dir);+/*+*dir->basebufgetsreusedbythetraversal,butwe+*needfnametoremainunchangedtoensurethesrc+*memberofeachstructexcludecorrectly+*back-referencesitssourcefile.Otherinvocations+*ofadd_exclude_listprovidestablestrings,sowe+*strdup()andfree()hereinthecaller.+*/+el=add_exclude_list(dir,EXC_DIRS,strdup(dir->basebuf));+stk->exclude_ix=group->nr-1;+add_excludes_from_file_to_list(dir->basebuf,+dir->basebuf,stk->baselen,+el,1);+dir->exclude_stack=stk;+current=stk->baselen;+}+dir->basebuf[baselen]='\0';+}++/**Loadstheexcludelistsforthedirectorycontainingpathname,then*scansallexcludeliststodeterminewhetherpathnameisexcluded.*Returnstheexclude_listelementwhichmatched,orNULLfor
The is_excluded and is_path_excluded APIs are very similar, except for a
few noteworthy differences:
is_excluded doesn't handle ignored directories, results for paths within
ignored directories are incorrect. This is probably based on the premise
that recursive directory scans should stop at ignored directories, which
is no longer true (in certain cases, read_directory_recursive currently
calls is_excluded *and* is_path_excluded to get correct ignored state).
is_excluded caches parsed .gitignore files of the last directory in struct
dir_struct. If the directory changes, it finds a common parent directory
and is very careful to drop only as much state as necessary. On the other
hand, is_excluded will also read and parse .gitignore files in already
ignored directories, which are completely irrelevant.
is_path_excluded correctly handles ignored directories by checking if any
component in the path is excluded. As it uses is_excluded internally, this
unfortunately forces is_excluded to drop and re-read all .gitignore files,
as there is no common parent directory for the root dir.
is_path_excluded tracks state in a separate struct path_exclude_check,
which is essentially a wrapper of dir_struct with two more fields. However,
as is_path_excluded also modifies dir_struct, it is not possible to e.g.
use multiple path_exclude_check structures with the same dir_struct in
parallel. The additional structure just unnecessarily complicates the API.
Teach is_excluded / prep_exclude about ignored directories: whenever
entering a new directory, first check if the entire directory is excluded.
Remember the excluded state in dir_struct. Don't traverse into already
ignored directories (i.e. don't read irrelevant .gitignore files).
Directories could also be excluded by exclude patterns specified on the
command line or .git/info/exclude, so we cannot simply skip prep_exclude
entirely if there's no .gitignore file name (dir_struct.exclude_per_dir).
Move this check to just before actually reading the file.
is_path_excluded is now equivalent to is_excluded, so we can simply
redirect to it (the public API is cleaned up in the next patch).
The performance impact of the additional ignored check per directory is
hardly noticeable when reading directories recursively (e.g. 'git status').
However, performance of git commands using the is_path_excluded API (e.g.
'git ls-files --cached --ignored --exclude-standard') is greatly improved
as this no longer re-reads .gitignore files on each call.
Here's some performance data from the linux and WebKit repos (best of 10
runs on a Debian Linux on SSD, core.preloadIndex=true):
| ls-files -ci | status | status --ignored
| linux | WebKit | linux | WebKit | linux | WebKit
-------+-------+--------+-------+--------+-------+---------
before | 0.506 | 6.539 | 0.212 | 1.555 | 0.323 | 2.541
after | 0.080 | 1.191 | 0.218 | 1.583 | 0.321 | 2.579
gain | 6.325 | 5.490 | 0.972 | 0.982 | 1.006 | 0.985
Signed-off-by: Karsten Blees <redacted>
---
dir.c | 107 +++++++++++++++++++++++++++---------------------------------------
dir.h | 6 ++--
2 files changed, 46 insertions(+), 67 deletions(-)
@@ -710,10 +710,6 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)structexclude_stack*stk=NULL;intcurrent;-if((!dir->exclude_per_dir)||-(baselen+strlen(dir->exclude_per_dir)>=PATH_MAX))-return;/* too long a path -- ignore */-group=&dir->exclude_list_group[EXC_DIRS];/* Pop the exclude lists from the EXCL_DIRS exclude_list_group
@@ -725,12 +721,17 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)break;el=&group->el[dir->exclude_stack->exclude_ix];dir->exclude_stack=stk->prev;+dir->exclude=NULL;free((char*)el->src);/* see strdup() below */clear_exclude_list(el);free(stk);group->nr--;}+/* Skip traversing into sub directories if the parent is excluded */+if(dir->exclude)+return;+/* Read from the parent directories and push them down. */current=stk?stk->baselen:-1;while(current<baselen){
@@ -749,22 +750,43 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)}stk->prev=dir->exclude_stack;stk->baselen=cp-base;+stk->exclude_ix=group->nr;+el=add_exclude_list(dir,EXC_DIRS,NULL);memcpy(dir->basebuf+current,base+current,stk->baselen-current);-strcpy(dir->basebuf+stk->baselen,dir->exclude_per_dir);-/*-*dir->basebufgetsreusedbythetraversal,butwe-*needfnametoremainunchangedtoensurethesrc-*memberofeachstructexcludecorrectly-*back-referencesitssourcefile.Otherinvocations-*ofadd_exclude_listprovidestablestrings,sowe-*strdup()andfree()hereinthecaller.-*/-el=add_exclude_list(dir,EXC_DIRS,strdup(dir->basebuf));-stk->exclude_ix=group->nr-1;-add_excludes_from_file_to_list(dir->basebuf,-dir->basebuf,stk->baselen,-el,1);++/* Abort if the directory is excluded */+if(stk->baselen){+intdt=DT_DIR;+dir->basebuf[stk->baselen-1]=0;+dir->exclude=last_exclude_matching_from_lists(dir,+dir->basebuf,stk->baselen-1,+dir->basebuf+current,&dt);+dir->basebuf[stk->baselen-1]='/';+if(dir->exclude){+dir->basebuf[stk->baselen]=0;+dir->exclude_stack=stk;+return;+}+}++/* Try to read per-directory file unless path is too long */+if(dir->exclude_per_dir&&+stk->baselen+strlen(dir->exclude_per_dir)<PATH_MAX){+strcpy(dir->basebuf+stk->baselen,+dir->exclude_per_dir);+/*+*dir->basebufgetsreusedbythetraversal,butwe+*needfnametoremainunchangedtoensurethesrc+*memberofeachstructexcludecorrectly+*back-referencesitssourcefile.Otherinvocations+*ofadd_exclude_listprovidestablestrings,sowe+*strdup()andfree()hereinthecaller.+*/+el->src=strdup(dir->basebuf);+add_excludes_from_file_to_list(dir->basebuf,+dir->basebuf,stk->baselen,el,1);+}dir->exclude_stack=stk;current=stk->baselen;}
@@ -831,49 +853,6 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,constchar*name,intnamelen,int*dtype){-inti;-structstrbuf*path=&check->path;-structexclude*exclude;--/*-*weallowthecallertopassnamelenasanoptimization;it-*mustmatchthelengthofthename,asweeventuallycall-*is_excluded()onthewholenamestring.-*/-if(namelen<0)-namelen=strlen(name);--/*-*Ifpathisnon-empty,andnameisequaltopathora-*subdirectoryofpath,nameshouldbeexcluded,because-*it'sinsideadirectorywhichisalreadyknowntobe-*excludedandwaspreviouslyleftincheck->path.-*/-if(path->len&&-path->len<=namelen&&-!memcmp(name,path->buf,path->len)&&-(!name[path->len]||name[path->len]=='/'))-returncheck->exclude;--strbuf_setlen(path,0);-for(i=0;name[i];i++){-intch=name[i];--if(ch=='/'){-intdt=DT_DIR;-exclude=last_exclude_matching(check->dir,-path->buf,&dt);-if(exclude){-check->exclude=exclude;-returnexclude;-}-}-strbuf_addch(path,ch);-}--/* An entry in the index; cannot be a directory with subentries */-strbuf_setlen(path,0);-returnlast_exclude_matching(check->dir,name,dtype);}
@@ -454,7 +452,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)&&!file_exists(pathspec[i])){if(ignore_missing){intdtype=DT_UNKNOWN;-if(is_path_excluded(&check,pathspec[i],-1,&dtype))+if(is_excluded(&dir,pathspec[i],&dtype))dir_add_ignored(&dir,pathspec[i],strlen(pathspec[i]));}elsedie(_("pathspec '%s' did not match any files"),
@@ -59,7 +59,6 @@ static int check_ignore(const char *prefix, const char **pathspec)constchar*path,*full_path;char*seen;intnum_ignored=0,dtype=DT_UNKNOWN,i;-structpath_exclude_checkcheck;structexclude*exclude;/* read_cache() is only necessary so we can watch out for submodules. */
@@ -201,19 +201,15 @@ static void show_ru_info(void)}}-staticintce_excluded(structpath_exclude_check*check,structcache_entry*ce)+staticintce_excluded(structdir_struct*dir,structcache_entry*ce){intdtype=ce_to_dtype(ce);-returnis_path_excluded(check,ce->name,ce_namelen(ce),&dtype);+returnis_excluded(dir,ce->name,&dtype);}staticvoidshow_files(structdir_struct*dir){inti;-structpath_exclude_checkcheck;--if((dir->flags&DIR_SHOW_IGNORED))-path_exclude_check_init(&check,dir);/* For cached/deleted files we don't need to even do the readdir */if(show_others||show_killed){
@@ -830,47 +830,6 @@ static int is_excluded(struct dir_struct *dir, const char *pathname, int *dtype_return0;}-voidpath_exclude_check_init(structpath_exclude_check*check,-structdir_struct*dir)-{-check->dir=dir;-}--voidpath_exclude_check_clear(structpath_exclude_check*check)-{-}--/*-*Foreachsubdirectoryinname,startingwiththetop-most,checks-*toseeifthatsubdirectoryisexcluded,andifso,returnsthe-*correspondingexcludestructure.Otherwise,checkswhethername-*itself(whichispresumablyafile)isexcluded.-*-*Apathtoadirectoryknowntobeexcludedisleftincheck->pathto-*optimizeforrepeatedchecksforfilesinthesameexcludeddirectory.-*/-structexclude*last_exclude_matching_path(structpath_exclude_check*check,-constchar*name,intnamelen,-int*dtype)-{-returnlast_exclude_matching(check->dir,name,dtype);-}--/*-*Isthisnameexcluded?Thisisforacallerlikeshow_files()that-*donothonordirectoryhierarchyanditeratethroughpathsthatare-*possiblyinanignoreddirectory.-*/-intis_path_excluded(structpath_exclude_check*check,-constchar*name,intnamelen,int*dtype)-{-structexclude*exclude=-last_exclude_matching_path(check,name,namelen,dtype);-if(exclude)-returnexclude->flags&EXC_FLAG_NEGATIVE?0:1;-return0;-}-staticstructdir_entry*dir_entry_new(constchar*pathname,intlen){structdir_entry*ent;
@@ -1042,15 +1001,6 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,/* This is the "show_other_directories" case */-/* might be a sub directory in an excluded directory */-if(!exclude){-structpath_exclude_checkcheck;-intdt=DT_DIR;-path_exclude_check_init(&check,dir);-exclude=is_path_excluded(&check,dirname,len,&dt);-path_exclude_check_clear(&check);-}-/**Wearelookingforignoredfilesandourdirectoryisnotignored,*checkifitcontainsuntrackedfiles(i.e.islistedasuntracked)
@@ -1375,7 +1367,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,return0;if(o->dir&&-is_path_excluded(o->path_exclude_check,name,-1,&dtype))+is_excluded(o->dir,name,&dtype))/**ce->nameisexplicitlyexcluded,soitisOkto*overwriteit.
Checking if a file is in the index is much faster (hashtable lookup) than
checking if the file is excluded (linear search over exclude patterns).
Skip is_excluded checks for files: move the cache_name_exists check from
treat_file to treat_one_path and return early if the file is tracked.
This can safely be done as all other code paths also return path_ignored
for tracked files, and dir_add_ignored skips tracked files as well.
There's just one line left in treat_file, so move this to treat_one_path
as well.
Here's some performance data for git-status from the linux and WebKit
repos (best of 10 runs on a Debian Linux on SSD, core.preloadIndex=true):
| status | status --ignored
| linux | WebKit | linux | WebKit
-------+-------+--------+-------+---------
before | 0.218 | 1.583 | 0.321 | 2.579
after | 0.156 | 0.988 | 0.202 | 1.279
gain | 1.397 | 1.602 | 1.589 | 2.016
Signed-off-by: Karsten Blees <redacted>
---
dir.c | 38 +++++++++++---------------------------
1 file changed, 11 insertions(+), 27 deletions(-)
'git-status --ignored' recursively scans directories up to three times:
1. To collect untracked files.
2. To collect ignored files.
3. When collecting ignored files, to check that an untracked directory
that potentially contains ignored files doesn't also contain untracked
files (i.e. isn't already listed as untracked).
Let's get rid of case 3 first.
Currently, read_directory_recursive returns a boolean whether a directory
contains the requested files or not (actually, it returns the number of
files, but no caller actually needs that), and DIR_SHOW_IGNORED specifies
what we're looking for.
To be able to test for both untracked and ignored files in a single scan,
we need to return a bit more info, and the result must be independent of
the DIR_SHOW_IGNORED flag.
Reuse the path_treatment enum as return value of read_directory_recursive.
Split path_handled in two separate values path_excluded and path_untracked
that don't change their meaning with the DIR_SHOW_IGNORED flag. We don't
need an extra value path_untracked_and_excluded, as directories with both
untracked and ignored files should be listed as untracked.
Rename path_ignored to path_none for clarity (i.e. "don't treat that path"
in contrast to "the path is ignored and should be treated according to
DIR_SHOW_IGNORED").
Replace enum directory_treatment with path_treatment. That's just another
enum with the same meaning, no need to translate back and forth.
In treat_directory, get rid of the extra read_directory_recursive call and
all the DIR_SHOW_IGNORED-specific code.
In read_directory_recursive, decide whether to dir_add_name path_excluded
or path_untracked paths based on the DIR_SHOW_IGNORED flag.
The return value of read_directory_recursive is the maximum path_treatment
of all files and sub-directories. In the check_only case, abort when we've
reached the most significant value (path_untracked).
Signed-off-by: Karsten Blees <redacted>
---
dir.c | 146 +++++++++++++++++++++++++++++++++---------------------------------
1 file changed, 72 insertions(+), 74 deletions(-)
@@ -958,35 +972,26 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len)**(a)if"show_other_directories"istrue,weshowitas*justadirectory,unless"hide_empty_directories"is-*alsotrueandthedirectoryisempty,inwhichcase-*wejustignoreitentirely.-*ifwearelookingforignoreddirectories,lookifit-*containsonlyignoredfilestodecideifitmustbeshownas-*ignoredornot.+*alsotrue,inwhichcaseweneedtocheckifitcontainsany+*untrackedand/orignoredfiles.*(b)ifitlookslikeagitdirectory,andwedon'thave*'no_gitlinks'setwetreatitasagitlink,andshowit*asadirectory.*(c)otherwise,werecurseintoit.*/-enumdirectory_treatment{-show_directory,-ignore_directory,-recurse_into_directory-};--staticenumdirectory_treatmenttreat_directory(structdir_struct*dir,+staticenumpath_treatmenttreat_directory(structdir_struct*dir,constchar*dirname,intlen,intexclude,conststructpath_simplify*simplify){/* The "len-1" is to strip the final '/' */switch(directory_exists_in_index(dirname,len-1)){caseindex_directory:-returnrecurse_into_directory;+returnpath_recurse;caseindex_gitdir:if(dir->flags&DIR_SHOW_OTHER_DIRECTORIES)-returnignore_directory;-returnshow_directory;+returnpath_none;+returnpath_untracked;caseindex_nonexistent:if(dir->flags&DIR_SHOW_OTHER_DIRECTORIES)
@@ -994,32 +999,17 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,if(!(dir->flags&DIR_NO_GITLINKS)){unsignedcharsha1[20];if(resolve_gitlink_ref(dirname,"HEAD",sha1)==0)-returnshow_directory;+returnpath_untracked;}-returnrecurse_into_directory;+returnpath_recurse;}/* This is the "show_other_directories" case */-/*-*Wearelookingforignoredfilesandourdirectoryisnotignored,-*checkifitcontainsuntrackedfiles(i.e.islistedasuntracked)-*/-if((dir->flags&DIR_SHOW_IGNORED)&&!exclude){-intignored;-dir->flags&=~DIR_SHOW_IGNORED;-ignored=read_directory_recursive(dir,dirname,len,1,simplify);-dir->flags|=DIR_SHOW_IGNORED;--if(ignored)-returnignore_directory;-}-if(!(dir->flags&DIR_HIDE_EMPTY_DIRECTORIES))-returnshow_directory;-if(!read_directory_recursive(dir,dirname,len,1,simplify))-returnignore_directory;-returnshow_directory;+returnexclude?path_excluded:path_untracked;++returnread_directory_recursive(dir,dirname,len,1,simplify);}/*
@@ -1134,12 +1124,6 @@ static int get_dtype(struct dirent *de, const char *path, int len)returndtype;}-enumpath_treatment{-path_ignored,-path_handled,-path_recurse-};-staticenumpath_treatmenttreat_one_path(structdir_struct*dir,structstrbuf*path,conststructpath_simplify*simplify,
@@ -1234,26 +1210,48 @@ static int read_directory_recursive(struct dir_struct *dir,gotoout;while((de=readdir(fdir))!=NULL){-switch(treat_path(dir,de,&path,baselen,simplify)){-casepath_recurse:-contents+=read_directory_recursive(dir,path.buf,+/* check how the file or directory should be treated */+state=treat_path(dir,de,&path,baselen,simplify);+if(state>dir_state)+dir_state=state;++/* recurse into subdir if instructed by treat_path */+if(state==path_recurse){+subdir_state=read_directory_recursive(dir,path.buf,path.len,check_only,simplify);+if(subdir_state>dir_state)+dir_state=subdir_state;+}++if(check_only){+/* abort early if maximum state has been reached */+if(dir_state==path_untracked)+break;+/* skip the dir_add_* part */continue;-casepath_ignored:-continue;-casepath_handled:-break;}-contents++;-if(check_only)++/* add the path to the appropriate result list */+switch(state){+casepath_excluded:+if(dir->flags&DIR_SHOW_IGNORED)+dir_add_name(dir,path.buf,path.len);+break;++casepath_untracked:+if(!(dir->flags&DIR_SHOW_IGNORED))+dir_add_name(dir,path.buf,path.len);break;-dir_add_name(dir,path.buf,path.len);++default:+break;+}}closedir(fdir);out:strbuf_release(&path);-returncontents;+returndir_state;}staticintcmp_name(constvoid*p1,constvoid*p2)
@@ -1324,7 +1322,7 @@ static int treat_leading_path(struct dir_struct *dir,if(simplify_away(sb.buf,sb.len,simplify))break;if(treat_one_path(dir,&sb,simplify,-DT_DIR,NULL)==path_ignored)+DT_DIR,NULL)==path_none)break;/* do not recurse into it */if(len<=baselen){rc=1;
'git-status --ignored' still scans the work tree twice to collect
untracked and ignored files, respectively.
fill_directory / read_directory already supports collecting untracked and
ignored files in a single directory scan. However, the DIR_COLLECT_IGNORED
flag to enable this has some git-add specific side-effects (e.g. it
doesn't recurse into ignored directories, so listing ignored files with
--untracked=all doesn't work).
The DIR_SHOW_IGNORED flag doesn't list untracked files and returns ignored
files in dir_struct.entries[] (instead of dir_struct.ignored[] as
DIR_COLLECT_IGNORED). DIR_SHOW_IGNORED is used all throughout git.
We don't want to break the existing API, so lets introduce a new flag
DIR_SHOW_IGNORED_TOO that lists untracked as well as ignored files similar
to DIR_COLLECT_FILES, but will recurse into sub-directories based on the
other flags as DIR_SHOW_IGNORED does.
In dir.c::read_directory_recursive, add ignored files to either
dir_struct.entries[] or dir_struct.ignored[] based on the flags. Also move
the DIR_COLLECT_IGNORED case here so that filling result lists is in a
common place.
In wt-status.c::wt_status_collect_untracked, use the new flag and read
results from dir_struct.ignored[]. Remove the extra fill_directory call.
builtin/check-ignore.c doesn't call fill_directory, setting the git-add
specific DIR_COLLECT_IGNORED flag has no effect here. Remove for clarity.
Update API documentation to reflect the changes.
Performance: with this patch, 'git-status --ignored' is typically as fast
as 'git-status'.
Signed-off-by: Karsten Blees <redacted>
---
Documentation/technical/api-directory-listing.txt | 25 ++++++++++++++++++++---
builtin/check-ignore.c | 1 -
dir.c | 10 +++++----
dir.h | 3 ++-
wt-status.c | 24 ++++++++++------------
5 files changed, 41 insertions(+), 22 deletions(-)
@@ -22,12 +22,23 @@ The notable options are: `flags`::- A bit-field of options:+ A bit-field of options (the `*IGNORED*` flags are mutually exclusive): `DIR_SHOW_IGNORED`:::- The traversal is for finding just ignored files, not unignored- files.+ Return just ignored files in `entries[]`, not untracked files.++`DIR_SHOW_IGNORED_TOO`:::++ Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`+ in addition to untracked files in `entries[]`.++`DIR_COLLECT_IGNORED`:::++ Special mode for git-add. Return ignored files in `ignored[]` and+ untracked files in `entries[]`. Only returns ignored files that match+ pathspec exactly (no wildcards). Does not recurse into ignored+ directories. `DIR_SHOW_OTHER_DIRECTORIES`:::
@@ -57,6 +68,14 @@ The result of the enumeration is left in these fields: Internal use; keeps track of allocation of `entries[]` array.+`ignored[]`::++ An array of `struct dir_entry`, used for ignored paths with the+ `DIR_SHOW_IGNORED_TOO` and `DIR_COLLECT_IGNORED` flags.++`ignored_nr`::++ The number of members in `ignored[]` array. Calling sequence ----------------
From: Thomas Rast <hidden> Date: 2016-06-15 22:56:51
read_revisions_from_stdin() has passed pointers to its read buffer
down to handle_revision_arg() since its inception way back in 42cabc3
(Teach rev-list an option to read revs from the standard input.,
2006-09-05). Even back then, this was a bug: through
add_pending_object, the argument was recorded in the object_array's
'name' field.
Fix it by making a copy whenever read_revisions_from_stdin() passes an
argument down the callchain. The other caller runs handle_revision_arg()
on argv[], where it would be redundant to make a copy.
Signed-off-by: Thomas Rast <redacted>
---
So I changed my mind. Your "easy fix" looks to me the right thing
to do.
So here's the same with a commit message and signoff. I hope I got my
history right; I didn't look too long if it had any users, but it was
definitely recorded.
revision.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Firstly, great work on the series! I've just started looking into it,
so please don't take my comments too seriously: some of them may be
queries, and others may be minor suggestions, but I can't say I
understand the area you're patching. I know Junio doesn't like me
mixing queries in reviews, but I don't fully agree with his policy.
Karsten Blees wrote:
'git-status --ignored' drops ignored directories if they contain untracked
files in an untracked sub directory.
Wait, ignored directories will always contain untracked
subdirectories, unless you add -f them, right? Why are you saying
untracked files in an _untracked_ subdirectory? We don't track
directories anyway, and I would call a directory "tracked" if there's
atleast one file inside it is tracked. So, my understanding of this
is:
quux/
bar
baz/
foo
In this example, if quux is ignored and untracked, git status
--ignored currently shows quux/. If quux/bar is tracked (say with add
-f), but baz/ is untracked, git status --ignored doesn't show me
anything. What exactly is the bug you're fixing? I'll try to look at
the tests to infer this, but your commit message could probably be
clearer.
Nit: please s/git-status/git status/
Fix it by getting exact (recursive) excluded status in treat_directory.
Okay, so you're patching treat_directory() in dir.c to do some
recursive exclude handling. Let's see what this is.
@@ -1060,6 +1060,15 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,/* This is the "show_other_directories" case */+/* might be a sub directory in an excluded directory */+if(!exclude){+structpath_exclude_checkcheck;+intdt=DT_DIR;+path_exclude_check_init(&check,dir);+exclude=is_path_excluded(&check,dirname,len,&dt);+path_exclude_check_clear(&check);+}+
So, I'm guessing that DT_DIR refers to a value that a field in struct
dirent can take; that value could be one of DIR (directory), REG
(regular file?), LNK (symbolic link?). I don't get much of this, but
what I do get is that you're setting exclude for the rest of the code
in this function.
Sorry that I'm not able to do a more thorough review.
Please put these segments inside the test_expect_success block, so
it's easy to think about those blocks in isolation. I know you're
just following the existing conventions existing in this test, but
those are not necessarily good conventions.
+test_expect_success 'status ignored tracked directory with uncommitted file in untracked subdir with --ignore' '
+ rm -rf tracked/uncommitted &&
+ mkdir tracked/ignored &&
+ : >tracked/ignored/uncommitted &&
+ git status --porcelain --ignored >actual &&
+ test_cmp expected actual
+'
This is very confusing. How is tracked a tracked directory? Oh,
right: some previous test git add'ed tracked/committed. How do I know
about that in this test?
Yeah, changes to tracked ignored directories are not shown, but the
commit message didn't tell me this.
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! tracked/ignored/uncommitted
+EOF
+
+test_expect_success 'status ignored tracked directory with uncommitted file in untracked subdir with --ignore -u' '
+ git status --porcelain --ignored -u >actual &&
+ test_cmp expected actual
+'
+
test_done
I suppose the commit message told me about this one vaguely, but I
think it could be much clearer overall.
Thanks.
'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.
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.
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?
@@ -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&&+gitstatus--porcelain--ignored>actual&&+test_cmpexpectedactual+'
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?
+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.
Once again, apologies for my inexperienced comments: I'm contributing
whatever little I can to the review process.
Am 16.04.2013 19:48, schrieb Ramkumar Ramachandra:
Karsten Blees wrote:
quoted
'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.
quoted
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?
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.
@@ -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&&+gitstatus--porcelain--ignored>actual&&+test_cmpexpectedactual+'
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.
quoted
+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.
quoted
+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.
Am 16.04.2013 19:33, schrieb Ramkumar Ramachandra:
Firstly, great work on the series! I've just started looking into it,
so please don't take my comments too seriously: some of them may be
queries, and others may be minor suggestions, but I can't say I
understand the area you're patching. I know Junio doesn't like me
mixing queries in reviews, but I don't fully agree with his policy.
Karsten Blees wrote:
quoted
'git-status --ignored' drops ignored directories if they contain untracked
files in an untracked sub directory.
Wait, ignored directories will always contain untracked
subdirectories, unless you add -f them, right? Why are you saying
untracked files in an _untracked_ subdirectory? We don't track
IIRC i mentioned untracked subdirectory explicitly because the problem doesn't occur if the subdirectory is tracked (i.e. contains a tracked file).
directories anyway, and I would call a directory "tracked" if there's
atleast one file inside it is tracked. So, my understanding of this
is:
quux/
bar
baz/
foo
In this example, if quux is ignored and untracked, git status
--ignored currently shows quux/. If quux/bar is tracked (say with add
-f), but baz/ is untracked, git status --ignored doesn't show me
anything. What exactly is the bug you're fixing?
With your example, you get this:
1) git-status --ignored -uall: quux/baz/foo
2) git-status --ignored -unormal: <empty>
Without the untracked directory 'baz' between 'quux' and 'foo':
quux/
bar
foo
you get this:
3) git status --ignored -uall: quux/foo
4) git status --ignored -unormal: quux/
The bug is in case (2), this should be:
2) git-status --ignored -unormal: quux/
I.e. ignored directories with untracked files in untracked sub directories should be listed as ignored, because they contain ignored files. Currently these directories are not listed. I realize this sounds pretty much like the original commit message, but I don't know how to describe it any better.
I'll try to look at
the tests to infer this, but your commit message could probably be
clearer.
Nit: please s/git-status/git status/
Both notations (as well as just 'status') seem to be widely used. Does this justify a reroll of the entire series with changed commit messages (I believe I've used git-status / git-ls-files / git-clean consistently in all patches)?
quoted
Fix it by getting exact (recursive) excluded status in treat_directory.
Okay, so you're patching treat_directory() in dir.c to do some
recursive exclude handling. Let's see what this is.
@@ -1060,6 +1060,15 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,/* This is the "show_other_directories" case */+/* might be a sub directory in an excluded directory */+if(!exclude){+structpath_exclude_checkcheck;+intdt=DT_DIR;+path_exclude_check_init(&check,dir);+exclude=is_path_excluded(&check,dirname,len,&dt);+path_exclude_check_clear(&check);+}+
So, I'm guessing that DT_DIR refers to a value that a field in struct
dirent can take; that value could be one of DIR (directory), REG
(regular file?), LNK (symbolic link?). I don't get much of this, but
what I do get is that you're setting exclude for the rest of the code
in this function.
This simply does a recursive excluded check via is_path_excluded, because is_excluded is not good enough. The rest is just boilerplate code (i.e. initialize and clear the check structure and passing a dtype == DT_DIR because dirname obviously is a directory here). We already have similar code in treat_file (and all other places that use is_path_excluded).
Sorry that I'm not able to do a more thorough review.
Please put these segments inside the test_expect_success block, so
it's easy to think about those blocks in isolation. I know you're
just following the existing conventions existing in this test, but
those are not necessarily good conventions.
$ grep -e '^cat .*EOF$' t*.sh | wc -l
743
$ grep -e '^ cat .*EOF$' t*.sh | wc -l
22
Setting up expected results outside test_expect_* (i.e. unindented) seems to be the norm in git tests.
quoted
+test_expect_success 'status ignored tracked directory with uncommitted file in untracked subdir with --ignore' '
+ rm -rf tracked/uncommitted &&
+ mkdir tracked/ignored &&
+ : >tracked/ignored/uncommitted &&
+ git status --porcelain --ignored >actual &&
+ test_cmp expected actual
+'
This is very confusing. How is tracked a tracked directory? Oh,
right: some previous test git add'ed tracked/committed. How do I know
about that in this test?
This test expands on the previous test 'status ignored tracked directory and uncommitted file with --ignore', i.e. the test just adds the 'in untracked subdir' part (and thus moves the 'uncommitted' file into a sub directory). Expanding on previous state is the whole point of putting several tests in a single t*.sh file, right? If I wanted to set up the test fixture from scratch I would have started a new t7063-whatever.sh.
Yeah, changes to tracked ignored directories are not shown, but the
commit message didn't tell me this.
quoted
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! tracked/ignored/uncommitted
+EOF
+
+test_expect_success 'status ignored tracked directory with uncommitted file in untracked subdir with --ignore -u' '
+ git status --porcelain --ignored -u >actual &&
+ test_cmp expected actual
+'
+
test_done
I suppose the commit message told me about this one vaguely, but I
think it could be much clearer overall.
Note that all tests in t7061 come in pairs '--ignored' (i.e. --untracked-files=normal) and '--ignored -u' (i.e. --untracked-files=all). If '-uall' lists individual ignored files, then '-unormal' should list the ignored directory.