public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] testsuite: use cygpath to convert from Unix to Windows paths
@ 2020-03-11 18:32 Simon Marchi
  2020-03-11 18:40 ` Christian Biesinger
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2020-03-11 18:32 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

When on a MinGW host, standard_output_file uses a regular expression to
convert Unix-style paths of the form "/c/foo" to "c:/foo".  This is
needed because the paths we pass to GDB (for example, with the "file"
command) need to be in the Windows form.

However, the regexp only works if your binutils-gdb repo is under a
`/[a-z]/...` path (the Unix paths mapping to Windows drives).
Presumably, that works if you clone the repo in Windows, then access it
through `/c/...`.

In my case, I've cloned the repository directly inside my MinGW shell,
so in /home/smarchi.  The regexp therefore doesn't work for me.  The
path doesn't get transformed, and the file command fails when running
any test:

    (gdb) file /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent
    /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent: No such file or directory.

A safer way to do this conversion would be to use the cygpath utility.
It can be used to convert any MinGW path into its Windows equivalent.

Note that despite originally coming from Cygwin, the cygpath utility is
distributed by mingw-w64 and can be used in that environment.

With this, the file command in the test succeeds:

    (gdb) file C:/msys64/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent
    Reading symbols from C:/msys64/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent...

gdb/testsuite/ChangeLog:

	* lib/gdb.exp (standard_output_file): Use cygpath to convert
	from Unix to Windows path.
---
 gdb/testsuite/lib/gdb.exp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9614e8dc87cd..ac16e0e85c99 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4899,8 +4899,10 @@ proc standard_output_file {basename} {
     file mkdir $dir
     # If running on MinGW, replace /c/foo with c:/foo
     if { [ishost *-*-mingw*] } {
-        set dir [regsub {^/([a-z])/} $dir {\1:/}]
+       set dir [exec cygpath --mixed "${dir}"]
+
     }
+
     return [file join $dir $basename]
 }
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] testsuite: use cygpath to convert from Unix to Windows paths
  2020-03-11 18:32 [PATCH] testsuite: use cygpath to convert from Unix to Windows paths Simon Marchi
@ 2020-03-11 18:40 ` Christian Biesinger
  2020-03-11 19:09   ` Simon Marchi
  0 siblings, 1 reply; 9+ messages in thread
From: Christian Biesinger @ 2020-03-11 18:40 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Wed, Mar 11, 2020 at 1:32 PM Simon Marchi via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> When on a MinGW host, standard_output_file uses a regular expression to
> convert Unix-style paths of the form "/c/foo" to "c:/foo".  This is
> needed because the paths we pass to GDB (for example, with the "file"
> command) need to be in the Windows form.
>
> However, the regexp only works if your binutils-gdb repo is under a
> `/[a-z]/...` path (the Unix paths mapping to Windows drives).
> Presumably, that works if you clone the repo in Windows, then access it
> through `/c/...`.
>
> In my case, I've cloned the repository directly inside my MinGW shell,
> so in /home/smarchi.  The regexp therefore doesn't work for me.  The
> path doesn't get transformed, and the file command fails when running
> any test:
>
>     (gdb) file /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent
>     /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent: No such file or directory.
>
> A safer way to do this conversion would be to use the cygpath utility.
> It can be used to convert any MinGW path into its Windows equivalent.
>
> Note that despite originally coming from Cygwin, the cygpath utility is
> distributed by mingw-w64 and can be used in that environment.
>
> With this, the file command in the test succeeds:
>
>     (gdb) file C:/msys64/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent
>     Reading symbols from C:/msys64/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent...
>
> gdb/testsuite/ChangeLog:
>
>         * lib/gdb.exp (standard_output_file): Use cygpath to convert
>         from Unix to Windows path.
> ---
>  gdb/testsuite/lib/gdb.exp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 9614e8dc87cd..ac16e0e85c99 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -4899,8 +4899,10 @@ proc standard_output_file {basename} {
>      file mkdir $dir
>      # If running on MinGW, replace /c/foo with c:/foo
>      if { [ishost *-*-mingw*] } {
> -        set dir [regsub {^/([a-z])/} $dir {\1:/}]
> +       set dir [exec cygpath --mixed "${dir}"]

This is fine, but out of curiosity, why --mixed instead of --windows?

> +
>      }

Why the empty line above?

> +
>      return [file join $dir $basename]
>  }
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] testsuite: use cygpath to convert from Unix to Windows paths
  2020-03-11 18:40 ` Christian Biesinger
