diff mbox

[U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains

Message ID 871tn78t71.fsf@nbsps.com
State RFC
Headers show

Commit Message

Bill Pringlemeir Jan. 6, 2015, 10:01 p.m. UTC
>> On Tue, Jan 06, 2015 at 11:27:43AM +0100, Hans de Goede wrote:

>>> ping ? current master still has this regression, it is not fatal,
>>> but it is not
>>> pretty either.

> On  6 Jan 2015, trini@ti.com wrote:

>> Did you see my earlier reply?  It's OK with vanilla toolchains (see
>> ELDK) and Linaro ones, but breaking on how Fedora mangles the version
>> string.  I'm _not_ saying it's a Fedora problem, but can you poke at
>> this a bit?  If not, I'll play with echo and see if I can do it.

On  6 Jan 2015, bpringlemeir@nbsps.com wrote:

> diff --git a/scripts/binutils-version.sh b/scripts/binutils-version.sh
> index 0bc26cf..955267d 100755 --- a/scripts/binutils-version.sh +++
> b/scripts/binutils-version.sh @@ -5,7 +5,6 @@ # Prints the binutils
> version of `gas-command' in a canonical 4-digit form # such as `0222'
> for binutils 2.22 # - gas="$*"
>
> if [ ${#gas} -eq 0 ]; then
> @@ -14,9 +13,9 @@ if [ ${#gas} -eq 0 ]; then
> exit 1
> fi
>
> -version_string=$($gas --version | head -1 | sed -e 's/.*)
> *\([0-9.]*\).*/\1/' ) +version_string=$($gas --version | head -1 | sed
> -e 's/.*) *\([0-9\.]*\).*/\1/' )
>
> -MAJOR=$(echo $version_string | cut -d . -f 1)
> -MINOR=$(echo $version_string | cut -d . -f 2)
> +MAJOR=$(echo "$version_string" | cut -d . -f 1)
> +MINOR=$(echo "$version_string" | cut -d . -f 2)
>
> -printf "%02d%02d\\n" $MAJOR $MINOR
> +printf "%02d%02d\\n" "$MAJOR" "$MINOR"
>
>
> The main issue is quoting of the 'sed' expression.  We had regex
> [0-9.], but we want [0-9\.] so that we match a literal '.' as opposed
> to anything.  Or so I speculate.  I made a script that output the
> version info Hans has and after the patch it is fine.

Err none of the above. The RedHat binutils doesnt have a 'package'
string.  With brackets.  For example,

   GNU assembler (GNU Binutils for Ubuntu) 2.24
                                           ^^^^
   GNU assembler (crosstool-NG 1.20.0 - nbsps) 2.24
                                               ^^^^
It is just, 

   GNU assembler version 2.24.0-6.fc21 20140613
   -----------------------------------
                         ++++++

Below gets the first ##.## after a bracket or the first ##.## No
escaping of the . is needed inside the range of course.

Comments

Tom Rini Jan. 6, 2015, 10:07 p.m. UTC | #1
On Tue, Jan 06, 2015 at 05:01:22PM -0500, Bill Pringlemeir wrote:
> >> On Tue, Jan 06, 2015 at 11:27:43AM +0100, Hans de Goede wrote:
> 
> >>> ping ? current master still has this regression, it is not fatal,
> >>> but it is not
> >>> pretty either.
> 
> > On  6 Jan 2015, trini@ti.com wrote:
> 
> >> Did you see my earlier reply?  It's OK with vanilla toolchains (see
> >> ELDK) and Linaro ones, but breaking on how Fedora mangles the version
> >> string.  I'm _not_ saying it's a Fedora problem, but can you poke at
> >> this a bit?  If not, I'll play with echo and see if I can do it.
> 
> On  6 Jan 2015, bpringlemeir@nbsps.com wrote:
> 
> > diff --git a/scripts/binutils-version.sh b/scripts/binutils-version.sh
> > index 0bc26cf..955267d 100755 --- a/scripts/binutils-version.sh +++
> > b/scripts/binutils-version.sh @@ -5,7 +5,6 @@ # Prints the binutils
> > version of `gas-command' in a canonical 4-digit form # such as `0222'
> > for binutils 2.22 # - gas="$*"
> >
> > if [ ${#gas} -eq 0 ]; then
> > @@ -14,9 +13,9 @@ if [ ${#gas} -eq 0 ]; then
> > exit 1
> > fi
> >
> > -version_string=$($gas --version | head -1 | sed -e 's/.*)
> > *\([0-9.]*\).*/\1/' ) +version_string=$($gas --version | head -1 | sed
> > -e 's/.*) *\([0-9\.]*\).*/\1/' )
> >
> > -MAJOR=$(echo $version_string | cut -d . -f 1)
> > -MINOR=$(echo $version_string | cut -d . -f 2)
> > +MAJOR=$(echo "$version_string" | cut -d . -f 1)
> > +MINOR=$(echo "$version_string" | cut -d . -f 2)
> >
> > -printf "%02d%02d\\n" $MAJOR $MINOR
> > +printf "%02d%02d\\n" "$MAJOR" "$MINOR"
> >
> >
> > The main issue is quoting of the 'sed' expression.  We had regex
> > [0-9.], but we want [0-9\.] so that we match a literal '.' as opposed
> > to anything.  Or so I speculate.  I made a script that output the
> > version info Hans has and after the patch it is fine.
> 
> Err none of the above. The RedHat binutils doesnt have a 'package'
> string.  With brackets.  For example,
> 
>    GNU assembler (GNU Binutils for Ubuntu) 2.24
>                                            ^^^^
>    GNU assembler (crosstool-NG 1.20.0 - nbsps) 2.24
>                                                ^^^^
> It is just, 
> 
>    GNU assembler version 2.24.0-6.fc21 20140613
>    -----------------------------------
>                          ++++++
> 
> Below gets the first ##.## after a bracket or the first ##.## No
> escaping of the . is needed inside the range of course.
> 
> diff --git a/scripts/binutils-version.sh b/scripts/binutils-version.sh
> index 0bc26cf..b1afc98 100755
> --- a/scripts/binutils-version.sh
> +++ b/scripts/binutils-version.sh
> @@ -5,7 +5,6 @@
>  # Prints the binutils version of `gas-command' in a canonical 4-digit form
>  # such as `0222' for binutils 2.22
>  #
> -
>  gas="$*"
>  
>  if [ ${#gas} -eq 0 ]; then
> @@ -15,6 +14,7 @@ if [ ${#gas} -eq 0 ]; then
>  fi
>  
>  version_string=$($gas --version | head -1 | sed -e 's/.*) *\([0-9.]*\).*/\1/' )
> +version_string=$(echo "$version_string" | sed -e 's/[^0-9]*\([0-9.]*\).*/\1/')
>  
>  MAJOR=$(echo $version_string | cut -d . -f 1)
>  MINOR=$(echo $version_string | cut -d . -f 2)

Thanks!  Can you post a proper version and Hans can Tested-by it as
well?
diff mbox

Patch

diff --git a/scripts/binutils-version.sh b/scripts/binutils-version.sh
index 0bc26cf..b1afc98 100755
--- a/scripts/binutils-version.sh
+++ b/scripts/binutils-version.sh
@@ -5,7 +5,6 @@ 
 # Prints the binutils version of `gas-command' in a canonical 4-digit form
 # such as `0222' for binutils 2.22
 #
-
 gas="$*"
 
 if [ ${#gas} -eq 0 ]; then
@@ -15,6 +14,7 @@  if [ ${#gas} -eq 0 ]; then
 fi
 
 version_string=$($gas --version | head -1 | sed -e 's/.*) *\([0-9.]*\).*/\1/' )
+version_string=$(echo "$version_string" | sed -e 's/[^0-9]*\([0-9.]*\).*/\1/')
 
 MAJOR=$(echo $version_string | cut -d . -f 1)
 MINOR=$(echo $version_string | cut -d . -f 2)