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

From: Duy Nguyen <pclouds@gmail.com>
Date: Jun 1, 2013, 10:44 UTC
Message-ID: <CACsJy8D4wmhGkEsn8r5OEQv_hX=OFD5W8abnBnYFcFCQZfLOoQ@mail.gmail.com>
In-Reply-To: <51A665E4.9080307@gmail.com>

On Thu, May 30, 2013 at 3:32 AM, Karsten Blees <karsten.blees@gmail.com> wrote:

Show 15 quoted lines

> 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>

Show 5 quoted lines

> 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

Show 9 quoted lines

> +
> +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