[PATCH 1/4] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja

[PATCH 1/4] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja

[luoyonggang]

From: Yonggang Luo <luoyonggang@gmail.com>

SIMPLE_PATH_RE should match the full path token.
Or the $ and : contained in path would not matched if the path are start with C:/ and E:/
---
 scripts/ninjatool.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
index cc77d51aa8..6ca8be6f10 100755
--- a/scripts/ninjatool.py
+++ b/scripts/ninjatool.py
@@ -55,7 +55,7 @@ else:
 
 PATH_RE = r"[^$\s:|]+|\$[$ :]|\$[a-zA-Z0-9_-]+|\$\{[a-zA-Z0-9_.-]+\}"
 
-SIMPLE_PATH_RE = re.compile(r"[^$\s:|]+")
+SIMPLE_PATH_RE = re.compile(r"^[^$\s:|]+$")
 IDENT_RE = re.compile(r"[a-zA-Z0-9_.-]+$")
 STRING_RE = re.compile(r"(" + PATH_RE + r"|[\s:|])(?:\r?\n)?|.")
 TOPLEVEL_RE = re.compile(r"([=:#]|\|\|?|^ +|(?:" + PATH_RE + r")+)\s*|.")
-- 
2.27.0.windows.1

[PATCH 2/4] meson: fixes relpath may fail on win32.

[luoyonggang]

From: Yonggang Luo <luoyonggang@gmail.com>

On win32, os.path.relpath would raise exception when do the following relpath:
C:/msys64/mingw64/x.exe relative to E:/path/qemu-build would fail.
So we try catch it for stopping it from raise exception on msys2
---
 scripts/mtest2make.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
index bdb257bbd9..d7a51bf97e 100644
--- a/scripts/mtest2make.py
+++ b/scripts/mtest2make.py
@@ -53,9 +53,16 @@ i = 0
 for test in json.load(sys.stdin):
     env = ' '.join(('%s=%s' % (shlex.quote(k), shlex.quote(v))
                     for k, v in test['env'].items()))
-    executable = os.path.relpath(test['cmd'][0])
+    executable = test['cmd'][0]
+    try:
+        executable = os.path.relpath(executable)
+    except:
+        pass
     if test['workdir'] is not None:
-        test['cmd'][0] = os.path.relpath(test['cmd'][0], test['workdir'])
+        try:
+            test['cmd'][0] = os.path.relpath(executable, test['workdir'])
+        except:
+            test['cmd'][0] = executable
     else:
         test['cmd'][0] = executable
     cmd = '$(.test.env) %s %s' % (env, ' '.join((shlex.quote(x) for x in test['cmd'])))
-- 
2.27.0.windows.1

[PATCH 3/4] meson: Mingw64 gcc doesn't recognize system include_type for sdl2

[luoyonggang]

From: Yonggang Luo <luoyonggang@gmail.com>

Fixes this for msys2/mingw64 by remove the include_type for sdl2 discovery in meson
---
 meson.build | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index f0fe5f8799..1644bbd83c 100644
--- a/meson.build
+++ b/meson.build
@@ -224,8 +224,7 @@ if 'CONFIG_BRLAPI' in config_host
   brlapi = declare_dependency(link_args: config_host['BRLAPI_LIBS'].split())
 endif
 
-sdl = dependency('sdl2', required: get_option('sdl'), static: enable_static,
-                 include_type: 'system')
+sdl = dependency('sdl2', required: get_option('sdl'), static: enable_static)
 sdl_image = not_found
 if sdl.found()
   # work around 2.0.8 bug
-- 
2.27.0.windows.1

[PATCH 4/4] configure: Fix include and linkage issue on msys2

[luoyonggang]

From: Yonggang Luo <luoyonggang@gmail.com>

On msys2, the -I/e/path/to/qemu -L/e/path/to/qemu are not recognized by the compiler
Cause $PWD are result posix style path such as /e/path/to/qemu that can not be recognized
by mingw gcc, and `pwd -W` are result Windows style path such as E:/path/to/qemu that can
be recognized by the mingw gcc. So we replace all $PWD with $build_path that can
building qemu under msys2/mingw environment.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 configure | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index b1e11397a8..3b9e79923d 100755
--- a/configure
+++ b/configure
@@ -13,8 +13,13 @@ export CCACHE_RECACHE=yes
 
 # make source path absolute
 source_path=$(cd "$(dirname -- "$0")"; pwd)
