Re: bug: Kbuild seems to require "make prepare-objtool" in order to use (some) new config vars

[Masahiro Yamada]

On Fri, May 1, 2020 at 4:10 AM Mauro Carvalho Chehab
<mchehab+huawei@xxxxxxxxxx> wrote:
>
> Em Fri, 1 May 2020 02:20:13 +0900
> Masahiro Yamada <masahiroy@xxxxxxxxxx> escreveu:
>
> > On Fri, May 1, 2020 at 1:49 AM Mauro Carvalho Chehab
> > <mchehab+huawei@xxxxxxxxxx> wrote:
> > >
> > > Em Thu, 30 Apr 2020 22:51:48 +0900
> > > Masahiro Yamada <masahiroy@xxxxxxxxxx> escreveu:
> > >
> > > > Hi Mauro,
> > > >
> > > >
> > > > On Thu, Apr 30, 2020 at 8:17 PM Mauro Carvalho Chehab
> > > > <mchehab+huawei@xxxxxxxxxx> wrote:
> > > > >
> > > > > Hi Masahiro,
> > > > >
> > > > > Not sure if this was already reported (or eventually fixed) upstream.
> > > > >
> > > > > While doing a Kconfig reorg on media, I noticed a few weird things,
> > > > > requiring me to call, on a few situations, "make modules_prepare"
> > > > > manually after some changes.
> > > > >
> > > > > I'm now working on a patchset to yet to be merged upstream aiming to
> > > > > resurrect a driver from staging. It is currently on this tree
> > > > > (with is based at the media development tree, on the top of 5.7-rc1):
> > > > >
> > > > >         https://git.linuxtv.org/mchehab/experimental.git/log/?h=atomisp_v2
> > > > >
> > > > > There, I was able to identify a misbehavior that it is probably
> > > > > what forced me to need calling "make modules_prepare".
> > > > >
> > > > > The atomisp driver is on a very bad shape. Among its problems, it has a
> > > > > set of header definitions that should be different for two different
> > > > > variants of the supported devices. In order to be able to compile for
> > > > > either one of the variants, I added a new config var:
> > > > > CONFIG_VIDEO_ATOMISP_ISP2401.
> > > > >
> > > > > The problem is that calling just
> > > > >
> > > > >         ./scripts/config -e CONFIG_VIDEO_ATOMISP_ISP2401
> > > > >
> > > > > or
> > > > >         ./scripts/config -d CONFIG_VIDEO_ATOMISP_ISP2401
> > > >
> > > >
> > > > scripts/config does not take the dependency into consideration
> > > > at all.
> > > >
> > > > You need to enable/disable other options that it depends on.
> > >
> > > Yes, I know. on my tests, I did a "make allyesconfig" before
> > > the patch whose added this dependency. So, the only dependency
> > > left to be enabled or disabled was this one.
> > >
> > > The problem I'm pointing is not really do a select recursion
> > > (that would be a really cool feature, but I know it is not
> > > trivial), but, instead, that, despite the config var was
> > > there, when I tried to build it with:
> > >
> > >         make clean; make M=drivers/staging/media/atomisp
> > >
> > > It didn't do the right thing. Instead, it used ISP2401=y
> > > on make clean and ISP2401=n at the drivers build.
> > >
> > > So, in order to test if patches won't break building,
> > > depending on the value of this var, I'm currently doing:
> > >
> > >         cls;./scripts/config -d CONFIG_VIDEO_ATOMISP_ISP2401 && make prepare-objtool && make clean && make M=drivers/staging/media/atomisp && ./scripts/config -e CONFIG_VIDEO_ATOMISP_ISP2401 && make prepare-objtool && make clean && make M=drivers/staging/media/atomisp
> > >
> > > (note the alien "make prepare-objtool" in the middle)
> > >
> > >
> > > > ./scripts/config -e STAGING -e STAGING_MEDIA -e MEDIA_SUPPORT -e
> > > > MEDIA_CONTROLLER -e INTEL_ATOMISP -e VIDEOBUF_VMALLOC -e VIDEO_ATOMISP
> > > > -d MEDIA_SUPPORT_FILTER -e VIDEO_DEV -e VIDEO_V4L2 -e
> > > > VIDEO_ATOMISP_ISP2401
> > > >
> > > > allows me to enable VIDEO_ATOMISP_ISP2401.
> > > >
> > > >
> > > > It is painful to debug such complicated dependency graph, though.
> > >
> > > Yeah, dependencies at the media subsystem are usually quite complex.
> > >
> > > > >
> > > > > is not enough anymore for the build to actually use the new config value.
> > > > >
> > > > > It just keeps silently using the old config value.
> > > > >
> > > > > I double-checked it by adding this macro at the Makefile:
> > > > >
> > > > >         $(info ************ ISP2401: $(CONFIG_VIDEO_ATOMISP_ISP2401) ************)
> > > > >
> > > > > The Makefile doesn't see the change, except if I explicitly call
> > > > > "make modules_prepare" or "make prepare-objtool".
> > > > >
> > > > > Even calling "make oldconfig" doesn't make it use the new CONFIG_*
> > > >
> > > >
> > > > I do not know.
> > > >
> > > > I cannot look into it without detailed steps to reproduce it.
> > >
> > > I'm applying the enclosed patch to this branch:
> > >
> > >         https://git.linuxtv.org/mchehab/experimental.git/log/?h=atomisp_v2
> > >
> > > and running this:
> > >
> > >         $ make allyesconfig 2>/dev/null >/dev/null; echo "disable";./scripts/config -d CONFIG_VIDEO_ATOMISP_ISP2401 && make M=drivers/staging/media/atomisp && ./scripts/config -e CONFIG_VIDEO_ATOMISP_ISP2401 && echo "enable" && make clean && make M=drivers/staging/media/atomisp
> > >         disable
> > >
> > >           WARNING: Symbol version dump ./Module.symvers
> > >                    is missing; modules will have no dependencies and modversions.
> > >
> > >         ************ ISP2401: y ************
> > >           AR      drivers/staging/media/atomisp/built-in.a
> > >         ************ ISP2401: y ************
> > >           MODPOST 0 modules
> > >         enable
> > >         ************ ISP2401:  ************
> > >
> > >           WARNING: Symbol version dump ./Module.symvers
> > >                    is missing; modules will have no dependencies and modversions.
> > >
> > >         ************ ISP2401: y ************
> > >           AR      drivers/staging/media/atomisp/built-in.a
> > >         ************ ISP2401: y ************
> > >           MODPOST 0 modules
> > >
> > > PS.: the same occurs if I use "make allmodconfig".
> >
> >
> >
> > This is the expected behavior.
> >
> > The problem is that you immediately compile the module after
> > you tweak the .config file.
> >
> > Kbuild does not use the .config file directly.
> > Instead, it uses include/config/auto.conf.
> >
> >
> > The M= builds never touch the in-kernel build artifacts,
> > so it includes the stale include/config/auto.conf
>
> I'm pretty sure that this used to work in the past.
>
> Can't we have something similar to[1]:
>
>         include/config/auto.conf: .config
>
> in order to force auto.conf to be re-generated if the .config file
> was modified?
>
> [1] yeah, I know that the above won't work currently, because of the
> ifdefs, but perhaps something like this could be done inside the
> "if KBUILD_EXTMOD" part of the Makefile.
>
> > After you change the .config, you must run 'make modules_prepare'
> > at least.
> >
> > This is documented in 'make help'.
> >
> >
> >   modules_prepare - Set up for building external modules
>
> Yeah, I noticed this new target on more recent Kernels. I was not
> familiar with this "new" concept of "external"[2].


The meaning of "new" depends on people.

The 'modules_prepare' target was added in 2004.

This commit:
https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=95065ad3fa787c417008a36d3a5d9a3bab17ab98


I do not think it is new, but you may think differently.






One more thing,
please tell me the motivation to do:

 make M=drivers/staging/media/atomisp



The main usage of M= is to build external modules.
If you want to compile the individual directory,
you can do this:

 make drivers/staging/media/stomisp/





>
> Maybe the text should be changed to something like:
>
>    modules_prepare - Should be called before using "make M=<module dir>"
>
> To make it clearer. Yet, having to call it *every time* a
> Kconfig option has changed, doesn't seem right. The
> building system could be smarter and re-build auto.conf if
> it is older than .config file, or, at least, emit a WARNING, if
> the file is not synchronized.
>
>
> [2] Not sure if this still works, but, in the past (2.x), it was
> possible to setup an out-of-tree external tree with just a new
> driver. Then use "make modules" to build those external OOT
> modules. For historical reasons, still have at linuxtv.org
> one such tree:
>
>          https://linuxtv.org/hg/v4l-dvb/file/tip
>
>
> Thanks,
> Mauro


--
Best Regards
Masahiro Yamada