* [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR @ 2021-03-02 22:12 Bernhard Rosenkränzer 2021-03-03 4:02 ` Nathan Chancellor 2021-03-04 3:34 ` Masahiro Yamada 0 siblings, 2 replies; 7+ messages in thread From: Bernhard Rosenkränzer @ 2021-03-02 22:12 UTC (permalink / raw) To: linux-kernel; +Cc: Bernhard Rosenkränzer If LLD was built with -DLLD_VENDOR="xyz", ld.lld --version output will prefix LLD_VENDOR. Since LLD_VENDOR can contain spaces, the LLD identifier isn't guaranteed to be $2 either. Adjust the version checker to handle such versions of lld. Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch> --- scripts/ld-version.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh index a463273509b5..4c042a306e22 100755 --- a/scripts/ld-version.sh +++ b/scripts/ld-version.sh @@ -49,6 +49,18 @@ elif [ "$1" = LLD ]; then min_version=$lld_min_version name=LLD disp_name=LLD +elif echo "$@" |grep -q ' LLD '; then + # if LLD was built with -DLLD_VENDOR="xyz", it ld.lld --version + # says "xyz LLD [...]". Since LLD_VENDOR may contain spaces, we + # don't know the exact position of "LLD" and the version info + # at this point + while [ "$1" != "LLD" ]; do + shift + done + version=$2 + min_version=$lld_min_version + name=LLD + disp_name=LLD else echo "$orig_args: unknown linker" >&2 exit 1 -- 2.30.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR 2021-03-02 22:12 [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR Bernhard Rosenkränzer @ 2021-03-03 4:02 ` Nathan Chancellor 2021-03-03 11:38 ` Masahiro Yamada 2021-03-04 3:34 ` Masahiro Yamada 1 sibling, 1 reply; 7+ messages in thread From: Nathan Chancellor @ 2021-03-03 4:02 UTC (permalink / raw) To: Bernhard Rosenkränzer Cc: linux-kernel, Masahiro Yamada, linux-kbuild, clang-built-linux, Nick Desaulniers Hi Bernhard, I have added the ClangBuiltLinux mailing list, kbuild mailing list, and Masahiro and Nick to CC. Maybe ld-version.sh and cc-version.sh should be added to a MAINTAINERS entry to make sure we get CC'd (I can send one along tomorrow). On Tue, Mar 02, 2021 at 11:12:11PM +0100, Bernhard Rosenkränzer wrote: > If LLD was built with -DLLD_VENDOR="xyz", ld.lld --version output > will prefix LLD_VENDOR. Since LLD_VENDOR can contain spaces, the > LLD identifier isn't guaranteed to be $2 either. TIL about LLD_VENDOR... > Adjust the version checker to handle such versions of lld. > > Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch> > --- > scripts/ld-version.sh | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > index a463273509b5..4c042a306e22 100755 > --- a/scripts/ld-version.sh > +++ b/scripts/ld-version.sh > @@ -49,6 +49,18 @@ elif [ "$1" = LLD ]; then > min_version=$lld_min_version > name=LLD > disp_name=LLD > +elif echo "$@" |grep -q ' LLD '; then > + # if LLD was built with -DLLD_VENDOR="xyz", it ld.lld --version > + # says "xyz LLD [...]". Since LLD_VENDOR may contain spaces, we > + # don't know the exact position of "LLD" and the version info > + # at this point > + while [ "$1" != "LLD" ]; do > + shift > + done > + version=$2 > + min_version=$lld_min_version > + name=LLD > + disp_name=LLD > else > echo "$orig_args: unknown linker" >&2 > exit 1 > -- > 2.30.1 > I am not sure what a better fix would be of the top of my head but wouldn't it be better to avoid the duplication? This diff below works for me with or without LLD_VENDOR defined. diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh index a463273509b5..84f9fc741f09 100755 --- a/scripts/ld-version.sh +++ b/scripts/ld-version.sh @@ -44,7 +44,10 @@ if [ "$1" = GNU -a "$2" = ld ]; then elif [ "$1" = GNU -a "$2" = gold ]; then echo "gold linker is not supported as it is not capable of linking the kernel proper." >&2 exit 1 -elif [ "$1" = LLD ]; then +elif echo "$*" | grep -q LLD; then + while [ "$1" != "LLD" ]; do + shift + done version=$2 min_version=$lld_min_version name=LLD ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR 2021-03-03 4:02 ` Nathan Chancellor @ 2021-03-03 11:38 ` Masahiro Yamada 2021-03-03 17:45 ` Nathan Chancellor 0 siblings, 1 reply; 7+ messages in thread From: Masahiro Yamada @ 2021-03-03 11:38 UTC (permalink / raw) To: Nathan Chancellor Cc: Bernhard Rosenkränzer, Linux Kernel Mailing List, Linux Kbuild mailing list, clang-built-linux, Nick Desaulniers On Wed, Mar 3, 2021 at 1:02 PM Nathan Chancellor <nathan@kernel.org> wrote: > > Hi Bernhard, > > I have added the ClangBuiltLinux mailing list, kbuild mailing list, and > Masahiro and Nick to CC. Maybe ld-version.sh and cc-version.sh should be > added to a MAINTAINERS entry to make sure we get CC'd (I can send one > along tomorrow). > > On Tue, Mar 02, 2021 at 11:12:11PM +0100, Bernhard Rosenkränzer wrote: > > If LLD was built with -DLLD_VENDOR="xyz", ld.lld --version output > > will prefix LLD_VENDOR. Since LLD_VENDOR can contain spaces, the > > LLD identifier isn't guaranteed to be $2 either. > > TIL about LLD_VENDOR... > > > Adjust the version checker to handle such versions of lld. > > > > Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch> > > --- > > scripts/ld-version.sh | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > > index a463273509b5..4c042a306e22 100755 > > --- a/scripts/ld-version.sh > > +++ b/scripts/ld-version.sh > > @@ -49,6 +49,18 @@ elif [ "$1" = LLD ]; then > > min_version=$lld_min_version > > name=LLD > > disp_name=LLD > > +elif echo "$@" |grep -q ' LLD '; then > > + # if LLD was built with -DLLD_VENDOR="xyz", it ld.lld --version > > + # says "xyz LLD [...]". Since LLD_VENDOR may contain spaces, we > > + # don't know the exact position of "LLD" and the version info > > + # at this point > > + while [ "$1" != "LLD" ]; do > > + shift > > + done > > + version=$2 > > + min_version=$lld_min_version > > + name=LLD > > + disp_name=LLD > > else > > echo "$orig_args: unknown linker" >&2 > > exit 1 > > -- > > 2.30.1 > > > > I am not sure what a better fix would be of the top of my head but > wouldn't it be better to avoid the duplication? This diff below works > for me with or without LLD_VENDOR defined. > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > index a463273509b5..84f9fc741f09 100755 > --- a/scripts/ld-version.sh > +++ b/scripts/ld-version.sh > @@ -44,7 +44,10 @@ if [ "$1" = GNU -a "$2" = ld ]; then > elif [ "$1" = GNU -a "$2" = gold ]; then > echo "gold linker is not supported as it is not capable of linking the kernel proper." >&2 > exit 1 > -elif [ "$1" = LLD ]; then > +elif echo "$*" | grep -q LLD; then > + while [ "$1" != "LLD" ]; do > + shift > + done > version=$2 > min_version=$lld_min_version > name=LLD You do not need to use grep. How about this? ... else while [ $# -gt 1 -a "$1" != "LLD" ]; do shift done if [ "$1" = LLD ]; then version=$2 min_version=$lld_min_version name=LLD disp_name=LLD else echo "$orig_args: unknown linker" >&2 exit 1 fi fi -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR 2021-03-03 11:38 ` Masahiro Yamada @ 2021-03-03 17:45 ` Nathan Chancellor 0 siblings, 0 replies; 7+ messages in thread From: Nathan Chancellor @ 2021-03-03 17:45 UTC (permalink / raw) To: Masahiro Yamada Cc: Bernhard Rosenkränzer, Linux Kernel Mailing List, Linux Kbuild mailing list, clang-built-linux, Nick Desaulniers On Wed, Mar 03, 2021 at 08:38:06PM +0900, Masahiro Yamada wrote: > On Wed, Mar 3, 2021 at 1:02 PM Nathan Chancellor <nathan@kernel.org> wrote: > > > > Hi Bernhard, > > > > I have added the ClangBuiltLinux mailing list, kbuild mailing list, and > > Masahiro and Nick to CC. Maybe ld-version.sh and cc-version.sh should be > > added to a MAINTAINERS entry to make sure we get CC'd (I can send one > > along tomorrow). > > > > On Tue, Mar 02, 2021 at 11:12:11PM +0100, Bernhard Rosenkränzer wrote: > > > If LLD was built with -DLLD_VENDOR="xyz", ld.lld --version output > > > will prefix LLD_VENDOR. Since LLD_VENDOR can contain spaces, the > > > LLD identifier isn't guaranteed to be $2 either. > > > > TIL about LLD_VENDOR... > > > > > Adjust the version checker to handle such versions of lld. > > > > > > Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch> > > > --- > > > scripts/ld-version.sh | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > > > index a463273509b5..4c042a306e22 100755 > > > --- a/scripts/ld-version.sh > > > +++ b/scripts/ld-version.sh > > > @@ -49,6 +49,18 @@ elif [ "$1" = LLD ]; then > > > min_version=$lld_min_version > > > name=LLD > > > disp_name=LLD > > > +elif echo "$@" |grep -q ' LLD '; then > > > + # if LLD was built with -DLLD_VENDOR="xyz", it ld.lld --version > > > + # says "xyz LLD [...]". Since LLD_VENDOR may contain spaces, we > > > + # don't know the exact position of "LLD" and the version info > > > + # at this point > > > + while [ "$1" != "LLD" ]; do > > > + shift > > > + done > > > + version=$2 > > > + min_version=$lld_min_version > > > + name=LLD > > > + disp_name=LLD > > > else > > > echo "$orig_args: unknown linker" >&2 > > > exit 1 > > > -- > > > 2.30.1 > > > > > > > I am not sure what a better fix would be of the top of my head but > > wouldn't it be better to avoid the duplication? This diff below works > > for me with or without LLD_VENDOR defined. > > > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > > index a463273509b5..84f9fc741f09 100755 > > --- a/scripts/ld-version.sh > > +++ b/scripts/ld-version.sh > > @@ -44,7 +44,10 @@ if [ "$1" = GNU -a "$2" = ld ]; then > > elif [ "$1" = GNU -a "$2" = gold ]; then > > echo "gold linker is not supported as it is not capable of linking the kernel proper." >&2 > > exit 1 > > -elif [ "$1" = LLD ]; then > > +elif echo "$*" | grep -q LLD; then > > + while [ "$1" != "LLD" ]; do > > + shift > > + done > > version=$2 > > min_version=$lld_min_version > > name=LLD > > > > You do not need to use grep. > How about this? > > > > > ... > else > while [ $# -gt 1 -a "$1" != "LLD" ]; do > shift > done > > if [ "$1" = LLD ]; then > version=$2 > min_version=$lld_min_version > name=LLD > disp_name=LLD > else > echo "$orig_args: unknown linker" >&2 > exit 1 > fi > fi > > > > > -- > Best Regards > Masahiro Yamada Yes, that as the following diff works for me. diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh index a463273509b5..30debf78aa09 100755 --- a/scripts/ld-version.sh +++ b/scripts/ld-version.sh @@ -44,14 +44,20 @@ if [ "$1" = GNU -a "$2" = ld ]; then elif [ "$1" = GNU -a "$2" = gold ]; then echo "gold linker is not supported as it is not capable of linking the kernel proper." >&2 exit 1 -elif [ "$1" = LLD ]; then - version=$2 - min_version=$lld_min_version - name=LLD - disp_name=LLD else - echo "$orig_args: unknown linker" >&2 - exit 1 + while [ $# -gt 1 -a "$1" != "LLD" ]; do + shift + done + + if [ "$1" = LLD ]; then + version=$2 + min_version=$lld_min_version + name=LLD + disp_name=LLD + else + echo "$orig_args: unknown linker" >&2 + exit 1 + fi fi # Some distributions append a package release number, as in 2.34-4.fc32 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR 2021-03-02 22:12 [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR Bernhard Rosenkränzer 2021-03-03 4:02 ` Nathan Chancellor @ 2021-03-04 3:34 ` Masahiro Yamada 2021-03-09 17:09 ` Masahiro Yamada 1 sibling, 1 reply; 7+ messages in thread From: Masahiro Yamada @ 2021-03-04 3:34 UTC (permalink / raw) To: Bernhard Rosenkränzer Cc: Linux Kernel Mailing List, Nathan Chancellor, clang-built-linux, Linux Kbuild mailing list On Thu, Mar 4, 2021 at 9:18 AM Bernhard Rosenkränzer <bero@lindev.ch> wrote: > > If LLD was built with -DLLD_VENDOR="xyz", ld.lld --version output > will prefix LLD_VENDOR. Since LLD_VENDOR can contain spaces, the > LLD identifier isn't guaranteed to be $2 either. > > Adjust the version checker to handle such versions of lld. > > Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch> > --- Bernhard, Could you senv v2 with the suggested code change? Please make sure to add linux-kbuild@vger.kernel.org in the To: > scripts/ld-version.sh | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > index a463273509b5..4c042a306e22 100755 > --- a/scripts/ld-version.sh > +++ b/scripts/ld-version.sh > @@ -49,6 +49,18 @@ elif [ "$1" = LLD ]; then > min_version=$lld_min_version > name=LLD > disp_name=LLD > +elif echo "$@" |grep -q ' LLD '; then > + # if LLD was built with -DLLD_VENDOR="xyz", it ld.lld --version > + # says "xyz LLD [...]". Since LLD_VENDOR may contain spaces, we > + # don't know the exact position of "LLD" and the version info > + # at this point > + while [ "$1" != "LLD" ]; do > + shift > + done > + version=$2 > + min_version=$lld_min_version > + name=LLD > + disp_name=LLD > else > echo "$orig_args: unknown linker" >&2 > exit 1 > -- > 2.30.1 > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR 2021-03-04 3:34 ` Masahiro Yamada @ 2021-03-09 17:09 ` Masahiro Yamada 2021-03-09 17:31 ` Nathan Chancellor 0 siblings, 1 reply; 7+ messages in thread From: Masahiro Yamada @ 2021-03-09 17:09 UTC (permalink / raw) To: Bernhard Rosenkränzer Cc: Linux Kernel Mailing List, Nathan Chancellor, clang-built-linux, Linux Kbuild mailing list On Thu, Mar 4, 2021 at 12:34 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Thu, Mar 4, 2021 at 9:18 AM Bernhard Rosenkränzer <bero@lindev.ch> wrote: > > > > If LLD was built with -DLLD_VENDOR="xyz", ld.lld --version output > > will prefix LLD_VENDOR. Since LLD_VENDOR can contain spaces, the > > LLD identifier isn't guaranteed to be $2 either. > > > > Adjust the version checker to handle such versions of lld. > > > > Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch> > > --- > > > Bernhard, > > Could you senv v2 > with the suggested code change? > > Please make sure to add > linux-kbuild@vger.kernel.org > in the To: I did not get v2, but never mind. I locally modified the code and applied. I added Link: to your original patch just in case I make some mistake in the code refactoring. The final one looks as follows: commit 0b2813ba7b0f0a9ff273177e85cbc93d92e76212 Author: Bernhard Rosenkränzer <bero@lindev.ch> Date: Tue Mar 2 23:12:11 2021 +0100 kbuild: Fix ld-version.sh script if LLD was built with LLD_VENDOR If LLD was built with -DLLD_VENDOR="xyz", ld.lld --version output will prefix LLD_VENDOR. Since LLD_VENDOR can contain spaces, the LLD identifier isn't guaranteed to be $2 either. Adjust the version checker to handle such versions of lld. Link: https://lore.kernel.org/lkml/20210302221211.1620858-1-bero@lindev.ch/ Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch> [masahiro yamada: refactor the code] Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh index a463273509b5..30debf78aa09 100755 --- a/scripts/ld-version.sh +++ b/scripts/ld-version.sh @@ -44,14 +44,20 @@ if [ "$1" = GNU -a "$2" = ld ]; then elif [ "$1" = GNU -a "$2" = gold ]; then echo "gold linker is not supported as it is not capable of linking the kernel proper." >&2 exit 1 -elif [ "$1" = LLD ]; then - version=$2 - min_version=$lld_min_version - name=LLD - disp_name=LLD else - echo "$orig_args: unknown linker" >&2 - exit 1 + while [ $# -gt 1 -a "$1" != "LLD" ]; do + shift + done + + if [ "$1" = LLD ]; then + version=$2 + min_version=$lld_min_version + name=LLD + disp_name=LLD + else + echo "$orig_args: unknown linker" >&2 + exit 1 + fi fi # Some distributions append a package release number, as in 2.34-4.fc32 -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR 2021-03-09 17:09 ` Masahiro Yamada @ 2021-03-09 17:31 ` Nathan Chancellor 0 siblings, 0 replies; 7+ messages in thread From: Nathan Chancellor @ 2021-03-09 17:31 UTC (permalink / raw) To: Masahiro Yamada Cc: Bernhard Rosenkränzer, Linux Kernel Mailing List, Nathan Chancellor, clang-built-linux, Linux Kbuild mailing list On Wed, Mar 10, 2021 at 02:09:02AM +0900, Masahiro Yamada wrote: > On Thu, Mar 4, 2021 at 12:34 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Thu, Mar 4, 2021 at 9:18 AM Bernhard Rosenkränzer <bero@lindev.ch> wrote: > > > > > > If LLD was built with -DLLD_VENDOR="xyz", ld.lld --version output > > > will prefix LLD_VENDOR. Since LLD_VENDOR can contain spaces, the > > > LLD identifier isn't guaranteed to be $2 either. > > > > > > Adjust the version checker to handle such versions of lld. > > > > > > Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch> > > > --- > > > > > > Bernhard, > > > > Could you senv v2 > > with the suggested code change? > > > > Please make sure to add > > linux-kbuild@vger.kernel.org > > in the To: > > > I did not get v2, but never mind. > I locally modified the code and applied. > > I added Link: to your original patch > just in case I make some mistake in the > code refactoring. > > > The final one looks as follows: > > > > > > commit 0b2813ba7b0f0a9ff273177e85cbc93d92e76212 > Author: Bernhard Rosenkränzer <bero@lindev.ch> > Date: Tue Mar 2 23:12:11 2021 +0100 > > kbuild: Fix ld-version.sh script if LLD was built with LLD_VENDOR > > If LLD was built with -DLLD_VENDOR="xyz", ld.lld --version output > will prefix LLD_VENDOR. Since LLD_VENDOR can contain spaces, the > LLD identifier isn't guaranteed to be $2 either. > > Adjust the version checker to handle such versions of lld. > > Link: https://lore.kernel.org/lkml/20210302221211.1620858-1-bero@lindev.ch/ > Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch> > [masahiro yamada: refactor the code] If it is not too late: Reviewed-by: Nathan Chancellor <nathan@kernel.org> Tested-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > index a463273509b5..30debf78aa09 100755 > --- a/scripts/ld-version.sh > +++ b/scripts/ld-version.sh > @@ -44,14 +44,20 @@ if [ "$1" = GNU -a "$2" = ld ]; then > elif [ "$1" = GNU -a "$2" = gold ]; then > echo "gold linker is not supported as it is not capable of > linking the kernel proper." >&2 > exit 1 > -elif [ "$1" = LLD ]; then > - version=$2 > - min_version=$lld_min_version > - name=LLD > - disp_name=LLD > else > - echo "$orig_args: unknown linker" >&2 > - exit 1 > + while [ $# -gt 1 -a "$1" != "LLD" ]; do > + shift > + done > + > + if [ "$1" = LLD ]; then > + version=$2 > + min_version=$lld_min_version > + name=LLD > + disp_name=LLD > + else > + echo "$orig_args: unknown linker" >&2 > + exit 1 > + fi > fi > > # Some distributions append a package release number, as in 2.34-4.fc32 > > > > > -- > Best Regards > Masahiro Yamada ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-03-09 17:32 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-02 22:12 [PATCH] Fix ld-version.sh script if LLD was built with LLD_VENDOR Bernhard Rosenkränzer 2021-03-03 4:02 ` Nathan Chancellor 2021-03-03 11:38 ` Masahiro Yamada 2021-03-03 17:45 ` Nathan Chancellor 2021-03-04 3:34 ` Masahiro Yamada 2021-03-09 17:09 ` Masahiro Yamada 2021-03-09 17:31 ` Nathan Chancellor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®