+build_path=$PWD
+if [ "$MSYSTEM" = "MINGW64" -o  "$MSYSTEM" = "MINGW32" ]; then
+source_path=$(cd "$(dirname -- "$0")"; pwd -W)
+build_path=`pwd -W`
+fi
 
-if test "$PWD" = "$source_path"
+if test "$build_path" = "$source_path"
 then
     echo "Using './build' as the directory for build output"
 
@@ -346,7 +351,12 @@ ld_has() {
     $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
 }
 
-if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
+check_valid_build_path="[[:space:]:]"
+if [ "$MSYSTEM" = "MINGW64" -o  "$MSYSTEM" = "MINGW32" ]; then
+check_valid_build_path="[[:space:]]"
+fi
+
+if printf %s\\n "$source_path" "$build_path" | grep -q "$check_valid_build_path";
 then
   error_exit "main directory cannot contain spaces nor colons"
 fi
@@ -942,7 +952,7 @@ Linux)
   linux="yes"
   linux_user="yes"
   kvm="yes"
-  QEMU_INCLUDES="-isystem ${source_path}/linux-headers -I$PWD/linux-headers $QEMU_INCLUDES"
+  QEMU_INCLUDES="-isystem ${source_path}/linux-headers -I${build_path}/linux-headers $QEMU_INCLUDES"
   libudev="yes"
 ;;
 esac
@@ -4283,7 +4293,7 @@ EOF
               symlink "$source_path/dtc/Makefile" "dtc/Makefile"
           fi
           fdt_cflags="-I${source_path}/dtc/libfdt"
-          fdt_ldflags="-L$PWD/dtc/libfdt"
+          fdt_ldflags="-L${build_path}/dtc/libfdt"
           fdt_libs="$fdt_libs"
       elif test "$fdt" = "yes" ; then
           # Not a git build & no libfdt found, prompt for system install
@@ -5268,7 +5278,7 @@ case "$capstone" in
     else
       LIBCAPSTONE=libcapstone.a
     fi
-    capstone_libs="-L$PWD/capstone -lcapstone"
+    capstone_libs="-L${build_path}/capstone -lcapstone"
     capstone_cflags="-I${source_path}/capstone/include"
     ;;
 
@@ -6268,8 +6278,8 @@ case "$slirp" in
       git_submodules="${git_submodules} slirp"
     fi
     mkdir -p slirp
-    slirp_cflags="-I${source_path}/slirp/src -I$PWD/slirp/src"
-    slirp_libs="-L$PWD/slirp -lslirp"
+    slirp_cflags="-I${source_path}/slirp/src -I${build_path}/slirp/src"
+    slirp_libs="-L${build_path}/slirp -lslirp"
     if test "$mingw32" = "yes" ; then
       slirp_libs="$slirp_libs -lws2_32 -liphlpapi"
     fi
@@ -8212,7 +8222,7 @@ fi
 mv $cross config-meson.cross
 
 rm -rf meson-private meson-info meson-logs
-NINJA=$PWD/ninjatool $meson setup \
+NINJA="${build_path}/ninjatool" $meson setup \
         --prefix "${pre_prefix}$prefix" \
         --libdir "${pre_prefix}$libdir" \
         --libexecdir "${pre_prefix}$libexecdir" \
@@ -8232,7 +8242,7 @@ NINJA=$PWD/ninjatool $meson setup \
 	-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
 	-Dgettext=$gettext -Dxkbcommon=$xkbcommon \
         $cross_arg \
-        "$PWD" "$source_path"
+        "$build_path" "$source_path"
 
 if test "$?" -ne 0 ; then
     error_exit "meson setup failed"
-- 
2.27.0.windows.1

Re: [PATCH 1/4] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja

[Mark Cave-Ayland]

On 25/08/2020 17:53, luoyonggang@gmail.com wrote:

