On Thu, Nov 28, 2024 at 12:29 AM Nicolas Schier <n.schier@xxxxxx> wrote: > > On Tue, Nov 19, 2024 at 02:02:05AM +0900, Masahiro Yamada wrote: > > On Mon, Nov 18, 2024 at 11:47 PM Nicolas Schier <n.schier@xxxxxx> wrote: > > > > > > 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 > > Nit: Adding a space between ':=1' for consistency? > > > > > endif > > > > > > > > -ifneq ($(KBUILD_ABS_SRCTREE),) > > > > -srctree := $(abs_srctree) > > > > +ifdef KBUILD_ABS_SRCTREE > > > > + # Do not nothing. Use the absolute path. > > Is this a superfluous "not"? > > "Do nothing. Use the absolute path."? Thanks. I folded both. > > > Thanks, again! > > Reviewed-by: Nicolas Schier <n.schier@xxxxxx> Thanks for the review! -- Best Regards Masahiro Yamada