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

[Nicolas Schier <n.schier@xxxxxx>]

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