> From: Yonggang Luo <luoyonggang@gmail.com>
> 
> SIMPLE_PATH_RE should match the full path token.
> Or the $ and : contained in path would not matched if the path are start with C:/ and E:/
> ---
>  scripts/ninjatool.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
> index cc77d51aa8..6ca8be6f10 100755
> --- a/scripts/ninjatool.py
> +++ b/scripts/ninjatool.py
> @@ -55,7 +55,7 @@ else:
>  
>  PATH_RE = r"[^$\s:|]+|\$[$ :]|\$[a-zA-Z0-9_-]+|\$\{[a-zA-Z0-9_.-]+\}"
>  
> -SIMPLE_PATH_RE = re.compile(r"[^$\s:|]+")
> +SIMPLE_PATH_RE = re.compile(r"^[^$\s:|]+$")
>  IDENT_RE = re.compile(r"[a-zA-Z0-9_.-]+$")
>  STRING_RE = re.compile(r"(" + PATH_RE + r"|[\s:|])(?:\r?\n)?|.")
>  TOPLEVEL_RE = re.compile(r"([=:#]|\|\|?|^ +|(?:" + PATH_RE + r")+)\s*|.")

I've tested this and it changes build.ninja so instead of Windows paths beginning C$$
they now begin C$ instead e.g.:

build qemu-version.h: CUSTOM_COMMAND  |
C$:/msys64/home/Mark/qemu/scripts/qemu-version.sh PHONY

I was expecting this not to work, however it seems in the next stage of
transformation from build.ninja to Makefile.ninja the extra $ is removed correctly:

qemu-version.h: qemu-version.h.stamp; @:
qemu-version.h.stamp: C:/msys64/home/Mark/qemu/scripts/qemu-version.sh PHONY | ;
${ninja-command-restat}

It feels like the extra $ shouldn't be present in build.ninja, but the patch does
generate a Makefile.ninja that works.


ATB,

Mark.

Re: [PATCH 2/4] meson: fixes relpath may fail on win32.

[Mark Cave-Ayland]

On 25/08/2020 17:53, luoyonggang@gmail.com wrote:

> From: Yonggang Luo <luoyonggang@gmail.com>
> 
> On win32, os.path.relpath would raise exception when do the following relpath:
> C:/msys64/mingw64/x.exe relative to E:/path/qemu-build would fail.
> So we try catch it for stopping it from raise exception on msys2
> ---
>  scripts/mtest2make.py | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
> index bdb257bbd9..d7a51bf97e 100644
> --- a/scripts/mtest2make.py
> +++ b/scripts/mtest2make.py
> @@ -53,9 +53,16 @@ i = 0
>  for test in json.load(sys.stdin):
>      env = ' '.join(('%s=%s' % (shlex.quote(k), shlex.quote(v))
>                      for k, v in test['env'].items()))
> -    executable = os.path.relpath(test['cmd'][0])
> +    executable = test['cmd'][0]
> +    try:
> +        executable = os.path.relpath(executable)
> +    except:
> +        pass
>      if test['workdir'] is not None:
> -        test['cmd'][0] = os.path.relpath(test['cmd'][0], test['workdir'])
> +        try:
> +            test['cmd'][0] = os.path.relpath(executable, test['workdir'])
> +        except:
> +            test['cmd'][0] = executable
>      else:
>          test['cmd'][0] = executable
>      cmd = '$(.test.env) %s %s' % (env, ' '.join((shlex.quote(x) for x in test['cmd'])))

I don't think this is relevant in my particular environment, however it didn't seem
to break the build. I'm curious as to why os.path.relpath throws an exception in this
particular case on Windows though - can you give us a bit more information about the
Exception that is being thrown?


ATB,

Mark.

Re: [PATCH 3/4] meson: Mingw64 gcc doesn't recognize system include_type for sdl2

[Mark Cave-Ayland]

On 25/08/2020 17:53, luoyonggang@gmail.com wrote:

> From: Yonggang Luo <luoyonggang@gmail.com>
> 
> Fixes this for msys2/mingw64 by remove the include_type for sdl2 discovery in meson
> ---
>  meson.build | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index f0fe5f8799..1644bbd83c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -224,8 +224,7 @@ if 'CONFIG_BRLAPI' in config_host
>    brlapi = declare_dependency(link_args: config_host['BRLAPI_LIBS'].split())
>  endif
>  
> -sdl = dependency('sdl2', required: get_option('sdl'), static: enable_static,
> -                 include_type: 'system')
> +sdl = dependency('sdl2', required: get_option('sdl'), static: enable_static)
>  sdl_image = not_found
>  if sdl.found()
>    # work around 2.0.8 bug

