Re: [PATCH v2 2/2] kbuild: handle excessively long argument lists

[Masahiro Yamada]

On Fri, Jan 15, 2021 at 6:50 AM Jeff Johnson <jjohnson@xxxxxxxxxxxxxx> wrote:
>
> From: Mahesh Kumar Kalikot Veetil <mkalikot@xxxxxxxxxxxxxx>
>
> Modules with a large number of compilation units may be
> exceeding AR and LD command argument list. Handle this gracefully by
> writing the long argument list in a file. The command line options
> read from file are inserted in place of the original @file option.
>
> The usage is well documented at
> https://www.gnu.org/software/make/manual/html_node/File-Function.html
>
> Signed-off-by: Mahesh Kumar Kalikot Veetil <mkalikot@xxxxxxxxxxxxxx>
> Signed-off-by: Jeff Johnson <jjohnson@xxxxxxxxxxxxxx>
> ---




First, is this a real problem?
If so, which module is exceeding the command line limit?


$(file ) is only supported by GNU Make 4.0 or later.

The current minimum version is GNU Make 3.81.

If we need this feature,
Documentation/process/changes.rst must be updated.




But, more importantly, none of your patches
works correctly.



Since $(file ...) is evaluated into an empty string,
your patches would break the Kbuild ability
that detects the command changes.




Steps to reproduce the problem
------------------------------



[1] add a module foo that consists of
    three objects foo1.o, foo2.o, foo3.o

For example, like follows:


obj-m += foo.o
foo-objs := foo1.o foo2.o foo3.o



[2] Run 'make modules'

You will get the module foo.



[3] Drop foo3.o from the module members

Change Makefile as follows:

obj-m += foo.o
foo-objs := foo1.o foo2.o



[4] Re-run 'make modules'





The current build system cleverly
notices the Makefile change, and
correctly rebuilds the foo module.

With your patch set applied,
the build system would not rebuild
the module.







> Changes in v2:
>   - Remove spurious endif
>
> scripts/Makefile.build | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 252b7d2..787dca2 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -425,7 +425,10 @@ $(obj)/lib.a: $(lib-y) FORCE
>  # module is turned into a multi object module, $^ will contain header file
>  # dependencies recorded in the .*.cmd file.
>  quiet_cmd_link_multi-m = LD [M]  $@
> -      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
> +      cmd_link_multi-m =                                       \
> +       $(file >$@.in,$(filter %.o,$^))                         \
> +       $(LD) $(ld_flags) -r -o $@ @$@.in;                      \
> +       rm -f $@.in
>
>  $(multi-used-m): FORCE
>         $(call if_changed,link_multi-m)
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>


-- 
Best Regards
Masahiro Yamada