public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains
@ 2014-12-30 10:55 Hans de Goede
  2014-12-30 13:50 ` Tom Rini
  2015-01-06 10:27 ` Hans de Goede
  0 siblings, 2 replies; 15+ messages in thread
From: Hans de Goede @ 2014-12-30 10:55 UTC (permalink / raw)
  To: u-boot

Hi,

I noticed $subject while doing a MAKEALL.

It seems that this commit:
http://git.denx.de/?p=u-boot.git;a=commit;h=73c25753060c58e4c339fba306ed0ded0c335748

Breaks things with Fedora's arm toolchain:

[hans at shalem u-boot]$ scripts/binutils-version.sh arm-linux-gnu-as
scripts/binutils-version.sh: line 22: printf: GNU: invalid number
scripts/binutils-version.sh: line 22: printf: assembler: invalid number
0000
scripts/binutils-version.sh: line 22: printf: version: invalid number
0002
2400

And:

[hans at shalem ~]$ arm-linux-gnu-as --version
GNU assembler version 2.24.0-6.fc21 20140613
Copyright 2013 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or later.
This program has absolutely no warranty.
This assembler was configured for a target of `arm-linux-gnueabi'.

Which makes $version_string: "GNU assembler version 2.24.0-6.fc21 20140613"
and $MAJOR: "GNU assembler version 2"

Regards,

Hans

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains
  2014-12-30 10:55 [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains Hans de Goede
@ 2014-12-30 13:50 ` Tom Rini
  2015-01-06 10:27 ` Hans de Goede
  1 sibling, 0 replies; 15+ messages in thread
From: Tom Rini @ 2014-12-30 13:50 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 30, 2014 at 11:55:27AM +0100, Hans de Goede wrote:
> Hi,
> 
> I noticed $subject while doing a MAKEALL.
> 
> It seems that this commit:
> http://git.denx.de/?p=u-boot.git;a=commit;h=73c25753060c58e4c339fba306ed0ded0c335748
> 
> Breaks things with Fedora's arm toolchain:
> 
> [hans at shalem u-boot]$ scripts/binutils-version.sh arm-linux-gnu-as
> scripts/binutils-version.sh: line 22: printf: GNU: invalid number
> scripts/binutils-version.sh: line 22: printf: assembler: invalid number
> 0000
> scripts/binutils-version.sh: line 22: printf: version: invalid number
> 0002
> 2400
> 
> And:
> 
> [hans at shalem ~]$ arm-linux-gnu-as --version
> GNU assembler version 2.24.0-6.fc21 20140613
> Copyright 2013 Free Software Foundation, Inc.
> This program is free software; you may redistribute it under the terms of
> the GNU General Public License version 3 or later.
> This program has absolutely no warranty.
> This assembler was configured for a target of `arm-linux-gnueabi'.
> 
> Which makes $version_string: "GNU assembler version 2.24.0-6.fc21 20140613"
> and $MAJOR: "GNU assembler version 2"

