{"thread":{"id":"42178","subject":"Fwd: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","startedAt":"2016-04-29T12:53:49Z","lastAt":"2016-05-03T21:28:09Z","messageCount":10,"participants":["Jan Keromnes","Johannes Schindelin","Stefan Beller","Junio C Hamano","Jeff King","erik elfström"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"284915","messageId":"CAA6PgK4of46eEJOJstnVvPvDFqU_OC5QVz9LWxyKgqHt9V5ySQ@mail.gmail.com","threadId":"42178","inReplyTo":"CAA6PgK7b=ithSYREV5axaE3fmRG5Vp06UtWiZXD-aJuZKfEVYA@mail.gmail.com","subject":"Fwd: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"Jan Keromnes","fromEmail":"janx@linux.com","sentAt":"2016-04-29T12:53:49Z","receivedAt":"2016-04-29T12:53:49Z","isPatch":false,"sender":{"key":"janx@linux.com","avatar":"https://gravatar.com/avatar/2d842511ba2875cc54f5434fb9c6a6ef813eb53bd511b6fe05b645f6c151e344?d=mp&s=160"},"body":"Hello,\n\nI tried running a full profile build of Git 2.8.1, but it looks like\ntest #32 in `t7300-clean.sh` fails:\n\nCommands:\n\n> curl https://www.kernel.org/pub/software/scm/git/git-2.8.1.tar.xz | tar xJ\n> cd git-2.8.1\n> make prefix=/usr profile-install install-man -j18\n\nLogs of test-suite that fails:\n\n*** t7300-clean.sh ***\nok 1 - setup\nok 2 - git clean with skip-worktree .gitignore\nok 3 - git clean\nok 4 - git clean src/\nok 5 - git clean src/ src/\nok 6 - git clean with prefix\nok 7 - git clean with relative prefix\nok 8 - git clean with absolute path\nok 9 - git clean with out of work tree relative path\nok 10 - git clean with out of work tree absolute path\nok 11 - git clean -d with prefix and path\nok 12 - git clean symbolic link\nok 13 - git clean with wildcard\nok 14 - git clean -n\nok 15 - git clean -d\nok 16 - git clean -d src/ examples/\nok 17 - git clean -x\nok 18 - git clean -d -x\nok 19 - git clean -d -x with ignored tracked directory\nok 20 - git clean -X\nok 21 - git clean -d -X\nok 22 - git clean -d -X with ignored tracked directory\nok 23 - clean.requireForce defaults to true\nok 24 - clean.requireForce\nok 25 - clean.requireForce and -n\nok 26 - clean.requireForce and -f\nok 27 - core.excludesfile\nok 28 # skip removal failure (missing SANITY)\nok 29 - nested git work tree\nok 30 - should clean things that almost look like git but are not\nok 31 - should not clean submodules\nnot ok 32 - should avoid cleaning possible submodules\n#\n#               rm -fr to_clean possible_sub1 &&\n#               mkdir to_clean possible_sub1 &&\n#               test_when_finished \"rm -rf possible_sub*\" &&\n#               echo \"gitdir: foo\" >possible_sub1/.git &&\n#               >possible_sub1/hello.world &&\n#               chmod 0 possible_sub1/.git &&\n#               >to_clean/should_clean.this &&\n#               git clean -f -d &&\n#               test_path_is_file possible_sub1/.git &&\n#               test_path_is_file possible_sub1/hello.world &&\n#               test_path_is_missing to_clean\n#\n\nBest,\nJan\n"},{"id":"284916","messageId":"alpine.DEB.2.20.1604291456230.9313@virtualbox","threadId":"42178","inReplyTo":"CAA6PgK4of46eEJOJstnVvPvDFqU_OC5QVz9LWxyKgqHt9V5ySQ@mail.gmail.com","subject":"Re: Fwd: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-04-29T12:58:24Z","receivedAt":"2016-04-29T12:58:24Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jan,\n\nOn Fri, 29 Apr 2016, Jan Keromnes wrote:\n\n> I tried running a full profile build of Git 2.8.1, but it looks like\n> test #32 in `t7300-clean.sh` fails:\n> \n> Commands:\n> \n> > curl https://www.kernel.org/pub/software/scm/git/git-2.8.1.tar.xz | tar xJ\n> > cd git-2.8.1\n> > make prefix=/usr profile-install install-man -j18\n> \n> Logs of test-suite that fails:\n> \n> *** t7300-clean.sh ***\n> ok 1 - setup\n> ok 2 - git clean with skip-worktree .gitignore\n> ok 3 - git clean\n> ok 4 - git clean src/\n> ok 5 - git clean src/ src/\n> ok 6 - git clean with prefix\n> ok 7 - git clean with relative prefix\n> ok 8 - git clean with absolute path\n> ok 9 - git clean with out of work tree relative path\n> ok 10 - git clean with out of work tree absolute path\n> ok 11 - git clean -d with prefix and path\n> ok 12 - git clean symbolic link\n> ok 13 - git clean with wildcard\n> ok 14 - git clean -n\n> ok 15 - git clean -d\n> ok 16 - git clean -d src/ examples/\n> ok 17 - git clean -x\n> ok 18 - git clean -d -x\n> ok 19 - git clean -d -x with ignored tracked directory\n> ok 20 - git clean -X\n> ok 21 - git clean -d -X\n> ok 22 - git clean -d -X with ignored tracked directory\n> ok 23 - clean.requireForce defaults to true\n> ok 24 - clean.requireForce\n> ok 25 - clean.requireForce and -n\n> ok 26 - clean.requireForce and -f\n> ok 27 - core.excludesfile\n> ok 28 # skip removal failure (missing SANITY)\n> ok 29 - nested git work tree\n> ok 30 - should clean things that almost look like git but are not\n> ok 31 - should not clean submodules\n> not ok 32 - should avoid cleaning possible submodules\n> #\n> #               rm -fr to_clean possible_sub1 &&\n> #               mkdir to_clean possible_sub1 &&\n> #               test_when_finished \"rm -rf possible_sub*\" &&\n> #               echo \"gitdir: foo\" >possible_sub1/.git &&\n> #               >possible_sub1/hello.world &&\n> #               chmod 0 possible_sub1/.git &&\n> #               >to_clean/should_clean.this &&\n> #               git clean -f -d &&\n> #               test_path_is_file possible_sub1/.git &&\n> #               test_path_is_file possible_sub1/hello.world &&\n> #               test_path_is_missing to_clean\n> #\n\nThis log does not really help, in particular because your report does not\ninclude vital information such as host OS, installed libraries, etc.\n\nTo debug further on your side (which is really the only logical thing to\ndo from here), have a look at this documentation:\n\n\thttps://github.com/git-for-windows/git/wiki/Running-Git's-regression-tests#running-individual-tests\n\n(even if it is on Git for Windows' wiki, you will find that the\nsuggestions apply to any development environment.)\n\nCiao,\nJohannes\n"},{"id":"284947","messageId":"CAGZ79kYy=+xz5k0pN+MZTRBALd-sqbQ0TePNjkx6itjQpbRUvA@mail.gmail.com","threadId":"42178","inReplyTo":"CAA6PgK4of46eEJOJstnVvPvDFqU_OC5QVz9LWxyKgqHt9V5ySQ@mail.gmail.com","subject":"Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-29T17:06:31Z","receivedAt":"2016-04-29T17:06:31Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Apr 29, 2016 at 5:53 AM, Jan Keromnes <janx@linux.com> wrote:\n> Hello,\n>\n> I tried running a full profile build of Git 2.8.1, but it looks like\n> test #32 in `t7300-clean.sh` fails:\n>\n> Commands:\n>\n>> curl https://www.kernel.org/pub/software/scm/git/git-2.8.1.tar.xz | tar xJ\n>> cd git-2.8.1\n>> make prefix=/usr profile-install install-man -j18\n>\n> Logs of test-suite that fails:\n>\n> *** t7300-clean.sh ***\n> ok 1 - setup\n> ok 2 - git clean with skip-worktree .gitignore\n> ok 3 - git clean\n> ok 4 - git clean src/\n> ok 5 - git clean src/ src/\n> ok 6 - git clean with prefix\n> ok 7 - git clean with relative prefix\n> ok 8 - git clean with absolute path\n> ok 9 - git clean with out of work tree relative path\n> ok 10 - git clean with out of work tree absolute path\n> ok 11 - git clean -d with prefix and path\n> ok 12 - git clean symbolic link\n> ok 13 - git clean with wildcard\n> ok 14 - git clean -n\n> ok 15 - git clean -d\n> ok 16 - git clean -d src/ examples/\n> ok 17 - git clean -x\n> ok 18 - git clean -d -x\n> ok 19 - git clean -d -x with ignored tracked directory\n> ok 20 - git clean -X\n> ok 21 - git clean -d -X\n> ok 22 - git clean -d -X with ignored tracked directory\n> ok 23 - clean.requireForce defaults to true\n> ok 24 - clean.requireForce\n> ok 25 - clean.requireForce and -n\n> ok 26 - clean.requireForce and -f\n> ok 27 - core.excludesfile\n> ok 28 # skip removal failure (missing SANITY)\n> ok 29 - nested git work tree\n> ok 30 - should clean things that almost look like git but are not\n> ok 31 - should not clean submodules\n> not ok 32 - should avoid cleaning possible submodules\n> #\n> #               rm -fr to_clean possible_sub1 &&\n> #               mkdir to_clean possible_sub1 &&\n> #               test_when_finished \"rm -rf possible_sub*\" &&\n> #               echo \"gitdir: foo\" >possible_sub1/.git &&\n> #               >possible_sub1/hello.world &&\n> #               chmod 0 possible_sub1/.git &&\n> #               >to_clean/should_clean.this &&\n> #               git clean -f -d &&\n> #               test_path_is_file possible_sub1/.git &&\n> #               test_path_is_file possible_sub1/hello.world &&\n> #               test_path_is_missing to_clean\n> #\n>\n> Best,\n> Jan\n\nThanks for reporting the bug!\n\nHave a look at t/README to run the tests with command line arguments.\n(I usually run tests as ./tXXXfoo.sh -d -i -v -x with these arguments,\nthough I cannot remember what each of that does. One of it makes the\ntest suite stop on a failing test, such that you can cd into the testing\ndirectory and check the state of the file. (Which are present, which are gone?)\n\nWith these arguments it is also very verbose, and it would tell\nyou what is wrong (is the assertion wrong in the `test_path_is_file/missing`\nor is it `git clean` segfaulting?)\n\nAs Johannes said, it makes sense that you debug into that as\nno one could reproduce it thus far on their system.\n\nThanks,\nStefan\n"},{"id":"285270","messageId":"CAA6PgK4M2Ag6anEFYakW9NJe0ehTtS1UEQmDWe387W_uV-MCXA@mail.gmail.com","threadId":"42178","inReplyTo":"CAGZ79kYy=+xz5k0pN+MZTRBALd-sqbQ0TePNjkx6itjQpbRUvA@mail.gmail.com","subject":"Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"Jan Keromnes","fromEmail":"janx@linux.com","sentAt":"2016-05-03T09:19:07Z","receivedAt":"2016-05-03T09:19:07Z","isPatch":false,"sender":{"key":"janx@linux.com","avatar":"https://gravatar.com/avatar/2d842511ba2875cc54f5434fb9c6a6ef813eb53bd511b6fe05b645f6c151e344?d=mp&s=160"},"body":"Thanks for your replies! I was able to reproduce to failure on Git 2.8.2.\n\nSteps:\n\n# Build Git 2.8.2 and run t/t7300-clean.sh in a Dockerfile based on\nubuntu:14.04.\nRUN mkdir /tmp/git \\\n && cd /tmp/git \\\n && curl https://www.kernel.org/pub/software/scm/git/git-2.8.2.tar.xz | tar xJ \\\n && cd git-2.8.2 \\\n && make all && cd t && ./t7300-clean.sh -d -i -v -x\n\nLogs:\n\nexpecting success:\n        rm -fr to_clean possible_sub1 &&\n        mkdir to_clean possible_sub1 &&\n        test_when_finished \"rm -rf possible_sub*\" &&\n        echo \"gitdir: foo\" >possible_sub1/.git &&\n        >possible_sub1/hello.world &&\n        chmod 0 possible_sub1/.git &&\n        >to_clean/should_clean.this &&\n        git clean -f -d &&\n        test_path_is_file possible_sub1/.git &&\n        test_path_is_file possible_sub1/hello.world &&\n        test_path_is_missing to_clean\n\n+ rm -fr to_clean possible_sub1\n+ mkdir to_clean possible_sub1\n+ test_when_finished rm -rf possible_sub*\n+ test 0 = 0\n+ test_cleanup={ rm -rf possible_sub*\n                } && (exit \"$eval_ret\"); eval_ret=$?; :\n+ echo gitdir: foo\n+\n+ chmod 0 possible_sub1/.git\n+\n+ git clean -f -d\nSkipping repository baz/boo\nSkipping repository foo/\nRemoving possible_sub1/\nSkipping repository repo/\nSkipping repository sub2/\nRemoving to_clean/\n+ test_path_is_file possible_sub1/.git\n+ test -f possible_sub1/.git\n+ echo File possible_sub1/.git doesn't exist.\n+ false\nerror: last command exited with $?=1\nFile possible_sub1/.git doesn't exist.\nnot ok 32 - should avoid cleaning possible submodules\n#\n#               rm -fr to_clean possible_sub1 &&\n#               mkdir to_clean possible_sub1 &&\n#               test_when_finished \"rm -rf possible_sub*\" &&\n#               echo \"gitdir: foo\" >possible_sub1/.git &&\n#               >possible_sub1/hello.world &&\n#               chmod 0 possible_sub1/.git &&\n#               >to_clean/should_clean.this &&\n#               git clean -f -d &&\n#               test_path_is_file possible_sub1/.git &&\n#               test_path_is_file possible_sub1/hello.world &&\n#               test_path_is_missing to_clean\n#\n\nJudging from the line \"Removing possible_sub1/\", it looks like Git\n2.8.2 removes a possible submodule while executing `git clean -f -d`,\nwhereas the test expects it not to.\n\nIs it possible to make Git's output even more verbose, so that it\ntells why it decides to remove \"possible_sub1\"? Are you able to\nreproduce this test fail on your side?\n\nThis prevents doing a full profile build.\n\nBest,\nJan\n\nOn Fri, Apr 29, 2016 at 7:06 PM, Stefan Beller <sbeller@google.com> wrote:\n> On Fri, Apr 29, 2016 at 5:53 AM, Jan Keromnes <janx@linux.com> wrote:\n>> Hello,\n>>\n>> I tried running a full profile build of Git 2.8.1, but it looks like\n>> test #32 in `t7300-clean.sh` fails:\n>>\n>> Commands:\n>>\n>>> curl https://www.kernel.org/pub/software/scm/git/git-2.8.1.tar.xz | tar xJ\n>>> cd git-2.8.1\n>>> make prefix=/usr profile-install install-man -j18\n>>\n>> Logs of test-suite that fails:\n>>\n>> *** t7300-clean.sh ***\n>> ok 1 - setup\n>> ok 2 - git clean with skip-worktree .gitignore\n>> ok 3 - git clean\n>> ok 4 - git clean src/\n>> ok 5 - git clean src/ src/\n>> ok 6 - git clean with prefix\n>> ok 7 - git clean with relative prefix\n>> ok 8 - git clean with absolute path\n>> ok 9 - git clean with out of work tree relative path\n>> ok 10 - git clean with out of work tree absolute path\n>> ok 11 - git clean -d with prefix and path\n>> ok 12 - git clean symbolic link\n>> ok 13 - git clean with wildcard\n>> ok 14 - git clean -n\n>> ok 15 - git clean -d\n>> ok 16 - git clean -d src/ examples/\n>> ok 17 - git clean -x\n>> ok 18 - git clean -d -x\n>> ok 19 - git clean -d -x with ignored tracked directory\n>> ok 20 - git clean -X\n>> ok 21 - git clean -d -X\n>> ok 22 - git clean -d -X with ignored tracked directory\n>> ok 23 - clean.requireForce defaults to true\n>> ok 24 - clean.requireForce\n>> ok 25 - clean.requireForce and -n\n>> ok 26 - clean.requireForce and -f\n>> ok 27 - core.excludesfile\n>> ok 28 # skip removal failure (missing SANITY)\n>> ok 29 - nested git work tree\n>> ok 30 - should clean things that almost look like git but are not\n>> ok 31 - should not clean submodules\n>> not ok 32 - should avoid cleaning possible submodules\n>> #\n>> #               rm -fr to_clean possible_sub1 &&\n>> #               mkdir to_clean possible_sub1 &&\n>> #               test_when_finished \"rm -rf possible_sub*\" &&\n>> #               echo \"gitdir: foo\" >possible_sub1/.git &&\n>> #               >possible_sub1/hello.world &&\n>> #               chmod 0 possible_sub1/.git &&\n>> #               >to_clean/should_clean.this &&\n>> #               git clean -f -d &&\n>> #               test_path_is_file possible_sub1/.git &&\n>> #               test_path_is_file possible_sub1/hello.world &&\n>> #               test_path_is_missing to_clean\n>> #\n>>\n>> Best,\n>> Jan\n>\n> Thanks for reporting the bug!\n>\n> Have a look at t/README to run the tests with command line arguments.\n> (I usually run tests as ./tXXXfoo.sh -d -i -v -x with these arguments,\n> though I cannot remember what each of that does. One of it makes the\n> test suite stop on a failing test, such that you can cd into the testing\n> directory and check the state of the file. (Which are present, which are gone?)\n>\n> With these arguments it is also very verbose, and it would tell\n> you what is wrong (is the assertion wrong in the `test_path_is_file/missing`\n> or is it `git clean` segfaulting?)\n>\n> As Johannes said, it makes sense that you debug into that as\n> no one could reproduce it thus far on their system.\n>\n> Thanks,\n> Stefan\n"},{"id":"285349","messageId":"CAGZ79kav27h+_eswTd3hRWjYhH5aApVcERa1bSxKD8p2R50HTw@mail.gmail.com","threadId":"42178","inReplyTo":"CAA6PgK4M2Ag6anEFYakW9NJe0ehTtS1UEQmDWe387W_uV-MCXA@mail.gmail.com","subject":"Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-03T18:02:46Z","receivedAt":"2016-05-03T18:02:46Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 3, 2016 at 2:19 AM, Jan Keromnes <janx@linux.com> wrote:\n> Thanks for your replies! I was able to reproduce to failure on Git 2.8.2.\n>\n> Steps:\n>\n> # Build Git 2.8.2 and run t/t7300-clean.sh in a Dockerfile based on\n> ubuntu:14.04.\n> RUN mkdir /tmp/git \\\n>  && cd /tmp/git \\\n>  && curl https://www.kernel.org/pub/software/scm/git/git-2.8.2.tar.xz | tar xJ \\\n>  && cd git-2.8.2 \\\n>  && make all && cd t && ./t7300-clean.sh -d -i -v -x\n\nI am on a Ubuntu 14.04.1 LTS derivative (plain, not inside a docker) and running\nthat list of commands (getting that tarball from kernel.org and testing) doesn't\nreproduce the failure for me.\n\nexpecting success:\n         rm -fr to_clean possible_sub1 &&\n         mkdir to_clean possible_sub1 &&\n         test_when_finished \"rm -rf possible_sub*\" &&\n         echo \"gitdir: foo\" >possible_sub1/.git &&\n         >possible_sub1/hello.world &&\n         chmod 0 possible_sub1/.git &&\n         >to_clean/should_clean.this &&\n         git clean -f -d &&\n         test_path_is_file possible_sub1/.git &&\n         test_path_is_file possible_sub1/hello.world &&\n         test_path_is_missing to_clean\n\n++ rm -fr to_clean possible_sub1\n++ mkdir to_clean possible_sub1\n++ test_when_finished 'rm -rf possible_sub*'\n++ test 0 = 0\n++ test_cleanup='{ rm -rf possible_sub*\n} && (exit \"$eval_ret\"); eval_ret=$?; :'\n++ echo 'gitdir: foo'\n++ chmod 0 possible_sub1/.git\n++ git clean -f -d\nSkipping repository baz/boo\nSkipping repository foo/\nSkipping repository possible_sub1/\nSkipping repository repo/\nSkipping repository sub2/\nRemoving to_clean/\n++ test_path_is_file possible_sub1/.git\n++ test -f possible_sub1/.git\n++ test_path_is_file possible_sub1/hello.world\n++ test -f possible_sub1/hello.world\n++ test_path_is_missing to_clean\n++ test -e to_clean\n++ rm -rf possible_sub1\n++ exit 0\n++ eval_ret=0\n++ :\nok 32 - should avoid cleaning possible submodules\n\n>\n> Judging from the line \"Removing possible_sub1/\", it looks like Git\n> 2.8.2 removes a possible submodule while executing `git clean -f -d`,\n> whereas the test expects it not to.\n\nYeah that is my conclusion as well. (The successful test doesn't remove it)\nReading the clean code at [1], specifically the loop starting at line 958,\nthere are two cases. Either the entry is a directory (submodules are\ntreated as special directories) or files.\n\n\nSo the line 975 is executed for that submodule as remove_dirs has the printing\nin both cases.\n\nApparently the remove_dirs exits early for me in the condition\n\n    if ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\nis_nonbare_repository_dir(path)) {\n\nand your case doesn't. (I assume it falls through and later produces\nthe output in line 243.\n\nSo I wonder if is_nonbare_repository_dir() is the culprit here.\n(We do a chmod 0 on the .git before the `git clean` in the test to confuse Git)\n\nis_nonbare_repository_dir is only used in `git clean` as well as for the\nfiles backend (which is rather new. I don't know the state of the usage there).\n\nWhat I find suspicious about is_nonbare_repository_dir (found in setup.c:316),\nis the assumption of only READ_GITFILE_ERR_OPEN_FAILED and\nREAD_GITFILE_ERR_READ_FAILED\nleading to a repository.\n\nI have cc'd Jeff and Erik, who have touched the code there, maybe they\nhave a thought on it.\nIn the mean time I would be interested if this change helps for you:\n\n        diff --git a/setup.c b/setup.c\n        index 3439ec6..4cfba8f 100644\n        --- a/setup.c\n        +++ b/setup.c\n        @@ -323,8 +323,7 @@ int is_nonbare_repository_dir(struct strbuf *path)\n                strbuf_addstr(path, \".git\");\n                if (read_gitfile_gently(path->buf, &gitfile_error) ||\nis_git_directory(path->buf))\n                        ret = 1;\n        -       if (gitfile_error == READ_GITFILE_ERR_OPEN_FAILED ||\n        -           gitfile_error == READ_GITFILE_ERR_READ_FAILED)\n        +       if (gitfile_error)\n                        ret = 1;\n                strbuf_setlen(path, orig_path_len);\n                return ret;\n\nDo you do anything fancy with your file system?\n\nThanks,\nStefan\n\n\n[1] https://github.com/git/git/blob/v2.8.2/builtin/clean.c#L854\n\n\n>\n> Is it possible to make Git's output even more verbose, so that it\n> tells why it decides to remove \"possible_sub1\"? Are you able to\n> reproduce this test fail on your side?\n>\n> This prevents doing a full profile build.\n>\n> Best,\n> Jan\n>\n> On Fri, Apr 29, 2016 at 7:06 PM, Stefan Beller <sbeller@google.com> wrote:\n>> On Fri, Apr 29, 2016 at 5:53 AM, Jan Keromnes <janx@linux.com> wrote:\n>>> Hello,\n>>>\n>>> I tried running a full profile build of Git 2.8.1, but it looks like\n>>> test #32 in `t7300-clean.sh` fails:\n>>>\n>>> Commands:\n>>>\n>>>> curl https://www.kernel.org/pub/software/scm/git/git-2.8.1.tar.xz | tar xJ\n>>>> cd git-2.8.1\n>>>> make prefix=/usr profile-install install-man -j18\n>>>\n>>> Logs of test-suite that fails:\n>>>\n>>> *** t7300-clean.sh ***\n>>> ok 1 - setup\n>>> ok 2 - git clean with skip-worktree .gitignore\n>>> ok 3 - git clean\n>>> ok 4 - git clean src/\n>>> ok 5 - git clean src/ src/\n>>> ok 6 - git clean with prefix\n>>> ok 7 - git clean with relative prefix\n>>> ok 8 - git clean with absolute path\n>>> ok 9 - git clean with out of work tree relative path\n>>> ok 10 - git clean with out of work tree absolute path\n>>> ok 11 - git clean -d with prefix and path\n>>> ok 12 - git clean symbolic link\n>>> ok 13 - git clean with wildcard\n>>> ok 14 - git clean -n\n>>> ok 15 - git clean -d\n>>> ok 16 - git clean -d src/ examples/\n>>> ok 17 - git clean -x\n>>> ok 18 - git clean -d -x\n>>> ok 19 - git clean -d -x with ignored tracked directory\n>>> ok 20 - git clean -X\n>>> ok 21 - git clean -d -X\n>>> ok 22 - git clean -d -X with ignored tracked directory\n>>> ok 23 - clean.requireForce defaults to true\n>>> ok 24 - clean.requireForce\n>>> ok 25 - clean.requireForce and -n\n>>> ok 26 - clean.requireForce and -f\n>>> ok 27 - core.excludesfile\n>>> ok 28 # skip removal failure (missing SANITY)\n>>> ok 29 - nested git work tree\n>>> ok 30 - should clean things that almost look like git but are not\n>>> ok 31 - should not clean submodules\n>>> not ok 32 - should avoid cleaning possible submodules\n>>> #\n>>> #               rm -fr to_clean possible_sub1 &&\n>>> #               mkdir to_clean possible_sub1 &&\n>>> #               test_when_finished \"rm -rf possible_sub*\" &&\n>>> #               echo \"gitdir: foo\" >possible_sub1/.git &&\n>>> #               >possible_sub1/hello.world &&\n>>> #               chmod 0 possible_sub1/.git &&\n>>> #               >to_clean/should_clean.this &&\n>>> #               git clean -f -d &&\n>>> #               test_path_is_file possible_sub1/.git &&\n>>> #               test_path_is_file possible_sub1/hello.world &&\n>>> #               test_path_is_missing to_clean\n>>> #\n>>>\n>>> Best,\n>>> Jan\n>>\n>> Thanks for reporting the bug!\n>>\n>> Have a look at t/README to run the tests with command line arguments.\n>> (I usually run tests as ./tXXXfoo.sh -d -i -v -x with these arguments,\n>> though I cannot remember what each of that does. One of it makes the\n>> test suite stop on a failing test, such that you can cd into the testing\n>> directory and check the state of the file. (Which are present, which are gone?)\n>>\n>> With these arguments it is also very verbose, and it would tell\n>> you what is wrong (is the assertion wrong in the `test_path_is_file/missing`\n>> or is it `git clean` segfaulting?)\n>>\n>> As Johannes said, it makes sense that you debug into that as\n>> no one could reproduce it thus far on their system.\n>>\n>> Thanks,\n>> Stefan\n"},{"id":"285350","messageId":"CAPc5daX4noqHaOZCrZ78s0ytFBkeU7znXs0QY1LXEw8w3RnQ1A@mail.gmail.com","threadId":"42178","inReplyTo":"CAGZ79kav27h+_eswTd3hRWjYhH5aApVcERa1bSxKD8p2R50HTw@mail.gmail.com","subject":"Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-03T18:05:09Z","receivedAt":"2016-05-03T18:05:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Tue, May 3, 2016 at 11:02 AM, Stefan Beller <sbeller@google.com> wrote:\n>\n> So I wonder if is_nonbare_repository_dir() is the culprit here.\n> (We do a chmod 0 on the .git before the `git clean` in the test to confuse Git)\n\nAsk if the test is run as root; if so, then mark the test to require\nSANITY prerequisite.\n"},{"id":"285355","messageId":"20160503184841.GA30530@sigill.intra.peff.net","threadId":"42178","inReplyTo":"CAPc5daX4noqHaOZCrZ78s0ytFBkeU7znXs0QY1LXEw8w3RnQ1A@mail.gmail.com","subject":"Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-03T18:48:41Z","receivedAt":"2016-05-03T18:48:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 03, 2016 at 11:05:09AM -0700, Junio C Hamano wrote:\n\n> On Tue, May 3, 2016 at 11:02 AM, Stefan Beller <sbeller@google.com> wrote:\n> >\n> > So I wonder if is_nonbare_repository_dir() is the culprit here.\n> > (We do a chmod 0 on the .git before the `git clean` in the test to confuse Git)\n> \n> Ask if the test is run as root; if so, then mark the test to require\n> SANITY prerequisite.\n\nYeah, I can easily reproduce the failure with `sudo ./t7300-clean.sh`.\nSo the immediate fix is the SANITY prereq.\n\nLooking at Stefan's message, I wondered if the patch he came up with:\n\n        diff --git a/setup.c b/setup.c\n        index 3439ec6..4cfba8f 100644\n        --- a/setup.c\n        +++ b/setup.c\n        @@ -323,8 +323,7 @@ int is_nonbare_repository_dir(struct strbuf *path)\n                strbuf_addstr(path, \".git\");\n                if (read_gitfile_gently(path->buf, &gitfile_error) ||\nis_git_directory(path->buf))\n                        ret = 1;\n        -       if (gitfile_error == READ_GITFILE_ERR_OPEN_FAILED ||\n        -           gitfile_error == READ_GITFILE_ERR_READ_FAILED)\n        +       if (gitfile_error)\n                        ret = 1;\n                strbuf_setlen(path, orig_path_len);\n                return ret;\n\nis related or worth doing on top. But I don't think so. That code is\njust trying to convert some error-cases into \"let's err on the side of\nassuming it is a repo\". Doing that for all values of gitfile_error is\ndefinitely the wrong thing (it would treat a totally non-existent\n\".git\" file as \"yes, it's there\", which is clearly bogus).\n\n-Peff\n"},{"id":"285356","messageId":"CAGZ79kYxSCsXGLBoBTAOLfe_pOu7YNBQoccs_fA0btEzpnU=-w@mail.gmail.com","threadId":"42178","inReplyTo":"20160503184841.GA30530@sigill.intra.peff.net","subject":"Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-03T18:53:36Z","receivedAt":"2016-05-03T18:53:36Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 3, 2016 at 11:48 AM, Jeff King <peff@peff.net> wrote:\n> On Tue, May 03, 2016 at 11:05:09AM -0700, Junio C Hamano wrote:\n>\n>> On Tue, May 3, 2016 at 11:02 AM, Stefan Beller <sbeller@google.com> wrote:\n>> >\n>> > So I wonder if is_nonbare_repository_dir() is the culprit here.\n>> > (We do a chmod 0 on the .git before the `git clean` in the test to confuse Git)\n>>\n>> Ask if the test is run as root; if so, then mark the test to require\n>> SANITY prerequisite.\n>\n> Yeah, I can easily reproduce the failure with `sudo ./t7300-clean.sh`.\n> So the immediate fix is the SANITY prereq.\n>\n> Looking at Stefan's message, I wondered if the patch he came up with:\n>\n>         diff --git a/setup.c b/setup.c\n>         index 3439ec6..4cfba8f 100644\n>         --- a/setup.c\n>         +++ b/setup.c\n>         @@ -323,8 +323,7 @@ int is_nonbare_repository_dir(struct strbuf *path)\n>                 strbuf_addstr(path, \".git\");\n>                 if (read_gitfile_gently(path->buf, &gitfile_error) ||\n> is_git_directory(path->buf))\n>                         ret = 1;\n>         -       if (gitfile_error == READ_GITFILE_ERR_OPEN_FAILED ||\n>         -           gitfile_error == READ_GITFILE_ERR_READ_FAILED)\n>         +       if (gitfile_error)\n>                         ret = 1;\n>                 strbuf_setlen(path, orig_path_len);\n>                 return ret;\n>\n> is related or worth doing on top. But I don't think so. That code is\n> just trying to convert some error-cases into \"let's err on the side of\n> assuming it is a repo\". Doing that for all values of gitfile_error is\n> definitely the wrong thing (it would treat a totally non-existent\n> \".git\" file as \"yes, it's there\", which is clearly bogus).\n\nThe proposed change is overly eager indeed.\nWhat if we get back a READ_GITFILE_ERR_STAT_FAILED ?\nI would think that is a reasonable indicator of a submodule being there?\n(The stat failure may be transient ENOMEM Out of memory (i.e., kernel memory).)\n\nBy being overly eager I wanted to make sure the assumptions I had were\nwrong.\n\nI'm about to send the SANITY prerequisite patch,\n\nThanks,\nStefan\n\n>\n> -Peff\n"},{"id":"285359","messageId":"20160503190053.GC30530@sigill.intra.peff.net","threadId":"42178","inReplyTo":"CAGZ79kYxSCsXGLBoBTAOLfe_pOu7YNBQoccs_fA0btEzpnU=-w@mail.gmail.com","subject":"Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-03T19:00:53Z","receivedAt":"2016-05-03T19:00:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 03, 2016 at 11:53:36AM -0700, Stefan Beller wrote:\n\n> > is related or worth doing on top. But I don't think so. That code is\n> > just trying to convert some error-cases into \"let's err on the side of\n> > assuming it is a repo\". Doing that for all values of gitfile_error is\n> > definitely the wrong thing (it would treat a totally non-existent\n> > \".git\" file as \"yes, it's there\", which is clearly bogus).\n> \n> The proposed change is overly eager indeed.\n> What if we get back a READ_GITFILE_ERR_STAT_FAILED ?\n> I would think that is a reasonable indicator of a submodule being there?\n> (The stat failure may be transient ENOMEM Out of memory (i.e., kernel memory).)\n\nThat would certainly be wrong with read_gitfile_gently() as it is today;\nit does not distinguish various values of errno for stat(), so that\nwould get the \"there's not even a .git file here at all\" case wrong.\n\nSo the first step would be to have read_gitfile_gently() start looking\nfor ENOENT versus other errors. I don't know if that's worth the\ntrouble; we're pretty cavalier about treating stat failure as \"file does\nnot exist\" in the rest of the code.\n\n-Peff\n"},{"id":"285381","messageId":"CAMpP7NbeELYQvZ1A1acA-V6jZVRvNzjbAqD6zd2YoJvGT5Dxxw@mail.gmail.com","threadId":"42178","inReplyTo":"20160503190053.GC30530@sigill.intra.peff.net","subject":"Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build","fromName":"erik elfström","fromEmail":"erik.elfstrom@gmail.com","sentAt":"2016-05-03T21:28:09Z","receivedAt":"2016-05-03T21:28:09Z","isPatch":false,"sender":{"key":"erik.elfstrom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13905184?v=4"},"body":"Thanks for fixing the missing SANITY prerequisite Stefan.\n\nAs for the error handling logic in setup.c: is_nonbare_repository_dir\n(was clean.c: is_git_repository) my reasoning is as follows:\n\nREAD_GITFILE_ERR_STAT_FAILED\nREAD_GITFILE_ERR_NOT_A_FILE:\n\nWhen checking random paths for .git files these are the common error\nmodes, file is not there or it is a directory. This should not be\ninterpreted as a valid .git file.\n\n\nREAD_GITFILE_ERR_OPEN_FAILED\nREAD_GITFILE_ERR_READ_FAILED:\n\nHere we found a .git file but could not open and read it to verify\nthat it is valid. Treating it as valid is the safest option for clean.\nFor resolve_gitlink_ref I think it maybe leads to the creation of a\nredundant ref cache entries but I don't think this is a problem unless\nsomeone has a huge amount of unreadable .git files lying around.\n\n\nREAD_GITFILE_ERR_TOO_LARGE:\n\nFile is absurdly large (1MB), very unlikely to be a valid .git file.\n\n\nREAD_GITFILE_ERR_INVALID_FORMAT\nREAD_GITFILE_ERR_NO_PATH:\n\nFile is malformed in some way, either the \"gitdir:\" prefix is missing\nor the path is missing. Could theoretically be a corrupted .git file\nbut seems unlikely.\n\n\nREAD_GITFILE_ERR_NOT_A_REPO:\n\nThis is maybe a bit more suspicious. Here we have found a .git file,\nit has the format \"gitdir: some/path\" but\nis_git_directory(\"some/path\") returned false, meaning that \"some/path\"\ndoes not fulfill:\n\n/*\n * Test if it looks like we're at a git directory.\n * We want to see:\n *\n *  - either an objects/ directory _or_ the proper\n *    GIT_OBJECT_DIRECTORY environment variable\n *  - a refs/ directory\n *  - either a HEAD symlink or a HEAD file that is formatted as\n *    a proper \"ref:\", or a regular file HEAD that has a properly\n *    formatted sha1 object name.\n*/\n\nTechnically we don't have a valid .git file here but we have something\nthat really tries to be. I guess it is debatable what the correct\nconservative choice is here and if it is the same for all callers.\n\n/Erik\n"}]}