There has been a long-standing request to support building external
modules in a separate build directory.

The first half is cleanups of documents and Makefiles.

The last part adds KBUILD_EXTMOD_OUTPUT (MO=).
This is too big changes, and too late for the current MW.
(I did not test kselftest at all.)
I hope people test this and may uncover some issues.



Masahiro Yamada (23):
  kbuild: doc: update the description about Kbuild/Makefile split
  kbuild: doc: remove description about grepping CONFIG options
  kbuild: doc: remove outdated description of the limitation on -I usage
  kbuild: doc: remove the description about shipped files
  kbuild: doc: describe the -C option precisely for external module
    builds
  kbuild: doc: replace "gcc" in external module description
  kbuild: remove unnecessary prune of rust/alloc for rustfmt
  kbuild: simplify find command for rustfmt
  speakup: use SPKDIR=$(src) to specify the source directory
  kbuild: refactor the check for missing config files
  kbuild: check the presence of include/generated/rustc_cfg
  scripts/nsdeps: use VPATH as src_prefix
  kbuild: replace two $(abs_objtree) with $(CURDIR) in top Makefile
  kbuild: add $(objtree)/ prefix to some in-kernel build artifacts
  kbuild: rename abs_objtree to abs_output
  kbuild: use 'output' variable to create the output directory
  kbuild: build external modules in their directory
  kbuild: remove extmod_prefix, MODORDER, MODULES_NSDEPS variables
  kbuild: support building external modules in a separate build
    directory
  kbuild: support -fmacro-prefix-map for external modules
  kbuild: use absolute path in the generated wrapper Makefile
  kbuild: make wrapper Makefile more convenient for external modules
  kbuild: allow to start building external module in any directory

 Documentation/dev-tools/coccinelle.rst |  19 +--
 Documentation/kbuild/kbuild.rst        |  11 ++
 Documentation/kbuild/modules.rst       | 104 ++++----------
 Makefile                               | 184 ++++++++++++++++---------
 drivers/accessibility/speakup/Makefile |   4 +-
 rust/Makefile                          |   4 +-
 scripts/Kbuild.include                 |   2 +-
 scripts/Makefile.build                 |   4 +-
 scripts/Makefile.compiler              |   2 +-
 scripts/Makefile.modfinal              |  16 +--
 scripts/Makefile.modinst               |   4 +-
 scripts/Makefile.modpost               |  24 ++--
 scripts/coccicheck                     |   6 +-
 scripts/depmod.sh                      |   4 +-
 scripts/nsdeps                         |   6 +-
 scripts/package/install-extmod-build   |   7 +
 16 files changed, 209 insertions(+), 192 deletions(-)

-- 
2.43.0

On Tue, Sep 17, 2024 at 11:16:28PM +0900, Masahiro Yamada wrote:
> 
> There has been a long-standing request to support building external
> modules in a separate build directory.

Thanks a lot, you are making several of my colleages very happy with
your patch set!

> The first half is cleanups of documents and Makefiles.
> 
> The last part adds KBUILD_EXTMOD_OUTPUT (MO=).
> This is too big changes, and too late for the current MW.
> (I did not test kselftest at all.)
> I hope people test this and may uncover some issues.

I'm not through all the patches in detail yet, just one observation beforehand:

    $ make KBUILD_OUTPUT=build allnoconfig
    $ ./scripts/config --file build/.config --enable modules --enable accessibility
    $ make KBUILD_OUTPUT=build olddefconfig
    $ make KBUILD_OUTPUT=build
    $ make KBUILD_OUTPUT=build CONFIG_SPEAKUP=m MO=/tmp/build M=~+/drivers/accessibility/speakup modules
    /home/nschier/src/kbuild-review/drivers/accessibility/speakup/genmap.c:23:10: fatal error: mapdata.h: No such file or directory
       23 | #include "mapdata.h"
          |          ^~~~~~~~~~~
    compilation terminated.
    make[3]: *** [/home/nschier/src/kbuild-review/scripts/Makefile.host:133: genmap.o] Error 1
    make[3]: *** Waiting for unfinished jobs....
    make[2]: *** [/home/nschier/src/kbuild-review/Makefile:1971: .] Error 2
    make[1]: *** [/home/nschier/src/kbuild-review/Makefile:251: __sub-make] Error 2
    make: *** [Makefile:251: __sub-make] Error 2
    [exit code 2]

If I add "EXTRA_CFLAGS=-I${MO} and EXTRA_HOSTCFLAGS=-I${MO}" to the module
build command, it works as expected.

Patching this into kbuild works for me, too, but I haven't checked whether it
breaks some other scenarios:

diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index e01c13a588dd..056c7da2776f 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -97,10 +97,13 @@ hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
                  $(HOSTRUSTFLAGS_$(target-stem))
 
 # $(objtree)/$(obj) for including generated headers from checkin source files
-ifeq ($(KBUILD_EXTMOD),)
 ifdef building_out_of_srctree
+ifeq ($(KBUILD_EXTMOD),)
 hostc_flags   += -I $(objtree)/$(obj)
 hostcxx_flags += -I $(objtree)/$(obj)
+else
+hostc_flags   += -I $(CURDIR)
+hostcxx_flags   += -I $(CURDIR)
 endif
 endif
 
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 1bdd77f42289..428a9eb74381 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -190,11 +190,15 @@ endif
 
 # $(src) for including checkin headers from generated source files
 # $(obj) for including generated headers from checkin source files
-ifeq ($(KBUILD_EXTMOD),)
 ifdef building_out_of_srctree
+ifeq ($(KBUILD_EXTMOD),)
 _c_flags   += $(addprefix -I, $(src) $(obj))
 _a_flags   += $(addprefix -I, $(src) $(obj))
 _cpp_flags += $(addprefix -I, $(src) $(obj))
+else
+_c_flags   += $(addprefix -I, $(src) $(obj) $(CURDIR))
+_a_flags   += $(addprefix -I, $(src) $(obj) $(CURDIR))
+_cpp_flags += $(addprefix -I, $(src) $(obj) $(CURDIR))
 endif
 endif
 
Is '-I$(MO)' in CFLAGS/HOSTCFLAGS is something we should support by
default, or should this be added to the external module's Makefile by
the respective developers themselves?

Kind regards,
Nicolas

On Fri, Sep 20, 2024 at 7:39 PM Nicolas Schier <n.schier@avm.de> wrote:
>
> On Tue, Sep 17, 2024 at 11:16:28PM +0900, Masahiro Yamada wrote:
> >
> > There has been a long-standing request to support building external
> > modules in a separate build directory.
>
> Thanks a lot, you are making several of my colleages very happy with
> your patch set!
>
> > The first half is cleanups of documents and Makefiles.
> >
> > The last part adds KBUILD_EXTMOD_OUTPUT (MO=).
> > This is too big changes, and too late for the current MW.
> > (I did not test kselftest at all.)
> > I hope people test this and may uncover some issues.
>
> I'm not through all the patches in detail yet, just one observation beforehand:
>
>     $ make KBUILD_OUTPUT=build allnoconfig
>     $ ./scripts/config --file build/.config --enable modules --enable accessibility
>     $ make KBUILD_OUTPUT=build olddefconfig
>     $ make KBUILD_OUTPUT=build
>     $ make KBUILD_OUTPUT=build CONFIG_SPEAKUP=m MO=/tmp/build M=~+/drivers/accessibility/speakup modules
>     /home/nschier/src/kbuild-review/drivers/accessibility/speakup/genmap.c:23:10: fatal error: mapdata.h: No such file or directory
>        23 | #include "mapdata.h"
>           |          ^~~~~~~~~~~
>     compilation terminated.
>     make[3]: *** [/home/nschier/src/kbuild-review/scripts/Makefile.host:133: genmap.o] Error 1
>     make[3]: *** Waiting for unfinished jobs....
>     make[2]: *** [/home/nschier/src/kbuild-review/Makefile:1971: .] Error 2
>     make[1]: *** [/home/nschier/src/kbuild-review/Makefile:251: __sub-make] Error 2
>     make: *** [Makefile:251: __sub-make] Error 2
>     [exit code 2]
>
> If I add "EXTRA_CFLAGS=-I${MO} and EXTRA_HOSTCFLAGS=-I${MO}" to the module
> build command, it works as expected.
>
> Patching this into kbuild works for me, too, but I haven't checked whether it
> breaks some other scenarios:
>
> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> index e01c13a588dd..056c7da2776f 100644
> --- a/scripts/Makefile.host
> +++ b/scripts/Makefile.host
> @@ -97,10 +97,13 @@ hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
>                   $(HOSTRUSTFLAGS_$(target-stem))
>
>  # $(objtree)/$(obj) for including generated headers from checkin source files
> -ifeq ($(KBUILD_EXTMOD),)
>  ifdef building_out_of_srctree
> +ifeq ($(KBUILD_EXTMOD),)
>  hostc_flags   += -I $(objtree)/$(obj)
>  hostcxx_flags += -I $(objtree)/$(obj)
> +else
> +hostc_flags   += -I $(CURDIR)
> +hostcxx_flags   += -I $(CURDIR)
>  endif
>  endif
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 1bdd77f42289..428a9eb74381 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -190,11 +190,15 @@ endif
>
>  # $(src) for including checkin headers from generated source files
>  # $(obj) for including generated headers from checkin source files
> -ifeq ($(KBUILD_EXTMOD),)
>  ifdef building_out_of_srctree
> +ifeq ($(KBUILD_EXTMOD),)
>  _c_flags   += $(addprefix -I, $(src) $(obj))
>  _a_flags   += $(addprefix -I, $(src) $(obj))
>  _cpp_flags += $(addprefix -I, $(src) $(obj))
> +else
> +_c_flags   += $(addprefix -I, $(src) $(obj) $(CURDIR))
> +_a_flags   += $(addprefix -I, $(src) $(obj) $(CURDIR))
> +_cpp_flags += $(addprefix -I, $(src) $(obj) $(CURDIR))
>  endif
>  endif
>
> Is '-I$(MO)' in CFLAGS/HOSTCFLAGS is something we should support by
> default, or should this be added to the external module's Makefile by
> the respective developers themselves?
>
> Kind regards,
> Nicolas