@ 2020-03-11 19:09   ` Simon Marchi
  2020-03-11 19:14     ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2020-03-11 19:09 UTC (permalink / raw)
  To: Christian Biesinger; +Cc: gdb-patches, Eli Zaretskii

On 2020-03-11 2:40 p.m., Christian Biesinger wrote:
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 9614e8dc87cd..ac16e0e85c99 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -4899,8 +4899,10 @@ proc standard_output_file {basename} {
>>      file mkdir $dir
>>      # If running on MinGW, replace /c/foo with c:/foo
>>      if { [ishost *-*-mingw*] } {
>> -        set dir [regsub {^/([a-z])/} $dir {\1:/}]
>> +       set dir [exec cygpath --mixed "${dir}"]
> 
> This is fine, but out of curiosity, why --mixed instead of --windows?

--mixed produces paths with forward slashes:

  C:/hello/christian

rather than back slashes:

  C:\hello\christian

And the backslashes wreak havoc (we would need to escape them):

  C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/9.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: cannot open output file C:msys64homesmarchi^Huild^Hinutils-gdbgdb  estsuiteoutputsgdb.archi386-bp_permanent/i386-bp_permanent.exe: Invalid argument

Since the tools understand forward slashes, it's just easier to use that.

> 
>> +
>>      }
> 
> Why the empty line above?

A leftover from fighting with Windows, I'll make sure to remove it.

However, the solution I proposed won't work with the non-MinGW-w64 MinGW, as
it does not ship with cygpath.  Here is another version of the patch that uses
`pwd -W` to get the same information.


From b1da3b3b78b92887eeed61b72ca765be45ce3ce2 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Wed, 11 Mar 2020 14:16:17 -0400
Subject: [PATCH] testsuite: use `pwd -W` to convert from Unix to Windows paths

When on a MinGW host, standard_output_file uses a regular expression to
convert Unix-style paths of the form "/c/foo" to "c:/foo".  This is
needed because the paths we pass to GDB (for example, with the "file"
command) need to be in the Windows form.

However, the regexp only works if your binutils-gdb repo is under a
`/[a-z]/...` path (the Unix paths mapping to Windows drives).
Presumably, that works if you clone the repo in Windows, then access it
through `/c/...`.

In my case, I've cloned the repository directly inside my MinGW shell,
so in /home/smarchi.  The regexp therefore doesn't work for me.  The
path doesn't get transformed, and the file command fails when running
any test:

    (gdb) file /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent
    /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent: No such file or directory.

A safer way to do this is to execute `pwd -W` while in the directory we
want the path for, this is what this patch does.

I have also considered using the using the cygpath utility to do the
conversion.  It can be used to convert any MinGW path into its Windows
equivalent.  Despite originally coming from Cygwin, the cygpath utility
is distributed by MinGW-w64 and can be used in that environment.
However, it's not distributed with the non-MinGW-w64 MinGW.

The `pwd -W` trick only works with directories that exist, which is the
case here, so it's sufficient.

With this, the file command in the test succeeds:

    (gdb) file C:/msys64/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent
    Reading symbols from C:/msys64/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/i386-bp_permanent/i386-bp_permanent...

gdb/testsuite/ChangeLog:

	* lib/gdb.exp (standard_output_file): Use `pwd -W` to convert
	from Unix to Windows path.
---
 gdb/testsuite/lib/gdb.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9614e8dc87cd..9e903ba34776 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4899,7 +4899,7 @@ proc standard_output_file {basename} {
     file mkdir $dir
     # If running on MinGW, replace /c/foo with c:/foo
     if { [ishost *-*-mingw*] } {
-        set dir [regsub {^/([a-z])/} $dir {\1:/}]
+        set dir [exec sh -c "cd ${dir} && pwd -W"]
     }
     return [file join $dir $basename]
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] testsuite: use cygpath to convert from Unix to Windows paths
  2020-03-11 19:09   ` Simon Marchi
