Skip to contentSkip to conversation

Check for sdl-config before calling it

PATCH In review Loïc Minier 8 participants 38 messages Jan 17, 2010 – Jan 28, 2010

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Check for sdl-config before calling it

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,

Signature · 1 line
Loïc Minier
Quoted 1 line

From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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.

---
 configure |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
configure +3 -1
index 5631bbb..450e1a2 100755
@@ -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
Signature · 1 line
1.6.5
#1 lore raw
Quoted 36 lines · Loïc Minier

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,

Signature · 20 lines
Loïc Minier

From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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 <[email protected]>
---
 configure |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
configure +0 -0
index 5631bbb..450e1a2 100755

configure fi · line 1001

Quoted 10 lines
configure +3 -1
@@ -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".

Signature · 2 lines
Måns Rullgård
[email protected]
#2 lore raw
Quoted 46 lines · Måns Rullgård
Quoted 43 lines · Loïc Minier

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,

Signature · 20 lines
Loïc Minier

From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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 <[email protected]>
---
 configure |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
configure +1 -1
index 5631bbb..450e1a2 100755
@@ -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'`
Quoted 9 lines

configure fi · line 1002

Quoted 4 lines
configure +2 -0
+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

#3 lore raw
Quoted 57 lines · Stefan Weil
Quoted 46 lines · Måns Rullgård
Quoted 43 lines · Loïc Minier

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,

Signature · 20 lines
Loïc Minier

From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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 <[email protected]>
---
 configure |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
configure +1 -1
index 5631bbb..450e1a2 100755
@@ -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'`
Quoted 9 lines

configure fi · line 1002

Quoted 4 lines
configure +2 -0
+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".

Quoted 1 line

"which" is already used several times in configure,

Then those should be fixed.

Quoted 1 line

so this would not make things worse.

One error does not justify another.

Quoted 2 lines

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.

Signature · 2 lines
Måns Rullgård
[email protected]
#4 lore raw

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

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,

Signature · 1 line
Loïc Minier
Quoted 1 line

From 815ca36e9b3e2c56040d7fc7709c95d600ed6496 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +48 -9
index baa2800..8d67b9c 100755
@@ -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
Signature · 1 line
1.6.5
#5 lore raw

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

Quoted 5 lines · Loïc Minier

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.

Signature · 1 line
Loïc Minier
Quoted 1 line

From 5fc05ec61d87049ea0f29b2dd51c16e260698ef8 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +44 -9
index baa2800..711e335 100755
@@ -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
Signature · 1 line
1.6.5
#6 lore raw

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

Quoted 9 lines · Loïc Minier
Quoted 6 lines · Loïc Minier

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.)

Quoted 3 lines

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)

#7 lore raw

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

Quoted 2 lines · Stefan Weil

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

Signature · 1 line
Loïc Minier
Quoted 1 line

From 1c0b63fb9fc735a6d367a65a6ed1b998942fb6a4 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +44 -9
index baa2800..db97a2c 100755
@@ -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
Signature · 1 line
1.6.5
#8 lore raw

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

In reply to Loïc Minier
Quoted 31 lines · Loïc Minier
Quoted 5 lines · Loïc Minier

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.

Signature · 13 lines
Loïc Minier

From 5fc05ec61d87049ea0f29b2dd51c16e260698ef8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +0 -0
index baa2800..711e335 100755