We can fix it more simply.




diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index e01c13a588dd..c1dedf646a39 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -96,12 +96,10 @@ hostrust_flags = --out-dir $(dir $@)
--emit=dep-info=$(depfile) \
                  $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
                  $(HOSTRUSTFLAGS_$(target-stem))

-# $(objtree)/$(obj) for including generated headers from checkin source files
-ifeq ($(KBUILD_EXTMOD),)
+# $(obj) for including generated headers from checkin source files
 ifdef building_out_of_srctree
-hostc_flags   += -I $(objtree)/$(obj)
-hostcxx_flags += -I $(objtree)/$(obj)
-endif
+hostc_flags   += -I $(obj)
+hostcxx_flags += -I $(obj)
 endif

 #####
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 1bdd77f42289..d8ce0f59fd17 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -190,13 +190,11 @@ endif

 # $(src) for including checkin headers from generated source files
 # $(obj) for including generated headers from checkin source files
-ifeq ($(KBUILD_EXTMOD),)
 ifdef building_out_of_srctree
 _c_flags   += $(addprefix -I, $(src) $(obj))
 _a_flags   += $(addprefix -I, $(src) $(obj))
 _cpp_flags += $(addprefix -I, $(src) $(obj))
 endif
-endif

 # If $(is-kernel-object) is 'y', this object will be linked to
vmlinux or modules
 is-kernel-object = $(or $(part-of-builtin),$(part-of-module))





However, I'd rather fix each Makefile
to add necessary include paths explicitly.








-- 
Best Regards
Masahiro Yamada