@ 2020-03-11 19:14     ` Eli Zaretskii
  2020-03-11 19:26       ` Simon Marchi
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2020-03-11 19:14 UTC (permalink / raw)
  To: Simon Marchi; +Cc: cbiesinger, gdb-patches

> Cc: gdb-patches <gdb-patches@sourceware.org>, Eli Zaretskii <eliz@gnu.org>
> From: Simon Marchi <simon.marchi@efficios.com>
> Date: Wed, 11 Mar 2020 15:09:10 -0400
> 
> However, the solution I proposed won't work with the non-MinGW-w64 MinGW, as
> it does not ship with cygpath.  Here is another version of the patch that uses
> `pwd -W` to get the same information.

Right, using "pwd -W" was exactly what I wanted to propose for this
purpose.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] testsuite: use cygpath to convert from Unix to Windows paths
  2020-03-11 19:14     ` Eli Zaretskii
@ 2020-03-11 19:26       ` Simon Marchi
  2020-03-13 17:57         ` Tom Tromey
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2020-03-11 19:26 UTC (permalink / raw)
  To: Eli Zaretskii, Simon Marchi; +Cc: gdb-patches

On 2020-03-11 3:14 p.m., Eli Zaretskii wrote:
>> Cc: gdb-patches <gdb-patches@sourceware.org>, Eli Zaretskii <eliz@gnu.org>
>> From: Simon Marchi <simon.marchi@efficios.com>
>> Date: Wed, 11 Mar 2020 15:09:10 -0400
>>
>> However, the solution I proposed won't work with the non-MinGW-w64 MinGW, as
>> it does not ship with cygpath.  Here is another version of the patch that uses
>> `pwd -W` to get the same information.
> 
> Right, using "pwd -W" was exactly what I wanted to propose for this
> purpose.

Ok thanks, I have pushed this version of the patch.

Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] testsuite: use cygpath to convert from Unix to Windows paths
  2020-03-11 19:26       ` Simon Marchi
@ 2020-03-13 17:57         ` Tom Tromey
  2020-03-13 18:14           ` Christian Biesinger
  2020-03-13 19:11           ` Tom Tromey
  0 siblings, 2 replies; 9+ messages in thread
From: Tom Tromey @ 2020-03-13 17:57 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Eli Zaretskii, Simon Marchi, gdb-patches

>> Right, using "pwd -W" was exactly what I wanted to propose for this
>> purpose.

Simon> Ok thanks, I have pushed this version of the patch.

Today I tried something a little weird -- I did a mingw build on Linux
and then tried to run the gdb.server test cases.  Linux can run mingw
executables via wine...

However this fails with:

    ERROR: tcl error sourcing /home/tromey/gdb/build-mingw/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.server/wrapper.exp.
    ERROR: sh: line 0: pwd: -W: invalid option
    pwd: usage: pwd [-LP]

My site.exp says:

    set host_triplet i686-w64-mingw32
    set target_alias i686-w64-mingw32
    set target_triplet i686-w64-mingw32
    set build_triplet x86_64-pc-linux-gnu

I guess maybe this is expected, I wonder if there's a clean way to fix
the problem.  Christian suggested maybe we could have a wine board file
to work around it.  Or maybe we could check build_triplet?  I'm not sure
if that would be incorrect in some case.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] testsuite: use cygpath to convert from Unix to Windows paths
  2020-03-13 17:57         ` Tom Tromey
@ 2020-03-13 18:14           ` Christian Biesinger
  2020-03-13 19:11           ` Tom Tromey
  1 sibling, 0 replies; 9+ messages in thread
From: Christian Biesinger @ 2020-03-13 18:14 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi, Simon Marchi, gdb-patches

