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

Mirko Faina <mroik@delayed.space> writes:

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

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.