[PATCH] t0008: fix "large exclude file ignored in tree"

[PATCH] t0008: fix "large exclude file ignored in tree"

[Mirko Faina]

Add cleanup to previous test for file that is unrequired to test the
size of the ignored exclude file.

Signed-off-by: Mirko Faina <mroik@delayed.space>
---
 t/t0008-ignores.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index db8bde280e..18e048ee8c 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -946,7 +946,7 @@ test_expect_success SYMLINKS 'symlinks respected in info/exclude' '
 '
 
 test_expect_success SYMLINKS 'symlinks not respected in-tree' '
-	test_when_finished "rm .gitignore" &&
+	test_when_finished "rm -rf subdir .gitignore" &&
 	ln -s ignore .gitignore &&
 	mkdir subdir &&
 	ln -s ignore subdir/.gitignore &&
-- 
2.53.0.959.g497ff81fa9

Re: [PATCH] t0008: fix "large exclude file ignored in tree"

[Junio C Hamano]

Mirko Faina <mroik@delayed.space> writes:

> Subject: Re: [PATCH] t0008: fix "large exclude file ignored in tree"

Strange.  That is clearly not what this patch is touching.

  Subject: t0008: fix cleanup in 'symlinks not respected in-tree'

or something?

> Add cleanup to previous test for file that is unrequired to test the
> size of the ignored exclude file.

This description is also inaccurate. It seems to be talking about the
next test in the file ("large exclude file ignored in tree") rather than
the one it's actually changing. Worse, the next test does its own
creation of the "large" .gitignore file and also cleans it up itself,
so there seem to be no need to fix it, either.

    The test 'symlinks not respected in-tree' creates a 'subdir'
    directory and 'subdir/.gitignore' symlink, but only removes the
    top-level '.gitignore' file in its cleanup.

    Add 'subdir' to the test_when_finished command to ensure the
    worktree is properly cleaned up after the test.

or something, perhaps?

This patch has some disturbing characteristics.

- The subject line and commit message describe a fix for the "large
  exclude file ignored in tree" test, but the code change actually
  modifies the "symlinks not respected in-tree" test.
- The description mentions a file "unrequired to test the size", which
  doesn't logically apply to the change being made (adding a directory
  to a cleanup command in a symlink test).
- This kind of context-mixing (applying a correct fix for one test but
  attributing it to a neighboring one) is a common pattern in LLM
  outputs.

Is this generated with LLM sent without any sanity-checking by a
human?

> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> index db8bde280e..18e048ee8c 100755
> --- a/t/t0008-ignores.sh
> +++ b/t/t0008-ignores.sh
> @@ -946,7 +946,7 @@ test_expect_success SYMLINKS 'symlinks respected in info/exclude' '
>  '
>  
>  test_expect_success SYMLINKS 'symlinks not respected in-tree' '
> -	test_when_finished "rm .gitignore" &&
> +	test_when_finished "rm -rf subdir .gitignore" &&
>  	ln -s ignore .gitignore &&
>  	mkdir subdir &&
>  	ln -s ignore subdir/.gitignore &&

Re: [PATCH] t0008: fix "large exclude file ignored in tree"

[Mirko Faina]

On Sat, Mar 14, 2026 at 10:50:42PM -0700, Junio C Hamano wrote:
> > Subject: Re: [PATCH] t0008: fix "large exclude file ignored in tree"
> 
> Strange.  That is clearly not what this patch is touching.
> 
>   Subject: t0008: fix cleanup in 'symlinks not respected in-tree'
> 
> or something?

The test I'm touching is "symlinks not respected in-tree", but the test
failing is "large exclude file ignored in tree". That's why in the
following section it mentions "the previous test".

> > Add cleanup to previous test for file that is unrequired to test the
> > size of the ignored exclude file.
> 
> This description is also inaccurate. It seems to be talking about the
> next test in the file ("large exclude file ignored in tree") rather than
> the one it's actually changing. Worse, the next test does its own
> creation of the "large" .gitignore file and also cleans it up itself,
> so there seem to be no need to fix it, either.
> 
>     The test 'symlinks not respected in-tree' creates a 'subdir'
>     directory and 'subdir/.gitignore' symlink, but only removes the
>     top-level '.gitignore' file in its cleanup.
> 
>     Add 'subdir' to the test_when_finished command to ensure the
>     worktree is properly cleaned up after the test.
> 
> or something, perhaps?

