[patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Sandra Loosemore
This patch fixes a group of failures in gdb.base/shell.exp on Windows
host due to assumptions that GDB is launching a POSIX-like shell.  On
Windows, we get CMD.EXE instead.

There is a potential second group of failures on Windows host not
addressed by this patch, due to the dependence of some of these tests on
utilities like wc, sed, and grep.  These tests happen to work for me
because the test harness connects to the remote host using Cygwin ssh
and both GDB and CMD.EXE inherit the PATH setting from the parent Cygwin
shell.  I don't know if disabling those tests on Windows host too is
appropriate, or again if this is an instance of badly-designed tests
that could be rewritten to avoid those dependencies.  WDYT?

-Sandra

windows-shell.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Sourceware - gdb-patches mailing list
On Wed, Jun 24, 2020 at 8:35 PM Sandra Loosemore
<[hidden email]> wrote:
>
> This patch fixes a group of failures in gdb.base/shell.exp on Windows
> host due to assumptions that GDB is launching a POSIX-like shell.  On
> Windows, we get CMD.EXE instead.

Hmm, if I call runtest/make check from an msys2 shell, do I really get
cmd.exe here? I ask because i wonder how many people run this from
cmd.exe as opposed to msys's shell, so it may be better to check for
cmd more specifically somehow (maybe $SHELL?)


Christian
Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Sandra Loosemore
On 6/25/20 11:32 AM, Christian Biesinger wrote:

> On Wed, Jun 24, 2020 at 8:35 PM Sandra Loosemore
> <[hidden email]> wrote:
>>
>> This patch fixes a group of failures in gdb.base/shell.exp on Windows
>> host due to assumptions that GDB is launching a POSIX-like shell.  On
>> Windows, we get CMD.EXE instead.
>
> Hmm, if I call runtest/make check from an msys2 shell, do I really get
> cmd.exe here? I ask because i wonder how many people run this from
> cmd.exe as opposed to msys's shell, so it may be better to check for
> cmd more specifically somehow (maybe $SHELL?)

I'm not familiar with the msys2 shell, but....

On Windows host GDB uses the system() function provided by the host C
library to launch a shell and does not consult $SHELL at all.  This is
in shell_escape in gdb/cli/cli-cmds.c.

Here is the documentation I found via Google for system() in the Windows
library:

https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/system-wsystem?view=vs-2019

"system uses the COMSPEC and PATH environment variables to locate the
command-interpreter file CMD.exe."

I think the GDB manual doesn't accurately describe what GDB does in this
situation, BTW.

-Sandra
Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Eli Zaretskii
> From: Sandra Loosemore <[hidden email]>
> Date: Thu, 25 Jun 2020 11:57:08 -0600
> Cc: "[hidden email]" <[hidden email]>
>
> Here is the documentation I found via Google for system() in the Windows
> library:
>
> https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/system-wsystem?view=vs-2019
>
> "system uses the COMSPEC and PATH environment variables to locate the
> command-interpreter file CMD.exe."
>
> I think the GDB manual doesn't accurately describe what GDB does in this
> situation, BTW.

Which part of the manual did you allude to here, and what is in your
opinion inaccurate there?

Thanks.
Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Sandra Loosemore
On 6/25/20 12:07 PM, Eli Zaretskii wrote:

>> From: Sandra Loosemore <[hidden email]>
>> Date: Thu, 25 Jun 2020 11:57:08 -0600
>> Cc: "[hidden email]" <[hidden email]>
>>
>> Here is the documentation I found via Google for system() in the Windows
>> library:
>>
>> https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/system-wsystem?view=vs-2019
>>
>> "system uses the COMSPEC and PATH environment variables to locate the
>> command-interpreter file CMD.exe."
>>
>> I think the GDB manual doesn't accurately describe what GDB does in this
>> situation, BTW.
>
> Which part of the manual did you allude to here, and what is in your
> opinion inaccurate there?

https://sourceware.org/gdb/current/onlinedocs/gdb/Shell-Commands.html#Shell-Commands

"If it exists, the environment variable SHELL determines which shell to
run. Otherwise GDB uses the default shell (/bin/sh on Unix systems,
COMMAND.COM on MS-DOS, etc.). "

The SHELL environment variable is not consulted on Windows hosts.  It
just uses whatever shell the system() call uses -- look at the
implementation of shell_escape in gdb/cli/cli-cmds.c.

The references to MS-DOS and COMMAND.COM are certainly bit-rotten.
COMMAND.COM was replaced by CMD.EXE in Windows NT, 25+ years ago now.
And do you really expect to be able to build a modern GDB to run on a
16-bit MS-DOS host?  :-P

-Sandra
Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Sourceware - gdb-patches mailing list
In reply to this post by Sandra Loosemore
On Thu, Jun 25, 2020 at 12:57 PM Sandra Loosemore
<[hidden email]> wrote:

>
> On 6/25/20 11:32 AM, Christian Biesinger wrote:
> > On Wed, Jun 24, 2020 at 8:35 PM Sandra Loosemore
> > <[hidden email]> wrote:
> >>
> >> This patch fixes a group of failures in gdb.base/shell.exp on Windows
> >> host due to assumptions that GDB is launching a POSIX-like shell.  On
> >> Windows, we get CMD.EXE instead.
> >
> > Hmm, if I call runtest/make check from an msys2 shell, do I really get
> > cmd.exe here? I ask because i wonder how many people run this from
> > cmd.exe as opposed to msys's shell, so it may be better to check for
> > cmd more specifically somehow (maybe $SHELL?)
>
> I'm not familiar with the msys2 shell, but....
>
> On Windows host GDB uses the system() function provided by the host C
> library to launch a shell and does not consult $SHELL at all.  This is
> in shell_escape in gdb/cli/cli-cmds.c.

Ah sorry, I didn't realize that this shell use came from GDB, I had
assumed this was from dejagnu. This (unofficially) looks good then.

Christian
Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Eli Zaretskii
In reply to this post by Sandra Loosemore
> CC: <[hidden email]>, <[hidden email]>
> From: Sandra Loosemore <[hidden email]>
> Date: Thu, 25 Jun 2020 13:35:45 -0600
>
> >> I think the GDB manual doesn't accurately describe what GDB does in this
> >> situation, BTW.
> >
> > Which part of the manual did you allude to here, and what is in your
> > opinion inaccurate there?
>
> https://sourceware.org/gdb/current/onlinedocs/gdb/Shell-Commands.html#Shell-Commands
>
> "If it exists, the environment variable SHELL determines which shell to
> run. Otherwise GDB uses the default shell (/bin/sh on Unix systems,
> COMMAND.COM on MS-DOS, etc.). "
>
> The SHELL environment variable is not consulted on Windows hosts.  It
> just uses whatever shell the system() call uses -- look at the
> implementation of shell_escape in gdb/cli/cli-cmds.c.

It isn't factually wrong: $SHELL isn't supposed to be set on non-Posix
hosts.  However, I made the text more accurate with the patch below,
which I just installed.

> The references to MS-DOS and COMMAND.COM are certainly bit-rotten.
> COMMAND.COM was replaced by CMD.EXE in Windows NT, 25+ years ago now.
> And do you really expect to be able to build a modern GDB to run on a
> 16-bit MS-DOS host?  :-P

You'd be surprised:

  ftp://ftp.delorie.com/pub/djgpp/current/v2gnu/gdb801b.zip
  https://groups.google.com/forum/#!msg/comp.os.msdos.djgpp/MIm6bIlJ8b8/yNuLXDoxBQAJ

The DJGPP project has MS-DOS ports of all the main GNU utilities, most
of them of fairly recent versions (the above is GDB 8.0.1).

As for the 16-bit nature of MS-DOS and the ability to run a modern GDB
on it, you remind me of a short dialogue between RMS and DJ Delorie,
which happened more than 30 years ago and started the DJGPP project;
see the first paragraphs on this page:

  http://www.delorie.com/djgpp/history.html

Here's the patch I installed on the master branch:

diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 4b1a4c0..c591934 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2020-06-26  Eli Zaretskii  <[hidden email]>
+
+ * gdb.texinfo (Shell Commands): More accurate description of use
+ of $SHELL.  Reported by Sandra Loosemore <[hidden email]>.
+
 2020-06-23  Andrew Burgess  <[hidden email]>
 
  * gdb.texinfo (Maintenance Commands): Document new 'maint print
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7f8c77a..fbe9f85 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1451,9 +1451,10 @@
 @itemx !@var{command-string}
 Invoke a standard shell to execute @var{command-string}.
 Note that no space is needed between @code{!} and @var{command-string}.
-If it exists, the environment variable @code{SHELL} determines which
-shell to run.  Otherwise @value{GDBN} uses the default shell
-(@file{/bin/sh} on Unix systems, @file{COMMAND.COM} on MS-DOS, etc.).
+On GNU and Unix systems, the environment variable @code{SHELL}, if it
+exists, determines which shell to run.  Otherwise @value{GDBN} uses
+the default shell (@file{/bin/sh} on GNU and Unix systems,
+@file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
 @end table
 
 The utility @code{make} is often needed in development environments.
Reply | Threaded
Open this post in threaded view
|

Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Sandra Loosemore
On 6/26/20 1:08 AM, Eli Zaretskii wrote:

> [snip]
>
> Here's the patch I installed on the master branch:
>
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 4b1a4c0..c591934 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-06-26  Eli Zaretskii  <[hidden email]>
> +
> + * gdb.texinfo (Shell Commands): More accurate description of use
> + of $SHELL.  Reported by Sandra Loosemore <[hidden email]>.
> +
>   2020-06-23  Andrew Burgess  <[hidden email]>
>  
>   * gdb.texinfo (Maintenance Commands): Document new 'maint print
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 7f8c77a..fbe9f85 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -1451,9 +1451,10 @@
>   @itemx !@var{command-string}
>   Invoke a standard shell to execute @var{command-string}.
>   Note that no space is needed between @code{!} and @var{command-string}.
> -If it exists, the environment variable @code{SHELL} determines which
> -shell to run.  Otherwise @value{GDBN} uses the default shell
> -(@file{/bin/sh} on Unix systems, @file{COMMAND.COM} on MS-DOS, etc.).
> +On GNU and Unix systems, the environment variable @code{SHELL}, if it
> +exists, determines which shell to run.  Otherwise @value{GDBN} uses
> +the default shell (@file{/bin/sh} on GNU and Unix systems,
> +@file{cmd.exe} on MS-Windows, @file{COMMAND.COM} on MS-DOS, etc.).
>   @end table
>  
>   The utility @code{make} is often needed in development environments.
>

This looks fine to me -- thanks.

-Sandra
Reply | Threaded
Open this post in threaded view
|

[ping] Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Sandra Loosemore
In reply to this post by Sandra Loosemore
Ping?  I'd like to get this patch into the GDB 10 branch.

https://sourceware.org/pipermail/gdb-patches/2020-June/169865.html

On 6/24/20 7:35 PM, Sandra Loosemore wrote:

> This patch fixes a group of failures in gdb.base/shell.exp on Windows
> host due to assumptions that GDB is launching a POSIX-like shell.  On
> Windows, we get CMD.EXE instead.
>
> There is a potential second group of failures on Windows host not
> addressed by this patch, due to the dependence of some of these tests on
> utilities like wc, sed, and grep.  These tests happen to work for me
> because the test harness connects to the remote host using Cygwin ssh
> and both GDB and CMD.EXE inherit the PATH setting from the parent Cygwin
> shell.  I don't know if disabling those tests on Windows host too is
> appropriate, or again if this is an instance of badly-designed tests
> that could be rewritten to avoid those dependencies.  WDYT?
>
> -Sandra

Reply | Threaded
Open this post in threaded view
|

Re: [ping] Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Simon Marchi-4
On 2020-07-15 3:53 p.m., Sandra Loosemore wrote:

> Ping?  I'd like to get this patch into the GDB 10 branch.
>
> https://sourceware.org/pipermail/gdb-patches/2020-June/169865.html
>
> On 6/24/20 7:35 PM, Sandra Loosemore wrote:
>> This patch fixes a group of failures in gdb.base/shell.exp on Windows
>> host due to assumptions that GDB is launching a POSIX-like shell.  On
>> Windows, we get CMD.EXE instead.
>>
>> There is a potential second group of failures on Windows host not
>> addressed by this patch, due to the dependence of some of these tests on
>> utilities like wc, sed, and grep.  These tests happen to work for me
>> because the test harness connects to the remote host using Cygwin ssh
>> and both GDB and CMD.EXE inherit the PATH setting from the parent Cygwin
>> shell.  I don't know if disabling those tests on Windows host too is
>> appropriate, or again if this is an instance of badly-designed tests
>> that could be rewritten to avoid those dependencies.  WDYT?
>>
>> -Sandra
>

The patch looks raisonnable to me... but Eli, as the de facto Windows specialist,
could you give your OK?

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [ping] Re: [patch] gdb/testsuite: Fix POSIX-isms in gdb.base/shell.exp

Eli Zaretskii
> From: Simon Marchi <[hidden email]>
> Date: Wed, 15 Jul 2020 22:41:59 -0400
>
> The patch looks raisonnable to me... but Eli, as the de facto Windows specialist,
> could you give your OK?

Yes, OK.