{"thread":{"id":"58497","subject":"[BUG] Git 2.28.0-rc1 t1800 message text comparison","startedAt":"2022-09-22T19:01:52Z","lastAt":"2023-06-12T18:01:59Z","messageCount":13,"participants":["rsbecker@nexbridge.com","René Scharfe","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"463463","messageId":"022e01d8ceb5$c2da52c0$488ef840$@nexbridge.com","threadId":"58497","inReplyTo":null,"subject":"[BUG] Git 2.28.0-rc1 t1800 message text comparison","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-09-22T19:01:40Z","receivedAt":"2022-09-22T19:01:52Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"Rc1 is looking good except for this test.\n\nWe get a diff as follows:\n\n-fatal: cannot run bad-hooks/test-hook: ...\n+fatal: cannot exec 'bad-hooks/test-hook': Permission denied\n\nIt looks like the pattern \nsed -e s/test-hook: .*/test-hook: .../\n\nneeds to be a bit extended. Adding\n\nsed -e s/exec/run/ | send -e s/[\"']//g\n\nmight help clear off the other noise.\n\nRegards,\nRandall\n\n"},{"id":"463464","messageId":"023101d8ceb5$f1c28850$d54798f0$@nexbridge.com","threadId":"58497","inReplyTo":"022e01d8ceb5$c2da52c0$488ef840$@nexbridge.com","subject":"RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-09-22T19:02:59Z","receivedAt":"2022-09-22T19:03:15Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On September 22, 2022 3:02 PM, I wrote:\n>Rc1 is looking good except for this test.\n>\n>We get a diff as follows:\n>\n>-fatal: cannot run bad-hooks/test-hook: ...\n>+fatal: cannot exec 'bad-hooks/test-hook': Permission denied\n>\n>It looks like the pattern\n>sed -e s/test-hook: .*/test-hook: .../\n>\n>needs to be a bit extended. Adding\n>\n>sed -e s/exec/run/ | send -e s/[\"']//g\n>\n>might help clear off the other noise.\n\nThe subject should have been 2.38.0, not 2.28.0.\n\n"},{"id":"463550","messageId":"003501d8cf8d$21ce5ba0$656b12e0$@nexbridge.com","threadId":"58497","inReplyTo":"022e01d8ceb5$c2da52c0$488ef840$@nexbridge.com","subject":"RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-09-23T20:43:21Z","receivedAt":"2022-09-23T20:47:03Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On September 22, 2022 3:03 PM, I wrote:\n>On September 22, 2022 3:02 PM, I wrote:\n>>Rc1 is looking good except for this test.\n>>\n>>We get a diff as follows:\n>>\n>>-fatal: cannot run bad-hooks/test-hook: ...\n>>+fatal: cannot exec 'bad-hooks/test-hook': Permission denied\n>>\n>>It looks like the pattern\n>>sed -e s/test-hook: .*/test-hook: .../\n>>\n>>needs to be a bit extended. Adding\n>>\n>>sed -e s/exec/run/ | send -e s/[\"']//g\n>>\n>>might help clear off the other noise.\n\nA patch that might work is as follows:\n\ndiff --git a/t/t1800-hook.sh b/t/t1800-hook.sh\nindex 43fcb7c0bf..9a723631a2 100755\n--- a/t/t1800-hook.sh\n+++ b/t/t1800-hook.sh\n@@ -173,7 +173,10 @@ test_expect_success 'git hook run a hook with a bad\nshebang' '\n                -c core.hooksPath=bad-hooks \\\n                hook run test-hook >out 2>err &&\n        test_must_be_empty out &&\n-       sed -e \"s/test-hook: .*/test-hook: .../\" <err >actual &&\n+       quot=`echo \"\\047\"` &&\n+       sed -e \"s/exec/run/\" <err | \\\n+               sed -e \"s/$quot//g\" | \\\n+               sed -e \"s/test-hook: .*/test-hook: .../\" >actual &&\n        test_cmp expect actual\n '\n\nThis does not require setting up a prerequisite for NonStop and the\ntechnique might make the MING code easier but adding a change from spawn to\nrun.\n\n-Randall\n\n"},{"id":"469002","messageId":"013701d90f80$692ba9c0$3b82fd40$@nexbridge.com","threadId":"58497","inReplyTo":"003501d8cf8d$21ce5ba0$656b12e0$@nexbridge.com","subject":"RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-12-14T05:53:31Z","receivedAt":"2022-12-14T05:53:50Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On September 23, 2022 4:43 PM, I wrote:\n>On September 22, 2022 3:03 PM, I wrote:\n>>On September 22, 2022 3:02 PM, I wrote:\n>>>Rc1 is looking good except for this test.\n>>>\n>>>We get a diff as follows:\n>>>\n>>>-fatal: cannot run bad-hooks/test-hook: ...\n>>>+fatal: cannot exec 'bad-hooks/test-hook': Permission denied\n>>>\n>>>It looks like the pattern\n>>>sed -e s/test-hook: .*/test-hook: .../\n>>>\n>>>needs to be a bit extended. Adding\n>>>\n>>>sed -e s/exec/run/ | send -e s/[\"']//g\n>>>\n>>>might help clear off the other noise.\n>\n>A patch that might work is as follows:\n>\n>diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 43fcb7c0bf..9a723631a2\n>100755\n>--- a/t/t1800-hook.sh\n>+++ b/t/t1800-hook.sh\n>@@ -173,7 +173,10 @@ test_expect_success 'git hook run a hook with a bad\n>shebang' '\n>                -c core.hooksPath=bad-hooks \\\n>                hook run test-hook >out 2>err &&\n>        test_must_be_empty out &&\n>-       sed -e \"s/test-hook: .*/test-hook: .../\" <err >actual &&\n>+       quot=`echo \"\\047\"` &&\n>+       sed -e \"s/exec/run/\" <err | \\\n>+               sed -e \"s/$quot//g\" | \\\n>+               sed -e \"s/test-hook: .*/test-hook: .../\" >actual &&\n>        test_cmp expect actual\n> '\n>\n>This does not require setting up a prerequisite for NonStop and the\ntechnique\n>might make the MING code easier but adding a change from spawn to run.\n\nThis is still broken on NonStop. Is there any hope of a resolution?\n\nThanks,\nRandall\n\n"},{"id":"477652","messageId":"2e3be2e9-191d-fbfd-54c6-a3958e35637c@web.de","threadId":"58497","inReplyTo":"013701d90f80$692ba9c0$3b82fd40$@nexbridge.com","subject":"Re: [BUG] Git 2.38.0-rc1 t1800 message text comparison","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-05-22T20:39:20Z","receivedAt":"2023-05-22T20:39:34Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 14.12.22 um 06:53 schrieb rsbecker@nexbridge.com:\n> On September 23, 2022 4:43 PM, I wrote:\n>> On September 22, 2022 3:03 PM, I wrote:\n>>> On September 22, 2022 3:02 PM, I wrote:\n>>>> Rc1 is looking good except for this test.\n>>>>\n>>>> We get a diff as follows:\n>>>>\n>>>> -fatal: cannot run bad-hooks/test-hook: ...\n>>>> +fatal: cannot exec 'bad-hooks/test-hook': Permission denied\n>>>>\n>>>> It looks like the pattern\n>>>> sed -e s/test-hook: .*/test-hook: .../\n>>>>\n>>>> needs to be a bit extended. Adding\n>>>>\n>>>> sed -e s/exec/run/ | send -e s/[\"']//g\n>>>>\n>>>> might help clear off the other noise.\n>>\n>> A patch that might work is as follows:\n>>\n>> diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 43fcb7c0bf..9a723631a2\n>> 100755\n>> --- a/t/t1800-hook.sh\n>> +++ b/t/t1800-hook.sh\n>> @@ -173,7 +173,10 @@ test_expect_success 'git hook run a hook with a bad\n>> shebang' '\n>>                -c core.hooksPath=bad-hooks \\\n>>                hook run test-hook >out 2>err &&\n>>        test_must_be_empty out &&\n>> -       sed -e \"s/test-hook: .*/test-hook: .../\" <err >actual &&\n>> +       quot=`echo \"\\047\"` &&\n>> +       sed -e \"s/exec/run/\" <err | \\\n>> +               sed -e \"s/$quot//g\" | \\\n>> +               sed -e \"s/test-hook: .*/test-hook: .../\" >actual &&\n>>        test_cmp expect actual\n>> '\n>>\n>> This does not require setting up a prerequisite for NonStop and the\n> technique\n>> might make the MING code easier but adding a change from spawn to run.\n>\n> This is still broken on NonStop. Is there any hope of a resolution?\n\nSo trying to execute an executable file consisting only of the line\n\"#!/bad/path/no/spaces\" causes NonStop to report \"Permission denied\"?\nThat message text belongs to error code EACCES, not to EPERM (\"Operation\nnot permitted\"), right?\n\nPOSIX allows execve to return EACCES if the file to execute is not a\nregular file and executing that file type is not supported or if\npermissions are missing to one of its path components.\n\nEither you have something called /bad that is not a regular file (e.g. a\ndirectory) -- then it's just a matter of changing the test to use a\ndifferent supposedly non-existing filename, perhaps by creating and\ndeleting a temporary file for just that purpose.\n\nOr NonStop correctly returns ENOEXEC on the first execve(2) call in\nrun-command.c and returns ENOACCES on the fallback with SHELL_PATH\n(default value /usr/coreutils/bin/bash), because you are not allowed to\nexecute that shell.  Then SHELL_PATH needs to be corrected.\n\nOr valid shells need to be placed in some kind of allow list to be\naccepted in #!-lines, lest NonStop returns ENOACCES on them.  Or NonStop\nis simply reporting a bogus error code for some reason.  In those cases\nyou probably need an execve) compat/ wrapper that corrects the error\ncode.\n\nOr I'm missing something here, which is a relatively safe bet.  Anyway,\ndepending on the cause of the \"Permission denied\" message, loosening the\ntextual comparison in the test might not be enough.  There may not be a\npoint for end users to distinguish between \"exec\" vs. \"run\", but the\nsilent_exec_failure feature of run-command depends on the error code\nbeing ENOENT.\n\nRené\n\n"},{"id":"477653","messageId":"013501d98cee$e58dc980$b0a95c80$@nexbridge.com","threadId":"58497","inReplyTo":"2e3be2e9-191d-fbfd-54c6-a3958e35637c@web.de","subject":"RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-05-22T20:49:20Z","receivedAt":"2023-05-22T20:49:31Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Monday, May 22, 2023 4:39 PM, René Scharfe wrote:\n>Am 14.12.22 um 06:53 schrieb rsbecker@nexbridge.com:\n>> On September 23, 2022 4:43 PM, I wrote:\n>>> On September 22, 2022 3:03 PM, I wrote:\n>>>> On September 22, 2022 3:02 PM, I wrote:\n>>>>> Rc1 is looking good except for this test.\n>>>>>\n>>>>> We get a diff as follows:\n>>>>>\n>>>>> -fatal: cannot run bad-hooks/test-hook: ...\n>>>>> +fatal: cannot exec 'bad-hooks/test-hook': Permission denied\n>>>>>\n>>>>> It looks like the pattern\n>>>>> sed -e s/test-hook: .*/test-hook: .../\n>>>>>\n>>>>> needs to be a bit extended. Adding\n>>>>>\n>>>>> sed -e s/exec/run/ | send -e s/[\"']//g\n>>>>>\n>>>>> might help clear off the other noise.\n>>>\n>>> A patch that might work is as follows:\n>>>\n>>> diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index\n>>> 43fcb7c0bf..9a723631a2\n>>> 100755\n>>> --- a/t/t1800-hook.sh\n>>> +++ b/t/t1800-hook.sh\n>>> @@ -173,7 +173,10 @@ test_expect_success 'git hook run a hook with a\n>>> bad shebang' '\n>>>                -c core.hooksPath=bad-hooks \\\n>>>                hook run test-hook >out 2>err &&\n>>>        test_must_be_empty out &&\n>>> -       sed -e \"s/test-hook: .*/test-hook: .../\" <err >actual &&\n>>> +       quot=`echo \"\\047\"` &&\n>>> +       sed -e \"s/exec/run/\" <err | \\\n>>> +               sed -e \"s/$quot//g\" | \\\n>>> +               sed -e \"s/test-hook: .*/test-hook: .../\" >actual &&\n>>>        test_cmp expect actual\n>>> '\n>>>\n>>> This does not require setting up a prerequisite for NonStop and the\n>> technique\n>>> might make the MING code easier but adding a change from spawn to run.\n>>\n>> This is still broken on NonStop. Is there any hope of a resolution?\n>\n>So trying to execute an executable file consisting only of the line\n>\"#!/bad/path/no/spaces\" causes NonStop to report \"Permission denied\"?\n>That message text belongs to error code EACCES, not to EPERM (\"Operation not\n>permitted\"), right?\n\nThat should be correct, although the OS Devs I spoke to about this said \"EPERM\". I am experimenting.\n\n>POSIX allows execve to return EACCES if the file to execute is not a regular file and\n>executing that file type is not supported or if permissions are missing to one of its\n>path components.\n\nPart of the OS Dev's response was that POSIX is actually ambiguous on this point. Linux made the decision to use ENOENT. NonStop decided to use EPERM (although it may actually be EACCESS - I will report back.\n\n>Either you have something called /bad that is not a regular file (e.g. a\n>directory) -- then it's just a matter of changing the test to use a different supposedly\n>non-existing filename, perhaps by creating and deleting a temporary file for just that\n>purpose.\n>\n>Or NonStop correctly returns ENOEXEC on the first execve(2) call in run-command.c\n>and returns ENOACCES on the fallback with SHELL_PATH (default value\n>/usr/coreutils/bin/bash), because you are not allowed to execute that shell.  Then\n>SHELL_PATH needs to be corrected.\n>\n>Or valid shells need to be placed in some kind of allow list to be accepted in #!-lines,\n>lest NonStop returns ENOACCES on them.  Or NonStop is simply reporting a bogus\n>error code for some reason.  In those cases you probably need an execve) compat/\n>wrapper that corrects the error code.\n>\n>Or I'm missing something here, which is a relatively safe bet.  Anyway, depending on\n>the cause of the \"Permission denied\" message, loosening the textual comparison in\n>the test might not be enough.  There may not be a point for end users to distinguish\n>between \"exec\" vs. \"run\", but the silent_exec_failure feature of run-command\n>depends on the error code being ENOENT.\n\nI will report my debug findings.\n--Randall\n\n"},{"id":"477654","messageId":"013601d98cf2$392153c0$ab63fb40$@nexbridge.com","threadId":"58497","inReplyTo":"013501d98cee$e58dc980$b0a95c80$@nexbridge.com","subject":"RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-05-22T21:13:09Z","receivedAt":"2023-05-22T21:13:19Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Monday, May 22, 2023 4:49 PM, I wrote:\n>On Monday, May 22, 2023 4:39 PM, René Scharfe wrote:\n>>Am 14.12.22 um 06:53 schrieb rsbecker@nexbridge.com:\n>>> On September 23, 2022 4:43 PM, I wrote:\n>>>> On September 22, 2022 3:03 PM, I wrote:\n>>>>> On September 22, 2022 3:02 PM, I wrote:\n>>>>>> Rc1 is looking good except for this test.\n>>>>>>\n>>>>>> We get a diff as follows:\n>>>>>>\n>>>>>> -fatal: cannot run bad-hooks/test-hook: ...\n>>>>>> +fatal: cannot exec 'bad-hooks/test-hook': Permission denied\n>>>>>>\n>>>>>> It looks like the pattern\n>>>>>> sed -e s/test-hook: .*/test-hook: .../\n>>>>>>\n>>>>>> needs to be a bit extended. Adding\n>>>>>>\n>>>>>> sed -e s/exec/run/ | send -e s/[\"']//g\n>>>>>>\n>>>>>> might help clear off the other noise.\n>>>>\n>>>> A patch that might work is as follows:\n>>>>\n>>>> diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index\n>>>> 43fcb7c0bf..9a723631a2\n>>>> 100755\n>>>> --- a/t/t1800-hook.sh\n>>>> +++ b/t/t1800-hook.sh\n>>>> @@ -173,7 +173,10 @@ test_expect_success 'git hook run a hook with a\n>>>> bad shebang' '\n>>>>                -c core.hooksPath=bad-hooks \\\n>>>>                hook run test-hook >out 2>err &&\n>>>>        test_must_be_empty out &&\n>>>> -       sed -e \"s/test-hook: .*/test-hook: .../\" <err >actual &&\n>>>> +       quot=`echo \"\\047\"` &&\n>>>> +       sed -e \"s/exec/run/\" <err | \\\n>>>> +               sed -e \"s/$quot//g\" | \\\n>>>> +               sed -e \"s/test-hook: .*/test-hook: .../\" >actual &&\n>>>>        test_cmp expect actual\n>>>> '\n>>>>\n>>>> This does not require setting up a prerequisite for NonStop and the\n>>> technique\n>>>> might make the MING code easier but adding a change from spawn to run.\n>>>\n>>> This is still broken on NonStop. Is there any hope of a resolution?\n>>\n>>So trying to execute an executable file consisting only of the line\n>>\"#!/bad/path/no/spaces\" causes NonStop to report \"Permission denied\"?\n>>That message text belongs to error code EACCES, not to EPERM\n>>(\"Operation not permitted\"), right?\n>\n>That should be correct, although the OS Devs I spoke to about this said \"EPERM\". I\n>am experimenting.\n>\n>>POSIX allows execve to return EACCES if the file to execute is not a\n>>regular file and executing that file type is not supported or if\n>>permissions are missing to one of its path components.\n>\n>Part of the OS Dev's response was that POSIX is actually ambiguous on this point.\n>Linux made the decision to use ENOENT. NonStop decided to use EPERM (although it\n>may actually be EACCESS - I will report back.\n>\n>>Either you have something called /bad that is not a regular file (e.g.\n>>a\n>>directory) -- then it's just a matter of changing the test to use a\n>>different supposedly non-existing filename, perhaps by creating and\n>>deleting a temporary file for just that purpose.\n>>\n>>Or NonStop correctly returns ENOEXEC on the first execve(2) call in\n>>run-command.c and returns ENOACCES on the fallback with SHELL_PATH\n>>(default value /usr/coreutils/bin/bash), because you are not allowed to\n>>execute that shell.  Then SHELL_PATH needs to be corrected.\n>>\n>>Or valid shells need to be placed in some kind of allow list to be\n>>accepted in #!-lines, lest NonStop returns ENOACCES on them.  Or\n>>NonStop is simply reporting a bogus error code for some reason.  In\n>>those cases you probably need an execve) compat/ wrapper that corrects the error\n>code.\n>>\n>>Or I'm missing something here, which is a relatively safe bet.  Anyway,\n>>depending on the cause of the \"Permission denied\" message, loosening\n>>the textual comparison in the test might not be enough.  There may not\n>>be a point for end users to distinguish between \"exec\" vs. \"run\", but\n>>the silent_exec_failure feature of run-command depends on the error code being\n>ENOENT.\n>\n>I will report my debug findings.\n\nNonStop actually does report EACCES, not EPERM. The comment at the front of run-command.c does describe the situation. The following is a potential fix:\n\ndiff --git a/run-command.c b/run-command.c\nindex 60c9419866..b76e117d35 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -846,7 +846,7 @@ int start_command(struct child_process *cmd)\n                        execve(argv.v[0], (char *const *) argv.v,\n                               (char *const *) childenv);\n\n-               if (errno == ENOENT) {\n+               if (errno == ENOENT || errno == EACCES) {\n                        if (cmd->silent_exec_failure)\n                                child_die(CHILD_ERR_SILENT);\n                        child_die(CHILD_ERR_ENOENT);\n\nThis does pass and should cover all POSIX interpretations.\n\nRegards,\nRandall\n\n"},{"id":"478027","messageId":"f5519f93-bca7-ecbc-9da3-b25ce8aef618@web.de","threadId":"58497","inReplyTo":"013601d98cf2$392153c0$ab63fb40$@nexbridge.com","subject":"Re: [BUG] Git 2.38.0-rc1 t1800 message text comparison","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-06-04T12:13:06Z","receivedAt":"2023-06-04T12:13:21Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 22.05.23 um 23:13 schrieb rsbecker@nexbridge.com:\n> On Monday, May 22, 2023 4:49 PM, I wrote:\n>> On Monday, May 22, 2023 4:39 PM, René Scharfe wrote:\n>>> So trying to execute an executable file consisting only of the line\n>>> \"#!/bad/path/no/spaces\" causes NonStop to report \"Permission denied\"?\n>>> That message text belongs to error code EACCES, not to EPERM\n>>> (\"Operation not permitted\"), right?\n>>\n>> That should be correct, although the OS Devs I spoke to about this said \"EPERM\". I\n>> am experimenting.\n>>\n>>> POSIX allows execve to return EACCES if the file to execute is not a\n>>> regular file and executing that file type is not supported or if\n>>> permissions are missing to one of its path components.\n>>\n>> Part of the OS Dev's response was that POSIX is actually ambiguous on this point.\n>> Linux made the decision to use ENOENT. NonStop decided to use EPERM (although it\n>> may actually be EACCESS - I will report back.\n\nIn https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html\nI don't see how EACCES would be a valid response in this case.  And it\nmakes no intuitive sense -- which permission is missing in this\nsituation?\n\nThat doesn't change the fact that this issue needs to be dealt with in\nGit somehow.\n\n> NonStop actually does report EACCES, not EPERM. The comment at the\n> front of run-command.c does describe the situation.\nDo you mean the one about EACCES being reported if a directory in $PATH\nis inaccessible in sane_execvp()?  That does not apply here because\n\"#!/bad/path/no/spaces\" specifies an absolute path, so $PATH is not\nsearched.\n\n> The following is a potential fix:\n>\n> diff --git a/run-command.c b/run-command.c\n> index 60c9419866..b76e117d35 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -846,7 +846,7 @@ int start_command(struct child_process *cmd)\n>                         execve(argv.v[0], (char *const *) argv.v,\n>                                (char *const *) childenv);\n>\n> -               if (errno == ENOENT) {\n> +               if (errno == ENOENT || errno == EACCES) {\n>                         if (cmd->silent_exec_failure)\n>                                 child_die(CHILD_ERR_SILENT);\n>                         child_die(CHILD_ERR_ENOENT);\n>\n> This does pass and should cover all POSIX interpretations.\n\nThat would misreport execution failures due to missing permissions\n(think e.g. \"chmod 0 file\") as \"No such file or directory\".  So this\nshould be only done if really needed, perhaps guarded by an access(2)\ncheck for verifying that EACCES is bogus, and only on affected\nplatforms.\n\nRené\n\n\n\n"},{"id":"478035","messageId":"28d808e4-77b4-d91e-8a86-c21287019b56@web.de","threadId":"58497","inReplyTo":"f5519f93-bca7-ecbc-9da3-b25ce8aef618@web.de","subject":"Re: [BUG] Git 2.38.0-rc1 t1800 message text comparison","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-06-04T20:55:54Z","receivedAt":"2023-06-04T20:56:22Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 04.06.23 um 14:13 schrieb René Scharfe:\n> Am 22.05.23 um 23:13 schrieb rsbecker@nexbridge.com:\n>> The following is a potential fix:\n>>\n>> diff --git a/run-command.c b/run-command.c\n>> index 60c9419866..b76e117d35 100644\n>> --- a/run-command.c\n>> +++ b/run-command.c\n>> @@ -846,7 +846,7 @@ int start_command(struct child_process *cmd)\n>>                         execve(argv.v[0], (char *const *) argv.v,\n>>                                (char *const *) childenv);\n>>\n>> -               if (errno == ENOENT) {\n>> +               if (errno == ENOENT || errno == EACCES) {\n>>                         if (cmd->silent_exec_failure)\n>>                                 child_die(CHILD_ERR_SILENT);\n>>                         child_die(CHILD_ERR_ENOENT);\n>>\n>> This does pass and should cover all POSIX interpretations.\n>\n> That would misreport execution failures due to missing permissions\n> (think e.g. \"chmod 0 file\") as \"No such file or directory\".  So this\n> should be only done if really needed, perhaps guarded by an access(2)\n> check for verifying that EACCES is bogus, and only on affected\n> platforms.\n\nActually the difference is a bit more subtle.  With that patch and\nsilent_exec_failure on, missing permissions wouldn't be reported\nanymore, and with it off the eventual message command would change\nfrom:\n\n                error_errno(\"cannot exec '%s'\", cmd->args.v[0]);\n\nto:\n\n                error_errno(\"cannot run %s\", cmd->args.v[0]);\n\nSo the actual errno-based message would be the same, unless its\nsuppressed.\n\nWhat's the significance of \"run\" and \"exec\" here?  Why do we have both\nvariants?  Is it to tell apart errors of the actual execve(2) call from\nthose in code leading up to it (e.g. when searching the executable in\n$PATH)?  But at this point we are after the call, so why is ENOENT\nfrom execve(2) a \"run\" thing, not an \"exec\" thing?\n\nRené\n"},{"id":"478055","messageId":"xmqqwn0h4ek3.fsf@gitster.g","threadId":"58497","inReplyTo":"28d808e4-77b4-d91e-8a86-c21287019b56@web.de","subject":"Re: [BUG] Git 2.38.0-rc1 t1800 message text comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-06T00:31:08Z","receivedAt":"2023-06-06T00:31:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> What's the significance of \"run\" and \"exec\" here?  Why do we have both\n> variants?  Is it to tell apart errors of the actual execve(2) call from\n> those in code leading up to it (e.g. when searching the executable in\n> $PATH)?  But at this point we are after the call, so why is ENOENT\n> from execve(2) a \"run\" thing, not an \"exec\" thing?\n\nI was reading output from\n\n    $ git log --reverse -p -SNOENT 7e5d776854e.. run-command.c\n\n45c0961c (run_command(): handle missing command errors more\ngracefully, 2009-01-28) explains how we use NOENT to tell between\n\"we failed to run the command requested because it did not even\nexist\" and \"we failed to run the command and the reason why it\nfailed is *not* because it did not exist\" (the distinction matters\nto implement \"run command X on $PATH\").  To further that, 38f865c2\n(run-command: treat inaccessible directories as ENOENT, 2012-03-30)\nand a7855083 (sane_execvp(): ignore non-directory on $PATH,\n2012-07-31) dealt corner cases where entries on $PATH were\nunreadable or were non-directories, but the idea should be the same.\n\nAll of the above matters purely when we fail silently (because we\nexpect only some directories on $PATH contain the command), and I\nsuspect that the phrasing differences when we _do_ tell the human\nuser what we failed to run does not matter all that much.\n\n"},{"id":"478189","messageId":"14e1be60-3765-0ba5-00f5-2848bb57bf53@web.de","threadId":"58497","inReplyTo":"xmqqwn0h4ek3.fsf@gitster.g","subject":"[PATCH 1/2] t1800: loosen matching of error message for bad shebang","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-06-10T14:51:12Z","receivedAt":"2023-06-10T14:51:34Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"t1800.16 checks whether an attempt to run a hook script with a missing\nexecutable in its #! line fails and reports that error.  The expected\nerror message differs between platforms.  The test handles two common\nvariants, but on NonStop OS we get a third one: \"fatal: cannot exec\n'bad-hooks/test-hook': ...\", which causes the test to fail there.\n\nWe don't really care about the specific message text all that much here.\nUse grep and a single regex with alternations to ascertain that we get\nan error message (fatal or otherwise) about the failed invocation of the\nhook, but don't bother checking if we get the right variant for the\nplatform the test is running on or whether quoting is done.  This looser\ncheck let's the test pass on NonStop OS.\n\nReported-by: Randall S. Becker <randall.becker@nexbridge.ca>\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n t/t1800-hook.sh | 20 +++++---------------\n 1 file changed, 5 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t1800-hook.sh b/t/t1800-hook.sh\nindex 3506f627b6..c156d6decc 100755\n--- a/t/t1800-hook.sh\n+++ b/t/t1800-hook.sh\n@@ -156,25 +156,15 @@ test_expect_success 'git hook run a hook with a bad shebang' '\n \tmkdir bad-hooks &&\n \twrite_script bad-hooks/test-hook \"/bad/path/no/spaces\" </dev/null &&\n\n-\t# TODO: We should emit the same (or at least a more similar)\n-\t# error on MINGW (essentially Git for Windows) and all other\n-\t# platforms.. See the OS-specific code in start_command()\n-\tif test_have_prereq !MINGW\n-\tthen\n-\t\tcat >expect <<-\\EOF\n-\t\tfatal: cannot run bad-hooks/test-hook: ...\n-\t\tEOF\n-\telse\n-\t\tcat >expect <<-\\EOF\n-\t\terror: cannot spawn bad-hooks/test-hook: ...\n-\t\tEOF\n-\tfi &&\n \ttest_expect_code 1 git \\\n \t\t-c core.hooksPath=bad-hooks \\\n \t\thook run test-hook >out 2>err &&\n \ttest_must_be_empty out &&\n-\tsed -e \"s/test-hook: .*/test-hook: .../\" <err >actual &&\n-\ttest_cmp expect actual\n+\n+\t# TODO: We should emit the same (or at least a more similar)\n+\t# error on MINGW (essentially Git for Windows) and all other\n+\t# platforms.. See the OS-specific code in start_command()\n+\tgrep -E \"^(error|fatal): cannot (exec|run|spawn) .*bad-hooks/test-hook\" err\n '\n\n test_expect_success 'stdin to hooks' '\n--\n2.41.0\n"},{"id":"478190","messageId":"753d6b82-580d-bcff-c0c2-e7ad53f13b21@web.de","threadId":"58497","inReplyTo":"xmqqwn0h4ek3.fsf@gitster.g","subject":"[PATCH 2/2] run-command: report exec error even on ENOENT","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-06-10T14:51:21Z","receivedAt":"2023-06-10T14:52:00Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"If execve(2) fails with ENOENT and we report the error, we use the\nformat \"cannot run %s\", followed by the actual error message.  For other\nerrors we use \"cannot exec '%s'\".\n\nStop making this subtle distinction and use the second format for all\nexecve(2) errors.  This simplifies the code and makes the prefix more\nprecise by indicating the failed operation.  It also allows us to\nslightly simplify t1800.16.\n\nOn Windows -- which lacks execve(2) -- we already use a single format in\nall cases: \"cannot spawn %s\".\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\nPatch formatted with -U6 for easier review.\n\n run-command.c   | 14 +++-----------\n t/t1800-hook.sh |  2 +-\n 2 files changed, 4 insertions(+), 12 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 60c9419866..758f8534da 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -304,13 +304,12 @@ static int child_notifier = -1;\n\n enum child_errcode {\n \tCHILD_ERR_CHDIR,\n \tCHILD_ERR_DUP2,\n \tCHILD_ERR_CLOSE,\n \tCHILD_ERR_SIGPROCMASK,\n-\tCHILD_ERR_ENOENT,\n \tCHILD_ERR_SILENT,\n \tCHILD_ERR_ERRNO\n };\n\n struct child_err {\n \tenum child_errcode err;\n@@ -387,15 +386,12 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr)\n \tcase CHILD_ERR_CLOSE:\n \t\terror_errno(\"close() in child failed\");\n \t\tbreak;\n \tcase CHILD_ERR_SIGPROCMASK:\n \t\terror_errno(\"sigprocmask failed restoring signals\");\n \t\tbreak;\n-\tcase CHILD_ERR_ENOENT:\n-\t\terror_errno(\"cannot run %s\", cmd->args.v[0]);\n-\t\tbreak;\n \tcase CHILD_ERR_SILENT:\n \t\tbreak;\n \tcase CHILD_ERR_ERRNO:\n \t\terror_errno(\"cannot exec '%s'\", cmd->args.v[0]);\n \t\tbreak;\n \t}\n@@ -843,19 +839,15 @@ int start_command(struct child_process *cmd)\n \t\texecve(argv.v[1], (char *const *) argv.v + 1,\n \t\t       (char *const *) childenv);\n \t\tif (errno == ENOEXEC)\n \t\t\texecve(argv.v[0], (char *const *) argv.v,\n \t\t\t       (char *const *) childenv);\n\n-\t\tif (errno == ENOENT) {\n-\t\t\tif (cmd->silent_exec_failure)\n-\t\t\t\tchild_die(CHILD_ERR_SILENT);\n-\t\t\tchild_die(CHILD_ERR_ENOENT);\n-\t\t} else {\n-\t\t\tchild_die(CHILD_ERR_ERRNO);\n-\t\t}\n+\t\tif (cmd->silent_exec_failure && errno == ENOENT)\n+\t\t\tchild_die(CHILD_ERR_SILENT);\n+\t\tchild_die(CHILD_ERR_ERRNO);\n \t}\n \tatfork_parent(&as);\n \tif (cmd->pid < 0)\n \t\terror_errno(\"cannot fork() for %s\", cmd->args.v[0]);\n \telse if (cmd->clean_on_exit)\n \t\tmark_child_for_cleanup(cmd->pid, cmd);\ndiff --git a/t/t1800-hook.sh b/t/t1800-hook.sh\nindex c156d6decc..8b0234cf2d 100755\n--- a/t/t1800-hook.sh\n+++ b/t/t1800-hook.sh\n@@ -161,13 +161,13 @@ test_expect_success 'git hook run a hook with a bad shebang' '\n \t\thook run test-hook >out 2>err &&\n \ttest_must_be_empty out &&\n\n \t# TODO: We should emit the same (or at least a more similar)\n \t# error on MINGW (essentially Git for Windows) and all other\n \t# platforms.. See the OS-specific code in start_command()\n-\tgrep -E \"^(error|fatal): cannot (exec|run|spawn) .*bad-hooks/test-hook\" err\n+\tgrep -E \"^(error|fatal): cannot (exec|spawn) .*bad-hooks/test-hook\" err\n '\n\n test_expect_success 'stdin to hooks' '\n \twrite_script .git/hooks/test-hook <<-\\EOF &&\n \techo BEGIN stdin\n \tcat\n--\n2.41.0\n\n"},{"id":"478290","messageId":"xmqqv8fsa7an.fsf@gitster.g","threadId":"58497","inReplyTo":"14e1be60-3765-0ba5-00f5-2848bb57bf53@web.de","subject":"Re: [PATCH 1/2] t1800: loosen matching of error message for bad shebang","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-12T18:01:52Z","receivedAt":"2023-06-12T18:01:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> +\t# TODO: We should emit the same (or at least a more similar)\n> +\t# error on MINGW (essentially Git for Windows) and all other\n> +\t# platforms.. See the OS-specific code in start_command()\n> +\tgrep -E \"^(error|fatal): cannot (exec|run|spawn) .*bad-hooks/test-hook\" err\n\nLooking great.  Thanks.\n"}]}