1.8.3 - gitignore not being parsed correctly on OS X; regex support is broken?

> 
> Hi,
> 
> Gitignore parsing no longer seems to work properly in git 1.8.3.
> 
> One of my repositories has the following gitignore:
> 
> /*
> !/.gitignore
> !/Library/
> !/CONTRIBUTING.md
> !/README.md
> !/SUPPORTERS.md
> !/bin
> /bin/*
> !/bin/brew
> !/share/man/man1/brew.1
> .DS_Store
> /Library/LinkedKegs
> /Library/PinnedKegs
> /Library/Taps
> /Library/Formula/.gitignore
> 
> In 1.8.2.3 and earlier, this works as expected. However, in 1.8.3 I'm
> seeing every file in /bin/ being marked as an untracked file.
> 
> I asked about this in #git, and was told that the culprit was the
> regex support; apparently recompiling without regex support fixes the
> specific gitignore issue. However, this doesn't seem to have been
> reported anywhere on the mailing list that I can see. I was also told
> that the issue is OS X-specific, and doesn't happen on other
> platforms.
> 
> Thanks,
> Misty De Meo
> 

> Misty De Meo <misty <at> brew.sh> writes:
>
>> 
>> Hi,
>> 
>> Gitignore parsing no longer seems to work properly in git 1.8.3.
>> 
>> One of my repositories has the following gitignore:
>> 
>> /*
>> !/.gitignore
>> !/Library/
>> !/CONTRIBUTING.md
>> !/README.md
>> !/SUPPORTERS.md
>> !/bin
>> /bin/*
>> !/bin/brew
>> !/share/man/man1/brew.1
>> .DS_Store
>> /Library/LinkedKegs
>> /Library/PinnedKegs
>> /Library/Taps
>> /Library/Formula/.gitignore
>> 
>> In 1.8.2.3 and earlier, this works as expected. However, in 1.8.3 I'm
>> seeing every file in /bin/ being marked as an untracked file.
>> 
>> I asked about this in #git, and was told that the culprit was the
>> regex support; apparently recompiling without regex support fixes the
>> specific gitignore issue. However, this doesn't seem to have been
>> reported anywhere on the mailing list that I can see. I was also told
>> that the issue is OS X-specific, and doesn't happen on other
>> platforms.
>> 
>> Thanks,
>> Misty De Meo
>> 
>
> I see a similar problem using e.g. the following .gitignore to exclude
> everything except C source files and header files:
>
>     *
>     !*/
>     !*.c
>     !*.h
>
> In Git 1.8.3 'git status' will show other files as untracked while in
> Git 1.8.2.3 I don't have that problem. I bisected to find that the
> offending commit is v1.8.2.1-402-g95c6f27. 
>
> I am not on OSX, however, but on Linux (Ubuntu 12.04 and RHEL 5.8) so
> this may be a separate issue. I've also gotten the impression that this
> is intentional. In any case I cannot create a .gitignore that achieves
> the same for both older and newer versions of Git.

> Hi,
>
> Gitignore parsing no longer seems to work properly in git 1.8.3.
>
> One of my repositories has the following gitignore:
>
> /*
> !/.gitignore
> !/Library/
> !/CONTRIBUTING.md
> !/README.md
> !/SUPPORTERS.md
> !/bin
> /bin/*
> !/bin/brew
> !/share/man/man1/brew.1
> .DS_Store
> /Library/LinkedKegs
> /Library/PinnedKegs
> /Library/Taps
> /Library/Formula/.gitignore
>
> In 1.8.2.3 and earlier, this works as expected. However, in 1.8.3 I'm
> seeing every file in /bin/ being marked as an untracked file.

> I asked about this in #git, and was told that the culprit was the
> regex support; apparently recompiling without regex support fixes the
> specific gitignore issue. However, this doesn't seem to have been
> reported anywhere on the mailing list that I can see. I was also told
> that the issue is OS X-specific, and doesn't happen on other
> platforms.

> The changes in this area since 1.8.2.3 seem to be Karsten's (I'm not
> blaming, just wanted to narrow down the problem). The patterns of
> interest seem to be
>
> !/bin
> /bin/*
> !/bin/brew
>
> Without "!/bin" v1.8.3 seems to behave the same as v1.8.2.3.

> On Wed, May 29, 2013 at 10:41 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> The changes in this area since 1.8.2.3 seem to be Karsten's (I'm not
>> blaming, just wanted to narrow down the problem). The patterns of
>> interest seem to be
>>
>> !/bin
>> /bin/*
>> !/bin/brew
>>
>> Without "!/bin" v1.8.3 seems to behave the same as v1.8.2.3.
>
> Karsten, the block "/* Abort if the directory is excluded */" in
> prep_exclude() seems to cause this. I think it goes through the
> exclude patterns, hits "!/bin", believes the patterns do not make
> sense in this context and throws all away. I think Øystein's case
> falls into the same path. Commenting out the block seems to gain the
> old behavior back (and probably breaks other stuff). Contrary to what
> Junio said, I'm clueless about this. I wanted to read your series
> through and eventually gave up. I think I now have the motivation to
> look at it again this weekend.