FWIW it works on "vanilla" toolchains too so it's something in the
tweaking above that needs further tweaks to work for Fedora and Linaro:
$ /opt/eldk-5.5.2/armv7a/sysroots/i686-eldk-linux/usr/bin/arm-linux-gnueabi/arm-linux-gnueabi-as
--version
GNU assembler (GNU Binutils) 2.23.2

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141230/0680bb34/attachment.pgp>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains
  2014-12-30 10:55 [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains Hans de Goede
  2014-12-30 13:50 ` Tom Rini
@ 2015-01-06 10:27 ` Hans de Goede
  2015-01-06 15:46   ` Tom Rini
  1 sibling, 1 reply; 15+ messages in thread
From: Hans de Goede @ 2015-01-06 10:27 UTC (permalink / raw)
  To: u-boot

Hi all,

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

Regards,

Hans


On 30-12-14 11:55, Hans de Goede wrote:
> Hi,
>
> I noticed $subject while doing a MAKEALL.
>
> It seems that this commit:
> http://git.denx.de/?p=u-boot.git;a=commit;h=73c25753060c58e4c339fba306ed0ded0c335748
>
> Breaks things with Fedora's arm toolchain:
>
> [hans at shalem u-boot]$ scripts/binutils-version.sh arm-linux-gnu-as
> scripts/binutils-version.sh: line 22: printf: GNU: invalid number
> scripts/binutils-version.sh: line 22: printf: assembler: invalid number
> 0000
> scripts/binutils-version.sh: line 22: printf: version: invalid number
> 0002
> 2400
>
> And:
>
> [hans at shalem ~]$ arm-linux-gnu-as --version
> GNU assembler version 2.24.0-6.fc21 20140613
> Copyright 2013 Free Software Foundation, Inc.
> This program is free software; you may redistribute it under the terms of
> the GNU General Public License version 3 or later.
> This program has absolutely no warranty.
> This assembler was configured for a target of `arm-linux-gnueabi'.
>
> Which makes $version_string: "GNU assembler version 2.24.0-6.fc21 20140613"
> and $MAJOR: "GNU assembler version 2"
>
> Regards,
>
> Hans

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains
  2015-01-06 10:27 ` Hans de Goede
@ 2015-01-06 15:46   ` Tom Rini
  2015-01-06 21:32     ` Bill Pringlemeir
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Rini @ 2015-01-06 15:46 UTC (permalink / raw)
  To: u-boot

On Tue, Jan 06, 2015 at 11:27:43AM +0100, Hans de Goede wrote:
> Hi all,
> 
> ping ? current master still has this regression, it is not fatal, but it is not
> pretty either.

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.
Thanks!

> 
> Regards,
> 
> Hans
> 
> 
> On 30-12-14 11:55, Hans de Goede wrote:
> >Hi,
> >
> >I noticed $subject while doing a MAKEALL.
> >
> >It seems that this commit:
> >http://git.denx.de/?p=u-boot.git;a=commit;h=73c25753060c58e4c339fba306ed0ded0c335748
> >
> >Breaks things with Fedora's arm toolchain:
> >
> >[hans at shalem u-boot]$ scripts/binutils-version.sh arm-linux-gnu-as
> >scripts/binutils-version.sh: line 22: printf: GNU: invalid number
> >scripts/binutils-version.sh: line 22: printf: assembler: invalid number
> >0000
> >scripts/binutils-version.sh: line 22: printf: version: invalid number
> >0002
> >2400
> >
> >And:
> >
> >[hans at shalem ~]$ arm-linux-gnu-as --version
> >GNU assembler version 2.24.0-6.fc21 20140613
> >Copyright 2013 Free Software Foundation, Inc.
> >This program is free software; you may redistribute it under the terms of
> >the GNU General Public License version 3 or later.
> >This program has absolutely no warranty.
> >This assembler was configured for a target of `arm-linux-gnueabi'.
> >
> >Which makes $version_string: "GNU assembler version 2.24.0-6.fc21 20140613"
> >and $MAJOR: "GNU assembler version 2"
> >
> >Regards,
> >
> >Hans
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150106/d543b9f8/attachment.pgp>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains
  2015-01-06 15:46   ` Tom Rini
@ 2015-01-06 21:32     ` Bill Pringlemeir
  2015-01-06 22:01       ` Bill Pringlemeir
  0 siblings, 1 reply; 15+ messages in thread
From: Bill Pringlemeir @ 2015-01-06 21:32 UTC (permalink / raw)
  To: u-boot

On  6 Jan 2015, trini at ti.com wrote:

> On Tue, Jan 06, 2015 at 11:27:43AM +0100, Hans de Goede wrote:
>> Hi all,
>>
>> ping ? current master still has this regression, it is not fatal,
>> but it is not
>> pretty either.
>
> 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.

I did a shell check on this script,

shellcheck -f gcc binutils-version.sh 
binutils-version.sh:13:9: warning: Don't use variables in the printf format string. Use printf "..%s.." "$foo". [SC2059]
binutils-version.sh:19:14: note: Double quote to prevent globbing and word splitting. [SC2086]
binutils-version.sh:20:14: note: Double quote to prevent globbing and word splitting. [SC2086]
binutils-version.sh:22:22: note: Double quote to prevent globbing and word splitting. [SC2086]
binutils-version.sh:22:29: note: Double quote to prevent globbing and word splitting. [SC2086]


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.

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains
  2015-01-06 21:32     ` Bill Pringlemeir
@ 2015-01-06 22:01       ` Bill Pringlemeir
  2015-01-06 22:07         ` Tom Rini
  0 siblings, 1 reply; 15+ messages in thread
From: Bill Pringlemeir @ 2015-01-06 22:01 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains
  2015-01-06 22:01       ` Bill Pringlemeir
@ 2015-01-06 22:07         ` Tom Rini
  2015-01-06 22:25           ` [U-Boot] [PATCH] scripts: fix binutils-version.sh for 'as' without a package Bill Pringlemeir
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Rini @ 2015-01-06 22:07 UTC (permalink / raw)
  To: u-boot

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 at 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 at 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?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150106/898900fb/attachment.pgp>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH] scripts: fix binutils-version.sh for 'as' without a package.
  2015-01-06 22:07         ` Tom Rini
@ 2015-01-06 22:25           ` Bill Pringlemeir
  2015-01-06 22:38             ` Bill Pringlemeir
  0 siblings, 1 reply; 15+ messages in thread
From: Bill Pringlemeir @ 2015-01-06 22:25 UTC (permalink / raw)
  To: u-boot

Commit 73c25753 fixed the common issue that binutil packages (tool/organization
that packaged or built the bin-utils) are included in brackets and this may
falsely be recognized as a version.  However, some tools do not provide a
'package' and previously we add the 'Gnu assembler..' to the version.

Run a 2nd pass on the 'version_string' to strip off any leading characters when
a package is not provided in brackets.

Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
---
 scripts/binutils-version.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/binutils-version.sh b/scripts/binutils-version.sh
index 0bc26cf..68cd0fe 100755
--- a/scripts/binutils-version.sh
+++ b/scripts/binutils-version.sh
@@ -5,7 +5,7 @@
 # Prints the binutils version of `gas-command' in a canonical 4-digit form
 # such as `0222' for binutils 2.22
 #
-
+set -x
 gas="$*"
 
 if [ ${#gas} -eq 0 ]; then
@@ -15,6 +15,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)
-- 
1.8.0.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH] scripts: fix binutils-version.sh for 'as' without a package.
  2015-01-06 22:25           ` [U-Boot] [PATCH] scripts: fix binutils-version.sh for 'as' without a package Bill Pringlemeir
@ 2015-01-06 22:38             ` Bill Pringlemeir
  2015-01-07 12:01               ` Masahiro Yamada
  0 siblings, 1 reply; 15+ messages in thread
From: Bill Pringlemeir @ 2015-01-06 22:38 UTC (permalink / raw)
  To: u-boot

Commit 73c25753 fixed the common issue that binutil packages (tool/organization
that packaged or built the bin-utils) are included in brackets and this may
falsely be recognized as a version.  However, some tools do not provide a
'package' and previously we add the 'Gnu assembler..' to the version.

Run a 2nd pass on the 'version_string' to strip off any leading characters when
a package is not provided in brackets.

Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
---
 scripts/binutils-version.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH] scripts: fix binutils-version.sh for 'as' without a package.
  2015-01-06 22:38             ` Bill Pringlemeir
@ 2015-01-07 12:01               ` Masahiro Yamada
  2015-01-07 15:34                 ` [U-Boot] [PATCH v3] " Bill Pringlemeir
  2015-01-07 15:42                 ` [U-Boot] [PATCH] " Bill Pringlemeir
  0 siblings, 2 replies; 15+ messages in thread
From: Masahiro Yamada @ 2015-01-07 12:01 UTC (permalink / raw)
  To: u-boot

Hi Bill,



On Tue,  6 Jan 2015 17:38:19 -0500
Bill Pringlemeir <bpringlemeir@nbsps.com> wrote:

> Commit 73c25753 fixed the common issue that binutil packages (tool/organization
> that packaged or built the bin-utils) are included in brackets and this may
> falsely be recognized as a version.  However, some tools do not provide a
> 'package' and previously we add the 'Gnu assembler..' to the version.
> 
> Run a 2nd pass on the 'version_string' to strip off any leading characters when
> a package is not provided in brackets.
> 
> Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
> ---
>  scripts/binutils-version.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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)
> -- 
> 1.8.0.2


Thanks for your patch.

I think this works but could it be more simplified?

In your commit-log, you mentioned only some of tools provide
additional information surrounded by brackets.

If so, we can
[1] remove blackets
[2] and then take the first word that consists of numbers+period


Like this:



version_string=$($gas --version | head -1 | \
	sed -e 's/(.*)//' -e 's/[^0-9.]*\([0-9.]*\).*/\1/')

MAJOR=$(echo $version_string | cut -d . -f 1)
MINOR=$(echo $version_string | cut -d . -f 2)

printf "%02d%02d\\n" $MAJOR $MINOR





Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v3] scripts: fix binutils-version.sh for 'as' without a package.
  2015-01-07 12:01               ` Masahiro Yamada
@ 2015-01-07 15:34                 ` Bill Pringlemeir
  2015-01-08  8:56                   ` Masahiro Yamada
                                     ` (2 more replies)
  2015-01-07 15:42                 ` [U-Boot] [PATCH] " Bill Pringlemeir
  1 sibling, 3 replies; 15+ messages in thread
From: Bill Pringlemeir @ 2015-01-07 15:34 UTC (permalink / raw)
  To: u-boot

Commit 73c25753 fixed the common issue that binutil packages (tool/organization
that packaged or built the bin-utils) are included in brackets and this may
falsely be recognized as a version.  However, some tools do not provide a
'package' and previously we add the 'Gnu assembler..' to the version.

Strip out the '(package version text)' and then look for a ##.## string.

Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
---
 scripts/binutils-version.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/binutils-version.sh b/scripts/binutils-version.sh
index 0bc26cf..a343681 100755
--- a/scripts/binutils-version.sh
+++ b/scripts/binutils-version.sh
@@ -14,7 +14,8 @@ 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/(.*)//; s/[^0-9.]*\([0-9.]*\).*/\1/')
 
 MAJOR=$(echo $version_string | cut -d . -f 1)
 MINOR=$(echo $version_string | cut -d . -f 2)
-- 
1.8.0.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH] scripts: fix binutils-version.sh for 'as' without a package.
  2015-01-07 12:01               ` Masahiro Yamada
  2015-01-07 15:34                 ` [U-Boot] [PATCH v3] " Bill Pringlemeir
