v2015.01-rc4 REGRESSION: "scripts: fix binutils-version.sh" breaks things for non Linaro toolchains
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 0000scripts/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
Quoted 27 lines · Hans de Goede
Hi,
I noticed $subject while doing a MAKEALL.
It seems that this commit:
http://git.denx.de/?p=u-boot.git;a=commit;h=73c25753060c58e4c339fba306ed0ded0c335748Breaks things with Fedora's arm toolchain:
[hans at shalem u-boot]$
scripts/binutils-version.sharm-linux-gnu-asscripts/binutils-version.sh: line 22: printf: GNU: invalid numberscripts/binutils-version.sh: line 22: printf: assembler: invalid number 0000scripts/binutils-version.sh: line 22: printf: version: invalid number 0002
2400And:
[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'.
Quoted 2 lines
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
Signature · 8 lines
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>
Hi all,
ping ? current master still has this regression, it is not fatal, but it is not pretty either.
Regards,
Hans
Quoted 33 lines · Hans de Goede
Hi,
I noticed $subject while doing a MAKEALL.
It seems that this commit:
http://git.denx.de/?p=u-boot.git;a=commit;h=73c25753060c58e4c339fba306ed0ded0c335748Breaks things with Fedora's arm toolchain:
[hans at shalem u-boot]$
scripts/binutils-version.sharm-linux-gnu-asscripts/binutils-version.sh: line 22: printf: GNU: invalid numberscripts/binutils-version.sh: line 22: printf: assembler: invalid number 0000scripts/binutils-version.sh: line 22: printf: version: invalid number 0002
2400And:
[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
Quoted 4 lines · Hans de Goede
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!
Quoted 44 lines
Regards,
Hans
Quoted 27 lines · Hans de Goede
Hi,
I noticed $subject while doing a MAKEALL.
It seems that this commit:
http://git.denx.de/?p=u-boot.git;a=commit;h=73c25753060c58e4c339fba306ed0ded0c335748Breaks things with Fedora's arm toolchain:
[hans at shalem u-boot]$
scripts/binutils-version.sharm-linux-gnu-asscripts/binutils-version.sh: line 22: printf: GNU: invalid numberscripts/binutils-version.sh: line 22: printf: assembler: invalid number 0000scripts/binutils-version.sh: line 22: printf: version: invalid number 0002
2400And:
[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'.Quoted 6 lines
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
Signature · 8 lines
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>
Quoted 11 lines · trini at ti.com
Quoted 5 lines · Hans de Goede
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]
scripts/binutils-version.sh +4 -5
@@ -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.
Quoted 12 lines
Quoted 5 lines · Hans de Goede
Quoted 4 lines
ping ? current master still has this regression, it is not fatal, but it is not
pretty either.Quoted 4 lines · trini at ti.com
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.
Quoted 24 lines · bpringlemeir at nbsps.com
scripts/binutils-version.sh +0
-0
b/scripts/binutils-version.sh@@ -5,7 +5,6 @@ # Prints the binutils version ofgas-command' in a canonical 4-digit form # such as0222' for binutils 2.22 # - gas="$*"if [ ${#gas} -eq 0 ]; then
scripts/binutils-version.sh +0
-0@@ -14,9 +13,9 @@ if [ ${#gas} -eq 0 ]; thenexit 1
fi-version_string=$($gas --version | head -1 | sed -e 's/.*) *\([0-9.]*\).*/\1/' ) +version_string=$($gas --version | head -1 | sed
scripts/binutils-version.sh +3
-4-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"
Quoted 4 lines
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.
scripts/binutils-version.sh +1 -1
@@ -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)
Quoted 73 lines · Bill Pringlemeir
Quoted 12 lines
Quoted 5 lines · Hans de Goede
Quoted 4 lines
ping ? current master still has this regression, it is not fatal, but it is not
pretty either.Quoted 4 lines · trini at ti.com
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.
Quoted 24 lines · bpringlemeir at nbsps.com
scripts/binutils-version.sh +0
-0
b/scripts/binutils-version.sh@@ -5,7 +5,6 @@ # Prints the binutils version ofgas-command' in a canonical 4-digit form # such as0222' for binutils 2.22 # - gas="$*"if [ ${#gas} -eq 0 ]; then
scripts/binutils-version.sh +0
-0@@ -14,9 +13,9 @@ if [ ${#gas} -eq 0 ]; thenexit 1
fi-version_string=$($gas --version | head -1 | sed -e 's/.*) *\([0-9.]*\).*/\1/' ) +version_string=$($gas --version | head -1 | sed
scripts/binutils-version.sh +3
-4-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"Quoted 4 lines
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.
scripts/binutils-version.sh +0
-1@@ -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
scripts/binutils-version.sh if [ ${#gas} -eq 0 ]; then · line 20
Quoted 7 lines
scripts/binutils-version.sh +1
-0@@ -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?
Signature · 8 lines
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>
PATCH 1/1 scripts: fix binutils-version.sh for 'as' without a package.
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.
--- scripts/binutils-version.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
scripts/binutils-version.sh +2 -1
@@ -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)
Signature · 1 line
1.8.0.2
PATCH 1/1 scripts: fix binutils-version.sh for 'as' without a package.
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.
--- scripts/binutils-version.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/binutils-version.sh +1 -1
@@ -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)
Signature · 1 line
1.8.0.2
PATCH 1/1 scripts: fix binutils-version.sh for 'as' without a package.
Hi Bill,
Quoted 35 lines · On Tue, 6 Jan 2015 17:38:19 -0500 Bill Pringlemeir <[email protected]>…
Commit
73c25753fixed 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 <[email protected]>--- scripts/binutils-version.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)scripts/binutils-version.sh +1
-1@@ -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)Signature · 1 line
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
PATCH v3 1/1 scripts: fix binutils-version.sh for 'as' without a package.
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.
--- scripts/binutils-version.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
scripts/binutils-version.sh +2 -1
@@ -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)
Signature · 1 line
1.8.0.2
PATCH 1/1 scripts: fix binutils-version.sh for 'as' without a package.
In reply to Masahiro YamadaQuoted 14 lines · yamada.m at jp.panasonic.com
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+periodLike this:
Quoted 7 lines
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.
PATCH v3 1/1 scripts: fix binutils-version.sh for 'as' without a package.
In reply to Bill PringlemeirQuoted 28 lines · On Wed, 7 Jan 2015 10:34:15 -0500 Bill Pringlemeir <[email protected]>…
Commit
73c25753fixed 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 <[email protected]>--- scripts/binutils-version.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)scripts/binutils-version.sh +2
-1@@ -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)Signature · 1 line
1.8.0.2
PATCH v3 1/1 scripts: fix binutils-version.sh for 'as' without a package.
In reply to Bill PringlemeirHi,
Quoted 8 lines · Bill Pringlemeir
Commit
73c25753fixed 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 <[email protected]>
Thanks, this fixes the errors I was seeing:
Regards,
Hans
PATCH v3 1/1 scripts: fix binutils-version.sh for 'as' without a package.
In reply to Bill PringlemeirQuoted 10 lines · Bill Pringlemeir
Commit
73c25753fixed 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 <[email protected]>Tested-by Masahiro Yamada <[email protected]>Tested-by Hans de Goede <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
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>