diff mbox

[RFA:] fix configury version checks for in-tree binutils

Message ID alpine.BSF.2.00.1208261043400.14399@dair.pair.com
State New
Headers show

Commit Message

Hans-Peter Nilsson Aug. 26, 2012, 2:47 p.m. UTC 
Found while investigating PR54373.  A combined tree (in-tree binutils)
using binutils post-2.22 is semi-broken at the moment: the version of
the assembler and linker can't be found.  The configury doesn't expect
the single-quote that has "appeared"; i.e.

 VERSION=2.22.0
vs.
 VERSION='2.23.51'

You see this in the build log as lots of:

checking assembler flags...
checking assembler for .balign and .p2align... /x/combined/gcc/configure: line 21848: test: -ge: unary operator expected
no
checking assembler for .p2align with maximum skip... /x/combined/gcc/configure: line 21884: test: -ge: unary operator expected
no

obviously causing all optional features in the assembler and linker to
be turned off.  (This could have been the cause of not seeing PR54373
in a combined tree, but that's actually due to a combination of two
other bugs.)

Here's a patch to fix in-tree binutils and to sanity-check that the
version is extracted to avoid semi-silent failures like the above.  An
alternative would be to scrap the special-case and just go with
out-of-tree feature tests; it seems few enough people use it, that this
has gone undetected for a while.  I don't know how important this is
to canadian-cross support though (where you can't run the assembler
and linker to test features).

I tested this by building a combined tree for mmix-knuth-mmixware with
top-of-tree binutils as well as binutils-2.22 from CVS (see quotes
above) past the point of failure.  I also built gcc with out-of-tree
binutils past the point-of-failure, as well as versions of the patch
in-tree and out-tree with just the error check, to verify that it
works (is not executed, for out-of-tree binutils).

I don't need to tell build maintainers how much of a problem it is
matching a single-quote character portably; I just went for any
non-numeric/identifier character.  I also don't need to tell build
maintainers that "?" and "+" are unportables in sed regexps.  The
changequote calls in the ld version check in configure.ac, are
necessary to avoid autoconf terminating with a too-deep-recursion
message for the AC_MSG_ERROR call, which can't be helped by quoting
the message (duh).  Can be alternatively fixed by re/moving the
other/outer pair of changequotes visible in the context of this patch
and to add corresponding [] where needed.  Too much fun for me.

N.B. combining CVS binutils-2.20 in-tree is broken due to mismatching
libtool versions.  I did not test CVS binutils-2.21.

Ok to commit?  I'd suggest applying this to 4.7 as well.
Ok there after test?

gcc:

	* acinclude.m4 (_gcc_COMPUTE_GAS_VERSION): Allow a single
	character to quote the VERSION= contents.  Sanity-check contents.
	* configure.ac ("what linker to use" ld version extraction): Ditto.
	* configure: Regenerate.


brgds, H-P

Comments

Hans-Peter Nilsson Sept. 3, 2012, 1:52 a.m. UTC | #1 
Ping with CC to build maintainers.
(Further pings, if any are needed, will be with URL only.)

On Sun, 26 Aug 2012, Hans-Peter Nilsson wrote:

> Found while investigating PR54373.  A combined tree (in-tree binutils)
> using binutils post-2.22 is semi-broken at the moment: the version of
> the assembler and linker can't be found.  The configury doesn't expect
> the single-quote that has "appeared"; i.e.
>
>  VERSION=2.22.0
> vs.
>  VERSION='2.23.51'
>
> You see this in the build log as lots of:
>
> checking assembler flags...
> checking assembler for .balign and .p2align... /x/combined/gcc/configure: line 21848: test: -ge: unary operator expected
> no
> checking assembler for .p2align with maximum skip... /x/combined/gcc/configure: line 21884: test: -ge: unary operator expected
> no
>
> obviously causing all optional features in the assembler and linker to
> be turned off.  (This could have been the cause of not seeing PR54373
> in a combined tree, but that's actually due to a combination of two
> other bugs.)
>
> Here's a patch to fix in-tree binutils and to sanity-check that the
> version is extracted to avoid semi-silent failures like the above.  An
> alternative would be to scrap the special-case and just go with
> out-of-tree feature tests; it seems few enough people use it, that this
> has gone undetected for a while.  I don't know how important this is
> to canadian-cross support though (where you can't run the assembler
> and linker to test features).
>
> I tested this by building a combined tree for mmix-knuth-mmixware with
> top-of-tree binutils as well as binutils-2.22 from CVS (see quotes
> above) past the point of failure.  I also built gcc with out-of-tree
> binutils past the point-of-failure, as well as versions of the patch
> in-tree and out-tree with just the error check, to verify that it
> works (is not executed, for out-of-tree binutils).
>
> I don't need to tell build maintainers how much of a problem it is
> matching a single-quote character portably; I just went for any
> non-numeric/identifier character.  I also don't need to tell build
> maintainers that "?" and "+" are unportables in sed regexps.  The
> changequote calls in the ld version check in configure.ac, are
> necessary to avoid autoconf terminating with a too-deep-recursion
> message for the AC_MSG_ERROR call, which can't be helped by quoting
> the message (duh).  Can be alternatively fixed by re/moving the
> other/outer pair of changequotes visible in the context of this patch
> and to add corresponding [] where needed.  Too much fun for me.
>
> N.B. combining CVS binutils-2.20 in-tree is broken due to mismatching
> libtool versions.  I did not test CVS binutils-2.21.
>
> Ok to commit?  I'd suggest applying this to 4.7 as well.
> Ok there after test?
>
> gcc:
>
> 	* acinclude.m4 (_gcc_COMPUTE_GAS_VERSION): Allow a single
> 	character to quote the VERSION= contents.  Sanity-check contents.
> 	* configure.ac ("what linker to use" ld version extraction): Ditto.
> 	* configure: Regenerate.
>
> Index: gcc/acinclude.m4
> ===================================================================
> --- gcc/acinclude.m4	(revision 190682)
> +++ gcc/acinclude.m4	(working copy)
> @@ -393,11 +393,15 @@ for f in $gcc_cv_as_bfd_srcdir/configure
>           $gcc_cv_as_gas_srcdir/configure \
>           $gcc_cv_as_gas_srcdir/configure.in \
>           $gcc_cv_as_gas_srcdir/Makefile.in ; do
> -  gcc_cv_gas_version=`sed -n -e 's/^[[ 	]]*\(VERSION=[[0-9]]*\.[[0-9]]*.*\)/\1/p' < $f`
> +  gcc_cv_gas_version=`sed -n -e 's/^[[ 	]]*VERSION=[[^0-9A-Za-z_]]*\([[0-9]]*\.[[0-9]]*.*\)/VERSION=\1/p' < $f`
>    if test x$gcc_cv_gas_version != x; then
>      break
>    fi
>  done
> +case $gcc_cv_gas_version in
> +  VERSION=[[0-9]]*) ;;
> +  *) AC_MSG_ERROR([[cannot find version of in-tree assembler]]);;
> +esac
>  gcc_cv_gas_major_version=`expr "$gcc_cv_gas_version" : "VERSION=\([[0-9]]*\)"`
>  gcc_cv_gas_minor_version=`expr "$gcc_cv_gas_version" : "VERSION=[[0-9]]*\.\([[0-9]]*\)"`
>  gcc_cv_gas_patch_version=`expr "$gcc_cv_gas_version" : "VERSION=[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)"`
> Index: gcc/configure.ac
> ===================================================================
> --- gcc/configure.ac	(revision 190682)
> +++ gcc/configure.ac	(working copy)
> @@ -2030,11 +2030,17 @@ if test "$gcc_cv_ld" = ../ld/ld-new$buil
>  	for f in $gcc_cv_ld_bfd_srcdir/configure $gcc_cv_ld_gld_srcdir/configure $gcc_cv_ld_gld_srcdir/configure.in $gcc_cv_ld_gld_srcdir/Makefile.in
>  	do
>  changequote(,)dnl
> -		gcc_cv_gld_version=`sed -n -e 's/^[ 	]*\(VERSION=[0-9]*\.[0-9]*.*\)/\1/p' < $f`
> +		gcc_cv_gld_version=`sed -n -e 's/^[ 	]*VERSION=[^0-9A-Za-z_]*\([0-9]*\.[0-9]*.*\)/VERSION=\1/p' < $f`
>  		if test x$gcc_cv_gld_version != x; then
>  			break
>  		fi
>  	done
> +	case $gcc_cv_gld_version in
> +	  VERSION=[0-9]*) ;;
> +changequote([,])dnl
> +	  *) AC_MSG_ERROR([[cannot find version of in-tree linker]]) ;;
> +changequote(,)dnl
> +	esac
>  	gcc_cv_gld_major_version=`expr "$gcc_cv_gld_version" : "VERSION=\([0-9]*\)"`
>  	gcc_cv_gld_minor_version=`expr "$gcc_cv_gld_version" : "VERSION=[0-9]*\.\([0-9]*\)"`
>  changequote([,])dnl
>
> brgds, H-P
>
Paolo Bonzini Sept. 3, 2012, 6:47 a.m. UTC | #2 
Il 03/09/2012 03:52, Hans-Peter Nilsson ha scritto:
> Ping with CC to build maintainers.
> (Further pings, if any are needed, will be with URL only.)
> 
> On Sun, 26 Aug 2012, Hans-Peter Nilsson wrote:
> 
>> Found while investigating PR54373.  A combined tree (in-tree binutils)
>> using binutils post-2.22 is semi-broken at the moment: the version of
>> the assembler and linker can't be found.  The configury doesn't expect
>> the single-quote that has "appeared"; i.e.
>>
>>  VERSION=2.22.0
>> vs.
>>  VERSION='2.23.51'
>>
>> You see this in the build log as lots of:
>>
>> checking assembler flags...
>> checking assembler for .balign and .p2align... /x/combined/gcc/configure: line 21848: test: -ge: unary operator expected
>> no
>> checking assembler for .p2align with maximum skip... /x/combined/gcc/configure: line 21884: test: -ge: unary operator expected
>> no
>>
>> obviously causing all optional features in the assembler and linker to
>> be turned off.  (This could have been the cause of not seeing PR54373
>> in a combined tree, but that's actually due to a combination of two
>> other bugs.)
>>
>> Here's a patch to fix in-tree binutils and to sanity-check that the
>> version is extracted to avoid semi-silent failures like the above.  An
>> alternative would be to scrap the special-case and just go with
>> out-of-tree feature tests; it seems few enough people use it, that this
>> has gone undetected for a while.  I don't know how important this is
>> to canadian-cross support though (where you can't run the assembler
>> and linker to test features).
>>
>> I tested this by building a combined tree for mmix-knuth-mmixware with
>> top-of-tree binutils as well as binutils-2.22 from CVS (see quotes
>> above) past the point of failure.  I also built gcc with out-of-tree
>> binutils past the point-of-failure, as well as versions of the patch
>> in-tree and out-tree with just the error check, to verify that it
>> works (is not executed, for out-of-tree binutils).
>>
>> I don't need to tell build maintainers how much of a problem it is
>> matching a single-quote character portably; I just went for any
>> non-numeric/identifier character.  I also don't need to tell build
>> maintainers that "?" and "+" are unportables in sed regexps.  The
>> changequote calls in the ld version check in configure.ac, are
>> necessary to avoid autoconf terminating with a too-deep-recursion
>> message for the AC_MSG_ERROR call, which can't be helped by quoting
>> the message (duh).  Can be alternatively fixed by re/moving the
>> other/outer pair of changequotes visible in the context of this patch
>> and to add corresponding [] where needed.  Too much fun for me.
>>
>> N.B. combining CVS binutils-2.20 in-tree is broken due to mismatching
>> libtool versions.  I did not test CVS binutils-2.21.
>>
>> Ok to commit?  I'd suggest applying this to 4.7 as well.
>> Ok there after test?
>>
>> gcc:
>>
>> 	* acinclude.m4 (_gcc_COMPUTE_GAS_VERSION): Allow a single
>> 	character to quote the VERSION= contents.  Sanity-check contents.
>> 	* configure.ac ("what linker to use" ld version extraction): Ditto.
>> 	* configure: Regenerate.
>>
>> Index: gcc/acinclude.m4
>> ===================================================================
>> --- gcc/acinclude.m4	(revision 190682)
>> +++ gcc/acinclude.m4	(working copy)
>> @@ -393,11 +393,15 @@ for f in $gcc_cv_as_bfd_srcdir/configure
>>           $gcc_cv_as_gas_srcdir/configure \
>>           $gcc_cv_as_gas_srcdir/configure.in \
>>           $gcc_cv_as_gas_srcdir/Makefile.in ; do
>> -  gcc_cv_gas_version=`sed -n -e 's/^[[ 	]]*\(VERSION=[[0-9]]*\.[[0-9]]*.*\)/\1/p' < $f`
>> +  gcc_cv_gas_version=`sed -n -e 's/^[[ 	]]*VERSION=[[^0-9A-Za-z_]]*\([[0-9]]*\.[[0-9]]*.*\)/VERSION=\1/p' < $f`
>>    if test x$gcc_cv_gas_version != x; then
>>      break
>>    fi
>>  done
>> +case $gcc_cv_gas_version in
>> +  VERSION=[[0-9]]*) ;;
>> +  *) AC_MSG_ERROR([[cannot find version of in-tree assembler]]);;
>> +esac
>>  gcc_cv_gas_major_version=`expr "$gcc_cv_gas_version" : "VERSION=\([[0-9]]*\)"`
>>  gcc_cv_gas_minor_version=`expr "$gcc_cv_gas_version" : "VERSION=[[0-9]]*\.\([[0-9]]*\)"`
>>  gcc_cv_gas_patch_version=`expr "$gcc_cv_gas_version" : "VERSION=[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)"`
>> Index: gcc/configure.ac
>> ===================================================================
>> --- gcc/configure.ac	(revision 190682)
>> +++ gcc/configure.ac	(working copy)
>> @@ -2030,11 +2030,17 @@ if test "$gcc_cv_ld" = ../ld/ld-new$buil
>>  	for f in $gcc_cv_ld_bfd_srcdir/configure $gcc_cv_ld_gld_srcdir/configure $gcc_cv_ld_gld_srcdir/configure.in $gcc_cv_ld_gld_srcdir/Makefile.in
>>  	do
>>  changequote(,)dnl
>> -		gcc_cv_gld_version=`sed -n -e 's/^[ 	]*\(VERSION=[0-9]*\.[0-9]*.*\)/\1/p' < $f`
>> +		gcc_cv_gld_version=`sed -n -e 's/^[ 	]*VERSION=[^0-9A-Za-z_]*\([0-9]*\.[0-9]*.*\)/VERSION=\1/p' < $f`
>>  		if test x$gcc_cv_gld_version != x; then
>>  			break
>>  		fi
>>  	done
>> +	case $gcc_cv_gld_version in
>> +	  VERSION=[0-9]*) ;;
>> +changequote([,])dnl
>> +	  *) AC_MSG_ERROR([[cannot find version of in-tree linker]]) ;;
>> +changequote(,)dnl
>> +	esac
>>  	gcc_cv_gld_major_version=`expr "$gcc_cv_gld_version" : "VERSION=\([0-9]*\)"`
>>  	gcc_cv_gld_minor_version=`expr "$gcc_cv_gld_version" : "VERSION=[0-9]*\.\([0-9]*\)"`
>>  changequote([,])dnl
>>
>> brgds, H-P
>>
> 