@ 2015-01-07 15:42                 ` Bill Pringlemeir
  1 sibling, 0 replies; 15+ messages in thread
From: Bill Pringlemeir @ 2015-01-07 15:42 UTC (permalink / raw)
  To: u-boot

On  7 Jan 2015, yamada.m at jp.panasonic.com wrote:

> Thanks for your patch.
>
> I think this works but could it be more simplified?
>
> In your commit-log, you mentioned only some of tools provide
> additional information surrounded by brackets.
>
> If so, we can
> [1] remove blackets
> [2] and then take the first word that consists of numbers+period
>
>
> Like this:
>
> version_string=$($gas --version | head -1 | \
> 	sed -e 's/(.*)//' -e 's/[^0-9.]*\([0-9.]*\).*/\1/')
>
> MAJOR=$(echo $version_string | cut -d . -f 1)
> MINOR=$(echo $version_string | cut -d . -f 2)
>
> printf "%02d%02d\\n" $MAJOR $MINOR

I realized I didn't need to spawn sed twice, but removing the
'(package)' stuff seems better.  Thanks for giving me a reason to fix
the whitespace.

Also the string,

 GNU assembler version 2.24.0-6.fc21 20140613

Was originally reporting '2014061320140613' prior to your version.  The
new version reports '0224' like all the others.

Fwiw,
Bill.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v3] scripts: fix binutils-version.sh for 'as' without a package.
  2015-01-07 15:34                 ` [U-Boot] [PATCH v3] " Bill Pringlemeir