On Fri, Sep 20, 2024 at 09:58:47PM +0900, Masahiro Yamada wrote:
> On Fri, Sep 20, 2024 at 7:39 PM Nicolas Schier <n.schier@avm.de> wrote:
> >
> > On Tue, Sep 17, 2024 at 11:16:28PM +0900, Masahiro Yamada wrote:
> > >
> > > There has been a long-standing request to support building external
> > > modules in a separate build directory.
> >
> > Thanks a lot, you are making several of my colleages very happy with
> > your patch set!
> >
> > > The first half is cleanups of documents and Makefiles.
> > >
> > > The last part adds KBUILD_EXTMOD_OUTPUT (MO=).
> > > This is too big changes, and too late for the current MW.
> > > (I did not test kselftest at all.)
> > > I hope people test this and may uncover some issues.
> >
> > I'm not through all the patches in detail yet, just one observation beforehand:
> >
> >     $ make KBUILD_OUTPUT=build allnoconfig
> >     $ ./scripts/config --file build/.config --enable modules --enable accessibility
> >     $ make KBUILD_OUTPUT=build olddefconfig
> >     $ make KBUILD_OUTPUT=build
> >     $ make KBUILD_OUTPUT=build CONFIG_SPEAKUP=m MO=/tmp/build M=~+/drivers/accessibility/speakup modules
> >     /home/nschier/src/kbuild-review/drivers/accessibility/speakup/genmap.c:23:10: fatal error: mapdata.h: No such file or directory
> >        23 | #include "mapdata.h"
> >           |          ^~~~~~~~~~~
> >     compilation terminated.
> >     make[3]: *** [/home/nschier/src/kbuild-review/scripts/Makefile.host:133: genmap.o] Error 1
> >     make[3]: *** Waiting for unfinished jobs....
> >     make[2]: *** [/home/nschier/src/kbuild-review/Makefile:1971: .] Error 2
> >     make[1]: *** [/home/nschier/src/kbuild-review/Makefile:251: __sub-make] Error 2
> >     make: *** [Makefile:251: __sub-make] Error 2
> >     [exit code 2]
> >
> > If I add "EXTRA_CFLAGS=-I${MO} and EXTRA_HOSTCFLAGS=-I${MO}" to the module
> > build command, it works as expected.
> >
> > Patching this into kbuild works for me, too, but I haven't checked whether it
> > breaks some other scenarios:
> >
> > diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> > index e01c13a588dd..056c7da2776f 100644
> > --- a/scripts/Makefile.host
> > +++ b/scripts/Makefile.host
> > @@ -97,10 +97,13 @@ hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
> >                   $(HOSTRUSTFLAGS_$(target-stem))
> >
> >  # $(objtree)/$(obj) for including generated headers from checkin source files
> > -ifeq ($(KBUILD_EXTMOD),)
> >  ifdef building_out_of_srctree
> > +ifeq ($(KBUILD_EXTMOD),)
> >  hostc_flags   += -I $(objtree)/$(obj)
> >  hostcxx_flags += -I $(objtree)/$(obj)
> > +else
> > +hostc_flags   += -I $(CURDIR)
> > +hostcxx_flags   += -I $(CURDIR)
> >  endif
> >  endif
> >
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index 1bdd77f42289..428a9eb74381 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -190,11 +190,15 @@ endif
> >
> >  # $(src) for including checkin headers from generated source files
> >  # $(obj) for including generated headers from checkin source files
> > -ifeq ($(KBUILD_EXTMOD),)
> >  ifdef building_out_of_srctree
> > +ifeq ($(KBUILD_EXTMOD),)
> >  _c_flags   += $(addprefix -I, $(src) $(obj))
> >  _a_flags   += $(addprefix -I, $(src) $(obj))
> >  _cpp_flags += $(addprefix -I, $(src) $(obj))
> > +else
> > +_c_flags   += $(addprefix -I, $(src) $(obj) $(CURDIR))
> > +_a_flags   += $(addprefix -I, $(src) $(obj) $(CURDIR))
> > +_cpp_flags += $(addprefix -I, $(src) $(obj) $(CURDIR))
> >  endif
> >  endif
> >
> > Is '-I$(MO)' in CFLAGS/HOSTCFLAGS is something we should support by
> > default, or should this be added to the external module's Makefile by
> > the respective developers themselves?
> >
> > Kind regards,
> > Nicolas
> 
> 
> 
> We can fix it more simply.

Ah, yes.

> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> index e01c13a588dd..c1dedf646a39 100644
> --- a/scripts/Makefile.host
> +++ b/scripts/Makefile.host
> @@ -96,12 +96,10 @@ hostrust_flags = --out-dir $(dir $@)
> --emit=dep-info=$(depfile) \
>                   $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
>                   $(HOSTRUSTFLAGS_$(target-stem))
> 
> -# $(objtree)/$(obj) for including generated headers from checkin source files
> -ifeq ($(KBUILD_EXTMOD),)
> +# $(obj) for including generated headers from checkin source files
>  ifdef building_out_of_srctree
> -hostc_flags   += -I $(objtree)/$(obj)
> -hostcxx_flags += -I $(objtree)/$(obj)
> -endif
> +hostc_flags   += -I $(obj)
> +hostcxx_flags += -I $(obj)
>  endif
> 
>  #####
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 1bdd77f42289..d8ce0f59fd17 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -190,13 +190,11 @@ endif
> 
>  # $(src) for including checkin headers from generated source files
>  # $(obj) for including generated headers from checkin source files
> -ifeq ($(KBUILD_EXTMOD),)
>  ifdef building_out_of_srctree
>  _c_flags   += $(addprefix -I, $(src) $(obj))
>  _a_flags   += $(addprefix -I, $(src) $(obj))
>  _cpp_flags += $(addprefix -I, $(src) $(obj))
>  endif
> -endif
> 
>  # If $(is-kernel-object) is 'y', this object will be linked to
> vmlinux or modules
>  is-kernel-object = $(or $(part-of-builtin),$(part-of-module))
> 
> 
> 
> 
> 
> However, I'd rather fix each Makefile
> to add necessary include paths explicitly.

Sure, clearly makes sense.  Especially as the explicit include path
addition is also as simple as:

    HOSTCFLAGS_genmap.o += -I $(obj)
    CFLAGS_main.o += -I $(obj)

Kind regards,
Nicolas