configure compile_prog() { · line 35

Quoted 9 lines
configure +6 -0
@@ -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.

Quoted 43 lines
configure +31 -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"

configure if test "$solaris" = "yes" ; then · line 819

Quoted 8 lines
configure +1 -1
@@ -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.

Signature · 2 lines
Måns Rullgård
[email protected]
#9 lore raw

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

On Tue, Jan 19, 2010, Måns Rullgård wrote:
[...]

Quoted 1 line

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.

Quoted 2 lines

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.

Signature · 1 line
Loïc Minier
Quoted 1 line

From 27f151ef7be19fcffbbf9c2d3c6ee50750be854d Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +43 -9
index baa2800..90b3c18 100755
@@ -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
Signature · 1 line
1.6.5
Quoted 1 line

From 421c7a48b5e0a20ae09014cd67e94f3c0ba26a20 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +3 -5
index 90b3c18..7e842ce 100755
@@ -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"
Signature · 1 line
1.6.5
#10 lore raw

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

configure compile_prog() { · line 62

Quoted 2 lines · Loïc Minier
configure +2 -0
+    # 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

#11 lore raw

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

Quoted 8 lines · Paolo Bonzini

configure compile_prog() { · line 62

Quoted 2 lines · Loïc Minier
configure +2 -0
+    # 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

Quoted 4 lines

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.

Signature · 1 line
Loïc Minier
#12 lore raw

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

Quoted 4 lines · Loïc Minier
Quoted 3 lines · Paolo Bonzini
Quoted 2 lines · Loïc Minier
configure +2 -0
+    # not found
+    IFS="$local_ifs"
Quoted 8 lines
Quoted 5 lines

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).

Quoted 7 lines
Quoted 4 lines

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

#13 lore raw

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

Quoted 11 lines · Paolo Bonzini
Quoted 2 lines

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
Quoted 2 lines

$
(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?

Signature · 1 line
Loïc Minier
#14 lore raw

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

Quoted 12 lines · Loïc Minier
Quoted 11 lines · Paolo Bonzini
Quoted 2 lines

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
Quoted 6 lines
Quoted 2 lines

$
(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>

Quoted 9 lines

+ 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.

Signature · 2 lines
Måns Rullgård
[email protected]
#15 lore raw

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

Quoted 2 lines · Måns Rullgård

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

Signature · 1 line
Loïc Minier
Quoted 1 line

From cccdcaeacc2214390c0c6c198ed875ac59d10669 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +43 -9
index baa2800..eb02de3 100755
@@ -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
Signature · 1 line
1.6.5
Quoted 1 line

From 430b06dcc84e82987d0146ef92dddbe838d6a117 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +3 -5
index eb02de3..f59f3e2 100755
@@ -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"
Signature · 1 line
1.6.5
#16 lore raw

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

Quoted 5 lines · Loïc Minier
Quoted 2 lines · Måns Rullgård

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.

#17 lore raw

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

Quoted 5 lines · Juan Quintela
Quoted 4 lines · Loïc Minier
Quoted 2 lines · Måns Rullgård

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

Quoted 8 lines
Quoted 1 line

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.

Signature · 2 lines
Måns Rullgård
[email protected]
#18 lore raw

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

Quoted 4 lines · Måns Rullgård

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

Signature · 1 line
Loïc Minier
#19 lore raw

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

Quoted 10 lines · Loïc Minier
Quoted 4 lines · Måns Rullgård

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.

#20 lore raw

PATCH 1/3 Check for sdl-config before calling it

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.

---
 configure |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
configure +6 -1
index 1f6de41..27ab724 100755
@@ -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
Signature · 1 line
1.6.5
#21 lore raw

PATCH 2/3 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(-)
configure +43 -9
index 27ab724..6bdd2b7 100755
@@ -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
Signature · 1 line
1.6.5
#22 lore raw

PATCH 3/3 Solaris: test for presence of commands with has()

---
 configure |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
configure +3 -5
index 6bdd2b7..1d0915a 100755
@@ -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"
Signature · 1 line
1.6.5
#23 lore raw
In reply to Loïc Minier
Quoted 30 lines · 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.

---
 configure |    7 ++++++-

 1 files changed, 6 insertions(+), 1 deletions(-)
configure +0 -0
index 1f6de41..27ab724 100755
@@ -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'

configure +1 -1
-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

Quoted 1 line

 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.

#24 lore raw
Quoted 4 lines

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

#25 lore raw

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

In reply to Blue Swirl
Quoted 2 lines · Blue Swirl

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.

Signature · 1 line
Loïc Minier
#26 lore raw

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

Quoted 6 lines · Loïc Minier
Quoted 2 lines · Blue Swirl

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.

#27 lore raw

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

Quoted 1 line · Blue Swirl

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?

Quoted 2 lines

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.

Signature · 1 line
Loïc Minier
Quoted 1 line

From 6fef73a3b890ec736cdfa1ff817230c8e6d760e3 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +7 -5
index 6bdd2b7..42ef628 100755
@@ -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"
Signature · 1 line
1.6.5
#28 lore raw

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

Quoted 7 lines · Loïc Minier
Quoted 1 line · Blue Swirl

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
Quoted 10 lines
Quoted 3 lines

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.

#29 lore raw

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

In reply to Måns Rullgård
Quoted 5 lines · Måns Rullgård

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

#30 lore raw

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

Quoted 4 lines · Jamie Lokier
Quoted 2 lines
Quoted 2 lines

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

#31 lore raw

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

In reply to Loïc Minier
Quoted 8 lines · Loïc Minier

On Tue, Jan 19, 2010, Måns Rullgård wrote:
[...]

Quoted 1 line

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.

Quoted 6 lines
Quoted 2 lines

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?

Quoted 1 line

From 4580f69b022e7861bf3be1c694222c0ee82889b7 Mon Sep 17 00:00:00 2001

Message-Id: <4580f69b022e7861bf3be1c694222c0ee82889b7.1263991676.git.quintela@redhat.com>
In-Reply-To: <[email protected]>
References: <[email protected]>
From: Juan Quintela <[email protected]>
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.

---
 configure |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)
configure +14 -11
index 5631bbb..c1c9bb4 100755
@@ -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
Signature · 1 line
1.6.6
#32 lore raw

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

configure fi · line 1696

Quoted 5 lines · Juan Quintela
configure +4 -0
+      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

#33 lore raw

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

In reply to Juan Quintela

configure compile_prog() { · line 33

Quoted 4 lines · Juan Quintela
configure +4 -0
+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.

configure fi · line 777

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

install2?

configure EOF · line 1426

Quoted 2 lines
configure +2 -0
+      if prog_exist "awk" -a \
+         prog_exist "grep"; then

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

configure fi · line 1696

Quoted 4 lines
configure +2 -2
-  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?

Signature · 1 line
Loïc Minier
#34 lore raw

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

In reply to Loïc Minier
Quoted 3 lines · Loïc Minier

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.

Signature · 1 line
Loïc Minier
Quoted 1 line

From 8b17a25e0f16390a6afed3e030a11ad290f413d7 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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(-)
configure +3 -5
index 90b3c18..4d48341 100755
@@ -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"
Signature · 1 line
1.6.5
#35 lore raw
In reply to Loïc Minier
Quoted 9 lines · Loïc Minier

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

#36 lore raw
Quoted 9 lines · Stefan Weil
Quoted 3 lines

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.

Quoted 4 lines

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?

Signature · 1 line
Loïc Minier
Quoted 1 line

From eb4ba45b0d71d13cb89f6362443ffbe50b65eba0 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <[email protected]>
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.

---
 configure |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
configure +6 -1
index 5631bbb..baa2800 100755
@@ -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
Signature · 1 line
1.6.5
Acked-by
#37 lore raw
Quoted 11 lines · Loïc Minier
Quoted 9 lines · Stefan Weil
Quoted 3 lines

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.

Quoted 8 lines

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

Quoted 4 lines

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?

Paolo