| Message ID | 20230617153025.1653851-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: make clean rule robust against too long argument error | expand |
Hello Yamada-san, Many thanks for your feedback and for your patch. It indeed addresses the issue I have reported in [1]. On Sun, Jun 18, 2023 at 12:30:25AM +0900, Masahiro Yamada wrote: > Commit cd968b97c492 ("kbuild: make built-in.a rule robust against too > long argument error") made a build rule robust against "Argument list > too long" error. > > Eugeniu Rosca reported the same error occurred when cleaning an external > module. > > The $(obj)/ prefix can be a very long path for external modules. Confirmed. I am seeing an instance of $(obj) being 150 characters long, due to an out-of-tree module deeply buried in a specific Yocto build. In the current vanilla version of 'make clean' (w/o this patch), the $(obj) prefix is applied to each and every file being removed, dramatically increasing the strlen of arguments passed to 'rm -rf'. > > Apply a similar solution to 'make clean'. > > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > scripts/Makefile.clean | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/scripts/Makefile.clean b/scripts/Makefile.clean > index 3649900696dd..235408a44f90 100644 > --- a/scripts/Makefile.clean > +++ b/scripts/Makefile.clean > @@ -37,8 +37,9 @@ __clean-files := $(wildcard $(addprefix $(obj)/, $(__clean-files))) > > # ========================================================================== > > +# Use xargs to make this robust against "Argument list too long" error Please, correct me if I am wrong, but it looks like the magic/brilliance is in the 'patsubst' function, since below version also fails for me: NOK: cmd_clean = printf '%s ' $(__clean-files) | xargs rm -rf When running 'make clean' for my particular out-of-tree kernel module, 'patsubst' is able to decrease the total number of characters passed to the shell's argument list from ~204k to ~47k, which elegantly prevents the "Argument list too long" error. > quiet_cmd_clean = CLEAN $(obj) > - cmd_clean = rm -rf $(__clean-files) > + cmd_clean = printf '$(obj)/%s ' $(patsubst $(obj)/%,%,$(__clean-files)) | xargs rm -rf > > __clean: $(subdir-ymn) > ifneq ($(strip $(__clean-files)),) > -- > 2.39.2 > Since it fixes the problem reported in [1] (much appreciated): Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com> Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com> [1] https://lore.kernel.org/linux-kbuild/20230616194505.GA27753@lxhi-065/
On Sun, Jun 18, 2023 at 5:07 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > > Hello Yamada-san, > > Many thanks for your feedback and for your patch. > It indeed addresses the issue I have reported in [1]. > > On Sun, Jun 18, 2023 at 12:30:25AM +0900, Masahiro Yamada wrote: > > Commit cd968b97c492 ("kbuild: make built-in.a rule robust against too > > long argument error") made a build rule robust against "Argument list > > too long" error. > > > > Eugeniu Rosca reported the same error occurred when cleaning an external > > module. > > > > The $(obj)/ prefix can be a very long path for external modules. > > Confirmed. I am seeing an instance of $(obj) being 150 characters long, > due to an out-of-tree module deeply buried in a specific Yocto build. > > In the current vanilla version of 'make clean' (w/o this patch), the > $(obj) prefix is applied to each and every file being removed, > dramatically increasing the strlen of arguments passed to 'rm -rf'. > > > > > Apply a similar solution to 'make clean'. > > > > Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > scripts/Makefile.clean | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/Makefile.clean b/scripts/Makefile.clean > > index 3649900696dd..235408a44f90 100644 > > --- a/scripts/Makefile.clean > > +++ b/scripts/Makefile.clean > > @@ -37,8 +37,9 @@ __clean-files := $(wildcard $(addprefix $(obj)/, $(__clean-files))) > > > > # ========================================================================== > > > > +# Use xargs to make this robust against "Argument list too long" error > > Please, correct me if I am wrong, but it looks like the magic/brilliance > is in the 'patsubst' function, since below version also fails for me: > > NOK: cmd_clean = printf '%s ' $(__clean-files) | xargs rm -rf Right. Now, 'printf' (instead of 'rm') failed with the too long argument list. GNU Make does not have the length limit, but shell does. So, the full-path list must be passed via stdout instead of the command line. The comment might be confusing. I will repeat the same comment written in scripts/Makefile.build: # To make this rule robust against "Argument list too long" error, # remove $(obj)/ prefix, and restore it by a shell command.
Hello Yamada-san, On Mon, Jun 19, 2023 at 12:50:48AM +0900, Masahiro Yamada wrote: > On Sun, Jun 18, 2023 at 5:07 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: [snip] > > Please, correct me if I am wrong, but it looks like the magic/brilliance > > is in the 'patsubst' function, since below version also fails for me: > > > > NOK: cmd_clean = printf '%s ' $(__clean-files) | xargs rm -rf > > Right. > Now, 'printf' (instead of 'rm') failed with the too long argument list. > > GNU Make does not have the length limit, but shell does. > So, the full-path list must be passed via stdout > instead of the command line. > > > The comment might be confusing. > > I will repeat the same comment written in scripts/Makefile.build: > > # To make this rule robust against "Argument list too long" error, > # remove $(obj)/ prefix, and restore it by a shell command. Sounds good to me. No open questions from my end :)
diff --git a/scripts/Makefile.clean b/scripts/Makefile.clean index 3649900696dd..235408a44f90 100644 --- a/scripts/Makefile.clean +++ b/scripts/Makefile.clean @@ -37,8 +37,9 @@ __clean-files := $(wildcard $(addprefix $(obj)/, $(__clean-files))) # ========================================================================== +# Use xargs to make this robust against "Argument list too long" error quiet_cmd_clean = CLEAN $(obj) - cmd_clean = rm -rf $(__clean-files) + cmd_clean = printf '$(obj)/%s ' $(patsubst $(obj)/%,%,$(__clean-files)) | xargs rm -rf __clean: $(subdir-ymn) ifneq ($(strip $(__clean-files)),)
Commit cd968b97c492 ("kbuild: make built-in.a rule robust against too long argument error") made a build rule robust against "Argument list too long" error. Eugeniu Rosca reported the same error occurred when cleaning an external module. The $(obj)/ prefix can be a very long path for external modules. Apply a similar solution to 'make clean'. Reported-by: Eugeniu Rosca <erosca@de.adit-jv.com> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/Makefile.clean | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)