When running "GIT_TEST_OPTS='-l -v' git make t0008-ignores.sh", "large
exclude file ignored in tree" fails with the comparison failing due to
an extra warning "warning: unable to access 'subdir/.gitignore': Too
many levels of symbolic links".

I don't know if this tests fails only on my setup, but seeing that the
failing test is supposed to check for .gitignore size only, the "subdir"
directory is not strictly necessary, without it the test works just
fine.

> This patch has some disturbing characteristics.
> 
> - The subject line and commit message describe a fix for the "large
>   exclude file ignored in tree" test, but the code change actually
>   modifies the "symlinks not respected in-tree" test.
> - The description mentions a file "unrequired to test the size", which
>   doesn't logically apply to the change being made (adding a directory
>   to a cleanup command in a symlink test).
> - This kind of context-mixing (applying a correct fix for one test but
>   attributing it to a neighboring one) is a common pattern in LLM
>   outputs.

Sorry for very bad commit message. When writing it and knowing where the
problem is, I found it obvious. Now reading it again, it doesn't make
sense at all without additional context.

> Is this generated with LLM sent without any sanity-checking by a
> human?

No, this was all hand written. This mess is all due to my incompetency.

> > diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> > index db8bde280e..18e048ee8c 100755
> > --- a/t/t0008-ignores.sh
> > +++ b/t/t0008-ignores.sh
> > @@ -946,7 +946,7 @@ test_expect_success SYMLINKS 'symlinks respected in info/exclude' '
> >  '
> >  
> >  test_expect_success SYMLINKS 'symlinks not respected in-tree' '
> > -	test_when_finished "rm .gitignore" &&
> > +	test_when_finished "rm -rf subdir .gitignore" &&
> >  	ln -s ignore .gitignore &&
> >  	mkdir subdir &&
> >  	ln -s ignore subdir/.gitignore &&

Re: [PATCH] t0008: fix "large exclude file ignored in tree"

[Junio C Hamano]

Mirko Faina <mroik@delayed.space> writes:

> When running "GIT_TEST_OPTS='-l -v' git make t0008-ignores.sh", "large

This must be "make", not "git make", right?

> exclude file ignored in tree" fails with the comparison failing due to
> an extra warning "warning: unable to access 'subdir/.gitignore': Too
> many levels of symbolic links".

test_expect_success SYMLINKS 'symlinks not respected in-tree' '
	test_when_finished "rm .gitignore" &&
	ln -s ignore .gitignore &&
	mkdir subdir &&
	ln -s ignore subdir/.gitignore &&
	test_must_fail git check-ignore subdir/file >actual 2>err &&
	test_must_be_empty actual &&
	test_grep "unable to access.*gitignore" err
'

The above step creates symbolic links subdir/.gitignore and
.gitignore, each of which pointing at a "ignore" file next to it.
This test has extremely bad hygiene and depends on "ignore" having
preexisting contents "*" in it (so a bad version of Git that follows
symbolic links would ignore almost everything).

Files err and actual are created, and when the test finishes, only
".gitignore" is removed, everything else left behind.

test_expect_success EXPENSIVE 'large exclude file ignored in tree' '
	test_when_finished "rm .gitignore" &&
	dd if=/dev/zero of=.gitignore bs=101M count=1 &&
	git ls-files -o --exclude-standard 2>err &&
	echo "warning: ignoring excessively large pattern file: .gitignore" >expect &&
	test_cmp expect err
'

Ah, OK, you're right.

The problem is not the subdir/ directory itself, but the leftover
symbolic link subdir/.gitignore that would cause "ls-files -o" to
notice and complain about that symbolic link.

