Re: [PATCH 19/23] kbuild: support building external modules in a separate build directory

[Nicolas Schier]

On Thu, Oct 03, 2024 at 09:47:35PM +0200, Nicolas Schier wrote:
> On Tue, Sep 17, 2024 at 11:16:47PM +0900, Masahiro Yamada wrote:
> > There has been a long-standing request to support building external
> > modules in a separate build directory.
> > 
> > This commit introduces a new environment variable, KBUILD_EXTMOD_OUTPUT,
> > and its shorthand Make variable, MO.
> > 
> > A simple usage:
> > 
> >  $ make -C <kernel-dir> M=<module-src-dir> MO=<module-build-dir>
> > 
> > Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > ---
> > 
> >  Documentation/kbuild/kbuild.rst  |  8 +++++-
> >  Documentation/kbuild/modules.rst |  5 +++-
> >  Makefile                         | 44 +++++++++++++++++++++++---------
> >  3 files changed, 43 insertions(+), 14 deletions(-)
> > 
> > diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
> > index 716f6fb70829..66a9dc44ea28 100644
> > --- a/Documentation/kbuild/kbuild.rst
> > +++ b/Documentation/kbuild/kbuild.rst
> > @@ -132,12 +132,18 @@ Specify the output directory when building the kernel.
> >  This variable can also be used to point to the kernel output directory when
> >  building external modules using kernel build artifacts in a separate build
> >  directory. Please note that this does NOT specify the output directory for the
> > -external modules themselves.
> > +external modules themselves. (Use KBUILD_EXTMOD_OUTPUT for that purpose.)
> >  
> >  The output directory can also be specified using "O=...".
> >  
> >  Setting "O=..." takes precedence over KBUILD_OUTPUT.
> >  
> > +KBUILD_EXTMOD_OUTPUT
> > +--------------------
> > +Specify the output directory for external modules.
> > +
> > +Setting "MO=..." takes precedence over KBUILD_EXTMOD_OUTPUT.
> > +
> >  KBUILD_EXTRA_WARN
> >  -----------------
> >  Specify the extra build checks. The same value can be assigned by passing
> > diff --git a/Documentation/kbuild/modules.rst b/Documentation/kbuild/modules.rst
> > index 3a6e7bdc0889..03347e13eeb5 100644
> > --- a/Documentation/kbuild/modules.rst
> > +++ b/Documentation/kbuild/modules.rst
> > @@ -95,7 +95,7 @@ executed to make module versioning work.
> >  	of the kernel output directory if the kernel was built in a separate
> >  	build directory.)
> >  
> > -	make -C $KDIR M=$PWD
> > +	make -C $KDIR M=$PWD [MO=$BUILD_DIR]
> >  
> >  	-C $KDIR
> >  		The directory that contains the kernel and relevant build
> > @@ -109,6 +109,9 @@ executed to make module versioning work.
> >  		directory where the external module (kbuild file) is
> >  		located.
> >  
> > +	MO=$BUILD_DIR
> > +		Speficies a separate output directory for the external module.
> 
> s/Speficies/Specifies/
> 
> > +
> >  2.3 Targets
> >  ===========
> >  
> > diff --git a/Makefile b/Makefile
> > index 9fbf7ef6e394..b654baa0763a 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -134,6 +134,10 @@ ifeq ("$(origin M)", "command line")
> >    KBUILD_EXTMOD := $(M)
> >  endif
> >  
> > +ifeq ("$(origin MO)", "command line")
> > +  KBUILD_EXTMOD_OUTPUT := $(MO)
> > +endif
> > +
> >  $(if $(word 2, $(KBUILD_EXTMOD)), \
> >  	$(error building multiple external modules is not supported))
> 
> Should we also check against multiple output directories?
> 
> >  
> > @@ -187,7 +191,11 @@ ifdef KBUILD_EXTMOD
> >      else
> >          objtree := $(CURDIR)
> >      endif
> > -    output := $(KBUILD_EXTMOD)
> > +    output := $(or $(KBUILD_EXTMOD_OUTPUT),$(KBUILD_EXTMOD))
> > +    # KBUILD_EXTMOD might be a relative path. Remember its absolute path before
> > +    # Make changes the working directory.
> > +    export abs_extmodtree := $(realpath $(KBUILD_EXTMOD))
> > +    $(if $(abs_extmodtree),,$(error specified external module directory "$(KBUILD_EXTMOD)" does not exist))
> >  else
> >      objtree := .
> >      output := $(KBUILD_OUTPUT)
> > @@ -246,7 +254,6 @@ else # need-sub-make
> >  ifeq ($(abs_srctree),$(CURDIR))
> >          # building in the source tree
> >          srctree := .
> > -	building_out_of_srctree :=
> >  else
> >          ifeq ($(abs_srctree)/,$(dir $(CURDIR)))
> >                  # building in a subdirectory of the source tree
> > @@ -254,22 +261,23 @@ else
> >          else
> >                  srctree := $(abs_srctree)
> >          endif
> > -	building_out_of_srctree := 1
> >  endif
> >  
> >  ifneq ($(KBUILD_ABS_SRCTREE),)
> >  srctree := $(abs_srctree)
> >  endif
> >  
> > -VPATH		:=
> > +export srctree
> >  
> > -ifeq ($(KBUILD_EXTMOD),)
> > -ifdef building_out_of_srctree
> > -VPATH		:= $(srctree)
> > -endif
> > -endif
> > +_vpath = $(or $(abs_extmodtree),$(srctree))
> >  
> > -export building_out_of_srctree srctree VPATH
> > +ifeq ($(realpath $(_vpath)),$(CURDIR))
> 
> Just a style consistency question: 'ifeq (,)' with a space after ',' (as a few
> lines above) or without as used here?
> 
> > +building_out_of_srctree :=
> > +VPATH :=
> > +else
> > +export building_out_of_srctree := 1
> > +export VPATH := $(_vpath)
> > +endif
> >  
> >  # To make sure we do not include .config for any of the *config targets
> >  # catch them early, and hand them over to scripts/kconfig/Makefile
> > @@ -550,7 +558,7 @@ USERINCLUDE    := \
> >  LINUXINCLUDE    := \
> >  		-I$(srctree)/arch/$(SRCARCH)/include \
> >  		-I$(objtree)/arch/$(SRCARCH)/include/generated \
> > -		$(if $(building_out_of_srctree),-I$(srctree)/include) \
> > +		-I$(srctree)/include \
> >  		-I$(objtree)/include \
> >  		$(USERINCLUDE)
> >  
> > @@ -640,6 +648,7 @@ quiet_cmd_makefile = GEN     Makefile
> >  	} > Makefile
> >  
> >  outputmakefile:
> > +ifeq ($(KBUILD_EXTMOD),)
> >  	@if [ -f $(srctree)/.config -o \
> >  		 -d $(srctree)/include/config -o \
> >  		 -d $(srctree)/arch/$(SRCARCH)/include/generated ]; then \
> > @@ -649,7 +658,16 @@ outputmakefile:
> >  		echo >&2 "***"; \
> >  		false; \
> >  	fi
> > -	$(Q)ln -fsn $(srctree) source
> > +else
> > +	@if [ -f $(KBUILD_EXTMOD)/modules.order ]; then \

If I do

    make O=build
    touch fs/btrfs/modules.order
    make O=build M=fs/btrfs/ MO=/tmp/btrfs CONFIG_BTRFS_FS=m

the 'modules.order' check does not work correctly.  But a consequtive

    make -C /tmp/btrfs CONFIG_BTRFS_FS=m

shows the 'The external module source tree is not clean.' message as
expected.

If I replace $(KBUILD_EXTMOD) by $(abs_extmodtree), it works for me also
in the first case.


Kind regards,
Nicolas