Re: [PATCH] kernel: use oldnoconfig instead of yes '' | make oldconfig

[Alexandre Belloni <alexandre.belloni@free-electrons.com>]

[-- Attachment #1: Type: text/plain, Size: 6610 bytes --]

Hi,

So, I did the same tests with an x86_64 config and an ARM config.

On 06/02/2014 at 16:48:52 -0500, Bruce Ashfield wrote :
> > git branch --contains fbe98bb9ed3dae23e320c6b113e35f129538d14a
> * master
> 
> > grep -C 2 ^VERSION Makefile
> VERSION = 3
> PATCHLEVEL = 14
> SUBLEVEL = 0
> 

I'm also on 3.14, commit 9343224bfd4be6a02e6ae0c0d66426c955c7d76e

> So we have all the relevant commits, and a up to date tree. Operating
> on the .config from a 64 bit x86 build:
> 
> > grep USB_ETH .config
> CONFIG_USB_ETH=y
> CONFIG_USB_ETH_RNDIS=y
> CONFIG_USB_ETH_EEM=y
> 
> > make savedefconfig
> scripts/kconfig/conf --savedefconfig=defconfig Kconfig
> .config:5333:warning: override: USB_ETH changes choice state
> .config:5336:warning: USB_G_NCM creates inconsistent choice state
> .config:5337:warning: USB_GADGETFS creates inconsistent choice state
> .config:5338:warning: USB_FUNCTIONFS creates inconsistent choice state
> .config:5342:warning: USB_MASS_STORAGE creates inconsistent choice state
> .config:5343:warning: USB_GADGET_TARGET creates inconsistent choice state
> .config:5344:warning: USB_G_SERIAL creates inconsistent choice state
> .config:5345:warning: USB_MIDI_GADGET creates inconsistent choice state
> .config:5346:warning: USB_G_PRINTER creates inconsistent choice state
> .config:5347:warning: USB_CDC_COMPOSITE creates inconsistent choice state
> .config:5348:warning: USB_G_NOKIA creates inconsistent choice state
> .config:5349:warning: USB_G_ACM_MS creates inconsistent choice state
> .config:5351:warning: USB_G_HID creates inconsistent choice state
> .config:5352:warning: USB_G_DBGP creates inconsistent choice state
> .config:5355:warning: USB_G_WEBCAM creates inconsistent choice state
> .config:6201:warning: symbol value 'm' invalid for VME_BUS
> 

I don't get those warnings.

> > cp defconfig .config
> 
> > yes '' | make oldconfig
> 
> > grep USB_ETH .config
> CONFIG_USB_ETH=m
> CONFIG_USB_ETH_RNDIS=y
> CONFIG_USB_ETH_EEM=y
> 
> So far so good, this matches what I'd expect. Now with olddefconfig
> / oldnoconfig
> 

Exactly what I have and what I'm trying to fix.

> > cp defconfig .config
> > make oldnoconfig
> scripts/kconfig/conf --olddefconfig Kconfig
> #
> # configuration written to .config
> #
> yow-bashfiel-d3 [/home/bruc...rnel/linux]> grep USB_ETH .config
> CONFIG_USB_ETH=m
> CONFIG_USB_ETH_RNDIS=y
> CONFIG_USB_ETH_EEM=y
> 
> .. CONFIG_USB_ETH is still =m.
> 

That is where I end up with:

$ grep USB_ETH .config
CONFIG_USB_ETH=y
CONFIG_USB_ETH_RNDIS=y
CONFIG_USB_ETH_EEM=y

> I've also been running a whole series of configuration tests on the
> defconfigs found in the various arch/platforms in the mainline kernel,
> and on a modern (3.14-rc) kernel, getting the same results from
> yes "" | oldconfig and olddefconfig. i.e. migrating their configs with
> either technique results in the same resulting .config,
> 
> Which if you recall the above question was my big concern, was the
> migration between kernel versions using the different techniques going
> to yield different results on the same .config.
> 
> The answer to that is "it appears not", and really, taking a Kconfig's
> default IS the right thing to do if you must make a choice, and aren't
> familiar with the option.
> 

But it should be different because using 'yes "" | oldconfig' is wrong.
Let's have a look at qi_lb60_defconfig:

$ ARCH=mips make qi_lb60_defconfig
#
# configuration written to .config
#
$ grep USB_ETH .config
CONFIG_USB_ETH=y
# CONFIG_USB_ETH_RNDIS is not set
# CONFIG_USB_ETH_EEM is not set

$ cp arch/mips/configs/qi_lb60_defconfig .config
‘arch/mips/configs/qi_lb60_defconfig’ -> ‘.config’
$ yes '' | make oldconfig
[...]
$ grep USB_ETH .config
CONFIG_USB_ETH=m
# CONFIG_USB_ETH_RNDIS is not set
# CONFIG_USB_ETH_EEM is not set


> ** So, that leaves me with agreeing that the change should be ok from
> that front, and that we definitely need the fallback to the yes "" |
> oldconfig variant if the newer targets don't exist. But from my tests,
> the commit log is slightly off .. again, maybe I messed something up
> in my tests with 3.14, but it is worth double checking.
> 

I believe my commit message is actually right and something went wrong
in your tests.

> And finally, to allow someone who really doesn't like the command full
> flexibility, I'm running with the following change, and it offers
> an easy way to change behaviour .. opinions ?
> 
> --->----->--->----->--->----->--->----->
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index b76a65699755..9510996d52c1 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -313,6 +313,8 @@ python sysroot_stage_all () {
>      oe.path.copyhardlinktree(d.expand("${D}${KERNEL_SRC_PATH}"),
> d.expand("${SYSROOT_DESTDIR}${KERNEL_SRC_PATH}"))
>  }
> 
> +KERNEL_CONFIG_COMMAND ?= "yes '' | oe_runmake oldconfig"
> +

If we go that route, shouldn't we have sane default and use
KERNEL_CONFIG_COMMAND ?= "oe_runmake oldnoconfig || yes '' | oe_runmake oldconfig"

I understand that it may change the behaviour for some existing
configurations but I believe it is the correct behaviour to actually
respect what is in the kernel config provided by the user.

>  kernel_do_configure() {
>         # fixes extra + in /lib/modules/2.6.37+
>         # $ scripts/setlocalversion . => +
> @@ -325,7 +327,7 @@ kernel_do_configure() {
>         if [ -f "${WORKDIR}/defconfig" ] && [ ! -f "${B}/.config" ]; then
>                 cp "${WORKDIR}/defconfig" "${B}/.config"
>         fi
> -       yes '' | oe_runmake oldconfig
> +       eval ${KERNEL_CONFIG_COMMAND}
>  }
> --->----->--->----->--->----->--->----->
> 
> Doing a v3 of the fallback version, with a checked commit log and
> perhaps even the command flexibility would keep everyone happy and
> I can add my Acked-by to that version.
> 

BTW, I've been told by the kconfig maintainers that the preferred way of
doing it would be to copy the provided defconfig to
arch/${ARCH}/configs/oe_defconfig and then oe_runmake oe_defconfig. But
unfortunately, that doesn't work for architectures without a 'configs'
directory. For 3.14, this is only alpha and frv but x86 got it starting
with 2266cfd50de3872e877eeca3dd4a6f940f22ba60, this is v2.6.24 and I
remember being asked to support older kernels.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 853 bytes --]