If that is what is happening, then this should probably be fixed in
a belt-and-suspenders fashion.  The primary bug is that the
expensive test that does not protect against pre-existing files in
the working tree when it starts.

In the failing test, remove preexisting .gitignore everywhere in
tree.  With as bad hygiene as this entire script has, we do not know
what is left behind in the working tree by which other test piece
that comes before this last test, something like

test_expect_success EXPENSIVE 'large exclude file ignored in tree' '
	test_when_finished "rm .gitignore" &&
	find . -name .gitignore -exec rm "{}" ";" &&
	dd if=/dev/zero of=.gitignore bs=101M count=1 &&
	...

perhaps?

And in the previous test, as you said, you would need to remove at
least subdir/.gitignore in addition to .gitignore to make the next
test pass, but (1) the next test should not depend on it to work
correctly, and (2) it would be a good discipline to remove any and
all dropping you make.

So the change to the not-expensive piece would be

test_expect_success SYMLINKS 'symlinks not respected in-tree' '
	test_when_finished "rm -fr subdir .gitignore err actual" &&
	...

and the primary reason why we make such a change (to be described in
the proposed log message) is not about the next test, but is about
cleaning cruft created in each test before it finishes.  It would be
OK to make both changes in a single commit, as they are fairly small.

Thanks.

[PATCH v2] t0008: improve test cleanup to fix failing test

[Mirko Faina]

The "large exclude file ignored in tree" test fails. This is due to an
additional warning message that is generated in the test. "warning:
unable to access 'subdir/.gitignore': Too many levels of symbolic
links", the extra warning that is not supposed to be there, happens
because of some leftover files left by previous tests.

To fix this we improve cleanup on "symlinks not respected in-tree", and
because the tests in t0008 in general have poor cleanup, at the start of
"large exclude file ignored in tree" we search for any leftover
.gitignore and remove them before starting the test.

Improve post-test cleanup and add pre-test cleanup to make sure that we
have a workable environment for the test.

Signed-off-by: Mirko Faina <mroik@delayed.space>
---
Sorry again for the poorly written commit message in the previous patch.

 t/t0008-ignores.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index db8bde280e..e716b5cdfa 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -946,7 +946,7 @@ test_expect_success SYMLINKS 'symlinks respected in info/exclude' '
 '
 
 test_expect_success SYMLINKS 'symlinks not respected in-tree' '
-	test_when_finished "rm .gitignore" &&
+	test_when_finished "rm -rf subdir .gitignore err actual" &&
 	ln -s ignore .gitignore &&
 	mkdir subdir &&
 	ln -s ignore subdir/.gitignore &&
@@ -957,6 +957,7 @@ test_expect_success SYMLINKS 'symlinks not respected in-tree' '
 
 test_expect_success EXPENSIVE 'large exclude file ignored in tree' '
 	test_when_finished "rm .gitignore" &&
+	find . -name .gitignore -exec rm "{}" ";" &&
 	dd if=/dev/zero of=.gitignore bs=101M count=1 &&
 	git ls-files -o --exclude-standard 2>err &&
 	echo "warning: ignoring excessively large pattern file: .gitignore" >expect &&
-- 
2.53.0.959.g497ff81fa9

Re: [PATCH v2] t0008: improve test cleanup to fix failing test

[Mirko Faina]

A curiosity of mine. How often are the expensive tests ran? Is it when
there's an RC? "large exclude file ignored in tree" has been there since
2024, and "symlink not respected in-tree" since 2021. I find it hard to
believe that no one has noticed this test failing had they been ran.
Since no one noticed I'm assuming they're not ran on github's CI
neither.

Re: [PATCH v2] t0008: improve test cleanup to fix failing test

[Junio C Hamano]

Mirko Faina <mroik@delayed.space> writes:

> The "large exclude file ignored in tree" test fails. This is due to an
> additional warning message that is generated in the test. "warning:
> unable to access 'subdir/.gitignore': Too many levels of symbolic
> links", the extra warning that is not supposed to be there, happens
> because of some leftover files left by previous tests.

Correctly diagnosed and clearly written.  Very much appreciated.

Will queue.  Thanks.