On Fri, Mar 13, 2020 at 12:57 PM Tom Tromey <tom@tromey.com> wrote:
>
> >> Right, using "pwd -W" was exactly what I wanted to propose for this
> >> purpose.
>
> Simon> Ok thanks, I have pushed this version of the patch.
>
> Today I tried something a little weird -- I did a mingw build on Linux
> and then tried to run the gdb.server test cases.  Linux can run mingw
> executables via wine...
>
> However this fails with:
>
>     ERROR: tcl error sourcing /home/tromey/gdb/build-mingw/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.server/wrapper.exp.
>     ERROR: sh: line 0: pwd: -W: invalid option
>     pwd: usage: pwd [-LP]
>
> My site.exp says:
>
>     set host_triplet i686-w64-mingw32
>     set target_alias i686-w64-mingw32
>     set target_triplet i686-w64-mingw32
>     set build_triplet x86_64-pc-linux-gnu
>
> I guess maybe this is expected, I wonder if there's a clean way to fix
> the problem.  Christian suggested maybe we could have a wine board file
> to work around it.  Or maybe we could check build_triplet?  I'm not sure
> if that would be incorrect in some case.

Just to clarify... it is not enough to just bypass that call for wine.
You do need to convert the linux path to a Windows path, or GDB will
not be able to load the file. So since that translation has to be done
by the testrunner, it has to be aware that you're running GDB under
Wine.

It looks like by default, Z:\ is mapped to /, so /foo should become Z:/foo.

Christian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] testsuite: use cygpath to convert from Unix to Windows paths
  2020-03-13 17:57         ` Tom Tromey
  2020-03-13 18:14           ` Christian Biesinger
@ 2020-03-13 19:11           ` Tom Tromey
  2020-05-12 22:44             ` Simon Marchi
  1 sibling, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2020-03-13 19:11 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi, Eli Zaretskii, Simon Marchi, gdb-patches

Tom> Today I tried something a little weird -- I did a mingw build on Linux
Tom> and then tried to run the gdb.server test cases.  Linux can run mingw
Tom> executables via wine...

Tom> However this fails with:

Tom>     ERROR: tcl error sourcing /home/tromey/gdb/build-mingw/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.server/wrapper.exp.
Tom>     ERROR: sh: line 0: pwd: -W: invalid option
Tom>     pwd: usage: pwd [-LP]

It failed this way on the Windows machine I have access to as well.

I believe on that machine, I log in to a Cygwin environment, but
normally I do mingw-hosted builds.  The Cygwin "pwd" doesn't understand -W.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] testsuite: use cygpath to convert from Unix to Windows paths
  2020-03-13 19:11           ` Tom Tromey
@ 2020-05-12 22:44             ` Simon Marchi
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2020-05-12 22:44 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi, gdb-patches

On 2020-03-13 3:11 p.m., Tom Tromey wrote:
> Tom> Today I tried something a little weird -- I did a mingw build on Linux
> Tom> and then tried to run the gdb.server test cases.  Linux can run mingw
> Tom> executables via wine...
> 
> Tom> However this fails with:
> 
> Tom>     ERROR: tcl error sourcing /home/tromey/gdb/build-mingw/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.server/wrapper.exp.
> Tom>     ERROR: sh: line 0: pwd: -W: invalid option
> Tom>     pwd: usage: pwd [-LP]
> 
> It failed this way on the Windows machine I have access to as well.
> 
> I believe on that machine, I log in to a Cygwin environment, but
> normally I do mingw-hosted builds.  The Cygwin "pwd" doesn't understand -W.

Hey, I missed that somehow, sorry about that.

I think I could implement something where we try multiple methods in sequence:

1. cygpath
2. pwd -W
3. the original regsub that was there prior to this patch

Would this help?

Simon


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-05-12 22:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 18:32 [PATCH] testsuite: use cygpath to convert from Unix to Windows paths Simon Marchi
2020-03-11 18:40 ` Christian Biesinger
2020-03-11 19:09   ` Simon Marchi
2020-03-11 19:14     ` Eli Zaretskii
2020-03-11 19:26       ` Simon Marchi
2020-03-13 17:57         ` Tom Tromey
2020-03-13 18:14           ` Christian Biesinger
2020-03-13 19:11           ` Tom Tromey
2020-05-12 22:44             ` Simon Marchi

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).