@ 2015-01-08  8:56                   ` Masahiro Yamada
  2015-01-08 10:18                   ` Hans de Goede
  2015-01-09 13:34                   ` [U-Boot] [U-Boot, " Tom Rini
  2 siblings, 0 replies; 15+ messages in thread
From: Masahiro Yamada @ 2015-01-08  8:56 UTC (permalink / raw)
  To: u-boot


On Wed,  7 Jan 2015 10:34:15 -0500
Bill Pringlemeir <bpringlemeir@nbsps.com> wrote:

> Commit 73c25753 fixed the common issue that binutil packages (tool/organization
> that packaged or built the bin-utils) are included in brackets and this may
> falsely be recognized as a version.  However, some tools do not provide a
> 'package' and previously we add the 'Gnu assembler..' to the version.
> 
> Strip out the '(package version text)' and then look for a ##.## string.
> 
> Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
> ---
>  scripts/binutils-version.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/binutils-version.sh b/scripts/binutils-version.sh
> index 0bc26cf..a343681 100755
> --- a/scripts/binutils-version.sh
> +++ b/scripts/binutils-version.sh
> @@ -14,7 +14,8 @@ 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/(.*)//; s/[^0-9.]*\([0-9.]*\).*/\1/')
>  
>  MAJOR=$(echo $version_string | cut -d . -f 1)
>  MINOR=$(echo $version_string | cut -d . -f 2)
> -- 
> 1.8.0.2


Tested-by: Masahiro Yamada <yamada.m@jp.panasonic.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v3] scripts: fix binutils-version.sh for 'as' without a package.
  2015-01-07 15:34                 ` [U-Boot] [PATCH v3] " Bill Pringlemeir
  2015-01-08  8:56                   ` Masahiro Yamada
@ 2015-01-08 10:18                   ` Hans de Goede
  2015-01-09 13:34                   ` [U-Boot] [U-Boot, " Tom Rini
  2 siblings, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2015-01-08 10:18 UTC (permalink / raw)
  To: u-boot

Hi,

On 07-01-15 16:34, Bill Pringlemeir wrote:
> Commit 73c25753 fixed the common issue that binutil packages (tool/organization
> that packaged or built the bin-utils) are included in brackets and this may
> falsely be recognized as a version.  However, some tools do not provide a
> 'package' and previously we add the 'Gnu assembler..' to the version.
>
> Strip out the '(package version text)' and then look for a ##.## string.
>
> Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>

Thanks, this fixes the errors I was seeing:

Tested-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot, v3] scripts: fix binutils-version.sh for 'as' without a package.
  2015-01-07 15:34                 ` [U-Boot] [PATCH v3] " Bill Pringlemeir
  2015-01-08  8:56                   ` Masahiro Yamada
  2015-01-08 10:18                   ` Hans de Goede
