* [musl] [PATCH 2/2] configure: improve portability of test command
2020-10-10 18:51 [musl] [PATCH 1/2] configure: do not use obsolescent form of test -a|o Issam E. Maghni
@ 2020-10-10 18:51 ` Issam E. Maghni
2020-10-11 0:08 ` Rich Felker
2020-10-11 0:14 ` [musl] [PATCH 1/2] configure: do not use obsolescent form of test -a|o Rich Felker
1 sibling, 1 reply; 6+ messages in thread
From: Issam E. Maghni @ 2020-10-10 18:51 UTC (permalink / raw)
To: musl; +Cc: Issam E. Maghni
> The two commands:
> test "$1"
> test ! "$1"
> could not be used reliably on some historical systems. Unexpected results
> would occur if such a string expression were used and $1 expanded to '!', '(',
> or a known unary primary. Better constructs are:
> test -n "$1"
> test -z "$1"
> respectively.
> Historical systems have also been unreliable given the common construct:
> test "$response" = "expected string"
> One of the following is a more reliable form:
> test "X$response" = "Xexpected string"
> test "expected string" = "$response"
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html#tag_20_128_16
Signed-off-by: Issam E. Maghni <issam.e.maghni@mailbox.org>
---
configure | 88 +++++++++++++++++++++++++++----------------------------
1 file changed, 44 insertions(+), 44 deletions(-)
diff --git a/configure b/configure
index a5231a0e..a57ae6e1 100755
--- a/configure
+++ b/configure
@@ -203,8 +203,8 @@ stripdir srcdir
fi
abs_builddir="$(pwd)" || fail "$0: cannot determine working directory"
abs_srcdir="$(cd $srcdir && pwd)" || fail "$0: invalid source directory $srcdir"
-test "$abs_srcdir" = "$abs_builddir" && srcdir=.
-test "$srcdir" != "." && test -f Makefile && test ! -h Makefile && fail "$0: Makefile already exists in the working directory"
+test "x${abs_srcdir}" = "x${abs_builddir}" && srcdir=.
+test '.' != "$srcdir" && test -f Makefile && test ! -h Makefile && fail "$0: Makefile already exists in the working directory"
#
# Get a temp filename we can use
@@ -214,7 +214,7 @@ set -C
while : ; do i=$(($i+1))
tmpc="./conf$$-$PPID-$i.c"
2>|/dev/null > "$tmpc" && break
-test "$i" -gt 50 && fail "$0: cannot create temporary file $tmpc"
+test 50 -le "$i" && fail "$0: cannot create temporary file $tmpc"
done
set +C
trap 'rm "$tmpc"' EXIT INT QUIT TERM HUP
@@ -222,17 +222,17 @@ trap 'rm "$tmpc"' EXIT INT QUIT TERM HUP
#
# Check that the requested malloc implementation exists
#
-test -d "$srcdir/src/malloc/$malloc_dir" \
+test -d "${srcdir}/src/malloc/${malloc_dir}" \
|| fail "$0: error: chosen malloc implementation '$malloc_dir' does not exist"
#
# Check whether we are cross-compiling, and set a default
# CROSS_COMPILE prefix if none was provided.
#
-test "$target" && \
-test "$target" != "$build" && \
+test -n "$target" && \
+test "x${target}" != "x${build}" && \
test -z "$CROSS_COMPILE" && \
-CROSS_COMPILE="$target-"
+CROSS_COMPILE="${target}-"
#
# Find a C compiler to use
@@ -279,33 +279,33 @@ echo "$cc_family"
#
# Figure out toolchain wrapper to build
#
-if test "$wrapper" = auto || test "$wrapper" = detect ; then
+if test 'auto' = "$wrapper" || test 'detect' = "$wrapper" ; then
echo "#include <stdlib.h>" > "$tmpc"
echo "#if ! __GLIBC__" >> "$tmpc"
echo "#error no" >> "$tmpc"
echo "#endif" >> "$tmpc"
printf "checking for toolchain wrapper to build... "
-if test "$wrapper" = auto && ! $CC -c -o /dev/null "$tmpc" >/dev/null 2>&1 ; then
+if test 'auto' = "$wrapper" && ! $CC -c -o /dev/null "$tmpc" >/dev/null 2>&1 ; then
echo "none"
-elif test "$cc_family" = gcc ; then
+elif test 'gcc' = "$cc_family" ; then
gcc_wrapper=yes
echo "gcc"
-elif test "$cc_family" = clang ; then
+elif test 'clang' = "$cc_family" ; then
clang_wrapper=yes
echo "clang"
else
echo "none"
-if test "$wrapper" = detect ; then
+if test 'detect' = "$wrapper" ; then
fail "$0: could not find an appropriate toolchain wrapper"
fi
fi
fi
-if test "$gcc_wrapper" = yes ; then
+if test 'yes' = "$gcc_wrapper" ; then
tools="$tools obj/musl-gcc"
tool_libs="$tool_libs lib/musl-gcc.specs"
fi
-if test "$clang_wrapper" = yes ; then
+if test 'yes' = "$clang_wrapper" ; then
tools="$tools obj/musl-clang obj/ld.musl-clang"
fi
@@ -350,7 +350,7 @@ tryflag CFLAGS_C99FSE -nostdinc
tryflag CFLAGS_C99FSE -ffreestanding \
|| tryflag CFLAGS_C99FSE -fno-builtin
tryflag CFLAGS_C99FSE -fexcess-precision=standard \
-|| { test "$ARCH" = i386 && tryflag CFLAGS_C99FSE -ffloat-store ; }
+|| { test 'i386' = "$ARCH" && tryflag CFLAGS_C99FSE -ffloat-store ; }
tryflag CFLAGS_C99FSE -frounding-math
#
@@ -400,7 +400,7 @@ tryflag CFLAGS_MEMOPS -fno-tree-loop-distribute-patterns
#
# Enable debugging if requessted.
#
-test "$debug" = yes && CFLAGS_AUTO=-g
+test 'yes' = "$debug" && CFLAGS_AUTO=-g
#
# Preprocess asm files to add extra debugging information if debug is
@@ -409,7 +409,7 @@ test "$debug" = yes && CFLAGS_AUTO=-g
#
printf "checking whether we should preprocess assembly to add debugging information... "
if fnmatch '-g*|*\ -g*' "$CFLAGS_AUTO $CFLAGS" &&
- test -f "tools/add-cfi.$ARCH.awk" &&
+ test -f "tools/add-cfi.${ARCH}.awk" &&
printf ".file 1 \"srcfile.s\"\n.line 1\n.cfi_startproc\n.cfi_endproc" | $CC -g -x assembler -c -o /dev/null 2>/dev/null -
then
ADD_CFI=yes
@@ -436,13 +436,13 @@ xno|x) printf "disabled\n" ; optimize=no ;;
*) printf "custom\n" ;;
esac
-test "$optimize" = no || tryflag CFLAGS_AUTO -Os || tryflag CFLAGS_AUTO -O2
-test "$optimize" = yes && optimize="internal,malloc,string"
+test 'no' = "$optimize" || tryflag CFLAGS_AUTO -Os || tryflag CFLAGS_AUTO -O2
+test 'yes' = "$optimize" && optimize="internal,malloc,string"
if fnmatch 'no|size' "$optimize" ; then :
else
printf "components to be optimized for speed:"
-while test "$optimize" ; do
+while test -n "$optimize" ; do
case "$optimize" in
*,*) this=${optimize%%,*} optimize=${optimize#*,} ;;
*) this=$optimize optimize=
@@ -500,7 +500,7 @@ tryflag CFLAGS_AUTO -fdata-sections
# Some build environments pass -march and -mtune options via CC, so
# check both CC and CFLAGS.
#
-if test "$ARCH" = "i386" ; then
+if test 'i386' = "$ARCH" ; then
fnmatch '-march=*|*\ -march=*' "$CC $CFLAGS" || tryldflag CFLAGS_AUTO -march=i486
fnmatch '-mtune=*|*\ -mtune=*' "$CC $CFLAGS" || tryldflag CFLAGS_AUTO -mtune=generic
fi
@@ -511,7 +511,7 @@ fi
# to start from a clean slate. So use -w if building with clang. Also
# turn off a common on-by-default cast warning regardless of compiler.
#
-test "$cc_family" = clang && tryflag CFLAGS_AUTO -w
+test 'clang' = "$cc_family" && tryflag CFLAGS_AUTO -w
tryflag CFLAGS_AUTO -Wno-pointer-to-int-cast
@@ -535,9 +535,9 @@ tryflag CFLAGS_AUTO -Werror=discarded-array-qualifiers
# parameter to stop printing warnings about LDFLAGS passed during
# compiling stage and CFLAGS passed during linking stage.
#
-test "$cc_family" = clang && tryflag CFLAGS_AUTO -Qunused-arguments
+test 'clang' = "$cc_family" && tryflag CFLAGS_AUTO -Qunused-arguments
-if test "x$warnings" = xyes ; then
+if test 'yes' = "$warnings" ; then
tryflag CFLAGS_AUTO -Waddress
tryflag CFLAGS_AUTO -Warray-bounds
tryflag CFLAGS_AUTO -Wchar-subscripts
@@ -604,7 +604,7 @@ printf "using compiler runtime libraries: %s\n" "$LIBCC"
SUBARCH=
t="$CFLAGS_C99FSE $CPPFLAGS $CFLAGS"
-if test "$ARCH" = "i386" ; then
+if test 'i386' = "$ARCH" ; then
printf "checking whether compiler can use ebx in PIC asm constraints... "
cat > "$tmpc" <<EOF
int foo(int x) { __asm__ ( "" : "+b"(x) ); return x; }
@@ -618,11 +618,11 @@ CFLAGS_AUTO="$CFLAGS_AUTO -DBROKEN_EBX_ASM"
fi
fi
-if test "$ARCH" = "x86_64" ; then
+if test 'x86_64' = "$ARCH" ; then
trycppif __ILP32__ "$t" && ARCH=x32
fi
-if test "$ARCH" = "arm" ; then
+if test 'arm' = "$ARCH" ; then
if trycppif __thumb2__ "$t" ; then
tryflag CFLAGS_AUTO -mimplicit-it=always
tryflag CFLAGS_AUTO -Wa,-mimplicit-it=always
@@ -633,7 +633,7 @@ trycppif __ARM_PCS_VFP "$t" && SUBARCH=${SUBARCH}hf
# Versions of clang up until at least 3.8 have the wrong constraint codes
# for floating point operands to inline asm. Detect this so the affected
# source files can just disable the asm.
-if test "$cc_family" = clang ; then
+if test 'clang' = "$cc_family" ; then
printf "checking whether clang's vfp asm constraints work... "
echo 'float f(float x) { __asm__("":"+t"(x)); return x; }' > "$tmpc"
if $CC $CFLAGS_C99FSE $CPPFLAGS $CFLAGS -c -o /dev/null "$tmpc" >/dev/null 2>&1 ; then
@@ -646,31 +646,31 @@ fi
fi
fi
-if test "$ARCH" = "aarch64" ; then
+if test 'aarch64' = "$ARCH" ; then
trycppif __AARCH64EB__ "$t" && SUBARCH=${SUBARCH}_be
fi
-if test "$ARCH" = "m68k" ; then
+if test 'm68k' = "$ARCH" ; then
if trycppif "__HAVE_68881__" ; then : ;
elif trycppif "__mcffpu__" ; then SUBARCH="-fp64"
else SUBARCH="-sf"
fi
fi
-if test "$ARCH" = "mips" ; then
+if test 'mips' = "$ARCH" ; then
trycppif "__mips_isa_rev >= 6" "$t" && SUBARCH=${SUBARCH}r6
trycppif "_MIPSEL || __MIPSEL || __MIPSEL__" "$t" && SUBARCH=${SUBARCH}el
trycppif __mips_soft_float "$t" && SUBARCH=${SUBARCH}-sf
fi
-if test "$ARCH" = "mips64" ; then
+if test 'mips64' = "$ARCH" ; then
trycppif "_MIPS_SIM != _ABI64" "$t" && ARCH=mipsn32
trycppif "__mips_isa_rev >= 6" "$t" && SUBARCH=${SUBARCH}r6
trycppif "_MIPSEL || __MIPSEL || __MIPSEL__" "$t" && SUBARCH=${SUBARCH}el
trycppif __mips_soft_float "$t" && SUBARCH=${SUBARCH}-sf
fi
-if test "$ARCH" = "powerpc" ; then
+if test 'powerpc' = "$ARCH" ; then
trycppif "__NO_FPRS__ && !_SOFT_FLOAT" "$t" && fail \
"$0: error: compiler's floating point configuration is unsupported"
trycppif _SOFT_FLOAT "$t" && SUBARCH=${SUBARCH}-sf
@@ -685,21 +685,21 @@ CFLAGS_AUTO="${CFLAGS_AUTO# }"
fi
fi
-test "$ARCH" = "microblaze" && trycppif __MICROBLAZEEL__ "$t" \
+test 'microblaze' = "$ARCH" && trycppif __MICROBLAZEEL__ "$t" \
&& SUBARCH=${SUBARCH}el
-if test "$ARCH" = "powerpc64" ; then
+if test 'powerpc64' = "$ARCH" ; then
trycppif "_CALL_ELF == 2" "$t" || fail "$0: error: unsupported powerpc64 ABI"
trycppif __LITTLE_ENDIAN__ "$t" && SUBARCH=${SUBARCH}le
trycppif _SOFT_FLOAT "$t" && fail "$0: error: soft-float not supported on powerpc64"
fi
-if test "$ARCH" = "riscv64" ; then
+if test 'riscv64' = "$ARCH" ; then
trycppif __riscv_float_abi_soft "$t" && SUBARCH=${SUBARCH}-sf
trycppif __riscv_float_abi_single "$t" && SUBARCH=${SUBARCH}-sp
fi
-if test "$ARCH" = "sh" ; then
+if test 'sh' = "$ARCH" ; then
tryflag CFLAGS_AUTO -Wa,--isa=any
trycppif __BIG_ENDIAN__ "$t" && SUBARCH=${SUBARCH}eb
if trycppif "__SH_FPU_ANY__ || __SH4__" "$t" ; then
@@ -722,7 +722,7 @@ SUBARCH=${SUBARCH}-fdpic
fi
fi
-test "$SUBARCH" \
+test -n "$SUBARCH" \
&& printf "configured for %s variant: %s\n" "$ARCH" "$ARCH$SUBARCH"
case "$ARCH$SUBARCH" in
@@ -802,13 +802,13 @@ TOOL_LIBS = $tool_libs
ADD_CFI = $ADD_CFI
MALLOC_DIR = $malloc_dir
EOF
-test "x$static" = xno && echo "STATIC_LIBS ="
-test "x$shared" = xno && echo "SHARED_LIBS ="
-test "x$cc_family" = xgcc && echo 'WRAPCC_GCC = $(CC)'
-test "x$cc_family" = xclang && echo 'WRAPCC_CLANG = $(CC)'
-test "x$pic_default" = xyes && echo 'AOBJS = $(LOBJS)'
+test 'no' = "$static" && echo "STATIC_LIBS ="
+test 'no' = "$shared" && echo "SHARED_LIBS ="
+test 'gcc' = "$cc_family" && echo 'WRAPCC_GCC = $(CC)'
+test 'clang' = "$cc_family" && echo 'WRAPCC_CLANG = $(CC)'
+test 'yes' = "$pic_default" && echo 'AOBJS = $(LOBJS)'
exec 1>&3 3>&-
-test "$srcdir" = "." || ln -sf $srcdir/Makefile .
+test '.' = "$srcdir" || ln -sf $srcdir/Makefile .
printf "done\n"
--
2.28.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [musl] [PATCH 1/2] configure: do not use obsolescent form of test -a|o
2020-10-10 18:51 [musl] [PATCH 1/2] configure: do not use obsolescent form of test -a|o Issam E. Maghni
2020-10-10 18:51 ` [musl] [PATCH 2/2] configure: improve portability of test command Issam E. Maghni
@ 2020-10-11 0:14 ` Rich Felker
2020-10-11 16:46 ` Issam E. Maghni
1 sibling, 1 reply; 6+ messages in thread
From: Rich Felker @ 2020-10-11 0:14 UTC (permalink / raw)
To: musl
On Sat, Oct 10, 2020 at 06:51:09PM +0000, Issam E. Maghni wrote:
> > The XSI extensions specifying the -a and -o binary primaries and the '(' and
> > ')' operators have been marked obsolescent.
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html#tag_20_128_16
>
> Signed-off-by: Issam E. Maghni <issam.e.maghni@mailbox.org>
Please keep commit messages to at most 76 columns, preferably less ala
email conventions, so that git log is readable in 80, and avoid block
quoting or other formatting that might not work well in all contexts
it's displayed in. A description is also preferred to quoting in
general, e.g. here:
The -a and -o operators are obsolescent and not in baseline POSIX.
Also no need for S-o-b.
Otherwise the patch itself looks good. Thanks for catching this.
Rich
> ---
> configure | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 947adf41..a5231a0e 100755
> --- a/configure
> +++ b/configure
> @@ -204,7 +204,7 @@ fi
> abs_builddir="$(pwd)" || fail "$0: cannot determine working directory"
> abs_srcdir="$(cd $srcdir && pwd)" || fail "$0: invalid source directory $srcdir"
> test "$abs_srcdir" = "$abs_builddir" && srcdir=.
> -test "$srcdir" != "." -a -f Makefile -a ! -h Makefile && fail "$0: Makefile already exists in the working directory"
> +test "$srcdir" != "." && test -f Makefile && test ! -h Makefile && fail "$0: Makefile already exists in the working directory"
>
> #
> # Get a temp filename we can use
> @@ -279,7 +279,7 @@ echo "$cc_family"
> #
> # Figure out toolchain wrapper to build
> #
> -if test "$wrapper" = auto -o "$wrapper" = detect ; then
> +if test "$wrapper" = auto || test "$wrapper" = detect ; then
> echo "#include <stdlib.h>" > "$tmpc"
> echo "#if ! __GLIBC__" >> "$tmpc"
> echo "#error no" >> "$tmpc"
> @@ -468,7 +468,7 @@ tryflag CFLAGS_AUTO -pipe
> # pointer is no longer needed for debugging.
> #
> if fnmatch '-g*|*\ -g*' "$CFLAGS_AUTO $CFLAGS" ; then :
> -else
> +else
> tryflag CFLAGS_AUTO -fomit-frame-pointer
> fi
>
> --
> 2.28.0
^ permalink raw reply [flat|nested] 6+ messages in thread