RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison

[<rsbecker@nexbridge.com>]

On Monday, May 22, 2023 4:49 PM, I wrote:
>On Monday, May 22, 2023 4:39 PM, René Scharfe wrote:
>>Am 14.12.22 um 06:53 schrieb rsbecker@nexbridge.com:
>>> On September 23, 2022 4:43 PM, I wrote:
>>>> On September 22, 2022 3:03 PM, I wrote:
>>>>> On September 22, 2022 3:02 PM, I wrote:
>>>>>> Rc1 is looking good except for this test.
>>>>>>
>>>>>> We get a diff as follows:
>>>>>>
>>>>>> -fatal: cannot run bad-hooks/test-hook: ...
>>>>>> +fatal: cannot exec 'bad-hooks/test-hook': Permission denied
>>>>>>
>>>>>> It looks like the pattern
>>>>>> sed -e s/test-hook: .*/test-hook: .../
>>>>>>
>>>>>> needs to be a bit extended. Adding
>>>>>>
>>>>>> sed -e s/exec/run/ | send -e s/["']//g
>>>>>>
>>>>>> might help clear off the other noise.
>>>>
>>>> A patch that might work is as follows:
>>>>
>>>> diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index
>>>> 43fcb7c0bf..9a723631a2
>>>> 100755
>>>> --- a/t/t1800-hook.sh
>>>> +++ b/t/t1800-hook.sh
>>>> @@ -173,7 +173,10 @@ test_expect_success 'git hook run a hook with a
>>>> bad shebang' '
>>>>                -c core.hooksPath=bad-hooks \
>>>>                hook run test-hook >out 2>err &&
>>>>        test_must_be_empty out &&
>>>> -       sed -e "s/test-hook: .*/test-hook: .../" <err >actual &&
>>>> +       quot=`echo "\047"` &&
>>>> +       sed -e "s/exec/run/" <err | \
>>>> +               sed -e "s/$quot//g" | \
>>>> +               sed -e "s/test-hook: .*/test-hook: .../" >actual &&
>>>>        test_cmp expect actual
>>>> '
>>>>
>>>> This does not require setting up a prerequisite for NonStop and the
>>> technique
>>>> might make the MING code easier but adding a change from spawn to run.
>>>
>>> This is still broken on NonStop. Is there any hope of a resolution?
>>
>>So trying to execute an executable file consisting only of the line
>>"#!/bad/path/no/spaces" causes NonStop to report "Permission denied"?
>>That message text belongs to error code EACCES, not to EPERM
>>("Operation not permitted"), right?
>
>That should be correct, although the OS Devs I spoke to about this said "EPERM". I
>am experimenting.
>
>>POSIX allows execve to return EACCES if the file to execute is not a
>>regular file and executing that file type is not supported or if
>>permissions are missing to one of its path components.
>
>Part 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.
>
>>Either you have something called /bad that is not a regular file (e.g.
>>a
>>directory) -- then it's just a matter of changing the test to use a
>>different supposedly non-existing filename, perhaps by creating and
>>deleting a temporary file for just that purpose.
>>
>>Or NonStop correctly returns ENOEXEC on the first execve(2) call in
>>run-command.c and returns ENOACCES on the fallback with SHELL_PATH
>>(default value /usr/coreutils/bin/bash), because you are not allowed to
>>execute that shell.  Then SHELL_PATH needs to be corrected.
>>
>>Or valid shells need to be placed in some kind of allow list to be
>>accepted in #!-lines, lest NonStop returns ENOACCES on them.  Or
>>NonStop is simply reporting a bogus error code for some reason.  In
>>those cases you probably need an execve) compat/ wrapper that corrects the error
>code.
>>
>>Or I'm missing something here, which is a relatively safe bet.  Anyway,
>>depending on the cause of the "Permission denied" message, loosening
>>the textual comparison in the test might not be enough.  There may not
>>be a point for end users to distinguish between "exec" vs. "run", but
>>the silent_exec_failure feature of run-command depends on the error code being
>ENOENT.
>
>I will report my debug findings.

NonStop 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:

diff --git a/run-command.c b/run-command.c
index 60c9419866..b76e117d35 100644
--- a/run-command.c
+++ b/run-command.c
@@ -846,7 +846,7 @@ int start_command(struct child_process *cmd)
                        execve(argv.v[0], (char *const *) argv.v,
                               (char *const *) childenv);

-               if (errno == ENOENT) {
+               if (errno == ENOENT || errno == EACCES) {
                        if (cmd->silent_exec_failure)
                                child_die(CHILD_ERR_SILENT);
                        child_die(CHILD_ERR_ENOENT);

This does pass and should cover all POSIX interpretations.

Regards,
Randall