@ 2015-01-09 13:34                   ` Tom Rini
  2 siblings, 0 replies; 15+ messages in thread
From: Tom Rini @ 2015-01-09 13:34 UTC (permalink / raw)
  To: u-boot

On Wed, Jan 07, 2015 at 10:34:15AM -0500, Bill Pringlemeir wrote:

> Commit 73c25753 fixed the common issue that binutil packages (tool/organization
> that packaged or built the bin-utils) are included in brackets and this may
> falsely be recognized as a version.  However, some tools do not provide a
> 'package' and previously we add the 'Gnu assembler..' to the version.
> 
> Strip out the '(package version text)' and then look for a ##.## string.
> 
> Signed-off-by: Bill Pringlemeir <bpringlemeir@nbsps.com>
> Tested-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Tested-by: Hans de Goede <hdegoede@redhat.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150109/30f6fce6/attachment.pgp>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-01-09 13:34 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-30 10:55 [U-Boot] v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains Hans de Goede
2014-12-30 13:50 ` Tom Rini
2015-01-06 10:27 ` Hans de Goede
2015-01-06 15:46   ` Tom Rini
2015-01-06 21:32     ` Bill Pringlemeir
2015-01-06 22:01       ` Bill Pringlemeir
2015-01-06 22:07         ` Tom Rini
2015-01-06 22:25           ` [U-Boot] [PATCH] scripts: fix binutils-version.sh for 'as' without a package Bill Pringlemeir
2015-01-06 22:38             ` Bill Pringlemeir
2015-01-07 12:01               ` Masahiro Yamada
2015-01-07 15:34                 ` [U-Boot] [PATCH v3] " Bill Pringlemeir
2015-01-08  8:56                   ` Masahiro Yamada
2015-01-08 10:18                   ` Hans de Goede
2015-01-09 13:34                   ` [U-Boot] [U-Boot, " Tom Rini
2015-01-07 15:42                 ` [U-Boot] [PATCH] " Bill Pringlemeir

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox