linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for checking spatch version
@ 2020-12-12 16:54 Masahiro Yamada
  2020-12-12 16:54 ` [PATCH 2/3] kbuild: LD_VERSION redenomination Masahiro Yamada
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Masahiro Yamada @ 2020-12-12 16:54 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Dominique Martinet, Masahiro Yamada, Gilles Muller, Julia Lawall,
	Matthias Maennich, Michal Marek, Nicolas Palix, cocci,
	linux-kernel

scripts/ld-version.sh was, as its file name implies, originally intended
for the GNU ld version, but is (ab)used for the spatch version too.

Use 'sort -CV' for the version comparison, then coccicheck does not need
to use scripts/ld-version.sh. Fix nsdeps as well.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/coccicheck | 14 +++++---------
 scripts/nsdeps     |  4 +---
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index 209bb0427b43..d7f6b7ff130a 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -16,7 +16,6 @@ if [ ! -x "$SPATCH" ]; then
 fi
 
 SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}')
-SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh)
 
 USE_JOBS="no"
 $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes"
@@ -186,14 +185,11 @@ coccinelle () {
 
     OPT=`grep "Options:" $COCCI | cut -d':' -f2`
     REQ=`grep "Requires:" $COCCI | cut -d':' -f2 | sed "s| ||"`
-    REQ_NUM=$(echo $REQ | ${DIR}/scripts/ld-version.sh)
-    if [ "$REQ_NUM" != "0" ] ; then
-	    if [ "$SPATCH_VERSION_NUM" -lt "$REQ_NUM" ] ; then
-		    echo "Skipping coccinelle SmPL patch: $COCCI"
-		    echo "You have coccinelle:           $SPATCH_VERSION"
-		    echo "This SmPL patch requires:      $REQ"
-		    return
-	    fi
+    if [ -n "$REQ" ] && ! { echo "$REQ"; echo "$SPATCH_VERSION"; } | sort -CV ; then
+	    echo "Skipping coccinelle SmPL patch: $COCCI"
+	    echo "You have coccinelle:           $SPATCH_VERSION"
+	    echo "This SmPL patch requires:      $REQ"
+	    return
     fi
 
 #   The option '--parse-cocci' can be used to syntactically check the SmPL files.
diff --git a/scripts/nsdeps b/scripts/nsdeps
index dab4c1a0e27d..e8ce2a4d704a 100644
--- a/scripts/nsdeps
+++ b/scripts/nsdeps
@@ -12,11 +12,9 @@ if [ ! -x "$SPATCH" ]; then
 	exit 1
 fi
 
-SPATCH_REQ_VERSION_NUM=$(echo $SPATCH_REQ_VERSION | ${DIR}/scripts/ld-version.sh)
 SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}')
-SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh)
 
-if [ "$SPATCH_VERSION_NUM" -lt "$SPATCH_REQ_VERSION_NUM" ] ; then
+if ! { echo "$SPATCH_REQ_VERSION"; echo "$SPATCH_VERSION"; } | sort -CV ; then
 	echo "spatch needs to be version $SPATCH_REQ_VERSION or higher"
 	exit 1
 fi
-- 
2.27.0


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

* [PATCH 2/3] kbuild: LD_VERSION redenomination
  2020-12-12 16:54 [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for checking spatch version Masahiro Yamada
@ 2020-12-12 16:54 ` Masahiro Yamada
  2020-12-14 23:05   ` Will Deacon
                     ` (2 more replies)
  2020-12-12 16:54 ` [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script Masahiro Yamada
  2020-12-12 17:30 ` [Cocci] [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for checking spatch version Julia Lawall
  2 siblings, 3 replies; 15+ messages in thread
From: Masahiro Yamada @ 2020-12-12 16:54 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Dominique Martinet, Masahiro Yamada, Benjamin Herrenschmidt,
	Catalin Marinas, Huacai Chen, Jiaxun Yang, Michael Ellerman,
	Paul Mackerras, Thomas Bogendoerfer, Will Deacon,
	linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev

Commit ccbef1674a15 ("Kbuild, lto: add ld-version and ld-ifversion
macros") introduced scripts/ld-version.sh for GCC LTO.

At that time, this script handled 5 version fields because GCC LTO
needed the downstream binutils. (https://lkml.org/lkml/2014/4/8/272)

The code snippet from the submitted patch was as follows:

    # We need HJ Lu's Linux binutils because mainline binutils does not
    # support mixing assembler and LTO code in the same ld -r object.
    # XXX check if the gcc plugin ld is the expected one too
    # XXX some Fedora binutils should also support it. How to check for that?
    ifeq ($(call ld-ifversion,-ge,22710001,y),y)
        ...

However, GCC LTO was not merged into the mainline after all.
(https://lkml.org/lkml/2014/4/8/272)

So, the 4th and 5th fields were never used, and finally removed by
commit 0d61ed17dd30 ("ld-version: Drop the 4th and 5th version
components").

Since then, the last 4-digits returned by this script is always zeros.

Remove the meaningless last 4-digits. This makes the version format
consistent with GCC_VERSION, CLANG_VERSION, LLD_VERSION.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/arm64/Kconfig            | 2 +-
 arch/mips/loongson64/Platform | 2 +-
 arch/mips/vdso/Kconfig        | 2 +-
 arch/powerpc/Makefile         | 2 +-
 arch/powerpc/lib/Makefile     | 2 +-
 scripts/ld-version.sh         | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a6b5b7ef40ae..69d56b21a6ec 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1499,7 +1499,7 @@ config ARM64_PTR_AUTH
 	depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC
 	# Modern compilers insert a .note.gnu.property section note for PAC
 	# which is only understood by binutils starting with version 2.33.1.
-	depends on LD_IS_LLD || LD_VERSION >= 233010000 || (CC_IS_GCC && GCC_VERSION < 90100)
+	depends on LD_IS_LLD || LD_VERSION >= 23301 || (CC_IS_GCC && GCC_VERSION < 90100)
 	depends on !CC_IS_CLANG || AS_HAS_CFI_NEGATE_RA_STATE
 	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
 	help
diff --git a/arch/mips/loongson64/Platform b/arch/mips/loongson64/Platform
index ec42c5085905..cc0b9c87f9ad 100644
--- a/arch/mips/loongson64/Platform
+++ b/arch/mips/loongson64/Platform
@@ -35,7 +35,7 @@ cflags-$(CONFIG_CPU_LOONGSON64)	+= $(call as-option,-Wa$(comma)-mno-fix-loongson
 # can't easily be used safely within the kbuild framework.
 #
 ifeq ($(call cc-ifversion, -ge, 0409, y), y)
-  ifeq ($(call ld-ifversion, -ge, 225000000, y), y)
+  ifeq ($(call ld-ifversion, -ge, 22500, y), y)
     cflags-$(CONFIG_CPU_LOONGSON64)  += \
       $(call cc-option,-march=loongson3a -U_MIPS_ISA -D_MIPS_ISA=_MIPS_ISA_MIPS64)
   else
diff --git a/arch/mips/vdso/Kconfig b/arch/mips/vdso/Kconfig
index 7aec721398d5..a665f6108cb5 100644
--- a/arch/mips/vdso/Kconfig
+++ b/arch/mips/vdso/Kconfig
@@ -12,7 +12,7 @@
 # the lack of relocations. As such, we disable the VDSO for microMIPS builds.
 
 config MIPS_LD_CAN_LINK_VDSO
-	def_bool LD_VERSION >= 225000000 || LD_IS_LLD
+	def_bool LD_VERSION >= 22500 || LD_IS_LLD
 
 config MIPS_DISABLE_VDSO
 	def_bool CPU_MICROMIPS || (!CPU_MIPSR6 && !MIPS_LD_CAN_LINK_VDSO)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 5c8c06215dd4..6a9a852c3d56 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -65,7 +65,7 @@ UTS_MACHINE := $(subst $(space),,$(machine-y))
 ifdef CONFIG_PPC32
 KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
 else
-ifeq ($(call ld-ifversion, -ge, 225000000, y),y)
+ifeq ($(call ld-ifversion, -ge, 22500, y),y)
 # Have the linker provide sfpr if possible.
 # There is a corresponding test in arch/powerpc/lib/Makefile
 KBUILD_LDFLAGS_MODULE += --save-restore-funcs
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 69a91b571845..d4efc182662a 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -31,7 +31,7 @@ obj-$(CONFIG_FUNCTION_ERROR_INJECTION)	+= error-inject.o
 # 64-bit linker creates .sfpr on demand for final link (vmlinux),
 # so it is only needed for modules, and only for older linkers which
 # do not support --save-restore-funcs
-ifeq ($(call ld-ifversion, -lt, 225000000, y),y)
+ifeq ($(call ld-ifversion, -lt, 22500, y),y)
 extra-$(CONFIG_PPC64)	+= crtsavres.o
 endif
 
diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh
index f2be0ff9a738..0f8a2c0f9502 100755
--- a/scripts/ld-version.sh
+++ b/scripts/ld-version.sh
@@ -6,6 +6,6 @@
 	gsub(".*version ", "");
 	gsub("-.*", "");
 	split($1,a, ".");
-	print a[1]*100000000 + a[2]*1000000 + a[3]*10000;
+	print a[1]*10000 + a[2]*100 + a[3];
 	exit
 	}
-- 
2.27.0


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

* [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script
  2020-12-12 16:54 [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for checking spatch version Masahiro Yamada
  2020-12-12 16:54 ` [PATCH 2/3] kbuild: LD_VERSION redenomination Masahiro Yamada
@ 2020-12-12 16:54 ` Masahiro Yamada
  2020-12-12 17:48   ` Dominique Martinet
                     ` (4 more replies)
  2020-12-12 17:30 ` [Cocci] [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for checking spatch version Julia Lawall
  2 siblings, 5 replies; 15+ messages in thread
From: Masahiro Yamada @ 2020-12-12 16:54 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Dominique Martinet, Masahiro Yamada, Michal Marek, linux-kernel

This script was written in awk in spite of the file extension '.sh'.
Rewrite it as a shell script.

The code was mostly copy-pasted from scripts/lld-version.sh.
The two scripts can be merged, but I am keeping this as a separate
file for now.

I tested this script for some corner cases reported in the past:

 - GNU ld version 2.25-15.fc23
   as reported by commit 8083013fc320 ("ld-version: Fix it on Fedora")

 - GNU ld (GNU Binutils) 2.20.1.20100303
   as reported by commit 0d61ed17dd30 ("ld-version: Drop the 4th and
   5th version components")

I also cleaned up the macros in scripts/Kbuild.include. Remove
ld-version, which has no direct user. You can use CONFIG_LD_VERSION
if you need the version string in a Makefile.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 init/Kconfig           |  2 +-
 scripts/Kbuild.include |  6 +-----
 scripts/ld-version.sh  | 31 +++++++++++++++++++++----------
 3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 0872a5a2e759..a44a13a6b38d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -35,7 +35,7 @@ config GCC_VERSION
 
 config LD_VERSION
 	int
-	default $(shell,$(LD) --version | $(srctree)/scripts/ld-version.sh)
+	default $(shell,$(srctree)/scripts/ld-version.sh $(LD))
 
 config CC_IS_CLANG
 	def_bool $(success,echo "$(CC_VERSION_TEXT)" | grep -q clang)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 08e011175b4c..167a68bbe7be 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -141,13 +141,9 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e
 # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
 ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
 
-# ld-version
-# Note this is mainly for HJ Lu's 3 number binutil versions
-ld-version = $(shell $(LD) --version | $(srctree)/scripts/ld-version.sh)
-
 # ld-ifversion
 # Usage:  $(call ld-ifversion, -ge, 22252, y)
-ld-ifversion = $(shell [ $(ld-version) $(1) $(2) ] && echo $(3) || echo $(4))
+ld-ifversion = $(shell [ $(CONFIG_LD_VERSION) $(1) $(2) ] && echo $(3) || echo $(4))
 
 ######
 
diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh
index 0f8a2c0f9502..c214aeb3200d 100755
--- a/scripts/ld-version.sh
+++ b/scripts/ld-version.sh
@@ -1,11 +1,22 @@
-#!/usr/bin/awk -f
+#!/bin/sh
 # SPDX-License-Identifier: GPL-2.0
-# extract linker version number from stdin and turn into single number
-	{
-	gsub(".*\\)", "");
-	gsub(".*version ", "");
-	gsub("-.*", "");
-	split($1,a, ".");
-	print a[1]*10000 + a[2]*100 + a[3];
-	exit
-	}
+#
+# Usage: $ ./scripts/ld-version.sh ld
+#
+# Print the linker version of `ld' in a 5 or 6-digit form
+# such as `23501' for GNU ld 2.35.1 etc.
+
+first_line="$($* --version | head -n 1)"
+
+if ! ( echo $first_line | grep -q "GNU ld"); then
+	echo 0
+	exit 1
+fi
+
+# Distributions may append an extra string like 2.35-15.fc33
+# Take the part that consists of numbers and dots.
+VERSION=$(echo $first_line | sed 's/.* \([^ ]*\)$/\1/' | sed 's/^\(^[0-9.]*\).*/\1/')
+MAJOR=$(echo $VERSION | cut -d . -f 1)
+MINOR=$(echo $VERSION | cut -d . -f 2)
+PATCHLEVEL=$(echo $VERSION | cut -d . -f 3)
+printf "%d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL
-- 
2.27.0


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

* Re: [Cocci] [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for    checking spatch version
  2020-12-12 16:54 [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for checking spatch version Masahiro Yamada
  2020-12-12 16:54 ` [PATCH 2/3] kbuild: LD_VERSION redenomination Masahiro Yamada
  2020-12-12 16:54 ` [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script Masahiro Yamada
@ 2020-12-12 17:30 ` Julia Lawall
  2 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2020-12-12 17:30 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Michal Marek, Gilles Muller, Nicolas Palix,
	Dominique Martinet, Matthias Maennich, linux-kernel, cocci



On Sun, 13 Dec 2020, Masahiro Yamada wrote:

> scripts/ld-version.sh was, as its file name implies, originally intended
> for the GNU ld version, but is (ab)used for the spatch version too.
>
> Use 'sort -CV' for the version comparison, then coccicheck does not need
> to use scripts/ld-version.sh. Fix nsdeps as well.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Applied, thanks.

julia

> ---
>
>  scripts/coccicheck | 14 +++++---------
>  scripts/nsdeps     |  4 +---
>  2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index 209bb0427b43..d7f6b7ff130a 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -16,7 +16,6 @@ if [ ! -x "$SPATCH" ]; then
>  fi
>
>  SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}')
> -SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh)
>
>  USE_JOBS="no"
>  $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes"
> @@ -186,14 +185,11 @@ coccinelle () {
>
>      OPT=`grep "Options:" $COCCI | cut -d':' -f2`
>      REQ=`grep "Requires:" $COCCI | cut -d':' -f2 | sed "s| ||"`
> -    REQ_NUM=$(echo $REQ | ${DIR}/scripts/ld-version.sh)
> -    if [ "$REQ_NUM" != "0" ] ; then
> -	    if [ "$SPATCH_VERSION_NUM" -lt "$REQ_NUM" ] ; then
> -		    echo "Skipping coccinelle SmPL patch: $COCCI"
> -		    echo "You have coccinelle:           $SPATCH_VERSION"
> -		    echo "This SmPL patch requires:      $REQ"
> -		    return
> -	    fi
> +    if [ -n "$REQ" ] && ! { echo "$REQ"; echo "$SPATCH_VERSION"; } | sort -CV ; then
> +	    echo "Skipping coccinelle SmPL patch: $COCCI"
> +	    echo "You have coccinelle:           $SPATCH_VERSION"
> +	    echo "This SmPL patch requires:      $REQ"
> +	    return
>      fi
>
>  #   The option '--parse-cocci' can be used to syntactically check the SmPL files.
> diff --git a/scripts/nsdeps b/scripts/nsdeps
> index dab4c1a0e27d..e8ce2a4d704a 100644
> --- a/scripts/nsdeps
> +++ b/scripts/nsdeps
> @@ -12,11 +12,9 @@ if [ ! -x "$SPATCH" ]; then
>  	exit 1
>  fi
>
> -SPATCH_REQ_VERSION_NUM=$(echo $SPATCH_REQ_VERSION | ${DIR}/scripts/ld-version.sh)
>  SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}')
> -SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh)
>
> -if [ "$SPATCH_VERSION_NUM" -lt "$SPATCH_REQ_VERSION_NUM" ] ; then
> +if ! { echo "$SPATCH_REQ_VERSION"; echo "$SPATCH_VERSION"; } | sort -CV ; then
>  	echo "spatch needs to be version $SPATCH_REQ_VERSION or higher"
>  	exit 1
>  fi
> --
> 2.27.0
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

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

* Re: [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script
  2020-12-12 16:54 ` [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script Masahiro Yamada
@ 2020-12-12 17:48   ` Dominique Martinet
  2020-12-21 14:23     ` Masahiro Yamada
  2020-12-12 20:21   ` kernel test robot
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Dominique Martinet @ 2020-12-12 17:48 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: linux-kbuild, Michal Marek, linux-kernel

Masahiro Yamada wrote on Sun, Dec 13, 2020:
> This script was written in awk in spite of the file extension '.sh'.
> Rewrite it as a shell script.

Wow! I wasn't expecting so much, would have sent some rework after the
upcoming merge window.
Thank you.

Some comments below that you can probably ignore, this works for me.


> diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh
> index 0f8a2c0f9502..c214aeb3200d 100755
> --- a/scripts/ld-version.sh
> +++ b/scripts/ld-version.sh
> @@ -1,11 +1,22 @@
> -#!/usr/bin/awk -f
> +#!/bin/sh
>  # SPDX-License-Identifier: GPL-2.0
> -# extract linker version number from stdin and turn into single number
> -	{
> -	gsub(".*\\)", "");
> -	gsub(".*version ", "");
> -	gsub("-.*", "");
> -	split($1,a, ".");
> -	print a[1]*10000 + a[2]*100 + a[3];
> -	exit
> -	}
> +#
> +# Usage: $ ./scripts/ld-version.sh ld
> +#
> +# Print the linker version of `ld' in a 5 or 6-digit form
> +# such as `23501' for GNU ld 2.35.1 etc.
> +
> +first_line="$($* --version | head -n 1)"

Just nitpicking: this ($*) would fail if the argument contains spaces,
it's generally better to use "$@" or "$1" (with quotes)

Probably doesn't matter here as gcc/clang-version scripts have the same
problem, so if someone had a problem with that they probably would have
reported it there.

> +
> +if ! ( echo $first_line | grep -q "GNU ld"); then
> +	echo 0
> +	exit 1
> +fi
> +
> +# Distributions may append an extra string like 2.35-15.fc33
> +# Take the part that consists of numbers and dots.
> +VERSION=$(echo $first_line | sed 's/.* \([^ ]*\)$/\1/' | sed 's/^\(^[0-9.]*\).*/\1/')
> +MAJOR=$(echo $VERSION | cut -d . -f 1)
> +MINOR=$(echo $VERSION | cut -d . -f 2)
> +PATCHLEVEL=$(echo $VERSION | cut -d . -f 3)
> +printf "%d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL

There is a bug if there is no dot at all (e.g. if binutils ever releases
a version 3 and call it version 3 and not 3.0, the script would print
30303 because cut when no delimiter is found always returns the whole
string)
This can be fixed by artificially appending a dot to VERSION:
VERSION=$(echo $first_line | sed 's/.* \([^ ]*\)$/\1/' | sed 's/^\(^[0-9.]*\).*/\1./')

I'm not sure it's worth worrying about either.


Thanks again,
-- 
Dominique

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

* Re: [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script
  2020-12-12 16:54 ` [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script Masahiro Yamada
  2020-12-12 17:48   ` Dominique Martinet
@ 2020-12-12 20:21   ` kernel test robot
  2020-12-12 20:53   ` kernel test robot
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2020-12-12 20:21 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kbuild
  Cc: kbuild-all, Dominique Martinet, Masahiro Yamada, Michal Marek,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1829 bytes --]

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on kbuild/for-next arm64/for-next/core linus/master v5.10-rc7 next-20201211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-do-not-use-scripts-ld-version-sh-for-checking-spatch-version/20201213-010621
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-tqm8xx_defconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0eab60f5e1f804528a4d0dd9566cb30f62242d22
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Masahiro-Yamada/kbuild-do-not-use-scripts-ld-version-sh-for-checking-spatch-version/20201213-010621
        git checkout 0eab60f5e1f804528a4d0dd9566cb30f62242d22
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> /bin/sh: 1: [: -ge: unexpected operator
   /bin/sh: 1: [: -lt: unexpected operator
--
>> /bin/sh: 1: [: -ge: unexpected operator
--
>> /bin/sh: 1: [: -ge: unexpected operator

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10786 bytes --]

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

* Re: [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script
  2020-12-12 16:54 ` [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script Masahiro Yamada
  2020-12-12 17:48   ` Dominique Martinet
  2020-12-12 20:21   ` kernel test robot
@ 2020-12-12 20:53   ` kernel test robot
  2020-12-12 21:42   ` kernel test robot
  2020-12-12 21:47   ` David Laight
  4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2020-12-12 20:53 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kbuild
  Cc: kbuild-all, clang-built-linux, Dominique Martinet,
	Masahiro Yamada, Michal Marek, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on kbuild/for-next arm64/for-next/core linus/master v5.10-rc7 next-20201211]
[cannot apply to mmarek/misc]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-do-not-use-scripts-ld-version-sh-for-checking-spatch-version/20201213-010621
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-r014-20201213 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 84c09ab44599ece409e4e19761288ddf796fceec)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/0eab60f5e1f804528a4d0dd9566cb30f62242d22
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Masahiro-Yamada/kbuild-do-not-use-scripts-ld-version-sh-for-checking-spatch-version/20201213-010621
        git checkout 0eab60f5e1f804528a4d0dd9566cb30f62242d22
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> /bin/sh: 1: [: -ge: unexpected operator
   /bin/sh: 1: [: -lt: unexpected operator
--
>> /bin/sh: 1: [: -ge: unexpected operator
--
>> /bin/sh: 1: [: -ge: unexpected operator

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32331 bytes --]

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

* Re: [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script
  2020-12-12 16:54 ` [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script Masahiro Yamada
                     ` (2 preceding siblings ...)
  2020-12-12 20:53   ` kernel test robot
@ 2020-12-12 21:42   ` kernel test robot
  2020-12-12 21:47   ` David Laight
  4 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2020-12-12 21:42 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kbuild
  Cc: kbuild-all, Dominique Martinet, Masahiro Yamada, Michal Marek,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1739 bytes --]

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on kbuild/for-next arm64/for-next/core linus/master v5.10-rc7 next-20201211]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-do-not-use-scripts-ld-version-sh-for-checking-spatch-version/20201213-010621
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-amigaone_defconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0eab60f5e1f804528a4d0dd9566cb30f62242d22
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Masahiro-Yamada/kbuild-do-not-use-scripts-ld-version-sh-for-checking-spatch-version/20201213-010621
        git checkout 0eab60f5e1f804528a4d0dd9566cb30f62242d22
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   /bin/sh: 1: [: -ge: unexpected operator
>> /bin/sh: 1: [: -lt: unexpected operator

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 18645 bytes --]

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

* RE: [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script
  2020-12-12 16:54 ` [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script Masahiro Yamada
                     ` (3 preceding siblings ...)
  2020-12-12 21:42   ` kernel test robot
@ 2020-12-12 21:47   ` David Laight
  2020-12-21 14:29     ` Masahiro Yamada
  4 siblings, 1 reply; 15+ messages in thread
From: David Laight @ 2020-12-12 21:47 UTC (permalink / raw)
  To: 'Masahiro Yamada', linux-kbuild
  Cc: Dominique Martinet, Michal Marek, linux-kernel

From: Masahiro Yamada
> Sent: 12 December 2020 16:55
> 
> This script was written in awk in spite of the file extension '.sh'.
> Rewrite it as a shell script.
...
> +#
> +# Usage: $ ./scripts/ld-version.sh ld
> +#
> +# Print the linker version of `ld' in a 5 or 6-digit form
> +# such as `23501' for GNU ld 2.35.1 etc.
> +
> +first_line="$($* --version | head -n 1)"
> +
> +if ! ( echo $first_line | grep -q "GNU ld"); then
> +	echo 0
> +	exit 1
> +fi
> +
> +# Distributions may append an extra string like 2.35-15.fc33
> +# Take the part that consists of numbers and dots.
> +VERSION=$(echo $first_line | sed 's/.* \([^ ]*\)$/\1/' | sed 's/^\(^[0-9.]*\).*/\1/')
> +MAJOR=$(echo $VERSION | cut -d . -f 1)
> +MINOR=$(echo $VERSION | cut -d . -f 2)
> +PATCHLEVEL=$(echo $VERSION | cut -d . -f 3)
> +printf "%d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL


Hmmmm.....
You've managed to convert an awk script into something that requires
sh, head, grep, sed (twice), and cut (thrice).
Plus (probably) a few sub-shells.

It is quite ease to do it all in all in the shell.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


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

* Re: [PATCH 2/3] kbuild: LD_VERSION redenomination
  2020-12-12 16:54 ` [PATCH 2/3] kbuild: LD_VERSION redenomination Masahiro Yamada
@ 2020-12-14 23:05   ` Will Deacon
  2020-12-15 13:48   ` Thomas Bogendoerfer
  2021-01-28  6:38   ` Masahiro Yamada
  2 siblings, 0 replies; 15+ messages in thread
From: Will Deacon @ 2020-12-14 23:05 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Dominique Martinet, Benjamin Herrenschmidt,
	Catalin Marinas, Huacai Chen, Jiaxun Yang, Michael Ellerman,
	Paul Mackerras, Thomas Bogendoerfer, linux-arm-kernel,
	linux-kernel, linux-mips, linuxppc-dev

On Sun, Dec 13, 2020 at 01:54:30AM +0900, Masahiro Yamada wrote:
> Commit ccbef1674a15 ("Kbuild, lto: add ld-version and ld-ifversion
> macros") introduced scripts/ld-version.sh for GCC LTO.
> 
> At that time, this script handled 5 version fields because GCC LTO
> needed the downstream binutils. (https://lkml.org/lkml/2014/4/8/272)
> 
> The code snippet from the submitted patch was as follows:
> 
>     # We need HJ Lu's Linux binutils because mainline binutils does not
>     # support mixing assembler and LTO code in the same ld -r object.
>     # XXX check if the gcc plugin ld is the expected one too
>     # XXX some Fedora binutils should also support it. How to check for that?
>     ifeq ($(call ld-ifversion,-ge,22710001,y),y)
>         ...
> 
> However, GCC LTO was not merged into the mainline after all.
> (https://lkml.org/lkml/2014/4/8/272)
> 
> So, the 4th and 5th fields were never used, and finally removed by
> commit 0d61ed17dd30 ("ld-version: Drop the 4th and 5th version
> components").
> 
> Since then, the last 4-digits returned by this script is always zeros.
> 
> Remove the meaningless last 4-digits. This makes the version format
> consistent with GCC_VERSION, CLANG_VERSION, LLD_VERSION.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  arch/arm64/Kconfig            | 2 +-
>  arch/mips/loongson64/Platform | 2 +-
>  arch/mips/vdso/Kconfig        | 2 +-
>  arch/powerpc/Makefile         | 2 +-
>  arch/powerpc/lib/Makefile     | 2 +-
>  scripts/ld-version.sh         | 2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index a6b5b7ef40ae..69d56b21a6ec 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1499,7 +1499,7 @@ config ARM64_PTR_AUTH
>  	depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC
>  	# Modern compilers insert a .note.gnu.property section note for PAC
>  	# which is only understood by binutils starting with version 2.33.1.
> -	depends on LD_IS_LLD || LD_VERSION >= 233010000 || (CC_IS_GCC && GCC_VERSION < 90100)
> +	depends on LD_IS_LLD || LD_VERSION >= 23301 || (CC_IS_GCC && GCC_VERSION < 90100)
>  	depends on !CC_IS_CLANG || AS_HAS_CFI_NEGATE_RA_STATE
>  	depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
>  	help

Acked-by: Will Deacon <will@kernel.org>

Will

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

* Re: [PATCH 2/3] kbuild: LD_VERSION redenomination
  2020-12-12 16:54 ` [PATCH 2/3] kbuild: LD_VERSION redenomination Masahiro Yamada
  2020-12-14 23:05   ` Will Deacon
@ 2020-12-15 13:48   ` Thomas Bogendoerfer
  2021-01-28  6:38   ` Masahiro Yamada
  2 siblings, 0 replies; 15+ messages in thread
From: Thomas Bogendoerfer @ 2020-12-15 13:48 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Dominique Martinet, Benjamin Herrenschmidt,
	Catalin Marinas, Huacai Chen, Jiaxun Yang, Michael Ellerman,
	Paul Mackerras, Will Deacon, linux-arm-kernel, linux-kernel,
	linux-mips, linuxppc-dev

On Sun, Dec 13, 2020 at 01:54:30AM +0900, Masahiro Yamada wrote:
> Commit ccbef1674a15 ("Kbuild, lto: add ld-version and ld-ifversion
> macros") introduced scripts/ld-version.sh for GCC LTO.
> 
> At that time, this script handled 5 version fields because GCC LTO
> needed the downstream binutils. (https://lkml.org/lkml/2014/4/8/272)
> 
> The code snippet from the submitted patch was as follows:
> 
>     # We need HJ Lu's Linux binutils because mainline binutils does not
>     # support mixing assembler and LTO code in the same ld -r object.
>     # XXX check if the gcc plugin ld is the expected one too
>     # XXX some Fedora binutils should also support it. How to check for that?
>     ifeq ($(call ld-ifversion,-ge,22710001,y),y)
>         ...
> 
> However, GCC LTO was not merged into the mainline after all.
> (https://lkml.org/lkml/2014/4/8/272)
> 
> So, the 4th and 5th fields were never used, and finally removed by
> commit 0d61ed17dd30 ("ld-version: Drop the 4th and 5th version
> components").
> 
> Since then, the last 4-digits returned by this script is always zeros.
> 
> Remove the meaningless last 4-digits. This makes the version format
> consistent with GCC_VERSION, CLANG_VERSION, LLD_VERSION.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  arch/mips/loongson64/Platform | 2 +-
>  arch/mips/vdso/Kconfig        | 2 +-

Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

* Re: [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script
  2020-12-12 17:48   ` Dominique Martinet
@ 2020-12-21 14:23     ` Masahiro Yamada
  0 siblings, 0 replies; 15+ messages in thread
From: Masahiro Yamada @ 2020-12-21 14:23 UTC (permalink / raw)
  To: Dominique Martinet
  Cc: Linux Kbuild mailing list, Michal Marek, Linux Kernel Mailing List

On Sun, Dec 13, 2020 at 2:48 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> Masahiro Yamada wrote on Sun, Dec 13, 2020:
> > This script was written in awk in spite of the file extension '.sh'.
> > Rewrite it as a shell script.
>
> Wow! I wasn't expecting so much, would have sent some rework after the
> upcoming merge window.
> Thank you.
>
> Some comments below that you can probably ignore, this works for me.
>
>
> > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh
> > index 0f8a2c0f9502..c214aeb3200d 100755
> > --- a/scripts/ld-version.sh
> > +++ b/scripts/ld-version.sh
> > @@ -1,11 +1,22 @@
> > -#!/usr/bin/awk -f
> > +#!/bin/sh
> >  # SPDX-License-Identifier: GPL-2.0
> > -# extract linker version number from stdin and turn into single number
> > -     {
> > -     gsub(".*\\)", "");
> > -     gsub(".*version ", "");
> > -     gsub("-.*", "");
> > -     split($1,a, ".");
> > -     print a[1]*10000 + a[2]*100 + a[3];
> > -     exit
> > -     }
> > +#
> > +# Usage: $ ./scripts/ld-version.sh ld
> > +#
> > +# Print the linker version of `ld' in a 5 or 6-digit form
> > +# such as `23501' for GNU ld 2.35.1 etc.
> > +
> > +first_line="$($* --version | head -n 1)"
>
> Just nitpicking: this ($*) would fail if the argument contains spaces,
> it's generally better to use "$@" or "$1" (with quotes)

This is just a copy-paste work based on scripts/lld-version.sh.

"$@" is better, I agree.




> Probably doesn't matter here as gcc/clang-version scripts have the same
> problem, so if someone had a problem with that they probably would have
> reported it there.

Talking about gcc/clang-version, "$1" is not acceptable because the first
word of the arguments may not be the compiler.

For example, when CC="ccache gcc" is passed in,
scripts/gcc-version.sh  ccache  gcc
must return the gcc version.




Difference between "$@" and "$*" matters only
when it is directly passed to some command.


masahiro@grover:~$ set  "a   b"  c  d
masahiro@grover:~$ ls  "$*"
ls: cannot access 'a   b c d': No such file or directory
masahiro@grover:~$ ls  "$@"
ls: cannot access 'a   b': No such file or directory
ls: cannot access 'c': No such file or directory
ls: cannot access 'd': No such file or directory


"$*" was expanded into a single string, 'a   b c d'.
It forgot which spaces were the part of the argument,
and which spaces were argument delimiters.

In contrast, "$@" was expanded into three arguments,
'a   b', 'c', and 'd'.
It correctly preserved the original arguments.



Let me continue some more experiments...


masahiro@grover:~$ set  "a   b"  c  d
masahiro@grover:~$ compiler="$*"
masahiro@grover:~$ ls "$compiler"
ls: cannot access 'a   b c d': No such file or directory
masahiro@grover:~$ ls $compiler
ls: cannot access 'a': No such file or directory
ls: cannot access 'b': No such file or directory
ls: cannot access 'c': No such file or directory
ls: cannot access 'd': No such file or directory



masahiro@grover:~$ set  "a   b"  c  d
masahiro@grover:~$ compiler="$@"
masahiro@grover:~$ ls "$compiler"
ls: cannot access 'a   b c d': No such file or directory
masahiro@grover:~$ ls $compiler
ls: cannot access 'a': No such file or directory
ls: cannot access 'b': No such file or directory
ls: cannot access 'c': No such file or directory
ls: cannot access 'd': No such file or directory


The result is the same.
So, whichever we use, after it is assigned to the variable "compiler",
it forgets the origin of the spaces.


If we really want to preserve the passed arguments,
we need to use "$@" directly in scripts/gcc-version.sh
like this:

MAJOR=$(echo __GNUC__ | "$@" -E -x c - | tail -n 1)

If we do this,

  scripts/gcc-version.sh  ccache gcc

will succeed, and

  scripts/gcc-version.sh  "ccache gcc"

will fail.




> > +
> > +if ! ( echo $first_line | grep -q "GNU ld"); then
> > +     echo 0
> > +     exit 1
> > +fi
> > +
> > +# Distributions may append an extra string like 2.35-15.fc33
> > +# Take the part that consists of numbers and dots.
> > +VERSION=$(echo $first_line | sed 's/.* \([^ ]*\)$/\1/' | sed 's/^\(^[0-9.]*\).*/\1/')
> > +MAJOR=$(echo $VERSION | cut -d . -f 1)
> > +MINOR=$(echo $VERSION | cut -d . -f 2)
> > +PATCHLEVEL=$(echo $VERSION | cut -d . -f 3)
> > +printf "%d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL
>
> There is a bug if there is no dot at all (e.g. if binutils ever releases
> a version 3 and call it version 3 and not 3.0, the script would print
> 30303 because cut when no delimiter is found always returns the whole
> string)
> This can be fixed by artificially appending a dot to VERSION:
> VERSION=$(echo $first_line | sed 's/.* \([^ ]*\)$/\1/' | sed 's/^\(^[0-9.]*\).*/\1./')
>
> I'm not sure it's worth worrying about either.

Ah, good catch.



--
Best Regards
Masahiro Yamada

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

* Re: [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script
  2020-12-12 21:47   ` David Laight
@ 2020-12-21 14:29     ` Masahiro Yamada
  2020-12-21 14:51       ` David Laight
  0 siblings, 1 reply; 15+ messages in thread
From: Masahiro Yamada @ 2020-12-21 14:29 UTC (permalink / raw)
  To: David Laight; +Cc: linux-kbuild, Dominique Martinet, Michal Marek, linux-kernel

On Sun, Dec 13, 2020 at 6:47 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Masahiro Yamada
> > Sent: 12 December 2020 16:55
> >
> > This script was written in awk in spite of the file extension '.sh'.
> > Rewrite it as a shell script.
> ...
> > +#
> > +# Usage: $ ./scripts/ld-version.sh ld
> > +#
> > +# Print the linker version of `ld' in a 5 or 6-digit form
> > +# such as `23501' for GNU ld 2.35.1 etc.
> > +
> > +first_line="$($* --version | head -n 1)"
> > +
> > +if ! ( echo $first_line | grep -q "GNU ld"); then
> > +     echo 0
> > +     exit 1
> > +fi
> > +
> > +# Distributions may append an extra string like 2.35-15.fc33
> > +# Take the part that consists of numbers and dots.
> > +VERSION=$(echo $first_line | sed 's/.* \([^ ]*\)$/\1/' | sed 's/^\(^[0-9.]*\).*/\1/')
> > +MAJOR=$(echo $VERSION | cut -d . -f 1)
> > +MINOR=$(echo $VERSION | cut -d . -f 2)
> > +PATCHLEVEL=$(echo $VERSION | cut -d . -f 3)
> > +printf "%d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL
>
>
> Hmmmm.....
> You've managed to convert an awk script into something that requires
> sh, head, grep, sed (twice), and cut (thrice).
> Plus (probably) a few sub-shells.
>
> It is quite ease to do it all in all in the shell.
>
>         David
>

OK, please rewrite the code.



-- 
Best Regards
Masahiro Yamada

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

* RE: [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script
  2020-12-21 14:29     ` Masahiro Yamada
@ 2020-12-21 14:51       ` David Laight
  0 siblings, 0 replies; 15+ messages in thread
From: David Laight @ 2020-12-21 14:51 UTC (permalink / raw)
  To: 'Masahiro Yamada'
  Cc: linux-kbuild, Dominique Martinet, Michal Marek, linux-kernel

From: Masahiro Yamada
> Sent: 21 December 2020 14:29
> 
> On Sun, Dec 13, 2020 at 6:47 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Masahiro Yamada
> > > Sent: 12 December 2020 16:55
> > >
> > > This script was written in awk in spite of the file extension '.sh'.
> > > Rewrite it as a shell script.
> > ...
> > > +#
> > > +# Usage: $ ./scripts/ld-version.sh ld
> > > +#
> > > +# Print the linker version of `ld' in a 5 or 6-digit form
> > > +# such as `23501' for GNU ld 2.35.1 etc.
> > > +
> > > +first_line="$($* --version | head -n 1)"
> > > +
> > > +if ! ( echo $first_line | grep -q "GNU ld"); then
> > > +     echo 0
> > > +     exit 1
> > > +fi
> > > +
> > > +# Distributions may append an extra string like 2.35-15.fc33
> > > +# Take the part that consists of numbers and dots.
> > > +VERSION=$(echo $first_line | sed 's/.* \([^ ]*\)$/\1/' | sed 's/^\(^[0-9.]*\).*/\1/')
> > > +MAJOR=$(echo $VERSION | cut -d . -f 1)
> > > +MINOR=$(echo $VERSION | cut -d . -f 2)
> > > +PATCHLEVEL=$(echo $VERSION | cut -d . -f 3)
> > > +printf "%d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL
> >
> >
> > Hmmmm.....
> > You've managed to convert an awk script into something that requires
> > sh, head, grep, sed (twice), and cut (thrice).
> > Plus (probably) a few sub-shells.
> >
> > It is quite ease to do it all in all in the shell.
> >
> >         David
> >
> 
> OK, please rewrite the code.

I've posted a couple of versions before, how about this one.
I've added a few comments - which don't need to be in the final version.

# Get the first line of the 'ld --version' output
if input_from_from_stdin; then
	read line
else
	IFS='
'
	set -- $("$@")
	line="$1"
fi

# Split the line on 'space' and get the last word
IFS=' '
set -- $line
shift $(($# - 1))
version="$1"

# Split on '.' and '-'
IFS='.-'
set -- $version

# The three version components are now $1 $2 and $3
# so you can do either
printf "%d%02d%02d\\n" $1 $2 $3
# or
echo $(($1 * 10000 + $2 * 100 + $3))
# both are builtins on recent shells (printf is most likely not to be).

So this should be the same as the script above:

#! /bin/sh
IFS='
'
set -- $("$@")
# The last word contains the version
IFS=' '
set -- $1
shift $(($# - 1))
# Get the first 3 parts of the version
IFS='.-'
set -- $1
printf "%d%02d%02d\\n" $1 $2 $3

Seems to work for me.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

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

* Re: [PATCH 2/3] kbuild: LD_VERSION redenomination
  2020-12-12 16:54 ` [PATCH 2/3] kbuild: LD_VERSION redenomination Masahiro Yamada
  2020-12-14 23:05   ` Will Deacon
  2020-12-15 13:48   ` Thomas Bogendoerfer
@ 2021-01-28  6:38   ` Masahiro Yamada
  2 siblings, 0 replies; 15+ messages in thread
From: Masahiro Yamada @ 2021-01-28  6:38 UTC (permalink / raw)
  To: Linux Kbuild mailing list
  Cc: Dominique Martinet, Benjamin Herrenschmidt, Catalin Marinas,
	Huacai Chen, Jiaxun Yang, Michael Ellerman, Paul Mackerras,
	Thomas Bogendoerfer, Will Deacon, linux-arm-kernel,
	Linux Kernel Mailing List, linux-mips, linuxppc-dev

On Sun, Dec 13, 2020 at 1:54 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Commit ccbef1674a15 ("Kbuild, lto: add ld-version and ld-ifversion
> macros") introduced scripts/ld-version.sh for GCC LTO.
>
> At that time, this script handled 5 version fields because GCC LTO
> needed the downstream binutils. (https://lkml.org/lkml/2014/4/8/272)
>
> The code snippet from the submitted patch was as follows:
>
>     # We need HJ Lu's Linux binutils because mainline binutils does not
>     # support mixing assembler and LTO code in the same ld -r object.
>     # XXX check if the gcc plugin ld is the expected one too
>     # XXX some Fedora binutils should also support it. How to check for that?
>     ifeq ($(call ld-ifversion,-ge,22710001,y),y)
>         ...
>
> However, GCC LTO was not merged into the mainline after all.
> (https://lkml.org/lkml/2014/4/8/272)
>
> So, the 4th and 5th fields were never used, and finally removed by
> commit 0d61ed17dd30 ("ld-version: Drop the 4th and 5th version
> components").
>
> Since then, the last 4-digits returned by this script is always zeros.
>
> Remove the meaningless last 4-digits. This makes the version format
> consistent with GCC_VERSION, CLANG_VERSION, LLD_VERSION.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>

Applied to linux-kbuild.


>  arch/arm64/Kconfig            | 2 +-
>  arch/mips/loongson64/Platform | 2 +-
>  arch/mips/vdso/Kconfig        | 2 +-
>  arch/powerpc/Makefile         | 2 +-
>  arch/powerpc/lib/Makefile     | 2 +-
>  scripts/ld-version.sh         | 2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index a6b5b7ef40ae..69d56b21a6ec 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1499,7 +1499,7 @@ config ARM64_PTR_AUTH
>         depends on (CC_HAS_SIGN_RETURN_ADDRESS || CC_HAS_BRANCH_PROT_PAC_RET) && AS_HAS_PAC
>         # Modern compilers insert a .note.gnu.property section note for PAC
>         # which is only understood by binutils starting with version 2.33.1.
> -       depends on LD_IS_LLD || LD_VERSION >= 233010000 || (CC_IS_GCC && GCC_VERSION < 90100)
> +       depends on LD_IS_LLD || LD_VERSION >= 23301 || (CC_IS_GCC && GCC_VERSION < 90100)
>         depends on !CC_IS_CLANG || AS_HAS_CFI_NEGATE_RA_STATE
>         depends on (!FUNCTION_GRAPH_TRACER || DYNAMIC_FTRACE_WITH_REGS)
>         help
> diff --git a/arch/mips/loongson64/Platform b/arch/mips/loongson64/Platform
> index ec42c5085905..cc0b9c87f9ad 100644
> --- a/arch/mips/loongson64/Platform
> +++ b/arch/mips/loongson64/Platform
> @@ -35,7 +35,7 @@ cflags-$(CONFIG_CPU_LOONGSON64)       += $(call as-option,-Wa$(comma)-mno-fix-loongson
>  # can't easily be used safely within the kbuild framework.
>  #
>  ifeq ($(call cc-ifversion, -ge, 0409, y), y)
> -  ifeq ($(call ld-ifversion, -ge, 225000000, y), y)
> +  ifeq ($(call ld-ifversion, -ge, 22500, y), y)
>      cflags-$(CONFIG_CPU_LOONGSON64)  += \
>        $(call cc-option,-march=loongson3a -U_MIPS_ISA -D_MIPS_ISA=_MIPS_ISA_MIPS64)
>    else
> diff --git a/arch/mips/vdso/Kconfig b/arch/mips/vdso/Kconfig
> index 7aec721398d5..a665f6108cb5 100644
> --- a/arch/mips/vdso/Kconfig
> +++ b/arch/mips/vdso/Kconfig
> @@ -12,7 +12,7 @@
>  # the lack of relocations. As such, we disable the VDSO for microMIPS builds.
>
>  config MIPS_LD_CAN_LINK_VDSO
> -       def_bool LD_VERSION >= 225000000 || LD_IS_LLD
> +       def_bool LD_VERSION >= 22500 || LD_IS_LLD
>
>  config MIPS_DISABLE_VDSO
>         def_bool CPU_MICROMIPS || (!CPU_MIPSR6 && !MIPS_LD_CAN_LINK_VDSO)
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 5c8c06215dd4..6a9a852c3d56 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -65,7 +65,7 @@ UTS_MACHINE := $(subst $(space),,$(machine-y))
>  ifdef CONFIG_PPC32
>  KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
>  else
> -ifeq ($(call ld-ifversion, -ge, 225000000, y),y)
> +ifeq ($(call ld-ifversion, -ge, 22500, y),y)
>  # Have the linker provide sfpr if possible.
>  # There is a corresponding test in arch/powerpc/lib/Makefile
>  KBUILD_LDFLAGS_MODULE += --save-restore-funcs
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 69a91b571845..d4efc182662a 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -31,7 +31,7 @@ obj-$(CONFIG_FUNCTION_ERROR_INJECTION)        += error-inject.o
>  # 64-bit linker creates .sfpr on demand for final link (vmlinux),
>  # so it is only needed for modules, and only for older linkers which
>  # do not support --save-restore-funcs
> -ifeq ($(call ld-ifversion, -lt, 225000000, y),y)
> +ifeq ($(call ld-ifversion, -lt, 22500, y),y)
>  extra-$(CONFIG_PPC64)  += crtsavres.o
>  endif
>
> diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh
> index f2be0ff9a738..0f8a2c0f9502 100755
> --- a/scripts/ld-version.sh
> +++ b/scripts/ld-version.sh
> @@ -6,6 +6,6 @@
>         gsub(".*version ", "");
>         gsub("-.*", "");
>         split($1,a, ".");
> -       print a[1]*100000000 + a[2]*1000000 + a[3]*10000;
> +       print a[1]*10000 + a[2]*100 + a[3];
>         exit
>         }
> --
> 2.27.0
>


-- 
Best Regards
Masahiro Yamada

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

end of thread, other threads:[~2021-01-28  6:40 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-12 16:54 [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for checking spatch version Masahiro Yamada
2020-12-12 16:54 ` [PATCH 2/3] kbuild: LD_VERSION redenomination Masahiro Yamada
2020-12-14 23:05   ` Will Deacon
2020-12-15 13:48   ` Thomas Bogendoerfer
2021-01-28  6:38   ` Masahiro Yamada
2020-12-12 16:54 ` [PATCH 3/3] kbuild: rewrite ld-version.sh in shell script Masahiro Yamada
2020-12-12 17:48   ` Dominique Martinet
2020-12-21 14:23     ` Masahiro Yamada
2020-12-12 20:21   ` kernel test robot
2020-12-12 20:53   ` kernel test robot
2020-12-12 21:42   ` kernel test robot
2020-12-12 21:47   ` David Laight
2020-12-21 14:29     ` Masahiro Yamada
2020-12-21 14:51       ` David Laight
2020-12-12 17:30 ` [Cocci] [PATCH 1/3] kbuild: do not use scripts/ld-version.sh for checking spatch version Julia Lawall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).