This gets around the issue whereby "-isystem" paths are not escaped correctly on
Windows, presumably by changing them to "-iquote" instead.

Marc-André had a query about why this is marked as a system include, however I can
confirm that it fixes the missing "SDL.h" issue during build.


ATB,

Mark.

Re: [PATCH 1/4] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja

[Eric Blake]

On 8/25/20 11:53 AM, luoyonggang@gmail.com wrote:
> From: Yonggang Luo <luoyonggang@gmail.com>
> 
> SIMPLE_PATH_RE should match the full path token.
> Or the $ and : contained in path would not matched if the path are start with C:/ and E:/
> ---

Missing a Signed-off-by tag.  Without that, we cannot apply it.

Also missing a 0/4 cover letter; less essential, but useful for 
continuous integration tools and for replying to the series as a whole.

More patch submission hints can be found at 
https://wiki.qemu.org/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 4/4] configure: Fix include and linkage issue on msys2

[Mark Cave-Ayland]

On 25/08/2020 17:53, luoyonggang@gmail.com wrote:

> From: Yonggang Luo <luoyonggang@gmail.com>
> 
> On msys2, the -I/e/path/to/qemu -L/e/path/to/qemu are not recognized by the compiler
> Cause $PWD are result posix style path such as /e/path/to/qemu that can not be recognized
> by mingw gcc, and `pwd -W` are result Windows style path such as E:/path/to/qemu that can
> be recognized by the mingw gcc. So we replace all $PWD with $build_path that can
> building qemu under msys2/mingw environment.
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  configure | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/configure b/configure
> index b1e11397a8..3b9e79923d 100755
> --- a/configure
> +++ b/configure
> @@ -13,8 +13,13 @@ export CCACHE_RECACHE=yes
>  
>  # make source path absolute
>  source_path=$(cd "$(dirname -- "$0")"; pwd)
> +build_path=$PWD
> +if [ "$MSYSTEM" = "MINGW64" -o  "$MSYSTEM" = "MINGW32" ]; then
> +source_path=$(cd "$(dirname -- "$0")"; pwd -W)
> +build_path=`pwd -W`
> +fi

This is missing some indentation here, and also for other if statements introduced below.

I'm wondering if build_path is the right name for this variable, since it looks like
it returns another variant of the source directory?

> -if test "$PWD" = "$source_path"
> +if test "$build_path" = "$source_path"
>  then
>      echo "Using './build' as the directory for build output"
>  
> @@ -346,7 +351,12 @@ ld_has() {
>      $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
>  }
>  
> -if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
> +check_valid_build_path="[[:space:]:]"
> +if [ "$MSYSTEM" = "MINGW64" -o  "$MSYSTEM" = "MINGW32" ]; then
> +check_valid_build_path="[[:space:]]"
> +fi
> +
> +if printf %s\\n "$source_path" "$build_path" | grep -q "$check_valid_build_path";
>  then
>    error_exit "main directory cannot contain spaces nor colons"
>  fi
> @@ -942,7 +952,7 @@ Linux)
>    linux="yes"
>    linux_user="yes"
>    kvm="yes"
> -  QEMU_INCLUDES="-isystem ${source_path}/linux-headers -I$PWD/linux-headers $QEMU_INCLUDES"
> +  QEMU_INCLUDES="-isystem ${source_path}/linux-headers -I${build_path}/linux-headers $QEMU_INCLUDES"
>    libudev="yes"
>  ;;
>  esac
> @@ -4283,7 +4293,7 @@ EOF
>                symlink "$source_path/dtc/Makefile" "dtc/Makefile"
>            fi
>            fdt_cflags="-I${source_path}/dtc/libfdt"
> -          fdt_ldflags="-L$PWD/dtc/libfdt"
> +          fdt_ldflags="-L${build_path}/dtc/libfdt"
>            fdt_libs="$fdt_libs"
>        elif test "$fdt" = "yes" ; then
>            # Not a git build & no libfdt found, prompt for system install
> @@ -5268,7 +5278,7 @@ case "$capstone" in
>      else
>        LIBCAPSTONE=libcapstone.a
>      fi
> -    capstone_libs="-L$PWD/capstone -lcapstone"
> +    capstone_libs="-L${build_path}/capstone -lcapstone"
>      capstone_cflags="-I${source_path}/capstone/include"
>      ;;
>  
> @@ -6268,8 +6278,8 @@ case "$slirp" in
>        git_submodules="${git_submodules} slirp"
>      fi
>      mkdir -p slirp
> -    slirp_cflags="-I${source_path}/slirp/src -I$PWD/slirp/src"
> -    slirp_libs="-L$PWD/slirp -lslirp"
> +    slirp_cflags="-I${source_path}/slirp/src -I${build_path}/slirp/src"
> +    slirp_libs="-L${build_path}/slirp -lslirp"
>      if test "$mingw32" = "yes" ; then
>        slirp_libs="$slirp_libs -lws2_32 -liphlpapi"
>      fi
> @@ -8212,7 +8222,7 @@ fi
>  mv $cross config-meson.cross
>  
>  rm -rf meson-private meson-info meson-logs
> -NINJA=$PWD/ninjatool $meson setup \
> +NINJA="${build_path}/ninjatool" $meson setup \
>          --prefix "${pre_prefix}$prefix" \
>          --libdir "${pre_prefix}$libdir" \
>          --libexecdir "${pre_prefix}$libexecdir" \
> @@ -8232,7 +8242,7 @@ NINJA=$PWD/ninjatool $meson setup \
>  	-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
>  	-Dgettext=$gettext -Dxkbcommon=$xkbcommon \
>          $cross_arg \
> -        "$PWD" "$source_path"
> +        "$build_path" "$source_path"
>  
>  if test "$?" -ne 0 ; then
>      error_exit "meson setup failed"

