| Message ID | 1606828650-29841-1-git-send-email-asmadeus@codewreck.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] ld-version: use /usr/bin/env awk for shebank | expand |
On Tue, Dec 1, 2020 at 10:18 PM Dominique Martinet <asmadeus@codewreck.org> wrote: > > /usr/bin/awk is not garanteed to exist (and doesn't on e.g. nixos), > using /usr/bin/env to have it look in PATH is more robust > > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > --- Both applied to linux-kbuild. Thanks. > I've been carrying these two patchs for local kernel development on > nixos for a while, I don't think it'd break anything for anyone so > might as well submit these -- please consider it :) > > scripts/ld-version.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > index f2be0ff9a738..05476b8f8925 100755 > --- a/scripts/ld-version.sh > +++ b/scripts/ld-version.sh > @@ -1,4 +1,4 @@ > -#!/usr/bin/awk -f > +#!/usr/bin/env -S awk -f > # SPDX-License-Identifier: GPL-2.0 > # extract linker version number from stdin and turn into single number > { > -- > 2.28.0 >
Hi Dominique, On 12/1/20 1:17 PM, Dominique Martinet wrote: > /usr/bin/awk is not garanteed to exist (and doesn't on e.g. nixos), > using /usr/bin/env to have it look in PATH is more robust > This patch breaks the compilation on Ubuntu 16.04 and 18.04 in fact: make[1]: Entering directory '/data1/Projects/LinuxKernel/linux-out' GEN Makefile HOSTCC scripts/basic/fixdep HOSTCC scripts/kconfig/conf.o HOSTCC scripts/kconfig/confdata.o HOSTCC scripts/kconfig/expr.o LEX scripts/kconfig/lexer.lex.c YACC scripts/kconfig/parser.tab.[ch] HOSTCC scripts/kconfig/lexer.lex.o HOSTCC scripts/kconfig/parser.tab.o HOSTCC scripts/kconfig/preprocess.o HOSTCC scripts/kconfig/symbol.o HOSTCC scripts/kconfig/util.o HOSTLD scripts/kconfig/conf *** Default configuration is based on 'defconfig' /usr/bin/env: invalid option -- 'S' Try '/usr/bin/env --help' for more information. init/Kconfig:39: syntax error init/Kconfig:38: invalid statement /data1/Projects/LinuxKernel/linux-mte/scripts/kconfig/Makefile:80: recipe for target 'defconfig' failed make[2]: *** [defconfig] Error 1 /data1/Projects/LinuxKernel/linux-mte/Makefile:602: recipe for target 'defconfig' failed make[1]: *** [defconfig] Error 2 make[1]: Leaving directory '/data1/Projects/LinuxKernel/linux-out' Makefile:185: recipe for target '__sub-make' failed make: *** [__sub-make] Error 2 The problem seems to be that the minimum version of env that supports '-S' is 8.30. Reverting the patch fixes the issue. Thanks! > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > --- > I've been carrying these two patchs for local kernel development on > nixos for a while, I don't think it'd break anything for anyone so > might as well submit these -- please consider it :) > > scripts/ld-version.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > index f2be0ff9a738..05476b8f8925 100755 > --- a/scripts/ld-version.sh > +++ b/scripts/ld-version.sh > @@ -1,4 +1,4 @@ > -#!/usr/bin/awk -f > +#!/usr/bin/env -S awk -f > # SPDX-License-Identifier: GPL-2.0 > # extract linker version number from stdin and turn into single number > { >
Vincenzo Frascino wrote on Wed, Dec 09, 2020: > On 12/1/20 1:17 PM, Dominique Martinet wrote: > > /usr/bin/awk is not garanteed to exist (and doesn't on e.g. nixos), > > using /usr/bin/env to have it look in PATH is more robust > > This patch breaks the compilation on Ubuntu 16.04 and 18.04 in fact: Thanks for the report, I was told the same by Krzysztof Kozlowski (added to Ccs) earlier today, I don't have any "old" machines like this around so didn't notice when I checked unfortunately :( I've suggested either just reverting this (I'll keep my local workaround) or going through /bin/sh which is always safe like the following patch -- leaving this to maintainers. Thanks! ----- From d53ef3b4c55aa2ea5f9ae887b3e1ace368f30f66 Mon Sep 17 00:00:00 2001 From: Dominique Martinet <asmadeus@codewreck.org> Date: Wed, 15 Jul 2020 16:00:13 +0200 Subject: [PATCH] ld-version: use /bin/sh then awk for shebank /usr/bin/awk is not garanteed to exist (and doesn't on e.g. nixos), using /bin/sh and invoking awk to have it look in PATH is more robust. Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh index f2be0ff9a738..02dbad7b5613 100755 --- a/scripts/ld-version.sh +++ b/scripts/ld-version.sh @@ -1,11 +1,11 @@ -#!/usr/bin/awk -f +#!/bin/sh # SPDX-License-Identifier: GPL-2.0 # extract linker version number from stdin and turn into single number - { +awk '{ gsub(".*\\)", ""); gsub(".*version ", ""); gsub("-.*", ""); split($1,a, "."); print a[1]*100000000 + a[2]*1000000 + a[3]*10000; exit - } +}'
On Tue, Dec 01, 2020 at 02:17:29PM +0100, Dominique Martinet wrote: > /usr/bin/awk is not garanteed to exist (and doesn't on e.g. nixos), > using /usr/bin/env to have it look in PATH is more robust > > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > --- > I've been carrying these two patchs for local kernel development on > nixos for a while, I don't think it'd break anything for anyone so > might as well submit these -- please consider it :) > > scripts/ld-version.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > index f2be0ff9a738..05476b8f8925 100755 > --- a/scripts/ld-version.sh > +++ b/scripts/ld-version.sh > @@ -1,4 +1,4 @@ > -#!/usr/bin/awk -f > +#!/usr/bin/env -S awk -f This patch results in: /usr/bin/env: invalid option -- 'S' Try '/usr/bin/env --help' for more information. init/Kconfig:39: syntax error init/Kconfig:38: invalid statement scripts/kconfig/Makefile:80: recipe for target 'defconfig' failed when using: env (GNU coreutils) 8.28 or any other version of "env" which doesn't support "-S". Guenter
From: Dominique Martinet > Sent: 09 December 2020 17:43 > > I've suggested either just reverting this (I'll keep my local > workaround) or going through /bin/sh which is always safe like the > following patch -- leaving this to maintainers. > > Thanks! > ----- > From d53ef3b4c55aa2ea5f9ae887b3e1ace368f30f66 Mon Sep 17 00:00:00 2001 > From: Dominique Martinet <asmadeus@codewreck.org> > Date: Wed, 15 Jul 2020 16:00:13 +0200 > Subject: [PATCH] ld-version: use /bin/sh then awk for shebank > > /usr/bin/awk is not garanteed to exist (and doesn't on e.g. nixos), > using /bin/sh and invoking awk to have it look in PATH is more robust. > > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > index f2be0ff9a738..02dbad7b5613 100755 > --- a/scripts/ld-version.sh > +++ b/scripts/ld-version.sh > @@ -1,11 +1,11 @@ > -#!/usr/bin/awk -f > +#!/bin/sh > # SPDX-License-Identifier: GPL-2.0 > # extract linker version number from stdin and turn into single number > - { > +awk '{ > gsub(".*\\)", ""); > gsub(".*version ", ""); > gsub("-.*", ""); > split($1,a, "."); > print a[1]*100000000 + a[2]*1000000 + a[3]*10000; > exit > - } > +}' Why bother with awk? I think you can do it all in a shell function. Something like: read line line=${line##*)} line=${line##*version } IFS='.-' set $line echo $(($1*100000000 + $2*1000000 + $3*10000)) That will work on any recent shell. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Hi Dominique and Dave, On 12/9/20 10:03 PM, David Laight wrote: > From: Dominique Martinet >> Sent: 09 December 2020 17:43 >> >> I've suggested either just reverting this (I'll keep my local >> workaround) or going through /bin/sh which is always safe like the >> following patch -- leaving this to maintainers. >> >> Thanks! >> ----- >> From d53ef3b4c55aa2ea5f9ae887b3e1ace368f30f66 Mon Sep 17 00:00:00 2001 >> From: Dominique Martinet <asmadeus@codewreck.org> >> Date: Wed, 15 Jul 2020 16:00:13 +0200 >> Subject: [PATCH] ld-version: use /bin/sh then awk for shebank >> >> /usr/bin/awk is not garanteed to exist (and doesn't on e.g. nixos), >> using /bin/sh and invoking awk to have it look in PATH is more robust. >> >> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> >> >> diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh >> index f2be0ff9a738..02dbad7b5613 100755 >> --- a/scripts/ld-version.sh >> +++ b/scripts/ld-version.sh >> @@ -1,11 +1,11 @@ >> -#!/usr/bin/awk -f >> +#!/bin/sh >> # SPDX-License-Identifier: GPL-2.0 >> # extract linker version number from stdin and turn into single number >> - { >> +awk '{ >> gsub(".*\\)", ""); >> gsub(".*version ", ""); >> gsub("-.*", ""); >> split($1,a, "."); >> print a[1]*100000000 + a[2]*1000000 + a[3]*10000; >> exit >> - } >> +}' > > Why bother with awk? > I think you can do it all in a shell function. > Something like: > read line > line=${line##*)} > line=${line##*version } > IFS='.-' > set $line > echo $(($1*100000000 + $2*1000000 + $3*10000)) > > That will work on any recent shell. > I would suggest to revert the patch for now since we are close to the merge window and then maybe in -rc1 start discussing a better solution. What do you think? > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) >
Vincenzo Frascino wrote on Thu, Dec 10, 2020: > On 12/9/20 10:03 PM, David Laight wrote: >> Why bother with awk? I wanted to keep the patch minimal, I'm not opposed to rewriting but that always potentially has more impact (although as you say, this script is simple enough) > > I think you can do it all in a shell function. > > Something like: > > read line > > line=${line##*)} > > line=${line##*version } > > IFS='.-' > > set $line > > echo $(($1*100000000 + $2*1000000 + $3*10000)) > > > > That will work on any recent shell. Works for me. > I would suggest to revert the patch for now since we are close to the merge > window and then maybe in -rc1 start discussing a better solution. As far as I can see the patch is only in -next, as said earlier I'm in favor of just dropping the patch until a decision is taken for the next merge window (or the one after that); there's no hurry for me and nothing to revert.
From: 'Dominique Martinet' > Sent: 10 December 2020 12:22 > > Vincenzo Frascino wrote on Thu, Dec 10, 2020: > > On 12/9/20 10:03 PM, David Laight wrote: > >> Why bother with awk? > > I wanted to keep the patch minimal, I'm not opposed to rewriting but > that always potentially has more impact (although as you say, this > script is simple enough) > > > > I think you can do it all in a shell function. > > > Something like: > > > read line > > > line=${line##*)} > > > line=${line##*version } > > > IFS='.-' > > > set $line > > > echo $(($1*100000000 + $2*1000000 + $3*10000)) > > > > > > That will work on any recent shell. > > Works for me. That was a very quick rewrite of what I think the awk script did. However I think the version is in the last space-separated word. So you can do (untested): read line set line shift $(($#-1)) OIFS="$IFS" IFS='.-' set $1 IFS="$OIFS" echo $(($1*100000000 + $2*1000000 + $3*10000)) Now, if you want a version that will work with a real bourne shell (that doesn't support $((expr)) or $(x##b} it gets more interesting. Yes, but for now, revert first. It might even be that the whole file isn't needed. If it only used from a Makefile it can be gone with gmake commands. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh index f2be0ff9a738..05476b8f8925 100755 --- a/scripts/ld-version.sh +++ b/scripts/ld-version.sh @@ -1,4 +1,4 @@ -#!/usr/bin/awk -f +#!/usr/bin/env -S awk -f # SPDX-License-Identifier: GPL-2.0 # extract linker version number from stdin and turn into single number {
/usr/bin/awk is not garanteed to exist (and doesn't on e.g. nixos), using /usr/bin/env to have it look in PATH is more robust Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> --- I've been carrying these two patchs for local kernel development on nixos for a while, I don't think it'd break anything for anyone so might as well submit these -- please consider it :) scripts/ld-version.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)