Ok.

Paolo
diff mbox

Patch

Index: gcc/acinclude.m4
===================================================================
--- gcc/acinclude.m4	(revision 190682)
+++ gcc/acinclude.m4	(working copy)
@@ -393,11 +393,15 @@  for f in $gcc_cv_as_bfd_srcdir/configure
          $gcc_cv_as_gas_srcdir/configure \
          $gcc_cv_as_gas_srcdir/configure.in \
          $gcc_cv_as_gas_srcdir/Makefile.in ; do
-  gcc_cv_gas_version=`sed -n -e 's/^[[ 	]]*\(VERSION=[[0-9]]*\.[[0-9]]*.*\)/\1/p' < $f`
+  gcc_cv_gas_version=`sed -n -e 's/^[[ 	]]*VERSION=[[^0-9A-Za-z_]]*\([[0-9]]*\.[[0-9]]*.*\)/VERSION=\1/p' < $f`
   if test x$gcc_cv_gas_version != x; then
     break
   fi
 done
+case $gcc_cv_gas_version in
+  VERSION=[[0-9]]*) ;;
+  *) AC_MSG_ERROR([[cannot find version of in-tree assembler]]);;
+esac
 gcc_cv_gas_major_version=`expr "$gcc_cv_gas_version" : "VERSION=\([[0-9]]*\)"`
 gcc_cv_gas_minor_version=`expr "$gcc_cv_gas_version" : "VERSION=[[0-9]]*\.\([[0-9]]*\)"`
 gcc_cv_gas_patch_version=`expr "$gcc_cv_gas_version" : "VERSION=[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)"`
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 190682)
+++ gcc/configure.ac	(working copy)
@@ -2030,11 +2030,17 @@  if test "$gcc_cv_ld" = ../ld/ld-new$buil
 	for f in $gcc_cv_ld_bfd_srcdir/configure $gcc_cv_ld_gld_srcdir/configure $gcc_cv_ld_gld_srcdir/configure.in $gcc_cv_ld_gld_srcdir/Makefile.in
 	do
 changequote(,)dnl
-		gcc_cv_gld_version=`sed -n -e 's/^[ 	]*\(VERSION=[0-9]*\.[0-9]*.*\)/\1/p' < $f`
+		gcc_cv_gld_version=`sed -n -e 's/^[ 	]*VERSION=[^0-9A-Za-z_]*\([0-9]*\.[0-9]*.*\)/VERSION=\1/p' < $f`
 		if test x$gcc_cv_gld_version != x; then
 			break
 		fi
 	done
+	case $gcc_cv_gld_version in
+	  VERSION=[0-9]*) ;;
+changequote([,])dnl
+	  *) AC_MSG_ERROR([[cannot find version of in-tree linker]]) ;;
+changequote(,)dnl
+	esac
 	gcc_cv_gld_major_version=`expr "$gcc_cv_gld_version" : "VERSION=\([0-9]*\)"`
 	gcc_cv_gld_minor_version=`expr "$gcc_cv_gld_version" : "VERSION=[0-9]*\.\([0-9]*\)"`
 changequote([,])dnl