LinuxLists.cc - [PATCH 0/4] kbuild: replace $(srctree)/$(src) with $(src) in Makefiles

2024-04-16 12:20:47

2024-04-16 22:49:57

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 4/4] kbuild: use $(src) instead of $(srctree)/$(src) for source directory

Hi Masahiro,

kernel test robot noticed the following build warnings:

[auto build test WARNING on masahiroy-kbuild/for-next]
[cannot apply to masahiroy-kbuild/fixes wireless-next/main wireless/main linus/master v6.9-rc4 next-20240416]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/arch-use-obj-instead-of-src-for-preprocessed-linker-scripts/20240416-202308
base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
patch link: https://lore.kernel.org/r/20240416121838.95427-5-masahiroy%40kernel.org
patch subject: [PATCH 4/4] kbuild: use $(src) instead of $(srctree)/$(src) for source directory
config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240417/[email protected]/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240417/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> cc1: warning: -I: No such file or directory [-Wmissing-include-dirs]

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-04-17 04:20:36

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 4/4] kbuild: use $(src) instead of $(srctree)/$(src) for source directory

Hi Masahiro,

kernel test robot noticed the following build errors:

[auto build test ERROR on masahiroy-kbuild/for-next]
[cannot apply to masahiroy-kbuild/fixes wireless-next/main wireless/main linus/master v6.9-rc4 next-20240416]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/arch-use-obj-instead-of-src-for-preprocessed-linker-scripts/20240416-202308
base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
patch link: https://lore.kernel.org/r/20240416121838.95427-5-masahiroy%40kernel.org
patch subject: [PATCH 4/4] kbuild: use $(src) instead of $(srctree)/$(src) for source directory
config: arm64-randconfig-004-20240417 (https://download.01.org/0day-ci/archive/20240417/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 7089c359a3845323f6f30c44a47dd901f2edfe63)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240417/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> error: couldn't read rust/doctests_kernel_generated.rs: No such file or directory (os error 2)

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-04-18 13:43:57

by Rob Herring (Arm)

On Sat, Apr 20, 2024 at 12:07 AM Nicolas Schier <[email protected]> wrote:
>
> On Tue 16 Apr 2024 21:18:36 GMT, Masahiro Yamada wrote:
> > The include directive with the double-quote form, #include "...",
> > first searches for the specified header in the directory containing
> > the source file.
> >
> > If all local headers are included using the double-quote form instead
> > of the angle bracket form, there is no need to add the local directory
> > to the header search path.
> >
> > drivers/gpu/drm/imagination and drivers/net/ethernet/aquantia/atlantic
> > use only the double-quote form for including the local headers, and
> > there are no generated sources or headers in their directories. Hence,
> > the local header search path is unneeded.
> >
> > The same applies to arch/loongarch/kvm/ because TRACE_INCLUDE_PATH is
> > relative to include/trace/.
> >
> > I guess there exist more Makefiles with unnecessary header inclusion
> > paths (and more cases where it is possible to delete the header search
> > path by replacing #include <...> with #include "..."), but I do not have
> > an easy way to detect it.
> >
> > These are unneeded inclusion paths that I happened to find.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > arch/loongarch/kvm/Makefile | 2 --
> > drivers/gpu/drm/imagination/Makefile | 2 --
> > drivers/net/ethernet/aquantia/atlantic/Makefile | 2 --
> > 3 files changed, 6 deletions(-)
> >
> > diff --git a/arch/loongarch/kvm/Makefile b/arch/loongarch/kvm/Makefile
> > index 244467d7792a..7a0108a721c1 100644
> > --- a/arch/loongarch/kvm/Makefile
> > +++ b/arch/loongarch/kvm/Makefile
> > @@ -3,8 +3,6 @@
> > # Makefile for LoongArch KVM support
> > #
> >
> > -ccflags-y += -I $(srctree)/$(src)
> > -
> > include $(srctree)/virt/kvm/Makefile.kvm
> >
> > obj-$(CONFIG_KVM) += kvm.o
> > diff --git a/drivers/gpu/drm/imagination/Makefile b/drivers/gpu/drm/imagination/Makefile
> > index ec6db8e9b403..3d9d4d40fb80 100644
> > --- a/drivers/gpu/drm/imagination/Makefile
> > +++ b/drivers/gpu/drm/imagination/Makefile
> > @@ -1,8 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only OR MIT
> > # Copyright (c) 2023 Imagination Technologies Ltd.
> >
> > -subdir-ccflags-y := -I$(srctree)/$(src)
> > -
> > powervr-y := \
> > pvr_ccb.o \
> > pvr_cccb.o \
> > diff --git a/drivers/net/ethernet/aquantia/atlantic/Makefile b/drivers/net/ethernet/aquantia/atlantic/Makefile
> > index 8ebcc68e807f..268a055086c4 100644
> > --- a/drivers/net/ethernet/aquantia/atlantic/Makefile
> > +++ b/drivers/net/ethernet/aquantia/atlantic/Makefile
> > @@ -8,8 +8,6 @@
> >
> > obj-$(CONFIG_AQTION) += atlantic.o
> >
> > -ccflags-y += -I$(srctree)/$(src)
> > -
> > atlantic-objs := aq_main.o \
> > aq_nic.o \
> > aq_pci_func.o \
>
> I'm afraid drivers/net/ethernet/aquantia/atlantic/macsec/macsec_api.c,
> .../hw_atl2/hw_atl2.c and .../hw_atl2/hw_atl2_utills{,_fw}.[ch] need to
> be adjusted first.
>
> Except for the atlantic related patch diff:
> Reviewed-by: Nicolas Schier <[email protected]>

Right. I got build errors for atlantic.

I will drop this because it may cause regressions.

--
Best Regards
Masahiro Yamada

2024-04-20 08:57:32

by Masahiro Yamada

[permalink] [raw]

Subject: Re: [PATCH 4/4] kbuild: use $(src) instead of $(srctree)/$(src) for source directory

On Sat, Apr 20, 2024 at 12:09 AM Nicolas Schier <[email protected]> wrote:
>
> On Tue 16 Apr 2024 21:18:38 GMT, Masahiro Yamada wrote:
> > Kbuild conventionally uses $(obj)/ for generated files, and $(src)/ for
> > checked-in source files. It is merely a convention without any functional
> > difference. In fact, $(obj) and $(src) are exactly the same, as defined
> > in scripts/Makefile.build:
> >
> > src := $(obj)
> >
> > When the kernel is built in a separate output directory, $(src) does
> > not match the directory in the source tree. It still works because the
> > top Makefile sets VPATH to $(srctree). However, VPATH does not cover
> > all cases. There are many places where the exact path to the source
> > directory is needed. For example, when adding a header search path for
> > local headers, it is typical to pass -I$(srctree)/$(src) to the compiler.
> >
> > However, it introduces inconsistency between upstream Makefiles and
> > downstream Makefiles; when the source directory path is needed,
> > $(srctree)/$(src) is used in the upstream kernel, while $(src) is used
> > in external modules.
> >
> > To resolve the distortion, this commit changes the semantics of $(src)
> > so that it always points to the directory in the source tree.
> >
> > Going forward, the variables used in Makefiles will work as follows:
> >
> > $(obj) - directory in the object tree
> > $(src) - directory in the source tree (changed by this commit!)
> > $(objtree) - the top of the kernel object tree
> > $(srctree) - the top of the kernel source tree
> >
> > This is a tweewide Makefile conversion for replacing $(srctree)/$(src)
> > with $(src).
>
> tweewide -> treewide
>
> [...]
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index 3bf5cab4b451..fe9ceb0d2288 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -6,11 +6,6 @@
> > # Rewritten to use lists instead of if-statements.
> > #
> >
> > -# Some driver Makefiles miss $(srctree)/ for include directive.
> > -ifdef building_out_of_srctree
> > -MAKEFLAGS += --include-dir=$(srctree)
> > -endif
> > -
>
> Ah, it's nice to get rid of this.
>
> > obj-y += cache/
> > obj-y += irqchip/
> > obj-y += bus/
> [...]
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index 3179747cbd2c..c53d2f168801 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -209,13 +209,13 @@ _c_flags += $(if $(patsubst n%,, \
> > -D__KCSAN_INSTRUMENT_BARRIERS__)
> > endif
> >
> > -# $(srctree)/$(src) for including checkin headers from generated source files
> > -# $(objtree)/$(obj) for including generated headers from checkin source files
> > +# $(src) for including checkin headers from generated source files
> > +# $(obj) for including generated headers from checkin source files
>
> this is a funny criss-cross of words; but might the following be more
> accurate?
>
> # $(src) for including headers from checked-in source files
> # $(obj) for including generated headers

These are added only when building_out_of_srctree is defined.

When the kernel is built in-tree,
the include directive with the double-quote form
should work without the header inclusion path.

Anyway, this does not seem to be a good idea.
I want to get rid of these entirely.

>
>
>
> Anyway, thanks for the patch. I think it is quite a good way forward!
>
> Reviewed-by: Nicolas Schier <[email protected]>

--
Best Regards
Masahiro Yamada