On Sun, Nov 10, 2024 at 10:34:33AM +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>
> ---
>
> Changes in v2:
> - Introduce a new 'srcroot' variable and clean-up code
> - Reword Documentation/dev-tools/coccinelle.rst
>
> Documentation/dev-tools/coccinelle.rst | 20 ++-----
> Documentation/kbuild/makefiles.rst | 14 +++++
> Makefile | 80 +++++++++++++++-----------
> rust/Makefile | 4 +-
> scripts/Makefile.build | 2 +-
> scripts/Makefile.clean | 2 +-
> scripts/Makefile.compiler | 2 +-
> scripts/Makefile.modpost | 6 +-
> scripts/coccicheck | 6 +-
> scripts/nsdeps | 8 +--
> scripts/package/install-extmod-build | 7 +++
> 11 files changed, 85 insertions(+), 66 deletions(-)
>
> diff --git a/Documentation/dev-tools/coccinelle.rst b/Documentation/dev-tools/coccinelle.rst
> index 535ce126fb4f..6e70a1e9a3c0 100644
> --- a/Documentation/dev-tools/coccinelle.rst
> +++ b/Documentation/dev-tools/coccinelle.rst
> @@ -250,25 +250,17 @@ 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"
> - else
> - OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
> - fi
> + OPTIONS="--dir $srcroot $COCCIINCLUDE"
>
> -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.
> +Here, $srcroot refers to the source directory of the target: it points to the
> +external module's source directory when M= used, and otherwise, to the kernel
> +source directory. The third rule ensures the spatch reads the .cocciconfig from
> +the target directory, allowing external modules to have their own .cocciconfig
> +file.
>
> 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/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
> index 7964e0c245ae..d36519f194dc 100644
> --- a/Documentation/kbuild/makefiles.rst
> +++ b/Documentation/kbuild/makefiles.rst
> @@ -449,6 +449,20 @@ $(obj)
> to prerequisites are referenced with $(src) (because they are not
> generated files).
>
> +$(srcroot)
> + $(srcroot) refers to the root of the source you are building, which can be
> + either the kernel source or the external modules source, depending on whether
> + KBUILD_EXTMOD is set. This can be either a relative or an absolute path, but
> + if KBUILD_ABS_SRCTREE=1 is set, it is always an absolute path.
> +
> +$(srctree)
> + $(srctree) refers to the root of the kernel source tree. When building the
> + kernel, this is the same as $(srcroot).
> +
> +$(objtree)
> + $(objtree) refers to the root of the kernel object tree. It is ``.`` when
> + building the kernel, but it is different when building external modules.
> +
Thanks, I think it's nice that there is now such a clear definition.
$(srcroot) sounds fine to me.
> $(kecho)
> echoing information to user in a rule is often a good practice
> but when execution ``make -s`` one does not expect to see any output
> diff --git a/Makefile b/Makefile
> index cf1d55560ae2..e5f7ac7647a7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -180,7 +180,24 @@ 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)
> + # KBUILD_EXTMOD might be a relative path. Remember its absolute path before
> + # Make changes the working directory.
> + srcroot := $(realpath $(KBUILD_EXTMOD))
> + $(if $(srcroot),,$(error specified external module directory "$(KBUILD_EXTMOD)" does not exist))
> +else
> + objtree := .
> + output := $(KBUILD_OUTPUT)
> +endif
> +
> +export objtree srcroot
>
> # Do we want to change the working directory?
> ifneq ($(output),)
> @@ -230,35 +247,33 @@ else # need-sub-make
>
> # We process the rest of the Makefile if this is the final invocation of make
>
> -ifeq ($(abs_srctree),$(CURDIR))
> - # building in the source tree
> - srctree := .
> - building_out_of_srctree :=
> +ifndef KBUILD_EXTMOD
> +srcroot := $(abs_srctree)
> +endif
> +
> +ifeq ($(srcroot),$(CURDIR))
> +building_out_of_srctree :=
> else
> - ifeq ($(abs_srctree)/,$(dir $(CURDIR)))
> - # building in a subdirectory of the source tree
> - srctree := ..
> - else
> - srctree := $(abs_srctree)
> - endif
> - building_out_of_srctree := 1
> +export building_out_of_srctree :=1
> endif
>
> -ifneq ($(KBUILD_ABS_SRCTREE),)
> -srctree := $(abs_srctree)
> +ifdef KBUILD_ABS_SRCTREE
> + # Do not nothing. Use the absolute path.
> +else ifeq ($(srcroot),$(CURDIR))
> + # Building in the source.
> + srcroot := .
> +else ifeq ($(srcroot)/,$(dir $(CURDIR)))
> + # Building in a subdirectory of the source.
> + srcroot := ..
> endif
>
> -objtree := .
> +export srctree := $(if $(KBUILD_EXTMOD),$(abs_srctree),$(srcroot))
With this patch applied, the following breaks for me:
$ make O=build M=fs/btrfs CONFIG_BTRFS_FS=m
make[1]: Entering directory '/data/linux/kbuild-review/fs/btrfs'
CC [M] super.o
In file included from <command-line>:
/data/linux/kbuild-review/include/linux/compiler_types.h:89:10: fatal error: linux/compiler_attributes.h: No such file or directory
89 | #include <linux/compiler_attributes.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
Adding 'ccflags-y += -I$(srctree)/include' to fs/btrfs/Makefile breaks
the build loudly. I could make it build again with
diff --git a/Makefile b/Makefile
index e5f7ac7647a7b..3d95911f1a68f 100644
--- a/Makefile
+++ b/Makefile
@@ -555,7 +555,7 @@ USERINCLUDE := \
LINUXINCLUDE := \
-I$(srctree)/arch/$(SRCARCH)/include \
-I$(objtree)/arch/$(SRCARCH)/include/generated \
- $(if $(building_out_of_srctree),-I$(srctree)/include) \
+ $(if $(or $(building_out_of_srctree),$(filter $(srctree)/%, $(CURDIR))),-I$(srctree)/include) \
-I$(objtree)/include \
$(USERINCLUDE)
but this does not feel good. It building in-tree modules in this way a
valid thing to do?
Kind regards,
Nicolas