On Tue, Sep 17, 2024 at 11:16:28PM +0900, Masahiro Yamada wrote:
> 
> There has been a long-standing request to support building external
> modules in a separate build directory.
> 
> The first half is cleanups of documents and Makefiles.
> 
> The last part adds KBUILD_EXTMOD_OUTPUT (MO=).
> This is too big changes, and too late for the current MW.
> (I did not test kselftest at all.)
> I hope people test this and may uncover some issues.

thanks again for the whole series.  I really appreciated to go through
this patch set and am sorry for that it took so long.  I have tested
only with some in-tree kmods and with kmods' testsuite modules (but only
on amd64) and could not find any major issue, only the minor things
reported.  I also did not test the kselftest or similar.

Kind regards,
Nicolas

On Tue, Sep 17, 2024 at 11:17 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
>
> There has been a long-standing request to support building external
> modules in a separate build directory.
>
> The first half is cleanups of documents and Makefiles.
>
> The last part adds KBUILD_EXTMOD_OUTPUT (MO=).
> This is too big changes, and too late for the current MW.
> (I did not test kselftest at all.)
> I hope people test this and may uncover some issues.
>
>
>
> Masahiro Yamada (23):
>   kbuild: doc: update the description about Kbuild/Makefile split
>   kbuild: doc: remove description about grepping CONFIG options
>   kbuild: doc: remove outdated description of the limitation on -I usage
>   kbuild: doc: remove the description about shipped files
>   kbuild: doc: describe the -C option precisely for external module
>     builds
>   kbuild: doc: replace "gcc" in external module description
>   kbuild: remove unnecessary prune of rust/alloc for rustfmt
>   kbuild: simplify find command for rustfmt
>   speakup: use SPKDIR=$(src) to specify the source directory
>   kbuild: refactor the check for missing config files
>   kbuild: check the presence of include/generated/rustc_cfg
>   scripts/nsdeps: use VPATH as src_prefix
>   kbuild: replace two $(abs_objtree) with $(CURDIR) in top Makefile
>   kbuild: add $(objtree)/ prefix to some in-kernel build artifacts
>   kbuild: rename abs_objtree to abs_output
>   kbuild: use 'output' variable to create the output directory
>   kbuild: build external modules in their directory
>   kbuild: remove extmod_prefix, MODORDER, MODULES_NSDEPS variables
>   kbuild: support building external modules in a separate build
>     directory
>   kbuild: support -fmacro-prefix-map for external modules
>   kbuild: use absolute path in the generated wrapper Makefile
>   kbuild: make wrapper Makefile more convenient for external modules
>   kbuild: allow to start building external module in any directory



I CC'ed rust ML because Ack for the following patches are appreciated.


[07/23] kbuild: remove unnecessary prune of rust/alloc for rustfmt
[08/23] kbuild: simplify find command for rustfmt
[11/23] kbuild: check the presence of include/generated/rustc_cfg







-- 
Best Regards
Masahiro Yamada

On Sat, Sep 28, 2024 at 8:50 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> I CC'ed rust ML because Ack for the following patches are appreciated.
>
> [07/23] kbuild: remove unnecessary prune of rust/alloc for rustfmt
> [08/23] kbuild: simplify find command for rustfmt
> [11/23] kbuild: check the presence of include/generated/rustc_cfg

Sorry, it was in my backlog after the conferences.

I am not sure what the base of the series was, but ran my usual tests
on top of v6.11 and then on top of `rust-fixes` after some manual
adjustment in both cases, and things appear to still work fine (i.e.
what I usually build, without taking advantage of the separate build
directory support).

Moreover, I tested the separate build directory support (`MO=`), for a
trivial Rust out-of-tree module, with a subdir as well as with a
directory outside the out-of-tree source code.

I also tested the new approach suggested for the out-of-tree
`Makefile` (i.e. `export KBUILD_EXTMOD` and `include
$(KDIR)/Makefile`), and it all worked as expected, so:

Tested-by: Miguel Ojeda <ojeda@kernel.org>

...except for arm64, where I found that I needed this bit:

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index b058c4803efb..4cd9a1f2ec3d 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -71,7 +71,7 @@ stack_protector_prepare: prepare0
                                -mstack-protector-guard-reg=sp_el0        \
                                -mstack-protector-guard-offset=$(shell    \
                        awk '{if ($$2 == "TSK_STACK_CANARY") print $$3;}' \
-                                       include/generated/asm-offsets.h))
+
$(objtree)/include/generated/asm-offsets.h))
 endif

 ifeq ($(CONFIG_ARM64_BTI_KERNEL),y)

Cheers,
Miguel