{"thread":{"id":"50443","subject":"[Possible Breakage] t1308 - Bad return value from test-tool","startedAt":"2019-02-08T21:42:30Z","lastAt":"2019-02-10T00:16:08Z","messageCount":5,"participants":["Randall S. Becker","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"368886","messageId":"004901d4bff7$2e17dc50$8a4794f0$@nexbridge.com","threadId":"50443","inReplyTo":null,"subject":"[Possible Breakage] t1308 - Bad return value from test-tool","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2019-02-08T21:42:19Z","receivedAt":"2019-02-08T21:42:30Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"t1308 has me perplexed - this is an old breakage on the NonStop platform,\nthat I have just gotten around to checking with the new bash version we\nhave. When running sub-test 23, the following was reported:\n\nValue not found for \"foo.bar\"\ntest_expect_code: command exited with 1, we wanted 2 test-tool config\nconfigset_get_value foo.bar a-directory\n\nHowever, when I looked inside t/helper/test-config.c, every path reporting\n\"Value not found\" has a goto exit1 not exit2. It seems, from the code, that\nthe test is actually incorrect and should be expecting 1 not 2, and that it\nis working properly on NonStop (but the test fails as a result).\n\nWhat is perplexing is that someone else should be seeing this. The\ntest-config.c code is about 5 years old, but the test was modified a year or\ntwo ago. What am I missing here?\n\nCheers,\nRandall\n\n\n\n"},{"id":"368905","messageId":"20190209042404.GA18286@sigill.intra.peff.net","threadId":"50443","inReplyTo":"004901d4bff7$2e17dc50$8a4794f0$@nexbridge.com","subject":"Re: [Possible Breakage] t1308 - Bad return value from test-tool","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-09T04:24:04Z","receivedAt":"2019-02-09T04:27:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 08, 2019 at 04:42:19PM -0500, Randall S. Becker wrote:\n\n> t1308 has me perplexed - this is an old breakage on the NonStop platform,\n> that I have just gotten around to checking with the new bash version we\n> have. When running sub-test 23, the following was reported:\n> \n> Value not found for \"foo.bar\"\n> test_expect_code: command exited with 1, we wanted 2 test-tool config\n> configset_get_value foo.bar a-directory\n> \n> However, when I looked inside t/helper/test-config.c, every path reporting\n> \"Value not found\" has a goto exit1 not exit2. It seems, from the code, that\n> the test is actually incorrect and should be expecting 1 not 2, and that it\n> is working properly on NonStop (but the test fails as a result).\n\nWe're expecting it to report an error reading the directory, not \"value\nnot found\". Which would yield code 2.\n\nIt sounds like you might need to set FREAD_READS_DIRECTORIES in your\nconfig.mak.\n\n-Peff\n"},{"id":"368930","messageId":"000501d4c0a2$683b78f0$38b26ad0$@nexbridge.com","threadId":"50443","inReplyTo":"20190209042404.GA18286@sigill.intra.peff.net","subject":"RE: [Possible Breakage] t1308 - Bad return value from test-tool","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2019-02-09T18:08:01Z","receivedAt":"2019-02-09T18:08:17Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On February 8, 2019 23:24, Jeff King wrote:\n> On Fri, Feb 08, 2019 at 04:42:19PM -0500, Randall S. Becker wrote:\n> \n> > t1308 has me perplexed - this is an old breakage on the NonStop\n> > platform, that I have just gotten around to checking with the new bash\n> > version we have. When running sub-test 23, the following was reported:\n> >\n> > Value not found for \"foo.bar\"\n> > test_expect_code: command exited with 1, we wanted 2 test-tool config\n> > configset_get_value foo.bar a-directory\n> >\n> > However, when I looked inside t/helper/test-config.c, every path\n> > reporting \"Value not found\" has a goto exit1 not exit2. It seems, from\n> > the code, that the test is actually incorrect and should be expecting\n> > 1 not 2, and that it is working properly on NonStop (but the test fails as a\n> result).\n> \n> We're expecting it to report an error reading the directory, not \"value not\n> found\". Which would yield code 2.\n> \n> It sounds like you might need to set FREAD_READS_DIRECTORIES in your\n> config.mak.\n\nSetting FREAD_READS_DIRECTORIES = UnfortunatelyYes\n\nstill results in \n\nValue not found for \"foo.bar\"\ntest_expect_code: command exited with 1, we wanted 2 test-tool config configset_get_value foo.bar a-directory\nnot ok 23 - proper error on directory \"files\"\n#\n#               echo \"Error (-1) reading configuration file a-directory.\" >expect &&\n#               mkdir a-directory &&\n#               test_expect_code 2 test-tool config configset_get_value foo.bar a-directory 2>output &&\n#               grep \"^warning:\" output &&\n#               grep \"^Error\" output >actual &&\n#               test_cmp expect actual\n#\n\nI don't think that helped. While fopen can open a directory, fread does not return any data in this platform. readdir or nftw/ftw are pretty much the only options. However, the code still goes down the goto exit1 path in this situation. \n\nPerplexed,\nRandall\n\n"},{"id":"368949","messageId":"20190209233246.GB7135@sigill.intra.peff.net","threadId":"50443","inReplyTo":"000501d4c0a2$683b78f0$38b26ad0$@nexbridge.com","subject":"Re: [Possible Breakage] t1308 - Bad return value from test-tool","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-09T23:32:46Z","receivedAt":"2019-02-09T23:32:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 09, 2019 at 01:08:01PM -0500, Randall S. Becker wrote:\n\n> > It sounds like you might need to set FREAD_READS_DIRECTORIES in your\n> > config.mak.\n> \n> Setting FREAD_READS_DIRECTORIES = UnfortunatelyYes\n\nSilly question, but you did rebuild after setting that, not just re-run\nthe tests, right?\n\n> still results in \n> \n> Value not found for \"foo.bar\"\n> test_expect_code: command exited with 1, we wanted 2 test-tool config configset_get_value foo.bar a-directory\n> not ok 23 - proper error on directory \"files\"\n> #\n> #               echo \"Error (-1) reading configuration file a-directory.\" >expect &&\n> #               mkdir a-directory &&\n> #               test_expect_code 2 test-tool config configset_get_value foo.bar a-directory 2>output &&\n> #               grep \"^warning:\" output &&\n> #               grep \"^Error\" output >actual &&\n> #               test_cmp expect actual\n> #\n> \n> I don't think that helped. While fopen can open a directory, fread\n> does not return any data in this platform. readdir or nftw/ftw are\n> pretty much the only options. However, the code still goes down the\n> goto exit1 path in this situation.\n\nHrm. That's the exact case FREAD_READS_DIRECTORIES is supposed to help.\nIt does an fstat() after fopen()ing the file to see if it points to a\ndirectory, and if so closes it immediately and returns EISDIR.\n\nCan you confirm via debugger (or printf statements) that we make it into\ngit_fopen, and what the resulting st.st_mode we see looks like?\n\n-Peff\n"},{"id":"368950","messageId":"000001d4c0d5$c972ad90$5c5808b0$@nexbridge.com","threadId":"50443","inReplyTo":"20190209233246.GB7135@sigill.intra.peff.net","subject":"RE: [Possible Breakage] t1308 - Bad return value from test-tool","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2019-02-10T00:15:48Z","receivedAt":"2019-02-10T00:16:08Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On February 9, 2019 18:33, Jeff King wrote:\n> On Sat, Feb 09, 2019 at 01:08:01PM -0500, Randall S. Becker wrote:\n> \n> > > It sounds like you might need to set FREAD_READS_DIRECTORIES in your\n> > > config.mak.\n> >\n> > Setting FREAD_READS_DIRECTORIES = UnfortunatelyYes\n> \n> Silly question, but you did rebuild after setting that, not just re-run the tests,\n> right?\n\nYes make was run, but some doofus (me) did not run make tests first, so test-tool was out of date, post reconfigure. When just a code change is made, test-tool gets build. I was/am confused.\n\n> > still results in\n> >\n> > Value not found for \"foo.bar\"\n> > test_expect_code: command exited with 1, we wanted 2 test-tool config\n> > configset_get_value foo.bar a-directory not ok 23 - proper error on\n> directory \"files\"\n> > #\n> > #               echo \"Error (-1) reading configuration file a-directory.\" >expect\n> &&\n> > #               mkdir a-directory &&\n> > #               test_expect_code 2 test-tool config configset_get_value foo.bar\n> a-directory 2>output &&\n> > #               grep \"^warning:\" output &&\n> > #               grep \"^Error\" output >actual &&\n> > #               test_cmp expect actual\n> > #\n> >\n> > I don't think that helped. While fopen can open a directory, fread\n> > does not return any data in this platform. readdir or nftw/ftw are\n> > pretty much the only options. However, the code still goes down the\n> > goto exit1 path in this situation.\n> \n> Hrm. That's the exact case FREAD_READS_DIRECTORIES is supposed to help.\n> It does an fstat() after fopen()ing the file to see if it points to a directory, and\n> if so closes it immediately and returns EISDIR.\n> \n> Can you confirm via debugger (or printf statements) that we make it into\n> git_fopen, and what the resulting st.st_mode we see looks like?\n\nIt did not get into git_fopen at all. Subtest 1 did, but that's no surprise.\n\nt1308 is passes now, finally. Patch coming.\n\nThanks for the help,\nRandall\n\n\n\n"}]}