Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: <rsbecker@nexbridge.com>
To: rsbecker@nexbridge.com, "'René Scharfe'" <l.s.r@web.de>,
	git@vger.kernel.org
Subject: RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison
Date: Mon, 22 May 2023 17:13:09 -0400	[thread overview]
Message-ID: <013601d98cf2$392153c0$ab63fb40$@nexbridge.com> (raw)
In-Reply-To: <013501d98cee$e58dc980$b0a95c80$@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


  reply	other threads:[~2023-05-22 21:13 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-22 19:01 [BUG] Git 2.28.0-rc1 t1800 message text comparison rsbecker
2022-09-22 19:02 ` [BUG] Git 2.38.0-rc1 " rsbecker
2022-09-23 20:43 ` rsbecker
2022-12-14  5:53   ` rsbecker
2023-05-22 20:39     ` René Scharfe
2023-05-22 20:49       ` rsbecker
2023-05-22 21:13         ` rsbecker [this message]
2023-06-04 12:13           ` René Scharfe
2023-06-04 20:55             ` René Scharfe
2023-06-06  0:31               ` Junio C Hamano
2023-06-10 14:51                 ` [PATCH 1/2] t1800: loosen matching of error message for bad shebang René Scharfe
2023-06-12 18:01                   ` Junio C Hamano
2023-06-10 14:51                 ` [PATCH 2/2] run-command: report exec error even on ENOENT René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='013601d98cf2$392153c0$ab63fb40$@nexbridge.com' \
    --to=rsbecker@nexbridge.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).