* [BUG] Git 2.28.0-rc1 t1800 message text comparison
@ 2022-09-22 19:01 rsbecker
2022-09-22 19:02 ` [BUG] Git 2.38.0-rc1 " rsbecker
2022-09-23 20:43 ` rsbecker
0 siblings, 2 replies; 7+ messages in thread
From: rsbecker @ 2022-09-22 19:01 UTC (permalink / raw)
To: git
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.
Regards,
Randall
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison
2022-09-22 19:01 [BUG] Git 2.28.0-rc1 t1800 message text comparison rsbecker
@ 2022-09-22 19:02 ` rsbecker
2022-09-23 20:43 ` rsbecker
1 sibling, 0 replies; 7+ messages in thread
From: rsbecker @ 2022-09-22 19:02 UTC (permalink / raw)
To: rsbecker, git
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.
The subject should have been 2.38.0, not 2.28.0.
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison
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
1 sibling, 1 reply; 7+ messages in thread
From: rsbecker @ 2022-09-23 20:43 UTC (permalink / raw)
To: git
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.
-Randall
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison
2022-09-23 20:43 ` rsbecker
@ 2022-12-14 5:53 ` rsbecker
2023-05-22 20:39 ` René Scharfe
0 siblings, 1 reply; 7+ messages in thread
From: rsbecker @ 2022-12-14 5:53 UTC (permalink / raw)
To: rsbecker, git
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?
Thanks,
Randall
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] Git 2.38.0-rc1 t1800 message text comparison
2022-12-14 5:53 ` rsbecker
@ 2023-05-22 20:39 ` René Scharfe
2023-05-22 20:49 ` rsbecker
0 siblings, 1 reply; 7+ messages in thread
From: René Scharfe @ 2023-05-22 20:39 UTC (permalink / raw)
To: rsbecker, git
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?
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.
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.
René
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison
2023-05-22 20:39 ` René Scharfe
@ 2023-05-22 20:49 ` rsbecker
2023-05-22 21:13 ` rsbecker
0 siblings, 1 reply; 7+ messages in thread
From: rsbecker @ 2023-05-22 20:49 UTC (permalink / raw)
To: 'René Scharfe', git
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.
--Randall
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [BUG] Git 2.38.0-rc1 t1800 message text comparison
2023-05-22 20:49 ` rsbecker
@ 2023-05-22 21:13 ` rsbecker
0 siblings, 0 replies; 7+ messages in thread
From: rsbecker @ 2023-05-22 21:13 UTC (permalink / raw)
To: rsbecker, 'René Scharfe', git
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-05-22 21:13 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
Code repositories for project(s) associated with this public inbox
https://80x24.org/pub/scm/git/git.git/
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).