Re: [PATCH] dir.c: fix ignore processing within not-ignored directories

[Duy Nguyen <pclouds@gmail.com>]

On Thu, May 30, 2013 at 3:32 AM, Karsten Blees <karsten.blees@gmail.com> wrote:
> As of 95c6f271 "dir.c: unify is_excluded and is_path_excluded APIs", the
> is_excluded API no longer recurses into directories that match an ignore
> pattern, and returns the directory's ignored state for all contained paths.
>
> This is OK for normal ignore patterns, i.e. ignoring a directory affects
> the entire contents recursively.
>
> Unfortunately, this also "works" for negated ignore patterns ('!dir'), i.e.
> the entire contents is "not-ignored" recursively, regardless of ignore
> patterns that match the contents directly.
>
> In prep_exclude, skip recursing into a directory only if it is really
> ignored (i.e. the ignore pattern is not negated).
>
> Signed-off-by: Karsten Blees <blees@dcon.de>

I think I've got a hang on the "unify" patch now.

Reviewed-by: Duy Nguyen <pclouds@gmail.com>


> diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
> +test_expect_success 'excluded directory overrides content patterns' '
> +
> +       git ls-files --others --exclude="one" --exclude="!one/a.1" >output &&
> +       if grep "^one/a.1" output

Actually I think this is a shortcoming of gitignore. You ask to
"exclude one except one/a.1" and one/a.1 should show up. '!' is
designed from day one to deal with the other way around ("include one
except one/a.1"). And it's arguable (and it was in the mail archive)
that if you already exclude "one", we should never ever descend there
to pick up "!a.1" from one/.gitignore. But we should do it with
already collected patterns, at least if we detect there are negated
patterns following the pattern that excludes a directory, e.g.
!one/a.1 or even !*.c. For the latter case, the user can always move
"!*.c" up before "one" if they don't want git to misinterpret and
descend in every excluded directory.

> +       then
> +               false
> +       fi
> +'

Nit pick, maybe this instead?

test_must_fail grep "^one/a.1" output

> +
> +test_expect_success 'negated directory doesn'\''t affect content patterns' '
> +
> +       git ls-files --others --exclude="!one" --exclude="one/a.1" >output &&
> +       if grep "^one/a.1" output
> +       then
> +               false
> +       fi
> +'

Same.
--
Duy