> On Wed, May 29, 2013 at 10:41 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> The changes in this area since 1.8.2.3 seem to be Karsten's (I'm not
>> blaming, just wanted to narrow down the problem). The patterns of
>> interest seem to be
>>
>> !/bin
>> /bin/*
>> !/bin/brew
>>
>> Without "!/bin" v1.8.3 seems to behave the same as v1.8.2.3.
> 
> Karsten, the block "/* Abort if the directory is excluded */" in
> prep_exclude() seems to cause this. I think it goes through the
> exclude patterns, hits "!/bin", believes the patterns do not make
> sense in this context and throws all away.

> 
> --- 8< ---
> diff --git a/dir.c b/dir.c
> index a5926fb..13858fe 100644
> --- a/dir.c
> +++ b/dir.c
>  <at>  <at>  -821,6 +821,9  <at>  <at>  static void prep_exclude(struct 

>  				dir->basebuf, stk->baselen - 1,
>  				dir->basebuf + current, &dt);
>  			dir->basebuf[stk->baselen - 1] = '/';
> +			if (dir->exclude &&
> +			    dir->exclude->flags & EXC_FLAG_NEGATIVE)
> +				dir->exclude = NULL;
>  			if (dir->exclude) {
>  				dir->basebuf[stk->baselen] = 0;
>  				dir->exclude_stack = stk;

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

> 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

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

>> +       then
>> +               false
>> +       fi
>> +'
>
> Nit pick, maybe this instead?
>
> test_must_fail grep "^one/a.1" output

> Duy Nguyen <pclouds@gmail.com> writes:
> 
>>> +       then
>>> +               false
>>> +       fi
>>> +'
>>
>> Nit pick, maybe this instead?
>>
>> test_must_fail grep "^one/a.1" output
> 
> Neither.
> 
> 	! grep "^one/a.1" output
> 

>      "test_must_fail"
> is there only to test Git command and catch unusual deaths we know
> about (e.g. segv) as an error, not as an expected failure.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> t/README | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/t/README b/t/README
> index e669bb3..35b3c5c 100644
> --- a/t/README
> +++ b/t/README
> @@ -324,6 +324,9 @@ Don't:
>    use 'test_must_fail git cmd'.  This will signal a failure if git
>    dies in an unexpected way (e.g. segfault).
>
> +   On the other hand, don't use test_must_fail for running regular
> +   platform commands; just use '! cmd'.
> +
>  - use perl without spelling it as "$PERL_PATH". This is to help our
>    friends on Windows where the platform Perl often adds CR before
>    the end of line, and they bundle Git with a version of Perl that
> --

> From: "Junio C Hamano" <gitster@pobox.com>
> Sent: Tuesday, June 04, 2013 5:50 PM
>> When a test wants to make sure there is no <string> in an output
>> file, we should just say "! grep string output";
>
> Small nit: It took me two readings of the commit message to correctly
> parse this break point. The flowing together of the two parts with the
> semicolon fooled me. Separate them?
>
>>      "test_must_fail"
>> is there only to test Git command and catch unusual deaths we know
>> about (e.g. segv) as an error, not as an expected failure.

> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> From: "Junio C Hamano" <gitster@pobox.com>
>> Sent: Tuesday, June 04, 2013 5:50 PM
>>> When a test wants to make sure there is no <string> in an output
>>> file, we should just say "! grep string output";
>>
>> Small nit: It took me two readings of the commit message to correctly
>> parse this break point. The flowing together of the two parts with 
>> the
>> semicolon fooled me. Separate them?
>>
>>>      "test_must_fail"
>>> is there only to test Git command and catch unusual deaths we know
>>> about (e.g. segv) as an error, not as an expected failure.
>
> Thanks.  Does this read better?

>
>    t/README: test_must_fail is for testing Git
>
>    When a test wants to make sure there is no <string> in an output
>    file, we should just say "! grep string output".
>
>    "test_must_fail" is there only to test Git command and catch 
> unusual
>    deaths we know about (e.g. segv) as an error, not as an expected
>    failure.  "test_must_fail grep string output" is unnecessary, as
>    we are not making sure the system binaries do not dump core or
>    anything like that.
>
>    Signed-off-by: Junio C Hamano <gitster@pobox.com>
> --