Skip to contentSkip to conversation

Remove which use

PATCH In review Juan Quintela 2 participants 8 messages Jan 20, 2010

lore
Share & tools
lore
#1 lore raw

PATCH 1/5 substitute all uses of which by type

In reply to Juan Quintela

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..71edaf5 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 $prog > /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 "$install" 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" && \
+         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" && \
+     prog_exist "pod2man" ; then
     docs=yes
   else
     if test "$docs" = "yes" ; then
Signature · 1 line
1.6.6
#0 lore raw

cover Remove which use

This series substitute "which" use by "type" one.
It is similar to Loïc patch, just that I didn't use path_of() because it was used just once.

This addresses commets about using && instead of -a.
It also remove the search for "grep" in kvm code. grep is assumed in lots of other places in configure already.

Riku: are you going to pick the missing symbols patch (its is for linux-user)
or should Anthony pick it?

Later, Juan.

Juan Quintela (3):
substitute all uses of which by type
Check for sdl-config before calling it
Remove check for grep

Loïc Minier (2):
Add -static earlier to LDFLAGS for compile_prog() Fix missing symbols in .rel/.rela.plt sections

 configure |   37 +++++++++++++++++++++++--------------
 i386.ld   |   16 ++++++++++++++--
 x86_64.ld |   16 ++++++++++++++--
 3 files changed, 51 insertions(+), 18 deletions(-)
#4 lore raw

PATCH 2/5 Check for sdl-config before calling it

Adapted Loïc Minier version to not use which.

---
 configure |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
configure +7 -1
index 71edaf5..c671918 100755
@@ -996,9 +996,15 @@ 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
+else if prog_exist "sdl-config"; 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
 fi
 
 sdl_too_old=no
Signature · 1 line
1.6.6
#5 lore raw

PATCH 3/5 Add -static earlier to LDFLAGS for compile_prog()

In reply to Juan Quintela

From: Loïc Minier <[email protected]>

Add -static to LDFLAGS earlier as to run the compile_prog() tests with this flags, this will avoid turning on features for which a shared library is available but not a static one.

---
 configure |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
configure +3 -2
index c671918..83f8b26 100755
@@ -456,7 +456,9 @@ for opt do
   ;;
   --enable-gprof) gprof="yes"
   ;;
-  --static) static="yes"
+  --static)
+    static="yes"
+    LDFLAGS="-static $LDFLAGS"
   ;;
   --disable-sdl) sdl="no"
   ;;
@@ -1977,7 +1979,6 @@ if test "$solaris" = "yes" ; then
 fi
 if test "$static" = "yes" ; then
   echo "CONFIG_STATIC=y" >> $config_host_mak
-  LDFLAGS="-static $LDFLAGS"
 fi
 if test $profiler = "yes" ; then
   echo "CONFIG_PROFILER=y" >> $config_host_mak
Signature · 1 line
1.6.6
#6 lore raw

PATCH 4/5 Remove check for grep

In reply to Juan Quintela

The rest of configure assumes that grep is available already

---
 configure |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
configure +1 -2
index 83f8b26..f31be87 100755
@@ -1430,8 +1430,7 @@ EOF
     fi
   else
     if test "$kvm" = "yes" ; then
-      if prog_exist "awk" && \
-         prog_exist "grep"; then
+      if prog_exist "awk" ; 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);}'`
Signature · 1 line
1.6.6
#7 lore raw

PATCH 5/5 Fix missing symbols in .rel/.rela.plt sections

In reply to Juan Quintela

From: Loïc Minier <[email protected]>

Fix .rel.plt sections in the output to not only include .rel.plt sections from the input but also the .rel.iplt sections and to define the hidden symbols __rel_iplt_start and __rel_iplt_end around .rel.iplt as otherwise we get undefined references to these when linking statically to a multilib libc.a. This fixes the static build under i386.

Apply similar logic to rela.plt/.iplt and __rela_iplt/_plt_start/_end to fix the static build under amd64.

---
 i386.ld   |   16 ++++++++++++++--
 x86_64.ld |   16 ++++++++++++++--
 2 files changed, 28 insertions(+), 4 deletions(-)
i386.ld +14 -2
index f2dafec..f8df7bf 100644
@@ -39,8 +39,20 @@ SECTIONS
   .rela.fini     : { *(.rela.fini)	}
   .rel.bss       : { *(.rel.bss)		}
   .rela.bss      : { *(.rela.bss)		}
