| Message ID | 20200826151006.80-4-luoyonggang@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/6] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja | expand |
I'm a bit wary of this patch, the effects are quite wide-ranging. If we move all the detection of dependencies to meson, it will take a while but we should get a similar effect. However, I'm testing and queuing patches 1 to 3. Paolo On Wed, Aug 26, 2020 at 5:13 PM <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 > > -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 >
On Wed, Aug 26, 2020 at 11:24 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > I'm a bit wary of this patch, the effects are quite wide-ranging. If > we move all the detection of dependencies to meson, it will take a > while but we should get a similar effect. > Only on MINGW the $PWD sematic are changed, so I don't think there is side effect of this patch. > > However, I'm testing and queuing patches 1 to 3. > > Paolo > > On Wed, Aug 26, 2020 at 5:13 PM <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 > > > > -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 > > > >
On Wed, Aug 26, 2020 at 5:33 PM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> wrote: > > > On Wed, Aug 26, 2020 at 11:24 PM Paolo Bonzini <pbonzini@redhat.com> wrote: >> I'm a bit wary of this patch, the effects are quite wide-ranging. If >> we move all the detection of dependencies to meson, it will take a >> while but we should get a similar effect. > > Only on MINGW the $PWD sematic are changed, so I don't think there is side effect of this patch. Yes the side effect is only on MINGW but the affected variables are used everywhere so it's a rather hard to review change. Paolo
On Wed, Aug 26, 2020 at 11:36 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > On Wed, Aug 26, 2020 at 5:33 PM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> > wrote: > > > > > > On Wed, Aug 26, 2020 at 11:24 PM Paolo Bonzini <pbonzini@redhat.com> > wrote: > >> I'm a bit wary of this patch, the effects are quite wide-ranging. If > >> we move all the detection of dependencies to meson, it will take a > >> while but we should get a similar effect. > > > > Only on MINGW the $PWD sematic are changed, so I don't think there is > side effect of this patch. > > Yes the side effect is only on MINGW but the affected variables are > used everywhere so it's a rather hard to review change. > Gotcha, review it carefully > > Paolo > >
On 26/08/2020 16:10, 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 This still doesn't match the existing indentation as per my previous comment. > +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 Same again here too. > +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" Is the change to this last section for the NINJA variable really required to fix linking? It would be useful to keep the NINJA variable and executable detection fix as a separate patch if possible. ATB, Mark.
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"