On Tue, Sep 17, 2024 at 11:16:45PM +0900, Masahiro Yamada wrote:
> Currently, Kbuild always operates in the output directory of the kernel,
> even when building external modules. This increases the risk of external
> module Makefiles attempting to write to the kernel directory.
>
> This commit switches the working directory to the external module
> directory, allowing the removal of the $(KBUILD_EXTMOD)/ prefix from
> some build artifacts.
>
> The command for building external modules maintains backward
> compatibility, but Makefiles that rely on working in the kernel
> directory may break. In such cases, $(objtree) and $(srctree) should
> be used to refer to the output and source directories of the kernel.
>
> The appearance of the build log will change as follows:
>
> [Before]
>
> $ make -C /path/to/my/linux M=/path/to/my/externel/module
> make: Entering directory '/path/to/my/linux'
> CC [M] /path/to/my/externel/module/helloworld.o
> MODPOST /path/to/my/externel/module/Module.symvers
> CC [M] /path/to/my/externel/module/helloworld.mod.o
> CC [M] /path/to/my/externel/module/.module-common.o
> LD [M] /path/to/my/externel/module/helloworld.ko
> make: Leaving directory '/path/to/my/linux'
>
> [After]
>
> $ make -C /path/to/my/linux M=/path/to/my/externel/module
> make: Entering directory '/path/to/my/linux'
> make[1]: Entering directory '/path/to/my/externel/module'
> CC [M] helloworld.o
> MODPOST Module.symvers
> CC [M] helloworld.mod.o
> CC [M] .module-common.o
> LD [M] helloworld.ko
> make[1]: Leaving directory '/path/to/my/externel/module'
> make: Leaving directory '/path/to/my/linux'
>
> Printing "Entering directory" twice is cumbersome. This will be
> addressed later.
>
> Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> ---
>
> Documentation/dev-tools/coccinelle.rst | 19 +++++-------
> Makefile | 40 +++++++++++++++++---------
> rust/Makefile | 4 +--
> scripts/Makefile.compiler | 2 +-
> scripts/Makefile.modpost | 6 ++--
> scripts/coccicheck | 6 ++--
> scripts/package/install-extmod-build | 7 +++++
> 7 files changed, 49 insertions(+), 35 deletions(-)
>
> diff --git a/Documentation/dev-tools/coccinelle.rst b/Documentation/dev-tools/coccinelle.rst
> index 535ce126fb4f..80c83ce0babc 100644
> --- a/Documentation/dev-tools/coccinelle.rst
> +++ b/Documentation/dev-tools/coccinelle.rst
> @@ -250,25 +250,20 @@ variables for .cocciconfig is as follows:
> - Your directory from which spatch is called is processed next
> - The directory provided with the ``--dir`` option is processed last, if used
>
> -Since coccicheck runs through make, it naturally runs from the kernel
> -proper dir; as such the second rule above would be implied for picking up a
> -.cocciconfig when using ``make coccicheck``.
> -
> ``make coccicheck`` also supports using M= targets. If you do not supply
> any M= target, it is assumed you want to target the entire kernel.
> The kernel coccicheck script has::
>
> - if [ "$KBUILD_EXTMOD" = "" ] ; then
> - OPTIONS="--dir $srctree $COCCIINCLUDE"
> + if [ "$VPATH" ] ; then
> + OPTIONS="--dir $VPATH $COCCIINCLUDE"
> else
> - OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
> + OPTIONS="--dir . $COCCIINCLUDE"
> fi
>
> -KBUILD_EXTMOD is set when an explicit target with M= is used. For both cases
> -the spatch ``--dir`` argument is used, as such third rule applies when whether
> -M= is used or not, and when M= is used the target directory can have its own
> -.cocciconfig file. When M= is not passed as an argument to coccicheck the
> -target directory is the same as the directory from where spatch was called.
> +When an explicit target is executed with a separate output directory, VPATH is
> +set to the target source directory. The third rule ensures the spatch reads the
My limited English with German background likes to have "the target's
source directory" here, but I am not sure if this is more correct.
> +.cocciconfig from the target directory. When M= is used, the external module
> +directory can have its own.cocciconfig file.
A space is missing after 'own'.
>
> If not using the kernel's coccicheck target, keep the above precedence
> order logic of .cocciconfig reading. If using the kernel's coccicheck target,
> diff --git a/Makefile b/Makefile
> index 7a76452049ea..4db22c3a8555 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -180,7 +180,20 @@ ifeq ("$(origin O)", "command line")
> KBUILD_OUTPUT := $(O)
> endif
>
> -output := $(KBUILD_OUTPUT)
> +ifdef KBUILD_EXTMOD
> + ifdef KBUILD_OUTPUT
> + objtree := $(realpath $(KBUILD_OUTPUT))
> + $(if $(objtree),,$(error specified kernel directory "$(KBUILD_OUTPUT)" does not exist))
> + else
> + objtree := $(CURDIR)
> + endif
> + output := $(KBUILD_EXTMOD)
> +else
> + objtree := .
> + output := $(KBUILD_OUTPUT)
> +endif
> +
> +export objtree
>
> # Do we want to change the working directory?
> ifneq ($(output),)
> @@ -248,8 +261,6 @@ ifneq ($(KBUILD_ABS_SRCTREE),)
> srctree := $(abs_srctree)
> endif
>
> -objtree := .
> -
> VPATH :=
>
> ifeq ($(KBUILD_EXTMOD),)
> @@ -258,7 +269,7 @@ VPATH := $(srctree)
> endif
> endif
>
> -export building_out_of_srctree srctree objtree VPATH
> +export building_out_of_srctree srctree VPATH
>
> # 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
> @@ -708,7 +719,7 @@ endif
> # in addition to whatever we do anyway.
> # Just "make" or "make all" shall build modules as well
>
> -ifneq ($(filter all modules nsdeps %compile_commands.json clang-%,$(MAKECMDGOALS)),)
> +ifneq ($(filter all modules nsdeps compile_commands.json clang-%,$(MAKECMDGOALS)),)
Nit: I think this would better match to patch "kbuild: remove
extmod_prefix, MODORDER, MODULES_NSDEPS variables", but probably nobody
else will care.
Reviewed-by: Nicolas Schier <nicolas@xxxxxxxxx>
