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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK7LNAS7D_d+Kz5+rk3OuN0StY=d3iOOa4Un12HYyF4w89LbTQ@mail.gmail.com>
Date: Fri, 20 Sep 2024 21:58:47 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Nicolas Schier <n.schier@....de>
Cc: linux-kbuild@...r.kernel.org, Miguel Ojeda <ojeda@...nel.org>, 
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>
Subject: Re: [PATCH 00/23] kbuild: support building external modules in a
 separate build directory

On Fri, Sep 20, 2024 at 7:39 PM Nicolas Schier <n.schier@....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