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

[Junio C Hamano <gitster@pobox.com>]

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.