{"thread":{"id":"65250","subject":"[PATCH] t0008: fix \"large exclude file ignored in tree\"","startedAt":"2026-03-15T03:49:14Z","lastAt":"2026-03-16T19:48:04Z","messageCount":7,"participants":["Mirko Faina","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"539009","messageId":"20260315034851.2261530-1-mroik@delayed.space","threadId":"65250","inReplyTo":null,"subject":"[PATCH] t0008: fix \"large exclude file ignored in tree\"","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-03-15T03:48:50Z","receivedAt":"2026-03-15T03:49:14Z","isPatch":true,"sender":{"key":"mroik@delayed.space","avatar":"https://avatars.githubusercontent.com/u/25752903?v=4"},"body":"Add cleanup to previous test for file that is unrequired to test the\nsize of the ignored exclude file.\n\nSigned-off-by: Mirko Faina <mroik@delayed.space>\n---\n t/t0008-ignores.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex db8bde280e..18e048ee8c 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -946,7 +946,7 @@ test_expect_success SYMLINKS 'symlinks respected in info/exclude' '\n '\n \n test_expect_success SYMLINKS 'symlinks not respected in-tree' '\n-\ttest_when_finished \"rm .gitignore\" &&\n+\ttest_when_finished \"rm -rf subdir .gitignore\" &&\n \tln -s ignore .gitignore &&\n \tmkdir subdir &&\n \tln -s ignore subdir/.gitignore &&\n-- \n2.53.0.959.g497ff81fa9\n\n"},{"id":"539013","messageId":"xmqqv7extzd9.fsf@gitster.g","threadId":"65250","inReplyTo":"20260315034851.2261530-1-mroik@delayed.space","subject":"Re: [PATCH] t0008: fix \"large exclude file ignored in tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-15T05:50:42Z","receivedAt":"2026-03-15T05:50:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mirko Faina <mroik@delayed.space> writes:\n\n> Subject: Re: [PATCH] t0008: fix \"large exclude file ignored in tree\"\n\nStrange.  That is clearly not what this patch is touching.\n\n  Subject: t0008: fix cleanup in 'symlinks not respected in-tree'\n\nor something?\n\n> Add cleanup to previous test for file that is unrequired to test the\n> size of the ignored exclude file.\n\nThis description is also inaccurate. It seems to be talking about the\nnext test in the file (\"large exclude file ignored in tree\") rather than\nthe one it's actually changing. Worse, the next test does its own\ncreation of the \"large\" .gitignore file and also cleans it up itself,\nso there seem to be no need to fix it, either.\n\n    The test 'symlinks not respected in-tree' creates a 'subdir'\n    directory and 'subdir/.gitignore' symlink, but only removes the\n    top-level '.gitignore' file in its cleanup.\n\n    Add 'subdir' to the test_when_finished command to ensure the\n    worktree is properly cleaned up after the test.\n\nor something, perhaps?\n\nThis patch has some disturbing characteristics.\n\n- The subject line and commit message describe a fix for the \"large\n  exclude file ignored in tree\" test, but the code change actually\n  modifies the \"symlinks not respected in-tree\" test.\n- The description mentions a file \"unrequired to test the size\", which\n  doesn't logically apply to the change being made (adding a directory\n  to a cleanup command in a symlink test).\n- This kind of context-mixing (applying a correct fix for one test but\n  attributing it to a neighboring one) is a common pattern in LLM\n  outputs.\n\nIs this generated with LLM sent without any sanity-checking by a\nhuman?\n\n> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\n> index db8bde280e..18e048ee8c 100755\n> --- a/t/t0008-ignores.sh\n> +++ b/t/t0008-ignores.sh\n> @@ -946,7 +946,7 @@ test_expect_success SYMLINKS 'symlinks respected in info/exclude' '\n>  '\n>  \n>  test_expect_success SYMLINKS 'symlinks not respected in-tree' '\n> -\ttest_when_finished \"rm .gitignore\" &&\n> +\ttest_when_finished \"rm -rf subdir .gitignore\" &&\n>  \tln -s ignore .gitignore &&\n>  \tmkdir subdir &&\n>  \tln -s ignore subdir/.gitignore &&\n"},{"id":"539016","messageId":"abZwbCF1R0_bnFBv@exploit","threadId":"65250","inReplyTo":"xmqqv7extzd9.fsf@gitster.g","subject":"Re: [PATCH] t0008: fix \"large exclude file ignored in tree\"","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-03-15T08:50:06Z","receivedAt":"2026-03-15T08:50:11Z","isPatch":true,"sender":{"key":"mroik@delayed.space","avatar":"https://avatars.githubusercontent.com/u/25752903?v=4"},"body":"On Sat, Mar 14, 2026 at 10:50:42PM -0700, Junio C Hamano wrote:\n> > Subject: Re: [PATCH] t0008: fix \"large exclude file ignored in tree\"\n> \n> Strange.  That is clearly not what this patch is touching.\n> \n>   Subject: t0008: fix cleanup in 'symlinks not respected in-tree'\n> \n> or something?\n\nThe test I'm touching is \"symlinks not respected in-tree\", but the test\nfailing is \"large exclude file ignored in tree\". That's why in the\nfollowing section it mentions \"the previous test\".\n\n> > Add cleanup to previous test for file that is unrequired to test the\n> > size of the ignored exclude file.\n> \n> This description is also inaccurate. It seems to be talking about the\n> next test in the file (\"large exclude file ignored in tree\") rather than\n> the one it's actually changing. Worse, the next test does its own\n> creation of the \"large\" .gitignore file and also cleans it up itself,\n> so there seem to be no need to fix it, either.\n> \n>     The test 'symlinks not respected in-tree' creates a 'subdir'\n>     directory and 'subdir/.gitignore' symlink, but only removes the\n>     top-level '.gitignore' file in its cleanup.\n> \n>     Add 'subdir' to the test_when_finished command to ensure the\n>     worktree is properly cleaned up after the test.\n> \n> or something, perhaps?\n\nWhen running \"GIT_TEST_OPTS='-l -v' git make t0008-ignores.sh\", \"large\nexclude file ignored in tree\" fails with the comparison failing due to\nan extra warning \"warning: unable to access 'subdir/.gitignore': Too\nmany levels of symbolic links\".\n\nI don't know if this tests fails only on my setup, but seeing that the\nfailing test is supposed to check for .gitignore size only, the \"subdir\"\ndirectory is not strictly necessary, without it the test works just\nfine.\n\n> This patch has some disturbing characteristics.\n> \n> - The subject line and commit message describe a fix for the \"large\n>   exclude file ignored in tree\" test, but the code change actually\n>   modifies the \"symlinks not respected in-tree\" test.\n> - The description mentions a file \"unrequired to test the size\", which\n>   doesn't logically apply to the change being made (adding a directory\n>   to a cleanup command in a symlink test).\n> - This kind of context-mixing (applying a correct fix for one test but\n>   attributing it to a neighboring one) is a common pattern in LLM\n>   outputs.\n\nSorry for very bad commit message. When writing it and knowing where the\nproblem is, I found it obvious. Now reading it again, it doesn't make\nsense at all without additional context.\n\n> Is this generated with LLM sent without any sanity-checking by a\n> human?\n\nNo, this was all hand written. This mess is all due to my incompetency.\n\n> > diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\n> > index db8bde280e..18e048ee8c 100755\n> > --- a/t/t0008-ignores.sh\n> > +++ b/t/t0008-ignores.sh\n> > @@ -946,7 +946,7 @@ test_expect_success SYMLINKS 'symlinks respected in info/exclude' '\n> >  '\n> >  \n> >  test_expect_success SYMLINKS 'symlinks not respected in-tree' '\n> > -\ttest_when_finished \"rm .gitignore\" &&\n> > +\ttest_when_finished \"rm -rf subdir .gitignore\" &&\n> >  \tln -s ignore .gitignore &&\n> >  \tmkdir subdir &&\n> >  \tln -s ignore subdir/.gitignore &&\n"},{"id":"539034","messageId":"xmqq7brdt6y6.fsf@gitster.g","threadId":"65250","inReplyTo":"abZwbCF1R0_bnFBv@exploit","subject":"Re: [PATCH] t0008: fix \"large exclude file ignored in tree\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-15T16:04:33Z","receivedAt":"2026-03-15T16:04:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mirko Faina <mroik@delayed.space> writes:\n\n> When running \"GIT_TEST_OPTS='-l -v' git make t0008-ignores.sh\", \"large\n\nThis must be \"make\", not \"git make\", right?\n\n> exclude file ignored in tree\" fails with the comparison failing due to\n> an extra warning \"warning: unable to access 'subdir/.gitignore': Too\n> many levels of symbolic links\".\n\ntest_expect_success SYMLINKS 'symlinks not respected in-tree' '\n\ttest_when_finished \"rm .gitignore\" &&\n\tln -s ignore .gitignore &&\n\tmkdir subdir &&\n\tln -s ignore subdir/.gitignore &&\n\ttest_must_fail git check-ignore subdir/file >actual 2>err &&\n\ttest_must_be_empty actual &&\n\ttest_grep \"unable to access.*gitignore\" err\n'\n\nThe above step creates symbolic links subdir/.gitignore and\n.gitignore, each of which pointing at a \"ignore\" file next to it.\nThis test has extremely bad hygiene and depends on \"ignore\" having\npreexisting contents \"*\" in it (so a bad version of Git that follows\nsymbolic links would ignore almost everything).\n\nFiles err and actual are created, and when the test finishes, only\n\".gitignore\" is removed, everything else left behind.\n\ntest_expect_success EXPENSIVE 'large exclude file ignored in tree' '\n\ttest_when_finished \"rm .gitignore\" &&\n\tdd if=/dev/zero of=.gitignore bs=101M count=1 &&\n\tgit ls-files -o --exclude-standard 2>err &&\n\techo \"warning: ignoring excessively large pattern file: .gitignore\" >expect &&\n\ttest_cmp expect err\n'\n\nAh, OK, you're right.\n\nThe problem is not the subdir/ directory itself, but the leftover\nsymbolic link subdir/.gitignore that would cause \"ls-files -o\" to\nnotice and complain about that symbolic link.\n\nIf that is what is happening, then this should probably be fixed in\na belt-and-suspenders fashion.  The primary bug is that the\nexpensive test that does not protect against pre-existing files in\nthe working tree when it starts.\n\nIn the failing test, remove preexisting .gitignore everywhere in\ntree.  With as bad hygiene as this entire script has, we do not know\nwhat is left behind in the working tree by which other test piece\nthat comes before this last test, something like\n\ntest_expect_success EXPENSIVE 'large exclude file ignored in tree' '\n\ttest_when_finished \"rm .gitignore\" &&\n\tfind . -name .gitignore -exec rm \"{}\" \";\" &&\n\tdd if=/dev/zero of=.gitignore bs=101M count=1 &&\n\t...\n\nperhaps?\n\nAnd in the previous test, as you said, you would need to remove at\nleast subdir/.gitignore in addition to .gitignore to make the next\ntest pass, but (1) the next test should not depend on it to work\ncorrectly, and (2) it would be a good discipline to remove any and\nall dropping you make.\n\nSo the change to the not-expensive piece would be\n\ntest_expect_success SYMLINKS 'symlinks not respected in-tree' '\n\ttest_when_finished \"rm -fr subdir .gitignore err actual\" &&\n\t...\n\nand the primary reason why we make such a change (to be described in\nthe proposed log message) is not about the next test, but is about\ncleaning cruft created in each test before it finishes.  It would be\nOK to make both changes in a single commit, as they are fairly small.\n\nThanks.\n"},{"id":"539057","messageId":"20260316011544.13825-1-mroik@delayed.space","threadId":"65250","inReplyTo":"20260315034851.2261530-1-mroik@delayed.space","subject":"[PATCH v2] t0008: improve test cleanup to fix failing test","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-03-16T01:15:42Z","receivedAt":"2026-03-16T01:15:57Z","isPatch":true,"sender":{"key":"mroik@delayed.space","avatar":"https://avatars.githubusercontent.com/u/25752903?v=4"},"body":"The \"large exclude file ignored in tree\" test fails. This is due to an\nadditional warning message that is generated in the test. \"warning:\nunable to access 'subdir/.gitignore': Too many levels of symbolic\nlinks\", the extra warning that is not supposed to be there, happens\nbecause of some leftover files left by previous tests.\n\nTo fix this we improve cleanup on \"symlinks not respected in-tree\", and\nbecause the tests in t0008 in general have poor cleanup, at the start of\n\"large exclude file ignored in tree\" we search for any leftover\n.gitignore and remove them before starting the test.\n\nImprove post-test cleanup and add pre-test cleanup to make sure that we\nhave a workable environment for the test.\n\nSigned-off-by: Mirko Faina <mroik@delayed.space>\n---\nSorry again for the poorly written commit message in the previous patch.\n\n t/t0008-ignores.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex db8bde280e..e716b5cdfa 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -946,7 +946,7 @@ test_expect_success SYMLINKS 'symlinks respected in info/exclude' '\n '\n \n test_expect_success SYMLINKS 'symlinks not respected in-tree' '\n-\ttest_when_finished \"rm .gitignore\" &&\n+\ttest_when_finished \"rm -rf subdir .gitignore err actual\" &&\n \tln -s ignore .gitignore &&\n \tmkdir subdir &&\n \tln -s ignore subdir/.gitignore &&\n@@ -957,6 +957,7 @@ test_expect_success SYMLINKS 'symlinks not respected in-tree' '\n \n test_expect_success EXPENSIVE 'large exclude file ignored in tree' '\n \ttest_when_finished \"rm .gitignore\" &&\n+\tfind . -name .gitignore -exec rm \"{}\" \";\" &&\n \tdd if=/dev/zero of=.gitignore bs=101M count=1 &&\n \tgit ls-files -o --exclude-standard 2>err &&\n \techo \"warning: ignoring excessively large pattern file: .gitignore\" >expect &&\n-- \n2.53.0.959.g497ff81fa9\n\n"},{"id":"539058","messageId":"abdaD52qLrFz2B_M@exploit","threadId":"65250","inReplyTo":"20260316011544.13825-1-mroik@delayed.space","subject":"Re: [PATCH v2] t0008: improve test cleanup to fix failing test","fromName":"Mirko Faina","fromEmail":"mroik@delayed.space","sentAt":"2026-03-16T01:21:30Z","receivedAt":"2026-03-16T01:21:34Z","isPatch":true,"sender":{"key":"mroik@delayed.space","avatar":"https://avatars.githubusercontent.com/u/25752903?v=4"},"body":"A curiosity of mine. How often are the expensive tests ran? Is it when\nthere's an RC? \"large exclude file ignored in tree\" has been there since\n2024, and \"symlink not respected in-tree\" since 2021. I find it hard to\nbelieve that no one has noticed this test failing had they been ran.\nSince no one noticed I'm assuming they're not ran on github's CI\nneither.\n"},{"id":"539148","messageId":"xmqqms07o8sv.fsf@gitster.g","threadId":"65250","inReplyTo":"20260316011544.13825-1-mroik@delayed.space","subject":"Re: [PATCH v2] t0008: improve test cleanup to fix failing test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-16T19:48:00Z","receivedAt":"2026-03-16T19:48:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mirko Faina <mroik@delayed.space> writes:\n\n> The \"large exclude file ignored in tree\" test fails. This is due to an\n> additional warning message that is generated in the test. \"warning:\n> unable to access 'subdir/.gitignore': Too many levels of symbolic\n> links\", the extra warning that is not supposed to be there, happens\n> because of some leftover files left by previous tests.\n\nCorrectly diagnosed and clearly written.  Very much appreciated.\n\nWill queue.  Thanks.\n"}]}