[Qemu-devel] [PATCH] Check for sdl-config before calling it

[Qemu-devel] [PATCH] Check for sdl-config before calling it

[Loïc Minier]

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

        Hi

 On systems were sdl-config isn't installed, ./configure triggers this
 warning:
    ./configure: 957: sdl-config: not found

 The attached patch fixes the warning for me.

   Thanks,
-- 
Loïc Minier

[-- Attachment #2: 0001-Check-for-sdl-config-before-calling-it.patch --]
[-- Type: text/x-diff, Size: 1150 bytes --]

>From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Sun, 17 Jan 2010 13:42:04 +0100
Subject: [PATCH] Check for sdl-config before calling it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Check whether sdl-config is available before calling it, otherwise
./configure triggers a warning:
    ./configure: 957: sdl-config: not found

If neither the .pc file not sdl-config are present, disable SDL support.

Signed-off-by: Loïc Minier <lool@dooz.org>
---
 configure |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 5631bbb..450e1a2 100755
--- a/configure
+++ b/configure
@@ -993,9 +993,11 @@ fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-else
+elif which sdl-config >/dev/null 2>&1; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
+else
+  sdl=no
 fi
 
 sdl_too_old=no
-- 
1.6.5

[Qemu-devel] Re: [PATCH] Check for sdl-config before calling it

[Måns Rullgård]

Loïc Minier <lool@dooz.org> writes:

>         Hi
>
>  On systems were sdl-config isn't installed, ./configure triggers this
>  warning:
>     ./configure: 957: sdl-config: not found
>
>  The attached patch fixes the warning for me.
>
>    Thanks,
> -- 
> Loïc Minier
>
> From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
> Date: Sun, 17 Jan 2010 13:42:04 +0100
> Subject: [PATCH] Check for sdl-config before calling it
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Check whether sdl-config is available before calling it, otherwise
> ./configure triggers a warning:
>     ./configure: 957: sdl-config: not found
>
> If neither the .pc file not sdl-config are present, disable SDL support.
>
> Signed-off-by: Loïc Minier <lool@dooz.org>
> ---
>  configure |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index 5631bbb..450e1a2 100755
> --- a/configure
> +++ b/configure
> @@ -993,9 +993,11 @@ fi
>  if $pkgconfig sdl --modversion >/dev/null 2>&1; then
>    sdlconfig="$pkgconfig sdl"
>    _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
> -else
> +elif which sdl-config >/dev/null 2>&1; then
>    sdlconfig='sdl-config'
>    _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
> +else
> +  sdl=no
>  fi

"which" isn't a standard command.  Would simply 2>/dev/null do the
trick just as well?  If you really need to test for the existence of
something, use "type".

-- 
Måns Rullgård
mans@mansr.com

Re: [Qemu-devel] Re: [PATCH] Check for sdl-config before calling it

[Stefan Weil]

Måns Rullgård schrieb:
> Loïc Minier <lool@dooz.org> writes:
>
>   
>>         Hi
>>
>>  On systems were sdl-config isn't installed, ./configure triggers this
>>  warning:
>>     ./configure: 957: sdl-config: not found
>>
>>  The attached patch fixes the warning for me.
>>
>>    Thanks,
>> -- 
>> Loïc Minier
>>
>> From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
>> Date: Sun, 17 Jan 2010 13:42:04 +0100
>> Subject: [PATCH] Check for sdl-config before calling it
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> Check whether sdl-config is available before calling it, otherwise
>> ./configure triggers a warning:
>>     ./configure: 957: sdl-config: not found
>>
>> If neither the .pc file not sdl-config are present, disable SDL support.
>>
>> Signed-off-by: Loïc Minier <lool@dooz.org>
>> ---
>>  configure |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 5631bbb..450e1a2 100755
>> --- a/configure
>> +++ b/configure
>> @@ -993,9 +993,11 @@ fi
>>  if $pkgconfig sdl --modversion >/dev/null 2>&1; then
>>    sdlconfig="$pkgconfig sdl"
>>    _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
>> -else
>> +elif which sdl-config >/dev/null 2>&1; then
>>    sdlconfig='sdl-config'
>>    _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
>> +else
>> +  sdl=no
>>  fi
>>     
>
> "which" isn't a standard command.  Would simply 2>/dev/null do the
> trick just as well?  If you really need to test for the existence of
> something, use "type".
>   

"which" is already used several times in configure,
so this would not make things worse.

A macro for all these tests might be a better solution.
And that macro should use "type" or "type -t".

Regards,
Stefan

[Qemu-devel] Re: [PATCH] Check for sdl-config before calling it

[Måns Rullgård]

Stefan Weil <weil@mail.berlios.de> writes:

> Måns Rullgård schrieb:
>> Loïc Minier <lool@dooz.org> writes:
>>
>>   
>>>         Hi
>>>
>>>  On systems were sdl-config isn't installed, ./configure triggers this
>>>  warning:
>>>     ./configure: 957: sdl-config: not found
>>>
>>>  The attached patch fixes the warning for me.
>>>
>>>    Thanks,
>>> -- 
>>> Loïc Minier
>>>
>>> From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
>>> Date: Sun, 17 Jan 2010 13:42:04 +0100
>>> Subject: [PATCH] Check for sdl-config before calling it
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> Check whether sdl-config is available before calling it, otherwise
>>> ./configure triggers a warning:
>>>     ./configure: 957: sdl-config: not found
>>>
>>> If neither the .pc file not sdl-config are present, disable SDL support.
>>>
>>> Signed-off-by: Loïc Minier <lool@dooz.org>
>>> ---
>>>  configure |    4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index 5631bbb..450e1a2 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -993,9 +993,11 @@ fi
>>>  if $pkgconfig sdl --modversion >/dev/null 2>&1; then
>>>    sdlconfig="$pkgconfig sdl"
>>>    _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
>>> -else
>>> +elif which sdl-config >/dev/null 2>&1; then
>>>    sdlconfig='sdl-config'
>>>    _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
>>> +else
>>> +  sdl=no
>>>  fi
>>>     
>>
>> "which" isn't a standard command.  Would simply 2>/dev/null do the
>> trick just as well?  If you really need to test for the existence of
>> something, use "type".
>>   
>
> "which" is already used several times in configure,

Then those should be fixed.

> so this would not make things worse.

One error does not justify another.

> A macro for all these tests might be a better solution.
> And that macro should use "type" or "type -t".

"type -t" is also not standard.  The standard "type" has no options.

In most cases the simpler solution is still probably to try the
command and let it fail.

-- 
Måns Rullgård
mans@mansr.com

[Qemu-devel] Stop using "which" in ./configure

[Loïc Minier]

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

        Hi

 Following the thread on the sdl-config patch, please find attached a
 patch to add a couple of portable shell functions which allow testing
 whehter a command/builtin is available and to find the full pathname of
 an executable in the PATH.  This also replaces all uses of "which" in
 ./configure.  (This should be applied on top of the sdl-config patch.)

   Thanks,
-- 
Loïc Minier

[-- Attachment #2: 0001-Add-and-use-has-and-path_of-funcs.patch --]
[-- Type: text/x-diff, Size: 4257 bytes --]

>From 815ca36e9b3e2c56040d7fc7709c95d600ed6496 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Tue, 19 Jan 2010 11:05:00 +0100
Subject: [PATCH] Add and use has() and path_of() funcs

Add has() and path_of() funcs and use them across configure; has()
will test whether a command or builtin is available; path_of() will
search the PATH for executables and return the full pathname if found.
---
 configure |   57 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index baa2800..8d67b9c 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,47 @@ compile_prog() {
   $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null
 }
 
+# check whether a command is available to this shell (may be either an
+# executable or a builtin)
+has() {
+    local_command="$1"
+    type "$local_command" >/dev/null
+}
+
+# search for an executable in PATH
+path_of() {
+    local_command="$1"
+    local_path="$PATH"
+    local_dir=""
+    # absolute path?
+    if [ "${local_command#*/}" != "$local_command" ]; then
+        if [ -x "$local_command" ] && [ ! -d "$local_command" ]; then
+            echo "$local_command"
+            return 0
+        fi
+    fi
+    if [ -z "$local_path" ] || [ -z "$local_command" ]; then
+        return 1
+    fi
+    while :; do
+        local_dir="${local_path%%:*}"
+        if [ "${local_path#*:}" = "$local_path" ]; then
+            # last loop
+            local_path=""
+        else
+            local_path="${local_path#*:}"
+        fi
+        if [ -x "$local_dir/$local_command" ] && [ ! -d "$local_dir/$local_command" ]; then
+            echo "$local_dir/$local_command"
+            return 0
+        fi
+        if [ -z "$local_path" ]; then
+            # not found
+            return 1
+        fi
+    done
+}
+
 # default parameters
 cpu=""
 prefix=""
@@ -763,7 +804,7 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"`
+  solinst=`path_of $install`
   if test -z "$solinst" ; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
@@ -776,7 +817,7 @@ if test "$solaris" = "yes" ; then
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"`
+  sol_ar=`path_of ar`
   if test -z "$sol_ar" ; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
@@ -969,7 +1010,7 @@ fi
 # pkgconfig probe
 
 pkgconfig="${cross_prefix}pkg-config"
-if ! test -x "$(which $pkgconfig 2>/dev/null)"; then
+if ! has $pkgconfig; then
   # likely not cross compiling, or hope for the best
   pkgconfig=pkg-config
 fi
@@ -977,7 +1018,7 @@ fi
 ##########################################
 # Sparse probe
 if test "$sparse" != "no" ; then
-  if test -x "$(which cgcc 2>/dev/null)"; then
+  if has cgcc; then
     sparse=yes
   else
     if test "$sparse" = "yes" ; then
@@ -993,7 +1034,7 @@ fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-elif which sdl-config >/dev/null 2>&1; then
+elif has sdl-config; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
 else
@@ -1424,8 +1465,7 @@ EOF
     fi
   else
     if test "$kvm" = "yes" ; then
-      if [ -x "`which awk 2>/dev/null`" ] && \
-         [ -x "`which grep 2>/dev/null`" ]; then
+      if has awk && has grep; then
         kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \
 	| grep "error: " \
 	| awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'`
@@ -1694,8 +1734,7 @@ fi
 
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
-  if test -x "`which texi2html 2>/dev/null`" -a \
-          -x "`which pod2man 2>/dev/null`" ; then
+  if has texi2html && has pod2man; then
     docs=yes
   else
     if test "$docs" = "yes" ; then
-- 
1.6.5

Re: [Qemu-devel] Stop using "which" in ./configure

[Loïc Minier]

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

On Tue, Jan 19, 2010, Loïc Minier wrote:
>  Following the thread on the sdl-config patch, please find attached a
>  patch to add a couple of portable shell functions which allow testing
>  whehter a command/builtin is available and to find the full pathname of
>  an executable in the PATH.  This also replaces all uses of "which" in
>  ./configure.  (This should be applied on top of the sdl-config patch.)

 Please find attached a new version of the patch with a simpler version
 of path_of() which uses IFS instead of the ${foo#bar} and ${foo%%bar}
 constructs.  It also removes the special casing of an empty PATH.

-- 
Loïc Minier

[-- Attachment #2: 0001-Add-and-use-has-and-path_of-funcs.patch --]
[-- Type: text/x-diff, Size: 4040 bytes --]

>From 5fc05ec61d87049ea0f29b2dd51c16e260698ef8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Tue, 19 Jan 2010 11:05:00 +0100
Subject: [PATCH] Add and use has() and path_of() funcs

Add has() and path_of() funcs and use them across configure; has()
will test whether a command or builtin is available; path_of() will
search the PATH for executables and return the full pathname if found.
---
 configure |   53 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index baa2800..711e335 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,43 @@ compile_prog() {
   $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null
 }
 
+# check whether a command is available to this shell (may be either an
+# executable or a builtin)
+has() {
+    local_command="$1"
+    type "$local_command" >/dev/null
+}
+
+# search for an executable in PATH
+path_of() {
+    local_command="$1"
+    local_ifs="$IFS"
+    local_dir=""
+
+    # pathname has a dir component?
+    if [ "${local_command#*/}" != "$local_command" ]; then
+        if [ -x "$local_command" ] && [ ! -d "$local_command" ]; then
+            echo "$local_command"
+            return 0
+        fi
+    fi
+    if [ -z "$local_command" ]; then
+        return 1
+    fi
+
+    IFS=:
+    for local_dir in $PATH; do
+        if [ -x "$local_dir/$local_command" ] && [ ! -d "$local_dir/$local_command" ]; then
+            echo "$local_dir/$local_command"
+            IFS="$local_ifs"
+            return 0
+        fi
+    done
+    # not found
+    IFS="$local_ifs"
+    return 1
+}
+
 # default parameters
 cpu=""
 prefix=""
@@ -763,7 +800,7 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"`
+  solinst=`path_of $install`
   if test -z "$solinst" ; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
@@ -776,7 +813,7 @@ if test "$solaris" = "yes" ; then
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"`
+  sol_ar=`path_of ar`
   if test -z "$sol_ar" ; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
@@ -969,7 +1006,7 @@ fi
 # pkgconfig probe
 
 pkgconfig="${cross_prefix}pkg-config"
-if ! test -x "$(which $pkgconfig 2>/dev/null)"; then
+if ! has $pkgconfig; then
   # likely not cross compiling, or hope for the best
   pkgconfig=pkg-config
 fi
@@ -977,7 +1014,7 @@ fi
 ##########################################
 # Sparse probe
 if test "$sparse" != "no" ; then
-  if test -x "$(which cgcc 2>/dev/null)"; then
+  if has cgcc; then
     sparse=yes
   else
     if test "$sparse" = "yes" ; then
@@ -993,7 +1030,7 @@ fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-elif which sdl-config >/dev/null 2>&1; then
+elif has sdl-config; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
 else
@@ -1424,8 +1461,7 @@ EOF
     fi
   else
     if test "$kvm" = "yes" ; then
-      if [ -x "`which awk 2>/dev/null`" ] && \
-         [ -x "`which grep 2>/dev/null`" ]; then
+      if has awk && has grep; then
         kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \
 	| grep "error: " \
 	| awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'`
@@ -1694,8 +1730,7 @@ fi
 
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
-  if test -x "`which texi2html 2>/dev/null`" -a \
-          -x "`which pod2man 2>/dev/null`" ; then
+  if has texi2html && has pod2man; then
     docs=yes
   else
     if test "$docs" = "yes" ; then
-- 
1.6.5

Re: [Qemu-devel] Stop using "which" in ./configure

[Stefan Weil]

Loïc Minier schrieb:
> On Tue, Jan 19, 2010, Loïc Minier wrote:
>   
>>  Following the thread on the sdl-config patch, please find attached a
>>  patch to add a couple of portable shell functions which allow testing
>>  whehter a command/builtin is available and to find the full pathname of
>>  an executable in the PATH.  This also replaces all uses of "which" in
>>  ./configure.  (This should be applied on top of the sdl-config patch.)
>>     
>
>  Please find attached a new version of the patch with a simpler version
>  of path_of() which uses IFS instead of the ${foo#bar} and ${foo%%bar}
>  constructs.  It also removes the special casing of an empty PATH.

I did not test the whole patch, but I think this would be better:

+    type "$local_command" >/dev/null 2>&1

(type sends error messages to stderr, we don't want to see them)

Re: [Qemu-devel] Stop using "which" in ./configure

[Loïc Minier]

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

On Tue, Jan 19, 2010, Stefan Weil wrote:
> I did not test the whole patch, but I think this would be better:
> +    type "$local_command" >/dev/null 2>&1

 Attaching an updated patch

    Thanks
-- 
Loïc Minier

[-- Attachment #2: 0001-Add-and-use-has-and-path_of-funcs.patch --]
[-- Type: text/x-diff, Size: 4045 bytes --]

>From 1c0b63fb9fc735a6d367a65a6ed1b998942fb6a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Tue, 19 Jan 2010 11:05:00 +0100
Subject: [PATCH] Add and use has() and path_of() funcs

Add has() and path_of() funcs and use them across configure; has()
will test whether a command or builtin is available; path_of() will
search the PATH for executables and return the full pathname if found.
---
 configure |   53 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index baa2800..db97a2c 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,43 @@ compile_prog() {
   $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null
 }
 
+# check whether a command is available to this shell (may be either an
+# executable or a builtin)
+has() {
+    local_command="$1"
+    type "$local_command" >/dev/null 2>&1
+}
+
+# search for an executable in PATH
+path_of() {
+    local_command="$1"
+    local_ifs="$IFS"
+    local_dir=""
+
+    # pathname has a dir component?
+    if [ "${local_command#*/}" != "$local_command" ]; then
+        if [ -x "$local_command" ] && [ ! -d "$local_command" ]; then
+            echo "$local_command"
+            return 0
+        fi
+    fi
+    if [ -z "$local_command" ]; then
+        return 1
+    fi
+
+    IFS=:
+    for local_dir in $PATH; do
+        if [ -x "$local_dir/$local_command" ] && [ ! -d "$local_dir/$local_command" ]; then
+            echo "$local_dir/$local_command"
+            IFS="$local_ifs"
+            return 0
+        fi
+    done
+    # not found
+    IFS="$local_ifs"
+    return 1
+}
+
 # default parameters
 cpu=""
 prefix=""
@@ -763,7 +800,7 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"`
+  solinst=`path_of $install`
   if test -z "$solinst" ; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
@@ -776,7 +813,7 @@ if test "$solaris" = "yes" ; then
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"`
+  sol_ar=`path_of ar`
   if test -z "$sol_ar" ; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
@@ -969,7 +1006,7 @@ fi
 # pkgconfig probe
 
 pkgconfig="${cross_prefix}pkg-config"
-if ! test -x "$(which $pkgconfig 2>/dev/null)"; then
+if ! has $pkgconfig; then
   # likely not cross compiling, or hope for the best
   pkgconfig=pkg-config
 fi
@@ -977,7 +1014,7 @@ fi
 ##########################################
 # Sparse probe
 if test "$sparse" != "no" ; then
-  if test -x "$(which cgcc 2>/dev/null)"; then
+  if has cgcc; then
     sparse=yes
   else
     if test "$sparse" = "yes" ; then
@@ -993,7 +1030,7 @@ fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-elif which sdl-config >/dev/null 2>&1; then
+elif has sdl-config; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
 else
@@ -1424,8 +1461,7 @@ EOF
     fi
   else
     if test "$kvm" = "yes" ; then
-      if [ -x "`which awk 2>/dev/null`" ] && \
-         [ -x "`which grep 2>/dev/null`" ]; then
+      if has awk && has grep; then
         kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \
 	| grep "error: " \
 	| awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'`
@@ -1694,8 +1730,7 @@ fi
 
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
-  if test -x "`which texi2html 2>/dev/null`" -a \
-          -x "`which pod2man 2>/dev/null`" ; then
+  if has texi2html && has pod2man; then
     docs=yes
   else
     if test "$docs" = "yes" ; then
-- 
1.6.5

[Qemu-devel] Re: Stop using "which" in ./configure

[Måns Rullgård]

Loïc Minier <lool@dooz.org> writes:

> On Tue, Jan 19, 2010, Loïc Minier wrote:
>>  Following the thread on the sdl-config patch, please find attached a
>>  patch to add a couple of portable shell functions which allow testing
>>  whehter a command/builtin is available and to find the full pathname of
>>  an executable in the PATH.  This also replaces all uses of "which" in
>>  ./configure.  (This should be applied on top of the sdl-config patch.)
>
>  Please find attached a new version of the patch with a simpler version
>  of path_of() which uses IFS instead of the ${foo#bar} and ${foo%%bar}
>  constructs.  It also removes the special casing of an empty PATH.
>
> -- 
> Loïc Minier
>
> From 5fc05ec61d87049ea0f29b2dd51c16e260698ef8 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
> Date: Tue, 19 Jan 2010 11:05:00 +0100
> Subject: [PATCH] Add and use has() and path_of() funcs
>
> Add has() and path_of() funcs and use them across configure; has()
> will test whether a command or builtin is available; path_of() will
> search the PATH for executables and return the full pathname if found.
> ---
>  configure |   53 ++++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/configure b/configure
> index baa2800..711e335 100755
> --- a/configure
> +++ b/configure
> @@ -27,6 +27,43 @@ compile_prog() {
>    $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null
>  }
>  
> +# check whether a command is available to this shell (may be either an
> +# executable or a builtin)
> +has() {
> +    local_command="$1"
> +    type "$local_command" >/dev/null
> +}

Why the extra variable?  Using $1 directly seems just as obvious to me.

> +# search for an executable in PATH
> +path_of() {
> +    local_command="$1"
> +    local_ifs="$IFS"
> +    local_dir=""
> +
> +    # pathname has a dir component?
> +    if [ "${local_command#*/}" != "$local_command" ]; then
> +        if [ -x "$local_command" ] && [ ! -d "$local_command" ]; then
> +            echo "$local_command"
> +            return 0
> +        fi
> +    fi
> +    if [ -z "$local_command" ]; then
> +        return 1
> +    fi
> +
> +    IFS=:
> +    for local_dir in $PATH; do
> +        if [ -x "$local_dir/$local_command" ] && [ ! -d "$local_dir/$local_command" ]; then
> +            echo "$local_dir/$local_command"
> +            IFS="$local_ifs"
> +            return 0
> +        fi
> +    done
> +    # not found
> +    IFS="$local_ifs"
> +    return 1
> +}
> +
>  # default parameters
>  cpu=""
>  prefix=""
> @@ -763,7 +800,7 @@ fi
>  # Solaris specific configure tool chain decisions
>  #
>  if test "$solaris" = "yes" ; then
> -  solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"`
> +  solinst=`path_of $install`
>    if test -z "$solinst" ; then
>      echo "Solaris install program not found. Use --install=/usr/ucb/install or"
>      echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
> @@ -776,7 +813,7 @@ if test "$solaris" = "yes" ; then
>      echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
>      exit 1
>    fi
> -  sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"`
> +  sol_ar=`path_of ar`
>    if test -z "$sol_ar" ; then
>      echo "Error: No path includes ar"
>      if test -f /usr/ccs/bin/ar ; then

Is the full path of these tools really important?  Doesn't look like
it to me.

-- 
Måns Rullgård
mans@mansr.com

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Loïc Minier]

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

On Tue, Jan 19, 2010, Måns Rullgård wrote:
[...]
> Why the extra variable?  Using $1 directly seems just as obvious to me.
[...]

 I'm attaching an updated patch which doesn't use this variable.  Should
 be applied after the sdl-config patch.

> Is the full path of these tools really important?  Doesn't look like
> it to me.

 I'm attaching a new patch which changes the tests a bit; I would prefer
 if someone with access to a Solaris build environment would do this
 though.

-- 
Loïc Minier

[-- Attachment #2: 0001-Add-and-use-has-and-path_of-funcs.patch --]
[-- Type: text/plain, Size: 4012 bytes --]

>From 27f151ef7be19fcffbbf9c2d3c6ee50750be854d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Tue, 19 Jan 2010 11:05:00 +0100
Subject: [PATCH 1/2] Add and use has() and path_of() funcs

Add has() and path_of() funcs and use them across configure; has()
will test whether a command or builtin is available; path_of() will
search the PATH for executables and return the full pathname if found.
---
 configure |   52 +++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index baa2800..90b3c18 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,42 @@ compile_prog() {
   $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null
 }
 
+# check whether a command is available to this shell (may be either an
+# executable or a builtin)
+has() {
+    type "$1" >/dev/null 2>&1
+}
+
+# search for an executable in PATH
+path_of() {
+    local_command="$1"
+    local_ifs="$IFS"
+    local_dir=""
+
+    # pathname has a dir component?
+    if [ "${local_command#*/}" != "$local_command" ]; then
+        if [ -x "$local_command" ] && [ ! -d "$local_command" ]; then
+            echo "$local_command"
+            return 0
+        fi
+    fi
+    if [ -z "$local_command" ]; then
+        return 1
+    fi
+
+    IFS=:
+    for local_dir in $PATH; do
+        if [ -x "$local_dir/$local_command" ] && [ ! -d "$local_dir/$local_command" ]; then
+            echo "$local_dir/$local_command"
+            IFS="$local_ifs"
+            return 0
+        fi
+    done
+    # not found
+    IFS="$local_ifs"
+    return 1
+}
+
 # default parameters
 cpu=""
 prefix=""
@@ -763,7 +799,7 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"`
+  solinst=`path_of $install`
   if test -z "$solinst" ; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
@@ -776,7 +812,7 @@ if test "$solaris" = "yes" ; then
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"`
+  sol_ar=`path_of ar`
   if test -z "$sol_ar" ; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
@@ -969,7 +1005,7 @@ fi
 # pkgconfig probe
 
 pkgconfig="${cross_prefix}pkg-config"
-if ! test -x "$(which $pkgconfig 2>/dev/null)"; then
+if ! has $pkgconfig; then
   # likely not cross compiling, or hope for the best
   pkgconfig=pkg-config
 fi
@@ -977,7 +1013,7 @@ fi
 ##########################################
 # Sparse probe
 if test "$sparse" != "no" ; then
-  if test -x "$(which cgcc 2>/dev/null)"; then
+  if has cgcc; then
     sparse=yes
   else
     if test "$sparse" = "yes" ; then
@@ -993,7 +1029,7 @@ fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-elif which sdl-config >/dev/null 2>&1; then
+elif has sdl-config; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
 else
@@ -1424,8 +1460,7 @@ EOF
     fi
   else
     if test "$kvm" = "yes" ; then
-      if [ -x "`which awk 2>/dev/null`" ] && \
-         [ -x "`which grep 2>/dev/null`" ]; then
+      if has awk && has grep; then
         kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \
 	| grep "error: " \
 	| awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'`
@@ -1694,8 +1729,7 @@ fi
 
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
-  if test -x "`which texi2html 2>/dev/null`" -a \
-          -x "`which pod2man 2>/dev/null`" ; then
+  if has texi2html && has pod2man; then
     docs=yes
   else
     if test "$docs" = "yes" ; then
-- 
1.6.5


[-- Attachment #3: 0002-Solaris-test-for-presence-of-commands-with-has.patch --]
[-- Type: text/plain, Size: 1467 bytes --]

>From 421c7a48b5e0a20ae09014cd67e94f3c0ba26a20 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Wed, 20 Jan 2010 12:35:54 +0100
Subject: [PATCH 2/2] Solaris: test for presence of commands with has()

---
 configure |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 90b3c18..7e842ce 100755
--- a/configure
+++ b/configure
@@ -799,21 +799,19 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`path_of $install`
-  if test -z "$solinst" ; then
+  if has $install; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
     echo "to get ginstall which is used by default (which lives in /opt/csw/bin)"
     exit 1
   fi
-  if test "$solinst" = "/usr/sbin/install" ; then
+  if "`path_of $install`" = "/usr/sbin/install" ; then
     echo "Error: Solaris /usr/sbin/install is not an appropriate install program."
     echo "try ginstall from the GNU fileutils available from www.blastwave.org"
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`path_of ar`
-  if test -z "$sol_ar" ; then
+  if has ar; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
       echo "Add /usr/ccs/bin to your path and rerun configure"
-- 
1.6.5

[Qemu-devel] Re: Stop using "which" in ./configure

[Paolo Bonzini]

On 01/20/2010 12:37 PM, Loïc Minier wrote:
> +    # not found
> +    IFS="$local_ifs"

If you do this, you should set IFS to space-tab-lf at the beginning of 
the script, like this:

IFS=" ""	""
"

or this (better because it does not rely on embedding whitespace 
characters within the line):

IFS=`printf ' \t'`"
"

Paolo

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Loïc Minier]

On Wed, Jan 20, 2010, Paolo Bonzini wrote:
> On 01/20/2010 12:37 PM, Loïc Minier wrote:
> >+    # not found
> >+    IFS="$local_ifs"
> If you do this, you should set IFS to space-tab-lf at the beginning of 
> the script, like this:
> 
> IFS=" ""	""
> "

 Are you saying that I can't backup/restore IFS without setting it
 first?  That would be odd; it works with bash, dash, and zsh here.
 Pointers to affected shell would help me avoid other issues with them.

 Alternatively, I could set IFS in a subshell.

 I checked the autoconf Portable Shell Programming section, but didn't
 find a similar recommendation:
   http://www.gnu.org/software/autoconf/manual/autoconf.html#index-IFS-1639

> or this (better because it does not rely on embedding whitespace 
> characters within the line):
> IFS=`printf ' \t'`"
> "

 If we go that route, perhaps IFS="`printf ' \t\n'`" would be more
 readable?  I'm not sure how common printf is though.

-- 
Loïc Minier

[Qemu-devel] Re: Stop using "which" in ./configure

[Paolo Bonzini]

On 01/20/2010 02:49 PM, Loïc Minier wrote:
> On Wed, Jan 20, 2010, Paolo Bonzini wrote:
>> On 01/20/2010 12:37 PM, Loïc Minier wrote:
>>> +    # not found
>>> +    IFS="$local_ifs"
>> If you do this, you should set IFS to space-tab-lf at the beginning of
>> the script, like this:
>>
>> IFS=" ""	""
>> "
>
>   Are you saying that I can't backup/restore IFS without setting it
>   first?

Yes, it affects the behavior of read for example:

$ echo a b c | (read a b c; echo $a; echo $b; echo $c)
a
b
c
$ IFS=
$ echo a b c | (read a b c; echo $a; echo $b; echo $c)
a b c


$

(It's not used by QEMU's configure, but it's better to be defensive).

>> or this (better because it does not rely on embedding whitespace
>> characters within the line):
>> IFS=`printf ' \t'`"
>> "
>
>   If we go that route, perhaps IFS="`printf ' \t\n'`" would be more
>   readable?  I'm not sure how common printf is though.

I tried that, but backtick strips trailing newlines (or something like 
that but anyway it does not work).  IFS=`printf ' \n\t'` would work (the 
double quotes are not needed) but the Autoconf manual suggests 
space-tab-newline in that order (even though only the leading space 
seems important from the rest of the paragraph).

printf is fine, though it may not be a builtin.  It's actually more 
portable (even if slower on some shells) to use printf than echo if you 
have variable substitutions in the string, because it handles 
consistently the case when the output starts with a minus sign.

Paolo

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Loïc Minier]

On Wed, Jan 20, 2010, Paolo Bonzini wrote:
> >  Are you saying that I can't backup/restore IFS without setting it
> >  first?
> Yes, it affects the behavior of read for example:
> $ echo a b c | (read a b c; echo $a; echo $b; echo $c)
> a
> b
> c
> $ IFS=
> $ echo a b c | (read a b c; echo $a; echo $b; echo $c)
> a b c
> 
> $
> (It's not used by QEMU's configure, but it's better to be defensive).

 I *do* understand that changing IFS will affect the program, but the
 patch I sent will backup IFS and then restore it; perhaps you missed
 the backup/restore bits:

 +    local_ifs="$IFS"
 [...]
 +    IFS=:
 [...]
 +            IFS="$local_ifs"
 +            return 0
 [...]
 +    IFS="$local_ifs"
 +    return 1

 Do you have an example of how that breaks if IFS isn't ever set in
 ./configure at all?

-- 
Loïc Minier

[Qemu-devel] Re: Stop using "which" in ./configure

[Måns Rullgård]

Loïc Minier <lool@dooz.org> writes:

> On Wed, Jan 20, 2010, Paolo Bonzini wrote:
>> >  Are you saying that I can't backup/restore IFS without setting it
>> >  first?
>> Yes, it affects the behavior of read for example:
>> $ echo a b c | (read a b c; echo $a; echo $b; echo $c)
>> a
>> b
>> c
>> $ IFS=
>> $ echo a b c | (read a b c; echo $a; echo $b; echo $c)
>> a b c
>> 
>> $
>> (It's not used by QEMU's configure, but it's better to be defensive).
>
>  I *do* understand that changing IFS will affect the program, but the
>  patch I sent will backup IFS and then restore it; perhaps you missed
>  the backup/restore bits:

It might not be set at all to begin with, in which case you'd set it
the empty string when restoring, and that would change the behaviour.

The POSIX spec says this:

  If IFS is not set, the shell shall behave as if the value of IFS is
  <space>, <tab>, and <newline>

>  +    local_ifs="$IFS"
>  [...]
>  +    IFS=:
>  [...]
>  +            IFS="$local_ifs"
>  +            return 0
>  [...]
>  +    IFS="$local_ifs"
>  +    return 1

If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
Likewise if you set the value first.

-- 
Måns Rullgård
mans@mansr.com

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Loïc Minier]

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

On Wed, Jan 20, 2010, Måns Rullgård wrote:
> If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
> Likewise if you set the value first.

 Ok; see attached patches

-- 
Loïc Minier

[-- Attachment #2: 0001-Add-and-use-has-and-path_of-funcs.patch --]
[-- Type: text/x-diff, Size: 4054 bytes --]

>From cccdcaeacc2214390c0c6c198ed875ac59d10669 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Tue, 19 Jan 2010 11:05:00 +0100
Subject: [PATCH 1/2] Add and use has() and path_of() funcs

Add has() and path_of() funcs and use them across configure; has()
will test whether a command or builtin is available; path_of() will
search the PATH for executables and return the full pathname if found.
---
 configure |   52 +++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index baa2800..eb02de3 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,42 @@ compile_prog() {
   $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null
 }
 
+# check whether a command is available to this shell (may be either an
+# executable or a builtin)
+has() {
+    type "$1" >/dev/null 2>&1
+}
+
+# search for an executable in PATH
+path_of() {
+    local_command="$1"
+    local_ifs="$IFS"
+    local_dir=""
+
+    # pathname has a dir component?
+    if [ "${local_command#*/}" != "$local_command" ]; then
+        if [ -x "$local_command" ] && [ ! -d "$local_command" ]; then
+            echo "$local_command"
+            return 0
+        fi
+    fi
+    if [ -z "$local_command" ]; then
+        return 1
+    fi
+
+    IFS=:
+    for local_dir in $PATH; do
+        if [ -x "$local_dir/$local_command" ] && [ ! -d "$local_dir/$local_command" ]; then
+            echo "$local_dir/$local_command"
+            IFS="${local_ifs:-$(printf ' \t\n')}"
+            return 0
+        fi
+    done
+    # not found
+    IFS="${local_ifs:-$(printf ' \t\n')}"
+    return 1
+}
+
 # default parameters
 cpu=""
 prefix=""
@@ -763,7 +799,7 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"`
+  solinst=`path_of $install`
   if test -z "$solinst" ; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
@@ -776,7 +812,7 @@ if test "$solaris" = "yes" ; then
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"`
+  sol_ar=`path_of ar`
   if test -z "$sol_ar" ; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
@@ -969,7 +1005,7 @@ fi
 # pkgconfig probe
 
 pkgconfig="${cross_prefix}pkg-config"
-if ! test -x "$(which $pkgconfig 2>/dev/null)"; then
+if ! has $pkgconfig; then
   # likely not cross compiling, or hope for the best
   pkgconfig=pkg-config
 fi
@@ -977,7 +1013,7 @@ fi
 ##########################################
 # Sparse probe
 if test "$sparse" != "no" ; then
-  if test -x "$(which cgcc 2>/dev/null)"; then
+  if has cgcc; then
     sparse=yes
   else
     if test "$sparse" = "yes" ; then
@@ -993,7 +1029,7 @@ fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-elif which sdl-config >/dev/null 2>&1; then
+elif has sdl-config; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
 else
@@ -1424,8 +1460,7 @@ EOF
     fi
   else
     if test "$kvm" = "yes" ; then
-      if [ -x "`which awk 2>/dev/null`" ] && \
-         [ -x "`which grep 2>/dev/null`" ]; then
+      if has awk && has grep; then
         kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \
 	| grep "error: " \
 	| awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'`
@@ -1694,8 +1729,7 @@ fi
 
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
-  if test -x "`which texi2html 2>/dev/null`" -a \
-          -x "`which pod2man 2>/dev/null`" ; then
+  if has texi2html && has pod2man; then
     docs=yes
   else
     if test "$docs" = "yes" ; then
-- 
1.6.5


[-- Attachment #3: 0002-Solaris-test-for-presence-of-commands-with-has.patch --]
[-- Type: text/x-diff, Size: 1471 bytes --]

>From 430b06dcc84e82987d0146ef92dddbe838d6a117 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Wed, 20 Jan 2010 12:35:54 +0100
Subject: [PATCH 2/2] Solaris: test for presence of commands with has()

---
 configure |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index eb02de3..f59f3e2 100755
--- a/configure
+++ b/configure
@@ -799,21 +799,19 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`path_of $install`
-  if test -z "$solinst" ; then
+  if ! has $install; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
     echo "to get ginstall which is used by default (which lives in /opt/csw/bin)"
     exit 1
   fi
-  if test "$solinst" = "/usr/sbin/install" ; then
+  if "`path_of $install`" = "/usr/sbin/install" ; then
     echo "Error: Solaris /usr/sbin/install is not an appropriate install program."
     echo "try ginstall from the GNU fileutils available from www.blastwave.org"
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`path_of ar`
-  if test -z "$sol_ar" ; then
+  if ! has ar; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
       echo "Add /usr/ccs/bin to your path and rerun configure"
-- 
1.6.5

[Qemu-devel] Re: Stop using "which" in ./configure

[Juan Quintela]

Loïc Minier <lool@dooz.org> wrote:
> On Wed, Jan 20, 2010, Måns Rullgård wrote:
>> If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
>> Likewise if you set the value first.
>
>  Ok; see attached patches

I still think that path_of is a complication that we don't really need.

"type" command exist in posix, and althought its output is not defined,
if the output of a real command don't show the path that we need (in
this case /usr/bin/install), then type is pretty wrong in my humble
opinion.

With respect of the rest of the patch.  Only real difference is that
mine uses "prog_exists" vs yours use "has" name.

I obviosly think that prog_exists is a more descriptive name, but I
really don't care one way or another.

Can we agree that we don't need path_of, and then just stop playing with
IFS and friends?

Later, Juan.

[Qemu-devel] Re: Stop using "which" in ./configure

[Måns Rullgård]

Juan Quintela <quintela@redhat.com> writes:

> Loïc Minier <lool@dooz.org> wrote:
>> On Wed, Jan 20, 2010, Måns Rullgård wrote:
>>> If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
>>> Likewise if you set the value first.
>>
>>  Ok; see attached patches
>
> I still think that path_of is a complication that we don't really need.
>
> "type" command exist in posix, and althought its output is not defined,
> if the output of a real command don't show the path that we need (in
> this case /usr/bin/install), then type is pretty wrong in my humble
> opinion.

I think that entire test is wrong, in fact.  It is perfectly possible
for someone on Solaris to install a working "install" command in
/usr/bin.  It is better, if possible, to test whatever "install"
command is in the path, and complain only if it actually fails.

-- 
Måns Rullgård
mans@mansr.com

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Loïc Minier]

On Thu, Jan 21, 2010, Måns Rullgård wrote:
> I think that entire test is wrong, in fact.  It is perfectly possible
> for someone on Solaris to install a working "install" command in
> /usr/bin.  It is better, if possible, to test whatever "install"
> command is in the path, and complain only if it actually fails.

 As I said, I prefer not changing this without access to a Solaris
 platform; would you mind committing the patch as is, and requesting
 whoever is interested in the Solaris specific code in qemu to fix the
 remainder?  The patch is a net improvement over what's in ./configure.

   Thanks
-- 
Loïc Minier

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Blue Swirl]

On Tue, Jan 26, 2010 at 6:47 PM, Loïc Minier <lool@dooz.org> wrote:
> On Thu, Jan 21, 2010, Måns Rullgård wrote:
>> I think that entire test is wrong, in fact.  It is perfectly possible
>> for someone on Solaris to install a working "install" command in
>> /usr/bin.  It is better, if possible, to test whatever "install"
>> command is in the path, and complain only if it actually fails.
>
>  As I said, I prefer not changing this without access to a Solaris
>  platform; would you mind committing the patch as is, and requesting
>  whoever is interested in the Solaris specific code in qemu to fix the
>  remainder?  The patch is a net improvement over what's in ./configure.

The patches didn't apply. Also please send only one patch per message,
git am can't handle multiple patches.

[Qemu-devel] [PATCH 1/3] Check for sdl-config before calling it

[Loïc Minier]

Check whether sdl-config is available before calling it, otherwise
./configure triggers a warning:
    ./configure: 957: sdl-config: not found

If neither the .pc file not sdl-config are present, disable SDL support.

Signed-off-by: Loïc Minier <lool@dooz.org>
---
 configure |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 1f6de41..27ab724 100755
--- a/configure
+++ b/configure
@@ -997,9 +997,14 @@ fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-else
+elif which sdl-config >/dev/null 2>&1; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
+else
+  if test "$sdl" = "yes" ; then
+    feature_not_found "sdl"
+  fi
+  sdl=no
 fi
 
 sdl_too_old=no
-- 
1.6.5

[Qemu-devel] [PATCH 2/3] Add and use has() and path_of() funcs

[Loïc Minier]

Add has() and path_of() funcs and use them across configure; has()
will test whether a command or builtin is available; path_of() will
search the PATH for executables and return the full pathname if found.
---
 configure |   52 +++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index 27ab724..6bdd2b7 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,42 @@ compile_prog() {
   $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null
 }
 
+# check whether a command is available to this shell (may be either an
+# executable or a builtin)
+has() {
+    type "$1" >/dev/null 2>&1
+}
+
+# search for an executable in PATH
+path_of() {
+    local_command="$1"
+    local_ifs="$IFS"
+    local_dir=""
+
+    # pathname has a dir component?
+    if [ "${local_command#*/}" != "$local_command" ]; then
+        if [ -x "$local_command" ] && [ ! -d "$local_command" ]; then
+            echo "$local_command"
+            return 0
+        fi
+    fi
+    if [ -z "$local_command" ]; then
+        return 1
+    fi
+
+    IFS=:
+    for local_dir in $PATH; do
+        if [ -x "$local_dir/$local_command" ] && [ ! -d "$local_dir/$local_command" ]; then
+            echo "$local_dir/$local_command"
+            IFS="${local_ifs:-$(printf ' \t\n')}"
+            return 0
+        fi
+    done
+    # not found
+    IFS="${local_ifs:-$(printf ' \t\n')}"
+    return 1
+}
+
 # default parameters
 cpu=""
 prefix=""
@@ -767,7 +803,7 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"`
+  solinst=`path_of $install`
   if test -z "$solinst" ; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
@@ -780,7 +816,7 @@ if test "$solaris" = "yes" ; then
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"`
+  sol_ar=`path_of ar`
   if test -z "$sol_ar" ; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
@@ -973,7 +1009,7 @@ fi
 # pkgconfig probe
 
 pkgconfig="${cross_prefix}pkg-config"
-if ! test -x "$(which $pkgconfig 2>/dev/null)"; then
+if ! has $pkgconfig; then
   # likely not cross compiling, or hope for the best
   pkgconfig=pkg-config
 fi
@@ -981,7 +1017,7 @@ fi
 ##########################################
 # Sparse probe
 if test "$sparse" != "no" ; then
-  if test -x "$(which cgcc 2>/dev/null)"; then
+  if has cgcc; then
     sparse=yes
   else
     if test "$sparse" = "yes" ; then
@@ -997,7 +1033,7 @@ fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-elif which sdl-config >/dev/null 2>&1; then
+elif has sdl-config; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
 else
@@ -1428,8 +1464,7 @@ EOF
     fi
   else
     if test "$kvm" = "yes" ; then
-      if [ -x "`which awk 2>/dev/null`" ] && \
-         [ -x "`which grep 2>/dev/null`" ]; then
+      if has awk && has grep; then
         kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \
 	| grep "error: " \
 	| awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'`
@@ -1698,8 +1733,7 @@ fi
 
 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
-  if test -x "`which texi2html 2>/dev/null`" -a \
-          -x "`which pod2man 2>/dev/null`" ; then
+  if has texi2html && has pod2man; then
     docs=yes
   else
     if test "$docs" = "yes" ; then
-- 
1.6.5

[Qemu-devel] [PATCH 3/3] Solaris: test for presence of commands with has()

[Loïc Minier]

---
 configure |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 6bdd2b7..1d0915a 100755
--- a/configure
+++ b/configure
@@ -803,21 +803,19 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`path_of $install`
-  if test -z "$solinst" ; then
+  if ! has $install; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
     echo "to get ginstall which is used by default (which lives in /opt/csw/bin)"
     exit 1
   fi
-  if test "$solinst" = "/usr/sbin/install" ; then
+  if "`path_of $install`" = "/usr/sbin/install" ; then
     echo "Error: Solaris /usr/sbin/install is not an appropriate install program."
     echo "try ginstall from the GNU fileutils available from www.blastwave.org"
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`path_of ar`
-  if test -z "$sol_ar" ; then
+  if ! has ar; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
       echo "Add /usr/ccs/bin to your path and rerun configure"
-- 
1.6.5

Re: [Qemu-devel] [PATCH 1/3] Check for sdl-config before calling it

[Ben Taylor]

On Wed, Jan 27, 2010 at 7:10 AM, Loïc Minier <lool@dooz.org> wrote:
> Check whether sdl-config is available before calling it, otherwise
> ./configure triggers a warning:
>    ./configure: 957: sdl-config: not found
>
> If neither the .pc file not sdl-config are present, disable SDL support.
>
> Signed-off-by: Loïc Minier <lool@dooz.org>
> ---
>  configure |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index 1f6de41..27ab724 100755
> --- a/configure
> +++ b/configure
> @@ -997,9 +997,14 @@ fi
>  if $pkgconfig sdl --modversion >/dev/null 2>&1; then
>   sdlconfig="$pkgconfig sdl"
>   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
> -else
> +elif which sdl-config >/dev/null 2>&1; then
>   sdlconfig='sdl-config'
>   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
> +else
> +  if test "$sdl" = "yes" ; then
> +    feature_not_found "sdl"
> +  fi
> +  sdl=no
>  fi
>
>  sdl_too_old=no


Glad to see someone working on Solaris.  Wondering why you're using
the which command, when you just created a "has" function in the other
patch segments, and just replaced all the other instances of which.

[Qemu-devel] Re: [PATCH 1/3] Check for sdl-config before calling it

[Paolo Bonzini]

> Glad to see someone working on Solaris.  Wondering why you're using
> the which command, when you just created a "has" function in the other
> patch segments, and just replaced all the other instances of which.

This is patch 1/3, patch 2/3 creates "has" and uses it here too.

Paolo

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Loïc Minier]

On Tue, Jan 26, 2010, Blue Swirl wrote:
> The patches didn't apply. Also please send only one patch per message,
> git am can't handle multiple patches.

 They applied fine here, perhaps you didn't apply the sdl-config patch
 first?  I rebased them and resent them.

-- 
Loïc Minier

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Blue Swirl]

On Wed, Jan 27, 2010 at 12:41 PM, Loïc Minier <lool@dooz.org> wrote:
> On Tue, Jan 26, 2010, Blue Swirl wrote:
>> The patches didn't apply. Also please send only one patch per message,
>> git am can't handle multiple patches.
>
>  They applied fine here, perhaps you didn't apply the sdl-config patch
>  first?  I rebased them and resent them.

That must've been it. But I get this on Milax:

config-host.mak is out-of-date, running configure
/bin/ginstall: cannot stat `=': No such file or directory

Configuration and build succeeds though.

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Loïc Minier]

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

On Wed, Jan 27, 2010, Blue Swirl wrote:
> That must've been it. But I get this on Milax:

 I didn't know about MilaX; I had a hard time getting gcc/binutils (or
 event git) to work along with working headers or base libs such as
 zlib.h or unistd.h.  Do you have any instructions on how to get these
 working?

> config-host.mak is out-of-date, running configure
> /bin/ginstall: cannot stat `=': No such file or directory

 Ok; this was a typo in the Solaris patch (note that I was reluctant to
 touch the Solaris code in the first place); I could reproduce and also
 noticed that /bin/sh on MilaX doesn't support "if ! foo", so I replaced
 these with "if foo; then :; else ...; fi" constructs.

 Please find an attached third patch.

-- 
Loïc Minier

[-- Attachment #2: 0002-Solaris-test-for-presence-of-commands-with-has.patch --]
[-- Type: text/x-diff, Size: 1506 bytes --]

>From 6fef73a3b890ec736cdfa1ff817230c8e6d760e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Wed, 20 Jan 2010 12:35:54 +0100
Subject: [PATCH 2/2] Solaris: test for presence of commands with has()

---
 configure |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 6bdd2b7..42ef628 100755
--- a/configure
+++ b/configure
@@ -803,21 +803,23 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`path_of $install`
-  if test -z "$solinst" ; then
+  if has $install; then
+    :
+  else
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
     echo "to get ginstall which is used by default (which lives in /opt/csw/bin)"
     exit 1
   fi
-  if test "$solinst" = "/usr/sbin/install" ; then
+  if test "`path_of $install`" = "/usr/sbin/install" ; then
     echo "Error: Solaris /usr/sbin/install is not an appropriate install program."
     echo "try ginstall from the GNU fileutils available from www.blastwave.org"
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`path_of ar`
-  if test -z "$sol_ar" ; then
+  if has ar; then
+    :
+  else
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
       echo "Add /usr/ccs/bin to your path and rerun configure"
-- 
1.6.5

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Blue Swirl]

On Thu, Jan 28, 2010 at 8:33 PM, Loïc Minier <lool@dooz.org> wrote:
> On Wed, Jan 27, 2010, Blue Swirl wrote:
>> That must've been it. But I get this on Milax:
>
>  I didn't know about MilaX; I had a hard time getting gcc/binutils (or
>  event git) to work along with working headers or base libs such as
>  zlib.h or unistd.h.  Do you have any instructions on how to get these
>  working?

The packaging system is weird. Maybe this list is useful.

application CSWbdb4            berkeleydb4 - Embedded database
libraries and utilities
application CSWbinutils        binutils - GNU binary utilities such as
gar, gas, gld, gnm and others
application CSWbonobo2         libbonobo2 - GNOME component and
compound document system
application CSWbzip2           bzip2 - The bzip2 high-quality data compressor
system      CSWcommon          common - common files and dirs for CSW packages
application CSWdbusglib        dbus_glib - DBus GLib Bindings
application CSWexpat           expat -  XML Parser Toolkit
system      CSWfam             fam - File Alteration Monitor, daemon
and interface library
application CSWfconfig         fontconfig - A library for configuring
and customizing font access.
application CSWftype2          freetype2 - The free and portable high
quality font engine
application CSWgcc3            gcc3 - GNU Compiler Suite branch 3 Replacement
application CSWgcc3core        gcc3core - GNU C Compiler
application CSWgcc3corert      gcc3corert - GNU C Compiler Run Time
application CSWgcc3g++         gcc3g++ - GNU C++ Compiler
application CSWgcc3g++rt       gcc3g++rt - GNU C++ Compiler Run Time
application CSWgcc3rt          gcc3rt - GNU Compiler Suite branch 3
Run Time Replacement
application CSWgcc4core        gcc4core - GNU C Compiler
application CSWgcc4corert      gcc4corert - GNU C Compiler Run Time
application CSWgconf2          gconf2 - GNOME configuration database system
system      CSWgcrypt          libgcrypt - GNU libcrypt
application CSWgdb             gdb - The GNU Debugger
system      CSWggettext        ggettext - GNU gettext
application CSWglib2           glib2 - the low-level core library for
GTK+ and GNOME
system      CSWgmake           gmake - GNU make
application CSWgnomevfs2       gnomevfs2 - GNOME Virtual File System
application CSWgnupg           gnupg - RFC 2440 compliant tool for
secure communication and data storage. (gpg binary)
system      CSWgnutls          gnutls - GNU Transport Layer Security
lib and tools
system      CSWgpgerr          libgpg_error - GNU gpg related library
application CSWgtk2            gtk2 - GTK+, The multiplatform X11
toolkit for GUI development
system      CSWiconv           libiconv - GNU iconv library
system      CSWisaexec         isaexec - sneaky wrapper around Sun isaexec
application CSWjasper          jasper - An image data software toolkit
for JPEG-2000 codec standard
system      CSWjpeg            jpeg - JPEG library and tools by the
Independent JPEG Group
application CSWkrb5lib         krb5_lib - MIT Kerberos 5 core libraries
application CSWlibart          libart - libart - High performance 2D
graphics library
application CSWlibatk          libatk - Accessibility toolkit for GNOME
application CSWlibcairo        libcairo - The Cairo 2D Graphics Library
application CSWlibcroco        libcroco - A CSS parsing and manipulation toolkit
application CSWlibcups         libcups - Libraries for the Common UNIX
Printing System
application CSWlibdbus         libdbus - DBus Message Bus System -
dynamic libraries
application CSWlibglade2       libglade2 - library for loading glade
user interface desc files
application CSWlibgnomecanvas  libgnomecanvas
application CSWlibgnomecups    libgnomecups - GNOME library for CUPS interaction
application CSWlibgnomeprint   libgnomeprint - The GNOME 2 print library
application CSWlibgnomeprintui libgnomeprintui - The GNOME 2 print UI library
application CSWlibgsf          libgsf - GNOME Structured File Library
application CSWlibidl          libidl - Library for handling CORBA IDL
system      CSWlibnet          libnet - the libnet packet construction library
application CSWlibpopt         libpopt - Popt is a C library for
parsing command line parameters
system      CSWlibsdl          libsdl - Simple DirectMedia Layer library
application CSWlibx11          libx11 - X11 client-side library
application CSWlibxau          libxau - X11 authorisation library
application CSWlibxcb          libxcb - The XCB library
application CSWlibxdmcp        libxdmcp - X11 authorisation library
application CSWlibxft2         libxft2 - FreeType-based font drawing
library for X
application CSWlibxml2         libxml2 - XML Parser Library
application CSWlibxrender      libxrender - X Render extension library
application CSWncurses         ncurses - ncurses library and utilities
application CSWnessuslib       nessuslib - libraries used by nessus
application CSWoldaprt         openldap_rt - OpenLDAP runtime
libraries (oldaprt)
system      CSWopencdk         opencdk - open crypto development kit
application CSWorbit2          orbit2 - ORBit 2.x CORBA tools and libs
application CSWossl            openssl - Openssl meta package
application CSWossldevel       openssl_devel - Openssl development support
application CSWosslrt          openssl_rt - Openssl runtime libraries
application CSWosslutils       openssl_utils - Openssl binaries and
related tools
application CSWpango           libpango - A framework for the layout
and rendering of i18n text
application CSWpixman          pixman - The pixel-manipulation library
for X and cairo
application CSWpkgutil         pkgutil - Installs Solaris packages easily
system      CSWpng             png - library for Portable Network
Graphics format (PNG)
application CSWqemu            qemu - x86 PC emulator
application CSWreadline        readline - GNU readline
application CSWsambalib        samba_lib - Tools to access a server's
filespace and printers via SMB (lib)
application CSWsasl            sasl - Simple Authentication and Security Layer
system      CSWsunmath         libsunmath - Sun maths library
system      CSWtextutils       textutils - GNU text file processing utilities
application CSWtiff            tiff - lib and tools for Tag Image File Format
application CSWzlib            zlib - Zlib Data Compression Library
system      SUNWarc            Lint Libraries (usr)
system      SUNWarcr           Lint Libraries (root)
system      SUNWhea            SunOS Header Files
system      SUNWlibm           Math & Microtasking Library Headers &
Lint Files (Usr)
system      SUNWlibmr          Math Library Lint Files (Root)
system      SUNWxwinc          X Window System include files

>
>> config-host.mak is out-of-date, running configure
>> /bin/ginstall: cannot stat `=': No such file or directory
>
>  Ok; this was a typo in the Solaris patch (note that I was reluctant to
>  touch the Solaris code in the first place); I could reproduce and also
>  noticed that /bin/sh on MilaX doesn't support "if ! foo", so I replaced
>  these with "if foo; then :; else ...; fi" constructs.
>
>  Please find an attached third patch.

Now the error is gone. Thanks, applied.

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Jamie Lokier]

Måns Rullgård wrote:
>   If IFS is not set, the shell shall behave as if the value of IFS is
>   <space>, <tab>, and <newline>
> 
> If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
> Likewise if you set the value first.

Remove the colon.  The above will wrongly change empty IFS, which
is not the same as unset IFS.

That said, nobody should expect ./configure to work if IFS has an
unusual value anyway.  Probably .configure should just set it to the
standard value at the start.

-- Jamie

[Qemu-devel] Re: Stop using "which" in ./configure

[Paolo Bonzini]

On 01/21/2010 05:53 PM, Jamie Lokier wrote:
>> >  If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe.
>> >  Likewise if you set the value first.
> Remove the colon.  The above will wrongly change empty IFS, which
> is not the same as unset IFS.

local_ifs would never be unset anyway, so it would never trigger.

After all, Loic's code can be considered okay as it was in the first 
place (sorry).  Instead, we should just make sure that no code ever 
unsets IFS.  I committed this recommendation to the Autoconf manual's 
shell portability section, so it's not necessary to add any comment here.

Paolo

[Qemu-devel] Re: Stop using "which" in ./configure

[Juan Quintela]

Loïc Minier <lool@dooz.org> wrote:
> On Tue, Jan 19, 2010, Måns Rullgård wrote:
> [...]
>> Why the extra variable?  Using $1 directly seems just as obvious to me.
> [...]
>
>  I'm attaching an updated patch which doesn't use this variable.  Should
>  be applied after the sdl-config patch.
>
>> Is the full path of these tools really important?  Doesn't look like
>> it to me.
>
>  I'm attaching a new patch which changes the tests a bit; I would prefer
>  if someone with access to a Solaris build environment would do this
>  though.

Hi

could you test this patch?

I did it concurrently with yours.  my prog_exist() is equal to your
has(), and path_of() is only needed in a single place, that I think it
is better handled with a single grep in that place.

What do you think?

>From 4580f69b022e7861bf3be1c694222c0ee82889b7 Mon Sep 17 00:00:00 2001
Message-Id: <4580f69b022e7861bf3be1c694222c0ee82889b7.1263991676.git.quintela@redhat.com>
In-Reply-To: <cover.1263991676.git.quintela@redhat.com>
References: <cover.1263991676.git.quintela@redhat.com>
From: Juan Quintela <quintela@redhat.com>
Date: Wed, 20 Jan 2010 13:24:59 +0100
Subject: [PATCH 1/3] substitute all uses of which by type

Except in one case, we are only interested in knowing if a command
exist, not is path.  Just create prog_exists() function that does this
check.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 configure |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index 5631bbb..c1c9bb4 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,11 @@ compile_prog() {
   $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null
 }

+prog_exist() {
+    prog="$1"
+    type $1 > /dev/null 2> /dev/null
+}
+
 # default parameters
 cpu=""
 prefix=""
@@ -763,21 +768,19 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"`
-  if test -z "$solinst" ; then
+  if ! prog_exist "$install" ; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
     echo "to get ginstall which is used by default (which lives in /opt/csw/bin)"
     exit 1
   fi
-  if test "$solinst" = "/usr/sbin/install" ; then
+  if type install2 2> /dev/null | grep /usr/bin/install >/dev/null ; then
     echo "Error: Solaris /usr/sbin/install is not an appropriate install program."
     echo "try ginstall from the GNU fileutils available from www.blastwave.org"
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"`
-  if test -z "$sol_ar" ; then
+  if ! prog_exist "ar" ; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
       echo "Add /usr/ccs/bin to your path and rerun configure"
@@ -969,7 +972,7 @@ fi
 # pkgconfig probe

 pkgconfig="${cross_prefix}pkg-config"
-if ! test -x "$(which $pkgconfig 2>/dev/null)"; then
+if ! prog_exist "$pkgconfig" ; then
   # likely not cross compiling, or hope for the best
   pkgconfig=pkg-config
 fi
@@ -977,7 +980,7 @@ fi
 ##########################################
 # Sparse probe
 if test "$sparse" != "no" ; then
-  if test -x "$(which cgcc 2>/dev/null)"; then
+  if prog_exist "cgcc" ; then
     sparse=yes
   else
     if test "$sparse" = "yes" ; then
@@ -1419,8 +1422,8 @@ EOF
     fi
   else
     if test "$kvm" = "yes" ; then
-      if [ -x "`which awk 2>/dev/null`" ] && \
-         [ -x "`which grep 2>/dev/null`" ]; then
+      if prog_exist "awk" -a \
+         prog_exist "grep"; then
         kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \
 	| grep "error: " \
 	| awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'`
@@ -1689,8 +1692,8 @@ fi

 # Check if tools are available to build documentation.
 if test "$docs" != "no" ; then
-  if test -x "`which texi2html 2>/dev/null`" -a \
-          -x "`which pod2man 2>/dev/null`" ; then
+  if prog_exist "texi2html" -a \
+     prog_exist "pod2man" ; then
     docs=yes
   else
     if test "$docs" = "yes" ; then
-- 
1.6.6

[Qemu-devel] Re: Stop using "which" in ./configure

[Paolo Bonzini]

On 01/20/2010 01:49 PM, Juan Quintela wrote:
> +      if prog_exist "awk" -a \
> +         prog_exist "grep"; then

> +  if prog_exist "texi2html" -a \
> +     prog_exist "pod2man" ; then

-a is wrong here.  BTW, it's evil for test too because its precedence 
rules WRT ! and -f/-n/-z/-x/etc. are broken.

Paolo

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Loïc Minier]

On Wed, Jan 20, 2010, Juan Quintela wrote:
> +prog_exist() {
> +    prog="$1"
> +    type $1 > /dev/null 2> /dev/null
> +}

 You set prog but you use $1; also, it seems vars in functions should be
 prefixed with local_ in qemu's ./configure.  I was told not to use a
 local var here though.

> -  if test "$solinst" = "/usr/sbin/install" ; then
> +  if type install2 2> /dev/null | grep /usr/bin/install >/dev/null ; then

 install2?

> +      if prog_exist "awk" -a \
> +         prog_exist "grep"; then

 already pointed out, but -a is a feature of "test" and incorrect here;
 you want &&

> -  if test -x "`which texi2html 2>/dev/null`" -a \
> -          -x "`which pod2man 2>/dev/null`" ; then
> +  if prog_exist "texi2html" -a \
> +     prog_exist "pod2man" ; then

 ditto


 Perhaps we should avoid work duplication though?

-- 
Loïc Minier

Re: [Qemu-devel] Re: Stop using "which" in ./configure

[Loïc Minier]

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

On Wed, Jan 20, 2010, Loïc Minier wrote:
>  I'm attaching a new patch which changes the tests a bit; I would prefer
>  if someone with access to a Solaris build environment would do this
>  though.

 Sorry, there was a typo in the new patch, a test was reversed; updated
 patch attached.

-- 
Loïc Minier

[-- Attachment #2: 0002-Solaris-test-for-presence-of-commands-with-has.patch --]
[-- Type: text/plain, Size: 1471 bytes --]

>From 8b17a25e0f16390a6afed3e030a11ad290f413d7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Wed, 20 Jan 2010 12:35:54 +0100
Subject: [PATCH 2/2] Solaris: test for presence of commands with has()

---
 configure |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 90b3c18..4d48341 100755
--- a/configure
+++ b/configure
@@ -799,21 +799,19 @@ fi
 # Solaris specific configure tool chain decisions
 #
 if test "$solaris" = "yes" ; then
-  solinst=`path_of $install`
-  if test -z "$solinst" ; then
+  if ! has $install; then
     echo "Solaris install program not found. Use --install=/usr/ucb/install or"
     echo "install fileutils from www.blastwave.org using pkg-get -i fileutils"
     echo "to get ginstall which is used by default (which lives in /opt/csw/bin)"
     exit 1
   fi
-  if test "$solinst" = "/usr/sbin/install" ; then
+  if "`path_of $install`" = "/usr/sbin/install" ; then
     echo "Error: Solaris /usr/sbin/install is not an appropriate install program."
     echo "try ginstall from the GNU fileutils available from www.blastwave.org"
     echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install"
     exit 1
   fi
-  sol_ar=`path_of ar`
-  if test -z "$sol_ar" ; then
+  if ! has ar; then
     echo "Error: No path includes ar"
     if test -f /usr/ccs/bin/ar ; then
       echo "Add /usr/ccs/bin to your path and rerun configure"
-- 
1.6.5

Re: [Qemu-devel] [PATCH] Check for sdl-config before calling it

[Stefan Weil]

Loïc Minier schrieb:
> Hi
>
> On systems were sdl-config isn't installed, ./configure triggers this
> warning:
> ./configure: 957: sdl-config: not found
>
> The attached patch fixes the warning for me.
>
> Thanks,

Hi,

which version did you test?
Git master has no sdl-config call at configure:957.

But I get warning messages, too, when pkg-config or
sdl-config are missing.

Your patch fixes the warning for sdl-config and sets
sdl=no. If configure was called with --enable-sdl,
this solution is wrong: configure should abort with
an error message (feature_not_found).

Regards,
Stefan

Re: [Qemu-devel] [PATCH] Check for sdl-config before calling it

[Loïc Minier]

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

On Sun, Jan 17, 2010, Stefan Weil wrote:
> > On systems were sdl-config isn't installed, ./configure triggers this
> > warning:
> > ./configure: 957: sdl-config: not found
> 
> which version did you test?
> Git master has no sdl-config call at configure:957.
> 
> But I get warning messages, too, when pkg-config or
> sdl-config are missing.

 Yes, the line is bogus for me as well; not sure why.  I'm using master.

> Your patch fixes the warning for sdl-config and sets
> sdl=no. If configure was called with --enable-sdl,
> this solution is wrong: configure should abort with
> an error message (feature_not_found).

 Ack; how about the attached one instead?

-- 
Loïc Minier

[-- Attachment #2: 0001-Check-for-sdl-config-before-calling-it.patch --]
[-- Type: text/plain, Size: 1224 bytes --]

>From eb4ba45b0d71d13cb89f6362443ffbe50b65eba0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Sun, 17 Jan 2010 13:42:04 +0100
Subject: [PATCH] Check for sdl-config before calling it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Check whether sdl-config is available before calling it, otherwise
./configure triggers a warning:
    ./configure: 957: sdl-config: not found

If neither the .pc file not sdl-config are present, disable SDL support.

Signed-off-by: Loïc Minier <lool@dooz.org>
---
 configure |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 5631bbb..baa2800 100755
--- a/configure
+++ b/configure
@@ -993,9 +993,14 @@ fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-else
+elif which sdl-config >/dev/null 2>&1; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
+else
+  if test "$sdl" = "yes" ; then
+    feature_not_found "sdl"
+  fi
+  sdl=no
 fi
 
 sdl_too_old=no
-- 
1.6.5

[Qemu-devel] Re: [PATCH] Check for sdl-config before calling it

[Paolo Bonzini]

On 01/17/2010 05:06 PM, Loïc Minier wrote:
> On Sun, Jan 17, 2010, Stefan Weil wrote:
>>> On systems were sdl-config isn't installed, ./configure triggers this
>>> warning:
>>> ./configure: 957: sdl-config: not found
>>
>> which version did you test?
>> Git master has no sdl-config call at configure:957.
>>
>> But I get warning messages, too, when pkg-config or
>> sdl-config are missing.
>
>   Yes, the line is bogus for me as well; not sure why.  I'm using master.
>
>> Your patch fixes the warning for sdl-config and sets
>> sdl=no. If configure was called with --enable-sdl,
>> this solution is wrong: configure should abort with
>> an error message (feature_not_found).
>
>   Ack; how about the attached one instead?

Acked-By: Paolo Bonzini <pbonzini@redhat.com>

Paolo