| Message ID | 20190904160021.72d104f1@canb.auug.org.au |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | linux-next: build failure after merge of the net-next tree | expand |
On Wed, Sep 4, 2019 at 3:00 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > Hi all, > > After merging the net-next tree, today's linux-next build (arm > multi_v7_defconfig) failed like this: > > scripts/link-vmlinux.sh: 74: Bad substitution > > Caused by commit > > 341dfcf8d78e ("btf: expose BTF info through sysfs") > > interacting with commit > > 1267f9d3047d ("kbuild: add $(BASH) to run scripts with bash-extension") > > from the kbuild tree. I knew that they were using bash-extension in the #!/bin/sh script. :-D In fact, I wrote my patch in order to break their code and make btf people realize that they were doing wrong. > The change in the net-next tree turned link-vmlinux.sh into a bash script > (I think). > > I have applied the following patch for today: But, this is a temporary fix only for linux-next. scripts/link-vmlinux.sh does not need to use the bash-extension ${@:2} in the first place. I hope btf people will write the correct code. Thanks. > From: Stephen Rothwell <sfr@canb.auug.org.au> > Date: Wed, 4 Sep 2019 15:43:41 +1000 > Subject: [PATCH] link-vmlinux.sh is now a bash script > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > --- > Makefile | 4 ++-- > scripts/link-vmlinux.sh | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index ac97fb282d99..523d12c5cebe 100644 > --- a/Makefile > +++ b/Makefile > @@ -1087,7 +1087,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > # Final link of vmlinux with optional arch pass after final link > cmd_link-vmlinux = \ > - $(CONFIG_SHELL) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > + $(BASH) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) > > vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE > @@ -1403,7 +1403,7 @@ clean: rm-files := $(CLEAN_FILES) > PHONY += archclean vmlinuxclean > > vmlinuxclean: > - $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean > + $(Q)$(BASH) $(srctree)/scripts/link-vmlinux.sh clean > $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean) > > clean: archclean vmlinuxclean > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index f7edb75f9806..ea1f8673869d 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -1,4 +1,4 @@ > -#!/bin/sh > +#!/bin/bash > # SPDX-License-Identifier: GPL-2.0 > # > # link vmlinux > -- > 2.23.0.rc1 > > -- > Cheers, > Stephen Rothwell -- Best Regards Masahiro Yamada
On 9/3/19 11:00 PM, Stephen Rothwell wrote: > Hi all, > > After merging the net-next tree, today's linux-next build (arm > multi_v7_defconfig) failed like this: > > scripts/link-vmlinux.sh: 74: Bad substitution > > Caused by commit > > 341dfcf8d78e ("btf: expose BTF info through sysfs") > > interacting with commit > > 1267f9d3047d ("kbuild: add $(BASH) to run scripts with bash-extension") > > from the kbuild tree. > > The change in the net-next tree turned link-vmlinux.sh into a bash script > (I think). Hi Stephen, Sorry about this breakage. Indeed, ${@:2} is BASH-specific extension, unfortunately. I'm verifying a simple fix with shift and $@, I'll post and CC you as soon as I've verified everything. With that your temporary fix shouldn't be necessary. > > I have applied the following patch for today: > > From: Stephen Rothwell <sfr@canb.auug.org.au> > Date: Wed, 4 Sep 2019 15:43:41 +1000 > Subject: [PATCH] link-vmlinux.sh is now a bash script > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > --- > Makefile | 4 ++-- > scripts/link-vmlinux.sh | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index ac97fb282d99..523d12c5cebe 100644 > --- a/Makefile > +++ b/Makefile > @@ -1087,7 +1087,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > # Final link of vmlinux with optional arch pass after final link > cmd_link-vmlinux = \ > - $(CONFIG_SHELL) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > + $(BASH) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) > > vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE > @@ -1403,7 +1403,7 @@ clean: rm-files := $(CLEAN_FILES) > PHONY += archclean vmlinuxclean > > vmlinuxclean: > - $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean > + $(Q)$(BASH) $(srctree)/scripts/link-vmlinux.sh clean > $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean) > > clean: archclean vmlinuxclean > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index f7edb75f9806..ea1f8673869d 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -1,4 +1,4 @@ > -#!/bin/sh > +#!/bin/bash > # SPDX-License-Identifier: GPL-2.0 > # > # link vmlinux >
On Tue, Sep 3, 2019 at 11:20 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Wed, Sep 4, 2019 at 3:00 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > > > Hi all, > > > > After merging the net-next tree, today's linux-next build (arm > > multi_v7_defconfig) failed like this: > > > > scripts/link-vmlinux.sh: 74: Bad substitution > > > > Caused by commit > > > > 341dfcf8d78e ("btf: expose BTF info through sysfs") > > > > interacting with commit > > > > 1267f9d3047d ("kbuild: add $(BASH) to run scripts with bash-extension") > > > > from the kbuild tree. > > > I knew that they were using bash-extension > in the #!/bin/sh script. :-D > > In fact, I wrote my patch in order to break their code > and make btf people realize that they were doing wrong. Was there a specific reason to wait until this would break during Stephen's merge, instead of giving me a heads up (or just replying on original patch) and letting me fix it and save everyone's time and efforts? Either way, I've fixed the issue in https://patchwork.ozlabs.org/patch/1158620/ and will pay way more attention to BASH-specific features going forward (I found it pretty hard to verify stuff like this, unfortunately). But again, code review process is the best place to catch this and I really hope in the future we can keep this process productive. Thanks! > > > > > The change in the net-next tree turned link-vmlinux.sh into a bash script > > (I think). > > > > I have applied the following patch for today: > > > But, this is a temporary fix only for linux-next. > > scripts/link-vmlinux.sh does not need to use the > bash-extension ${@:2} in the first place. > > I hope btf people will write the correct code. I replaced ${@:2} with shift and ${@}, I hope that's a correct fix, but if you think it's not, please reply on the patch and let me know. > > Thanks. > > > > > > From: Stephen Rothwell <sfr@canb.auug.org.au> > > Date: Wed, 4 Sep 2019 15:43:41 +1000 > > Subject: [PATCH] link-vmlinux.sh is now a bash script > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > --- > > Makefile | 4 ++-- > > scripts/link-vmlinux.sh | 2 +- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index ac97fb282d99..523d12c5cebe 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1087,7 +1087,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > > > # Final link of vmlinux with optional arch pass after final link > > cmd_link-vmlinux = \ > > - $(CONFIG_SHELL) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > > + $(BASH) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) > > > > vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE > > @@ -1403,7 +1403,7 @@ clean: rm-files := $(CLEAN_FILES) > > PHONY += archclean vmlinuxclean > > > > vmlinuxclean: > > - $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean > > + $(Q)$(BASH) $(srctree)/scripts/link-vmlinux.sh clean > > $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean) > > > > clean: archclean vmlinuxclean > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > index f7edb75f9806..ea1f8673869d 100755 > > --- a/scripts/link-vmlinux.sh > > +++ b/scripts/link-vmlinux.sh > > @@ -1,4 +1,4 @@ > > -#!/bin/sh > > +#!/bin/bash > > # SPDX-License-Identifier: GPL-2.0 > > # > > # link vmlinux > > -- > > 2.23.0.rc1 > > > > -- > > Cheers, > > Stephen Rothwell > > > > -- > Best Regards > Masahiro Yamada
On Fri, Sep 6, 2019 at 4:26 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Sep 3, 2019 at 11:20 PM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > On Wed, Sep 4, 2019 at 3:00 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > > > > > Hi all, > > > > > > After merging the net-next tree, today's linux-next build (arm > > > multi_v7_defconfig) failed like this: > > > > > > scripts/link-vmlinux.sh: 74: Bad substitution > > > > > > Caused by commit > > > > > > 341dfcf8d78e ("btf: expose BTF info through sysfs") > > > > > > interacting with commit > > > > > > 1267f9d3047d ("kbuild: add $(BASH) to run scripts with bash-extension") > > > > > > from the kbuild tree. > > > > > > I knew that they were using bash-extension > > in the #!/bin/sh script. :-D > > > > In fact, I wrote my patch in order to break their code > > and make btf people realize that they were doing wrong. > > Was there a specific reason to wait until this would break during > Stephen's merge, instead of giving me a heads up (or just replying on > original patch) and letting me fix it and save everyone's time and > efforts? > > Either way, I've fixed the issue in > https://patchwork.ozlabs.org/patch/1158620/ and will pay way more > attention to BASH-specific features going forward (I found it pretty > hard to verify stuff like this, unfortunately). But again, code review > process is the best place to catch this and I really hope in the > future we can keep this process productive. Thanks! I could have pointed it out if I had noticed it in the review process. I actually noticed your patch by Stephen's former email. (i.e. when it appeared in linux-next) (I try my best to check kbuild ML, and also search for my name in LKML in case I am explicitly addressed, but a large number of emails fall off my filter) It was somewhat too late when I noticed it. Of course, I still could email you afterward, or even send a patch to btf ML, but I did not fix a particular instance of breakage because there are already the same type of breakages in code base. Then, I applied the all-or-nothing checker because I thought it was the only way to address the root cause of the problems. I admit I could have done the process better. Sorry if I made people uncomfortable and waste time. Thanks. > > > > > > > > > The change in the net-next tree turned link-vmlinux.sh into a bash script > > > (I think). > > > > > > I have applied the following patch for today: > > > > > > But, this is a temporary fix only for linux-next. > > > > scripts/link-vmlinux.sh does not need to use the > > bash-extension ${@:2} in the first place. > > > > I hope btf people will write the correct code. > > I replaced ${@:2} with shift and ${@}, I hope that's a correct fix, > but if you think it's not, please reply on the patch and let me know. > > > > > > Thanks. > > > > > > > > > > > From: Stephen Rothwell <sfr@canb.auug.org.au> > > > Date: Wed, 4 Sep 2019 15:43:41 +1000 > > > Subject: [PATCH] link-vmlinux.sh is now a bash script > > > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > --- > > > Makefile | 4 ++-- > > > scripts/link-vmlinux.sh | 2 +- > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/Makefile b/Makefile > > > index ac97fb282d99..523d12c5cebe 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -1087,7 +1087,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > > > > > # Final link of vmlinux with optional arch pass after final link > > > cmd_link-vmlinux = \ > > > - $(CONFIG_SHELL) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > > > + $(BASH) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > > > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) > > > > > > vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE > > > @@ -1403,7 +1403,7 @@ clean: rm-files := $(CLEAN_FILES) > > > PHONY += archclean vmlinuxclean > > > > > > vmlinuxclean: > > > - $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean > > > + $(Q)$(BASH) $(srctree)/scripts/link-vmlinux.sh clean > > > $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean) > > > > > > clean: archclean vmlinuxclean > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > > index f7edb75f9806..ea1f8673869d 100755 > > > --- a/scripts/link-vmlinux.sh > > > +++ b/scripts/link-vmlinux.sh > > > @@ -1,4 +1,4 @@ > > > -#!/bin/sh > > > +#!/bin/bash > > > # SPDX-License-Identifier: GPL-2.0 > > > # > > > # link vmlinux > > > -- > > > 2.23.0.rc1 > > > > > > -- > > > Cheers, > > > Stephen Rothwell > > > > > > > > -- > > Best Regards > > Masahiro Yamada -- Best Regards Masahiro Yamada
On Thu, Sep 5, 2019 at 7:53 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Fri, Sep 6, 2019 at 4:26 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Tue, Sep 3, 2019 at 11:20 PM Masahiro Yamada > > <yamada.masahiro@socionext.com> wrote: > > > > > > On Wed, Sep 4, 2019 at 3:00 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > > > > > > > Hi all, > > > > > > > > After merging the net-next tree, today's linux-next build (arm > > > > multi_v7_defconfig) failed like this: > > > > > > > > scripts/link-vmlinux.sh: 74: Bad substitution > > > > > > > > Caused by commit > > > > > > > > 341dfcf8d78e ("btf: expose BTF info through sysfs") > > > > > > > > interacting with commit > > > > > > > > 1267f9d3047d ("kbuild: add $(BASH) to run scripts with bash-extension") > > > > > > > > from the kbuild tree. > > > > > > > > > I knew that they were using bash-extension > > > in the #!/bin/sh script. :-D > > > > > > In fact, I wrote my patch in order to break their code > > > and make btf people realize that they were doing wrong. > > > > Was there a specific reason to wait until this would break during > > Stephen's merge, instead of giving me a heads up (or just replying on > > original patch) and letting me fix it and save everyone's time and > > efforts? > > > > Either way, I've fixed the issue in > > https://patchwork.ozlabs.org/patch/1158620/ and will pay way more > > attention to BASH-specific features going forward (I found it pretty > > hard to verify stuff like this, unfortunately). But again, code review > > process is the best place to catch this and I really hope in the > > future we can keep this process productive. Thanks! > > I could have pointed it out if I had noticed > it in the review process. > > I actually noticed your patch by Stephen's > former email. (i.e. when it appeared in linux-next) > > (I try my best to check kbuild ML, and also search for > my name in LKML in case I am explicitly addressed, > but a large number of emails fall off my filter) > > It was somewhat too late when I noticed it. > Of course, I still could email you afterward, or even send a patch to btf ML, > but I did not fix a particular instance of breakage > because there are already the same type of breakages in code base. > > Then, I applied the all-or-nothing checker because I thought it was > the only way to address the root cause of the problems. > > I admit I could have done the process better. > Sorry if I made people uncomfortable and waste time. No worries. Thanks for candid answer. I just wanted to make sure there are no hard feelings and I can engage your expertise effectively in the future for kbuild stuff to ensure issues like this don't slip through, if we ever have to do anything like this for BPF-related things again. I'll keep CC'ing you and will add kbuild ML as well. Thanks! > > Thanks. > > > > > > > > > > > > > > > > > The change in the net-next tree turned link-vmlinux.sh into a bash script > > > > (I think). > > > > > > > > I have applied the following patch for today: > > > > > > > > > But, this is a temporary fix only for linux-next. > > > > > > scripts/link-vmlinux.sh does not need to use the > > > bash-extension ${@:2} in the first place. > > > > > > I hope btf people will write the correct code. > > > > I replaced ${@:2} with shift and ${@}, I hope that's a correct fix, > > but if you think it's not, please reply on the patch and let me know. > > > > > > > > > > Thanks. > > > > > > > > > > > > > > > > From: Stephen Rothwell <sfr@canb.auug.org.au> > > > > Date: Wed, 4 Sep 2019 15:43:41 +1000 > > > > Subject: [PATCH] link-vmlinux.sh is now a bash script > > > > > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > --- > > > > Makefile | 4 ++-- > > > > scripts/link-vmlinux.sh | 2 +- > > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/Makefile b/Makefile > > > > index ac97fb282d99..523d12c5cebe 100644 > > > > --- a/Makefile > > > > +++ b/Makefile > > > > @@ -1087,7 +1087,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > > > > > > > # Final link of vmlinux with optional arch pass after final link > > > > cmd_link-vmlinux = \ > > > > - $(CONFIG_SHELL) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > > > > + $(BASH) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ > > > > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) > > > > > > > > vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE > > > > @@ -1403,7 +1403,7 @@ clean: rm-files := $(CLEAN_FILES) > > > > PHONY += archclean vmlinuxclean > > > > > > > > vmlinuxclean: > > > > - $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean > > > > + $(Q)$(BASH) $(srctree)/scripts/link-vmlinux.sh clean > > > > $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean) > > > > > > > > clean: archclean vmlinuxclean > > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > > > index f7edb75f9806..ea1f8673869d 100755 > > > > --- a/scripts/link-vmlinux.sh > > > > +++ b/scripts/link-vmlinux.sh > > > > @@ -1,4 +1,4 @@ > > > > -#!/bin/sh > > > > +#!/bin/bash > > > > # SPDX-License-Identifier: GPL-2.0 > > > > # > > > > # link vmlinux > > > > -- > > > > 2.23.0.rc1 > > > > > > > > -- > > > > Cheers, > > > > Stephen Rothwell > > > > > > > > > > > > -- > > > Best Regards > > > Masahiro Yamada > > > > -- > Best Regards > Masahiro Yamada
diff --git a/Makefile b/Makefile index ac97fb282d99..523d12c5cebe 100644 --- a/Makefile +++ b/Makefile @@ -1087,7 +1087,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link cmd_link-vmlinux = \ - $(CONFIG_SHELL) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ + $(BASH) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE @@ -1403,7 +1403,7 @@ clean: rm-files := $(CLEAN_FILES) PHONY += archclean vmlinuxclean vmlinuxclean: - $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean + $(Q)$(BASH) $(srctree)/scripts/link-vmlinux.sh clean $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean) clean: archclean vmlinuxclean diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index f7edb75f9806..ea1f8673869d 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -1,4 +1,4 @@ -#!/bin/sh +#!/bin/bash # SPDX-License-Identifier: GPL-2.0 # # link vmlinux
Hi all, After merging the net-next tree, today's linux-next build (arm multi_v7_defconfig) failed like this: scripts/link-vmlinux.sh: 74: Bad substitution Caused by commit 341dfcf8d78e ("btf: expose BTF info through sysfs") interacting with commit 1267f9d3047d ("kbuild: add $(BASH) to run scripts with bash-extension") from the kbuild tree. The change in the net-next tree turned link-vmlinux.sh into a bash script (I think). I have applied the following patch for today: From: Stephen Rothwell <sfr@canb.auug.org.au> Date: Wed, 4 Sep 2019 15:43:41 +1000 Subject: [PATCH] link-vmlinux.sh is now a bash script Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- Makefile | 4 ++-- scripts/link-vmlinux.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)