Re: Possible bug in git-completion.sh
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 8, 2010, 19:58 UTC
- Message-ID
- <7vvdfcfjxo.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <7v8wc8jw3k.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 13 quoted lines
> Nah, I should have checked the code. > > The implementation of ls-files does cd up and uses pathspec, so the intent > is to apply higher level gitignore. > > It however feeds paths from the already ignored directories, which _is_ > the real bug. > ... > I think a proper fix should be in dir.c::read_directory() where it calls > read_directory_recursive() without first checking if the directory itself > should be ignored. read_directory_recursive() itself has a logic to see > if the dirent it found is worth recursing into and a similar logic should > be in the toplevel caller.
Actually doing less in fill_directory() turned out to be a simpler solution.
builtin_add() cares about the return value from fill_directory() and performs prune_directory() optimization magic, and we may want to change it not to do so as well, but I didn't want to worry about too many things at once, so this version still runs the "common_prefix()" that forgets to take .gitignore at higher levels (or a $GIT_DIR/info/exclude entry that ignores the common prefix directory of given pathspecs) into account.
Another possibility is to fix common_prefix() and make it walk the path it returns one level at a time from the top, making sure they are not ignored, and that would probably be a better fix, but at least this patch will give you a starting point and tests to check the result against.
diff --git a/dir.c b/dir.c index d0999ba..56d3f60 100644 --- a/dir.c +++ b/dir.c @@ -53,7 +53,6 @@ static int common_prefix(const char **pathspec) int fill_directory(struct dir_struct *dir, const char **pathspec) { - const char *path; int len; /* @@ -61,13 +60,8 @@ int fill_directory(struct dir_struct *dir, const char **pathspec) * use that to optimize the directory walk */ len = common_prefix(pathspec); - path = ""; - - if (len) - path = xmemdupz(*pathspec, len); - /* Read the directory and prune it */ - read_directory(dir, path, len, pathspec); + read_directory(dir, "", 0, pathspec); return len; } diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh index c65bca8..17d1764 100755 --- a/t/t3001-ls-files-others-exclude.sh +++ b/t/t3001-ls-files-others-exclude.sh @@ -153,4 +153,42 @@ test_expect_success 'negated exclude matches can override previous ones' ' grep "^a.1" output ' +test_expect_success 'subdirectory ignore (setup)' ' + mkdir -p top/l1/l2 && + ( + cd top && + git init && + echo /.gitignore >.gitignore && + echo l1 >>.gitignore && + echo l2 >l1/.gitignore + ) +' + +test_expect_success 'subdirectory ignore (toplevel)' ' + ( + cd top && + git ls-files -o --exclude-standard + ) >actual && + >expect && + test_cmp expect actual +' + +test_expect_success 'subdirectory ignore (l1/l2)' ' + ( + cd top/l1/l2 && + git ls-files -o --exclude-standard + ) >actual && + >expect && + test_cmp expect actual +' + +test_expect_success 'subdirectory ignore (l1)' ' + ( + cd top/l1 && + git ls-files -o --exclude-standard + ) >actual && + >expect && + test_cmp expect actual +' + test_done