-  .rel.plt       : { *(.rel.plt)		}
-  .rela.plt      : { *(.rela.plt)		}
+  .rel.plt      :
+  {
+    *(.rel.plt)
+    PROVIDE_HIDDEN (__rel_iplt_start = .);
+    *(.rel.iplt)
+    PROVIDE_HIDDEN (__rel_iplt_end = .);
+  }
+  .rela.plt       :
+  {
+    *(.rela.plt)
+    PROVIDE_HIDDEN (__rela_iplt_start = .);
+    *(.rela.iplt)
+    PROVIDE_HIDDEN (__rela_iplt_end = .);
+  }
   .init          : { *(.init)	} =0x47ff041f
   .text      :
   {
x86_64.ld +14 -2
index 24ea77d..46d8d4d 100644
@@ -35,8 +35,20 @@ SECTIONS
   .rela.got       : { *(.rela.got) }
   .rel.bss        : { *(.rel.bss .rel.bss.* .rel.gnu.linkonce.b.*) }
   .rela.bss       : { *(.rela.bss .rela.bss.* .rela.gnu.linkonce.b.*) }
-  .rel.plt        : { *(.rel.plt) }
-  .rela.plt       : { *(.rela.plt) }
+  .rel.plt      :
+  {
+    *(.rel.plt)
+    PROVIDE_HIDDEN (__rel_iplt_start = .);
+    *(.rel.iplt)
+    PROVIDE_HIDDEN (__rel_iplt_end = .);
+  }
+  .rela.plt       :
+  {
+    *(.rela.plt)
+    PROVIDE_HIDDEN (__rela_iplt_start = .);
+    *(.rela.iplt)
+    PROVIDE_HIDDEN (__rela_iplt_end = .);
+  }
   .init           :
   {
     KEEP (*(.init))
Signature · 1 line
1.6.6
#2 lore raw

Re: [Qemu-devel] [PATCH 1/5] substitute all uses of which by type

In reply to Juan Quintela
Quoted 5 lines · Juan Quintela

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.

See comments below. The changes proposed are a suggestion, no real need.

Regards,
Stefan

Quoted 10 lines

---
configure | 25 ++++++++++++++-----------

1 files changed, 14 insertions(+), 11 deletions(-)
configure +0 -0
index 5631bbb..71edaf5 100755
@@ -27,6 +27,11 @@ compile_prog() {

$cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags

Quoted 6 lines
Quoted 1 line

/dev/null 2> /dev/null

}

+prog_exist() {
+ prog="$1"
+ type $prog > /dev/null 2> /dev/null

I personally prefer
+ type $prog >/dev/null 2>&1
because it is shorter (no whitespace after >, no duplicate /dev/null). Currently, all variants to write this pattern are used. Maybe we can reduce this.

Quoted 11 lines
configure +2 -0
+}
+
# default parameters
cpu=""
prefix=""
configure +0 -0
@@ -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"

2>/dev/null

Quoted 4 lines

- if test -z "$solinst" ; then
+ if ! prog_exist "$install" ; then
echo "Solaris install program not found. Use
--install=/usr/ucb/install or"

Quoted 8 lines

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 "$install" 2> /dev/null | grep /usr/bin/install >/dev/null ; then

2>/dev/null

Quoted 53 lines

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"

configure +0 -0
@@ -969,7 +972,7 @@ fi
 # pkgconfig probe
 

pkgconfig="${cross_prefix}pkg-config"

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

configure +0 -0
@@ -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" && \
+ 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);}'`

configure +0 -0
@@ -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" && \
+ prog_exist "pod2man" ; then
docs=yes
else
if test "$docs" = "yes" ; then

#3 lore raw

[Qemu-devel] Re: [PATCH 1/5] substitute all uses of which by type

Quoted 29 lines · Stefan Weil
Quoted 5 lines · Juan Quintela

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.

See comments below. The changes proposed are a suggestion, no real need.

Regards,
Stefan

Quoted 10 lines

---
configure | 25 ++++++++++++++-----------

1 files changed, 14 insertions(+), 11 deletions(-)
configure +0 -0
index 5631bbb..71edaf5 100755
@@ -27,6 +27,11 @@ compile_prog() {

$cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags

Quoted 6 lines
Quoted 1 line

/dev/null 2> /dev/null

}

+prog_exist() {
+ prog="$1"
+ type $prog > /dev/null 2> /dev/null

Quoted 5 lines

I personally prefer
+ type $prog >/dev/null 2>&1
because it is shorter (no whitespace after >, no duplicate /dev/null). Currently, all variants to write this pattern are used. Maybe we can reduce this.

Agreed. For next series if there is not needed a rebase.

Later, Juan.