As I don't have your meson MR applied here, instead of this change to NINJA I have
installed ninja via pacman and use the following diff instead:

diff --git a/configure b/configure
index 67832e3bab..58d76ae15a 100755
--- a/configure
+++ b/configure
@@ -8232,7 +8232,7 @@ fi
 mv $cross config-meson.cross

 rm -rf meson-private meson-info meson-logs
-NINJA=$PWD/ninjatool $meson setup \
+NINJA=ninja $meson setup \
         --prefix "${pre_prefix}$prefix" \
         --libdir "${pre_prefix}$libdir" \
         --libexecdir "${pre_prefix}$libexecdir" \


I can confirm that this patch solves the linking issue and produces a working
qemu-system-ppc.exe which I was using as a quick test.


ATB,

Mark.

Re: [PATCH 1/4] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja

[Paolo Bonzini]

On Tue, Aug 25, 2020 at 11:26 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> I've tested this and it changes build.ninja so instead of Windows paths beginning C$$
> they now begin C$ instead e.g.:
>
> build qemu-version.h: CUSTOM_COMMAND  |
> C$:/msys64/home/Mark/qemu/scripts/qemu-version.sh PHONY

The patch should not change build.ninja in any way, but indeed it will
fix the transformation so that the (correct) ninja quoting is removed.

Paolo

Re: [PATCH 2/4] meson: fixes relpath may fail on win32.

[Paolo Bonzini]

On Tue, Aug 25, 2020 at 11:31 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 25/08/2020 17:53, luoyonggang@gmail.com wrote:
>
> > From: Yonggang Luo <luoyonggang@gmail.com>
> >
> > On win32, os.path.relpath would raise exception when do the following relpath:
> > C:/msys64/mingw64/x.exe relative to E:/path/qemu-build would fail.
> > So we try catch it for stopping it from raise exception on msys2
> I don't think this is relevant in my particular environment, however it didn't seem
> to break the build. I'm curious as to why os.path.relpath throws an exception in this
> particular case on Windows though - can you give us a bit more information about the
> Exception that is being thrown?

I think it's because it's impossible to make a relative path between
two different drives.

The patch is correct but is missing the Signed-off-by line.

Paolo

Re: [PATCH 3/4] meson: Mingw64 gcc doesn't recognize system include_type for sdl2

[Paolo Bonzini]

On Tue, Aug 25, 2020 at 11:38 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> Marc-André had a query about why this is marked as a system include, however I can
> confirm that it fixes the missing "SDL.h" issue during build.

It was marked as a system include in an attempt to work around the SDL
2.0.8 bug that requires -Wno-undef. In general we enable lots of
warnings and sometimes they trip dependencies, so using include_type:
'system' in principle makes sense. But if it doesn't work with
Windows, it's not a regression to remove it.

Paolo

