Re: [Qemu-devel] [PATCH for-2.9] Fix check for target OS support

[Peter Maydell]

On 27 March 2017 at 21:11, Stefan Weil <address@hidden> wrote:
> This check had several problems which are fixed here:
>
> * Calling "configure --help" was no longer possible on Cygwin.
>   Fix  this by introducing a third state for supported_os and
>   by moving the error handling code.
>   Move the error handling code for supported_cpu, too.
>
> * Fix the error text. Use target cpu and target os because the
>   build cpu and build host don't matter here.

I think it is important that we print the deprecation
message last, after all the other configure output.
Otherwise it will scroll off the screen and most people
will miss it. That's why I put it where it is.

> * Support cross compilation with the most common cross prefixes
>   for Mingw-w64. Other cross builds are still broken!
>
> Signed-off-by: Stefan Weil <address@hidden>
> ---
>
> This fixes a regression: building QEMU for Windows with a cross
> build under Cygwin is currently broken.
>
> Can the patch be applied directly, or who should send a pull request?
>
> Stefan
>
>  configure | 68 
> +++++++++++++++++++++++++++++++++++----------------------------
>  1 file changed, 38 insertions(+), 30 deletions(-)
>
> diff --git a/configure b/configure
> index d1ce33bc79..902bd20070 100755
> --- a/configure
> +++ b/configure
> @@ -322,7 +322,7 @@ jemalloc="no"
>  replication="yes"
>
>  supported_cpu="no"
> -supported_os="no"
> +supported_os="deprecated"

I don't see any need for anything other than 'yes' or 'no' ?

>
>  # parse CC options first
>  for opt do
> @@ -440,7 +440,13 @@ int main(void) { return 0; }
>  EOF
>  }
>
> -if check_define __linux__ ; then
> +if test "$cross_prefix" = "i686_64-w64-mingw32-" ; then
> +  targetos='MINGW32'
> +elif test "$cross_prefix" = "x86_64-w64-mingw32-" ; then
> +  targetos='MINGW32'
> +elif test -n "$cross_prefix" ; then
> +  targetos="$cross_prefix"

This doesn't look right. We already have too many
cases where we determine the target OS by something
other than looking at check_defines; one thing
I'd like to do when we've got rid of unsupported
OSes is to make sure that we determine the target
OS by check_define and only by check_define.

> +elif check_define __linux__ ; then
>    targetos="Linux"
>  elif check_define _WIN32 ; then
>    targetos='MINGW32'
> @@ -694,7 +700,7 @@ Linux)
>    supported_os="yes"
>  ;;
>  *)
> -  error_exit "Unsupported host OS $targetos"
> +  supported_os="no"

This was deliberately an error_exit because anything
going down that path got the Linux defines/includes by
accident and we thought that would not work on
anything else. What was using it? If we need an extra
entry in the case statement we can add one.

>  ;;
>  esac
>
> @@ -1428,6 +1434,34 @@ EOF
>  exit 0
>  fi

>  config_host_mak="config-host.mak"
>
>  echo "# Automatically generated by configure - do not modify" 
> >config-all-disas.mak
> @@ -5212,7 +5220,7 @@ if test "$mingw32" = "yes" ; then
>      echo "CONFIG_QGA_NTDDDISK=y" >> $config_host_mak
>    fi
>    if test "$guest_agent_msi" = "yes"; then
> -    echo "QEMU_GA_MSI_ENABLED=yes" >> $config_host_mak
> +    echo "QEMU_GA_MSI_ENABLED=yes" >> $config_host_mak
>      echo "QEMU_GA_MSI_MINGW_DLL_PATH=${QEMU_GA_MSI_MINGW_DLL_PATH}" >> 
> $config_host_mak
>      echo "QEMU_GA_MSI_WITH_VSS=${QEMU_GA_MSI_WITH_VSS}" >> $config_host_mak
>      echo "QEMU_GA_MSI_ARCH=${QEMU_GA_MSI_ARCH}" >> $config_host_mak

Stray unintended change?

> --
> 2.11.0
>

thanks
-- PMM