Kirill Makurin wrote:
> I was going to just say that this patch is good and go ahead with it, but I 
> decided to check it and turns out it fixes this issue only partially, but the 
> patch itself is good.
> 
> Msys2 has multiple environments (see 
> https://www.msys2.org/docs/environments/) and this patch works only for 
> `MSYS`. For all other environments `uname` will report a string starting with 
> `MINGW64` and `compile` will set `file_conv` variable to `mingw`. This, 
> unfortunately, overlaps with mingw32 which does not have `cygpath`.
> 
> For `mingw` the filename is converted in `compile` as follows:
> 
> ```
> mingw/*)
>       file=`cmd //C echo "$file " | sed -e 's/"\(.*\) " *$/\1/'`
> ```
> 
> The resulting converted filename has forward slashes as with `cygpath -m` and 
> suffers from the same double conversion issue.
> 
> In theory we could prepend `cygpath` invocation in this branch like this:
> 
> ```
> mingw/*)
>       file=`cygpath -w  "$file" 2>/dev/null || cmd //C echo "$file " | sed -e 
> 's/"\(.*\) " *$/\1/'`
> ```
> 
> However, compilation is still going to fail if there is no `cygpath` (also 
> ,`cygpath` is not installed with Msys2 by default).
> 
> I think a better solution may be to add additional sed invocation in the pipe 
> to convert forward slashes to backslashes.
> 
> I also was wondering if passing `mingw` as the second argument into 
> `func_file_conv ` in this branch could help, but unfortunately `/filename` in 
> resulting `-Tp/filename` is not converted by Msys2.
> 
> ```
> *.cc | *.CC | *.cxx | *.CXX | *.[cC]++)
>         func_file_conv "$1"
> ```
> 
> I hope I didn't cause too much confusion about `MSYS2_ARG_CONV_EXCL`, I was a 
> bit in a hurry and probably didn't make it clear that I meant setting 
> `MSYS2_ARG_CONV_EXCL` more like an external user-side workaround (e.g. I was 
> setting it in my build script).
> 
> - Kirill Makurin

Thanks for testing.

> For all other environments `uname` will report a string starting with 
> `MINGW64`
> and `compile` will set `file_conv` variable to `mingw`.

OK, so what you saying is the detection / distinction of environments
is not working. Which is quite plausible, since the original code was
written in 2010 (when MSYS2 did not exist) and Paul's patch from 2019-11-11
was apparently not well tested (Paul is not using Windows environments).

So, let's fix this environment distinction. This is better than applying
workarounds that will affect behaviour on the original MinGW.

Here are 3 proposed patches:

0001) This is a cleanup of Paul's patch from 2019-11-11. Since $file_conv
      can only have one of the three values 'mingw', 'cygwin', 'wine',
      it is pointless to test whether this value is 'msys'.

0002) This patch extends the distinction of environments. Since I don't
      have first-hand info about these environments, I asked the prior
      knowledge summarization engine. Results are attached. I trust these
      answers, because MinGW + MSYS is a topic that is widely discussed
      on the web.

0003) This patch is the same as already proposed: use backslashed filenames
      instead of forward-slashes ones (that MSYS2 would interpret a second
      time).

Kirill: Testing of this patch series in your environment (MSYS2 + MSVC)
        is welcome.

Bruno
>From fa6a98993ea95ce576c79e597cda5e166f36ba5e Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Mon, 3 Feb 2025 06:02:07 +0100
Subject: [PATCH 1/3] compile: Simplify.

* lib/compile (func_file_conv): Remove unnecessary code, added on 2019-11-11.
* lib/ar-lib (func_file_conv): Likewise.
---
 lib/ar-lib  | 4 ++--
 lib/compile | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/ar-lib b/lib/ar-lib
index 7d62dea99..85761fbf1 100755
--- a/lib/ar-lib
+++ b/lib/ar-lib
@@ -2,7 +2,7 @@
 # Wrapper for Microsoft lib.exe
 
 me=ar-lib
-scriptversion=2024-06-19.01; # UTC
+scriptversion=2025-02-03.05; # UTC
 
 # Copyright (C) 2010-2025 Free Software Foundation, Inc.
 # Written by Peter Rosin <p...@lysator.liu.se>.
@@ -65,7 +65,7 @@ func_file_conv ()
 	mingw)
 	  file=`cmd //C echo "$file " | sed -e 's/"\(.*\) " *$/\1/'`
 	  ;;
-	cygwin | msys)
+	cygwin)
 	  file=`cygpath -m "$file" || echo "$file"`
 	  ;;
 	wine)
diff --git a/lib/compile b/lib/compile
index 14aec5621..e80b054a0 100755
--- a/lib/compile
+++ b/lib/compile
@@ -1,7 +1,7 @@
 #! /bin/sh
 # Wrapper for compilers which do not understand '-c -o'.
 
-scriptversion=2024-12-03.03; # UTC
+scriptversion=2025-02-03.05; # UTC
 
 # Copyright (C) 1999-2025 Free Software Foundation, Inc.
 # Written by Tom Tromey <tro...@cygnus.com>.
@@ -67,7 +67,7 @@ func_file_conv ()
 	mingw/*)
 	  file=`cmd //C echo "$file " | sed -e 's/"\(.*\) " *$/\1/'`
 	  ;;
-	cygwin/* | msys/*)
+	cygwin/*)
 	  file=`cygpath -m "$file" || echo "$file"`
 	  ;;
 	wine/*)
-- 
2.43.0

>From 294cedfe2548ef8ccdf5162e2074eb1edd0cb094 Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Mon, 3 Feb 2025 06:10:09 +0100
Subject: [PATCH 2/3] compile: Distinguish various MinGW, MSYS, MSYS2
 environments correctly.

Reported by Kirill Makurin <maiddais...@outlook.com> in
<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=75939>.

* lib/compile (func_file_conv): Use not only "uname -s", but also $MSYSTEM,
in order to distinguish the original MinGW and MSYS2.
* lib/ar-lib (func_file_conv): Likewise.
---
 lib/ar-lib  | 15 +++++++++++++--
 lib/compile | 23 ++++++++++++++++++-----
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/lib/ar-lib b/lib/ar-lib
index 85761fbf1..df0243a8c 100755
--- a/lib/ar-lib
+++ b/lib/ar-lib
@@ -51,9 +51,20 @@ func_file_conv ()
 	# lazily determine how to convert abs files
 	case `uname -s` in
 	  MINGW*)
-	    file_conv=mingw
+	    if test -n "$MSYSTEM"; then
+	      # MSYS2 environment.
+	      file_conv=cygwin
+	    else
+	      # Original MinGW environment.
+	      file_conv=mingw
+	    fi
 	    ;;
-	  CYGWIN* | MSYS*)
+	  MSYS*)
+	    # Old MSYS environment, or MSYS2 with 32-bit MSYS2 shell.
+	    file_conv=cygwin
+	    ;;
+	  CYGWIN*)
+	    # Cygwin environment.
 	    file_conv=cygwin
 	    ;;
 	  *)
diff --git a/lib/compile b/lib/compile
index e80b054a0..99637261f 100755
--- a/lib/compile
+++ b/lib/compile
@@ -37,11 +37,11 @@ IFS=" ""	$nl"
 
 file_conv=
 
-# func_file_conv build_file lazy
+# func_file_conv build_file unneeded_conversions
 # Convert a $build file to $host form and store it in $file
 # Currently only supports Windows hosts. If the determined conversion
-# type is listed in (the comma separated) LAZY, no conversion will
-# take place.
+# type is listed in (the comma separated) UNNEEDED_CONVERSIONS, no
+# conversion will take place.
 func_file_conv ()
 {
   file=$1
@@ -51,9 +51,20 @@ func_file_conv ()
 	# lazily determine how to convert abs files
 	case `uname -s` in
 	  MINGW*)
-	    file_conv=mingw
+	    if test -n "$MSYSTEM"; then
+	      # MSYS2 environment.
+	      file_conv=cygwin
+	    else
+	      # Original MinGW environment.
+	      file_conv=mingw
+	    fi
 	    ;;
-	  CYGWIN* | MSYS*)
+	  MSYS*)
+	    # Old MSYS environment, or MSYS2 with 32-bit MSYS2 shell.
+	    file_conv=cygwin
+	    ;;
+	  CYGWIN*)
+	    # Cygwin environment.
 	    file_conv=cygwin
 	    ;;
 	  *)
@@ -63,6 +74,8 @@ func_file_conv ()
       fi
       case $file_conv/,$2, in
 	*,$file_conv,*)
+	  # This is the optimization mentioned above:
+	  # If UNNEEDED_CONVERSIONS contains $file_conv, don't convert.
 	  ;;
 	mingw/*)
 	  file=`cmd //C echo "$file " | sed -e 's/"\(.*\) " *$/\1/'`
-- 
2.43.0

>From b7b7983b66666833bbe1caa37c19b4599c12c610 Mon Sep 17 00:00:00 2001
From: Bruno Haible <br...@clisp.org>
Date: Mon, 3 Feb 2025 06:11:37 +0100
Subject: [PATCH 3/3] compile: Improve support for C++ compilations on MSYS2.

Reported by Kirill Makurin <maiddais...@outlook.com> in
<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=75939>.

* lib/compile (func_file_conv): Use 'cygpath -w', not 'cygpath -m'.
* lib/ar-lib (func_file_conv): Likewise.
---
 lib/ar-lib  | 2 +-
 lib/compile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ar-lib b/lib/ar-lib
index df0243a8c..3aae316a7 100755
--- a/lib/ar-lib
+++ b/lib/ar-lib
@@ -77,7 +77,7 @@ func_file_conv ()
 	  file=`cmd //C echo "$file " | sed -e 's/"\(.*\) " *$/\1/'`
 	  ;;
 	cygwin)
-	  file=`cygpath -m "$file" || echo "$file"`
+	  file=`cygpath -w "$file" || echo "$file"`
 	  ;;
 	wine)
 	  file=`winepath -w "$file" || echo "$file"`
diff --git a/lib/compile b/lib/compile
index 99637261f..6d24ae019 100755
--- a/lib/compile
+++ b/lib/compile
@@ -81,7 +81,7 @@ func_file_conv ()
 	  file=`cmd //C echo "$file " | sed -e 's/"\(.*\) " *$/\1/'`
 	  ;;
 	cygwin/*)
-	  file=`cygpath -m "$file" || echo "$file"`
+	  file=`cygpath -w "$file" || echo "$file"`
 	  ;;
 	wine/*)
 	  file=`winepath -w "$file" || echo "$file"`
-- 
2.43.0

Attachment: mingw-msys-environments.odt
Description: application/vnd.oasis.opendocument.text

Reply via email to