Re: [PATCH 3/4] meson: Mingw64 gcc doesn't recognize system include_type for sdl2

[Daniel P. Berrangé]

On Wed, Aug 26, 2020 at 08:48:22AM +0200, Paolo Bonzini wrote:
> On Tue, Aug 25, 2020 at 11:38 PM Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
> > Marc-André had a query about why this is marked as a system include, however I can
> > confirm that it fixes the missing "SDL.h" issue during build.
> 
> It was marked as a system include in an attempt to work around the SDL
> 2.0.8 bug that requires -Wno-undef. In general we enable lots of
> warnings and sometimes they trip dependencies, so using include_type:
> 'system' in principle makes sense. But if it doesn't work with
> Windows, it's not a regression to remove it.

SDL code is isolated to just a couple of files, so if we nede to squelch
a -Wno-undef warning, we can just use pragma push/pop to disable the
warning selectively in the code.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 1/4] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja

[Mark Cave-Ayland]

On 26/08/2020 07:45, Paolo Bonzini wrote:

> On Tue, Aug 25, 2020 at 11:26 PM Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> I've tested this and it changes build.ninja so instead of Windows paths beginning C$$
>> they now begin C$ instead e.g.:
>>
>> build qemu-version.h: CUSTOM_COMMAND  |
>> C$:/msys64/home/Mark/qemu/scripts/qemu-version.sh PHONY
> 
> The patch should not change build.ninja in any way, but indeed it will
> fix the transformation so that the (correct) ninja quoting is removed.

It definitely changes build.ninja here, with the escaping changing from C$$:\ to C$:\
in my tests. But if you're saying that $: with just a single $ is the correct
escaping for colon then I guess the patch is fine (and indeed it works for me with
the $ now being removed in the transformation to Makefile.ninja).


ATB,

Mark.

Re: [PATCH 1/4] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

Il gio 27 ago 2020, 18:05 Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ha scritto:

> On 26/08/2020 07:45, Paolo Bonzini wrote:
>
> > On Tue, Aug 25, 2020 at 11:26 PM Mark Cave-Ayland
> > <mark.cave-ayland@ilande.co.uk> wrote:
> >> I've tested this and it changes build.ninja so instead of Windows paths
> beginning C$$
> >> they now begin C$ instead e.g.:
> >>
> >> build qemu-version.h: CUSTOM_COMMAND  |
> >> C$:/msys64/home/Mark/qemu/scripts/qemu-version.sh PHONY
> >
> > The patch should not change build.ninja in any way, but indeed it will
> > fix the transformation so that the (correct) ninja quoting is removed.
>
> It definitely changes build.ninja here, with the escaping changing from
> C$$:\ to C$:\
> in my tests.


Yes, the ^ makes no difference but I missed that it also adds a $.

Thanks for testing!!

Paolo

if you're saying that $: with just a single $ is the correct
> escaping for colon then I guess the patch is fine (and indeed it works for
> me with
> the $ now being removed in the transformation to Makefile.ninja).
>
>
> ATB,
>
> Mark.
>
>

[-- Attachment #2: Type: text/html, Size: 1903 bytes --]

[PATCH 1/4] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja

[luoyonggang]

From: Yonggang Luo <luoyonggang@gmail.com>

SIMPLE_PATH_RE should match the full path token.
Or the $ and : contained in path would not matched if the path are start with C:/ and E:/
---
 scripts/ninjatool.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
index cc77d51aa8..6ca8be6f10 100755
--- a/scripts/ninjatool.py
+++ b/scripts/ninjatool.py
@@ -55,7 +55,7 @@ else:
 
 PATH_RE = r"[^$\s:|]+|\$[$ :]|\$[a-zA-Z0-9_-]+|\$\{[a-zA-Z0-9_.-]+\}"
 
-SIMPLE_PATH_RE = re.compile(r"[^$\s:|]+")
+SIMPLE_PATH_RE = re.compile(r"^[^$\s:|]+$")
 IDENT_RE = re.compile(r"[a-zA-Z0-9_.-]+$")
 STRING_RE = re.compile(r"(" + PATH_RE + r"|[\s:|])(?:\r?\n)?|.")
 TOPLEVEL_RE = re.compile(r"([=:#]|\|\|?|^ +|(?:" + PATH_RE + r")+)\s*|.")
-- 
2.27.0.windows.1