diff mbox

[v6] core/pkg-kconfig: Use olddefconfig when available

Message ID 1461146052-13261-1-git-send-email-romain.izard.pro@gmail.com
State Superseded
Headers show

Commit Message

Romain Izard April 20, 2016, 9:54 a.m. UTC
Using 'yes "" | make oldconfig' to regenerate a .config file from an
existing defconfig does not work reliably in all cases. Specifically, it
does not work well with tristate choice entries.

The correct way to do it is to use 'make olddefconfig', but this target
is not supported in projects that use an old version of kconfig. As
most projects do not use tristate entries, there is no need for them to
upgrade.

'oldnoconfig' is an alias for 'olddefconfig', but it also is the only
name for this target between Linux versions 2.6.36 and 3.6.

Check which targets are supported by the current project, and use the
appropriate method to regenerate the .config file.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
Changelog:
v1: New patch
v2: Use oldnoconfig instead of olddefconfig
    Fix silent mode
    Change the pattern in both places where it is used
    Use a macro to avoid redundancy
v3: Do not use $(call ...)
    Use the $(2)_KCONFIG_MAKE variable
v4: No change
v5: Detect available methods and use the most recent one
v6: Remove colon from matched rule names

 package/pkg-kconfig.mk | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

Comments

Yann E. MORIN April 21, 2016, 10:47 p.m. UTC | #1
Romain, All,

On 2016-04-20 11:54 +0200, Romain Izard spake thusly:
> Using 'yes "" | make oldconfig' to regenerate a .config file from an
> existing defconfig does not work reliably in all cases. Specifically, it
> does not work well with tristate choice entries.
> 
> The correct way to do it is to use 'make olddefconfig', but this target
> is not supported in projects that use an old version of kconfig. As
> most projects do not use tristate entries, there is no need for them to
> upgrade.
> 
> 'oldnoconfig' is an alias for 'olddefconfig', but it also is the only
> name for this target between Linux versions 2.6.36 and 3.6.
> 
> Check which targets are supported by the current project, and use the
> appropriate method to regenerate the .config file.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> Changelog:
> v1: New patch
> v2: Use oldnoconfig instead of olddefconfig
>     Fix silent mode
>     Change the pattern in both places where it is used
>     Use a macro to avoid redundancy
> v3: Do not use $(call ...)
>     Use the $(2)_KCONFIG_MAKE variable
> v4: No change
> v5: Detect available methods and use the most recent one
> v6: Remove colon from matched rule names
> 
>  package/pkg-kconfig.mk | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index 4033915..478f5bb 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -60,18 +60,35 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
>  $(2)_KCONFIG_MAKE = \
>  	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS)
>  
> +$(2)_KCONFIG_RULES = \
> +	$$(shell $$($(2)_KCONFIG_MAKE) -pn config 2>/dev/null | \
> +		sed 's/^\([_0-9a-zA-Z]*config\):.*/\1/ p; d')

Ok, so... Well... Err...

Could you respin v4 of the patch, the one that used something like:

    make oldnoconfig || (yes "" |make oldconfig)

please? ;-)

OK, what's wrong with my proposal? It works in an unconfigured tree.
But as soon as the tree is configured, it breaks, notably with older
kernels, like 3.0.

    $ cd linux-3.0
    $ make -qpn oldefconfig 2>/dev/null |grep olddefconfig
    [nothing]
    $ touch .config
    $ make -qpn oldefconfig 2>/dev/null |grep olddefconfig
    olddefconfig: scripts_basic outputmakefile FORCE

But 3.0 does not have olddefconfig. It's jsut the %config generic rule
kicking in. So we'd believe there would be support for olddefconfig (or
oldnoconfig) when there isn't.

So, I have to apologise for the delusion. Your v4 is the best we can do.

Thank you! :-)

Regards,
Yann E. MORIN.

> +# The correct way to regenerate a .config file is to use 'make olddefconfig'.
> +# For historical reasons, the target name is 'oldnoconfig' between Linux kernel
> +# versions 2.6.36 and 3.6, and remains as an alias in later versions.
> +# In older versions, and in some other projects that use kconfig, the target is
> +# not supported at all, and we use 'yes "" | make oldconfig' as a fallback
> +# only, as this can fail in complex cases.
> +define $(2)_REGEN_DOT_CONFIG
> +	$$(if $$(filter olddefconfig,$$($(2)_KCONFIG_RULES)),
> +		$$(Q)$$($(2)_KCONFIG_MAKE) olddefconfig,
> +		$$(if $$(filter oldnoconfig,$$($(2)_KCONFIG_RULES)),
> +			$$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig,
> +			$$(Q)(yes "" | $$($(2)_KCONFIG_MAKE) oldconfig)))
> +endef
> +
>  # The specified source configuration file and any additional configuration file
>  # fragments are merged together to .config, after the package has been patched.
>  # Since the file could be a defconfig file it needs to be expanded to a
> -# full .config first. We use 'make oldconfig' because this can be safely
> -# done even when the package does not support defconfigs.
> +# full .config first.
>  $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
>  	$$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \
>  		$$($(2)_KCONFIG_MAKE) $$($(2)_KCONFIG_DEFCONFIG), \
>  		cp $$($(2)_KCONFIG_FILE) $$(@))
>  	$$(Q)support/kconfig/merge_config.sh -m -O $$(@D) \
>  		$$(@) $$($(2)_KCONFIG_FRAGMENT_FILES)
> -	$$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig
> +	$$($(2)_REGEN_DOT_CONFIG)
>  
>  # If _KCONFIG_FILE or _KCONFIG_FRAGMENT_FILES exists, this dependency is
>  # already implied, but if we only have a _KCONFIG_DEFCONFIG we have to add
> @@ -82,7 +99,7 @@ $$($(2)_DIR)/.config: | $(1)-patch
>  # The exact rules are specified by the package .mk file.
>  define $(2)_FIXUP_DOT_CONFIG
>  	$$($(2)_KCONFIG_FIXUP_CMDS)
> -	$$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig
> +	$$($(2)_REGEN_DOT_CONFIG)
>  	$$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done
>  endef
>  
> -- 
> 2.5.0
>
Romain Izard April 22, 2016, 1:51 p.m. UTC | #2
Yann, all,

2016-04-22 0:47 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
>
> Ok, so... Well... Err...
>
> Could you respin v4 of the patch, the one that used something like:
>
>     make oldnoconfig || (yes "" |make oldconfig)
>
> please? ;-)
>
> OK, what's wrong with my proposal? It works in an unconfigured tree.
> But as soon as the tree is configured, it breaks, notably with older
> kernels, like 3.0.
>
>     $ cd linux-3.0
>     $ make -qpn oldefconfig 2>/dev/null |grep olddefconfig
>     [nothing]
>     $ touch .config
>     $ make -qpn oldefconfig 2>/dev/null |grep olddefconfig
>     olddefconfig: scripts_basic outputmakefile FORCE
>
> But 3.0 does not have olddefconfig. It's jsut the %config generic rule
> kicking in. So we'd believe there would be support for olddefconfig
> (or oldnoconfig) when there isn't.
>
> So, I have to apologise for the delusion. Your v4 is the best we can
> do.

After some more testing, I believe it should work. I've tested the
following command on the latest commit for all stable branches since
2.6.11, and I can reliably detect which targets are supported. Moreover,
I have the same output whether I touched .config or not.

make -pn O=dir config 2>/dev/null | grep -oE '^old(no|def)config:'

I believe the problem with your tests is that you try to use both '-q'
and '-n', while GNU Make's manual states (Chapter 9.3, Instead of
executing recipes) that:

"It is an error to use more than one of these three flags [-t -q -n] in
the same invocation of 'make'."

Best regards,
Yann E. MORIN April 22, 2016, 5:27 p.m. UTC | #3
Romain, All,

On 2016-04-22 15:51 +0200, Romain Izard spake thusly:
> 2016-04-22 0:47 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
> >
> > Ok, so... Well... Err...
> >
> > Could you respin v4 of the patch, the one that used something like:
> >
> >     make oldnoconfig || (yes "" |make oldconfig)
> >
> > please? ;-)
> >
> > OK, what's wrong with my proposal? It works in an unconfigured tree.
> > But as soon as the tree is configured, it breaks, notably with older
> > kernels, like 3.0.
> >
> >     $ cd linux-3.0
> >     $ make -qpn oldefconfig 2>/dev/null |grep olddefconfig
> >     [nothing]
> >     $ touch .config
> >     $ make -qpn oldefconfig 2>/dev/null |grep olddefconfig
> >     olddefconfig: scripts_basic outputmakefile FORCE
> >
> > But 3.0 does not have olddefconfig. It's jsut the %config generic rule
> > kicking in. So we'd believe there would be support for olddefconfig
> > (or oldnoconfig) when there isn't.
> >
> > So, I have to apologise for the delusion. Your v4 is the best we can
> > do.
> 
> After some more testing, I believe it should work. I've tested the
> following command on the latest commit for all stable branches since
> 2.6.11, and I can reliably detect which targets are supported. Moreover,
> I have the same output whether I touched .config or not.
> 
> make -pn O=dir config 2>/dev/null | grep -oE '^old(no|def)config:'
> 
> I believe the problem with your tests is that you try to use both '-q'
> and '-n', while GNU Make's manual states (Chapter 9.3, Instead of
> executing recipes) that:
> 
> "It is an error to use more than one of these three flags [-t -q -n] in
> the same invocation of 'make'."

Weird, my make manpage does not state anything about that...

Anyway, I'll do some more testing following your input. Thanks for your
patience! ;-)

Regards,
Yann E. MORIN.
Arnout Vandecappelle April 22, 2016, 11:24 p.m. UTC | #4
On 04/22/16 15:51, Romain Izard wrote:
> Yann, all,
>
> 2016-04-22 0:47 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
>>
>> Ok, so... Well... Err...
>>
>> Could you respin v4 of the patch, the one that used something like:
>>
>>      make oldnoconfig || (yes "" |make oldconfig)
>>
>> please? ;-)
>>
>> OK, what's wrong with my proposal? It works in an unconfigured tree.
>> But as soon as the tree is configured, it breaks, notably with older
>> kernels, like 3.0.
>>
>>      $ cd linux-3.0
>>      $ make -qpn oldefconfig 2>/dev/null |grep olddefconfig
>>      [nothing]
>>      $ touch .config
>>      $ make -qpn oldefconfig 2>/dev/null |grep olddefconfig
>>      olddefconfig: scripts_basic outputmakefile FORCE
>>
>> But 3.0 does not have olddefconfig. It's jsut the %config generic rule
>> kicking in. So we'd believe there would be support for olddefconfig
>> (or oldnoconfig) when there isn't.
>>
>> So, I have to apologise for the delusion. Your v4 is the best we can
>> do.
>
> After some more testing, I believe it should work. I've tested the
> following command on the latest commit for all stable branches since
> 2.6.11, and I can reliably detect which targets are supported. Moreover,
> I have the same output whether I touched .config or not.
>
> make -pn O=dir config 2>/dev/null | grep -oE '^old(no|def)config:'

  This indeed seems to work reliably.

  Yann discovered that when you run exactly the same command twice you get 
different results [1]. But that's because he uses .DEFAULT as the target instead 
of config. The problem in that case is that oldno/defconfig don't appear in the 
top-level Makefile, so they won't be printed. Only the first time it is printed, 
because of include/config/auto.conf: this is include'd in the Makefile so it 
will be regenerated (because of the include/config/%.conf: $(KCONFIG_CONFIG) 
include/config/auto.conf.cmd dependency), which calls the sub-make in 
scripts/kconfig, which does have the olddefconfig target so it is printed. The 
second time, auto.conf is newer than .config so it is not regenerated, so the 
olddefconfig target is not printed.

  Using config as the target elegantly solves that because it forces calling the 
submake in scripts/kconfig.

  So yes, you v6 would work I think.

  However, I think we (= Yann and I) mentioned at some point that we preferred 
everything in one command, using grep instead of filter. Or was there a 
counter-argument against that?


>
> I believe the problem with your tests is that you try to use both '-q'
> and '-n', while GNU Make's manual states (Chapter 9.3, Instead of
> executing recipes) that:
>
> "It is an error to use more than one of these three flags [-t -q -n] in
> the same invocation of 'make'."

  I think the issue is that -q doesn't cause calling sub-make's, ever. That's 
because it's not needed: if a sub-make is going to be called, something is going 
to be run so it can return 1 immediately without actually calling the sub-make. 
So with -q it is in fact guaranteed never to print an olddefconfig.


  Regards,
  Arnout


[1] http://code.bulix.org/qwcju4-97419?raw
diff mbox

Patch

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index 4033915..478f5bb 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -60,18 +60,35 @@  $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
 $(2)_KCONFIG_MAKE = \
 	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS)
 
+$(2)_KCONFIG_RULES = \
+	$$(shell $$($(2)_KCONFIG_MAKE) -pn config 2>/dev/null | \
+		sed 's/^\([_0-9a-zA-Z]*config\):.*/\1/ p; d')
+
+# The correct way to regenerate a .config file is to use 'make olddefconfig'.
+# For historical reasons, the target name is 'oldnoconfig' between Linux kernel
+# versions 2.6.36 and 3.6, and remains as an alias in later versions.
+# In older versions, and in some other projects that use kconfig, the target is
+# not supported at all, and we use 'yes "" | make oldconfig' as a fallback
+# only, as this can fail in complex cases.
+define $(2)_REGEN_DOT_CONFIG
+	$$(if $$(filter olddefconfig,$$($(2)_KCONFIG_RULES)),
+		$$(Q)$$($(2)_KCONFIG_MAKE) olddefconfig,
+		$$(if $$(filter oldnoconfig,$$($(2)_KCONFIG_RULES)),
+			$$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig,
+			$$(Q)(yes "" | $$($(2)_KCONFIG_MAKE) oldconfig)))
+endef
+
 # The specified source configuration file and any additional configuration file
 # fragments are merged together to .config, after the package has been patched.
 # Since the file could be a defconfig file it needs to be expanded to a
-# full .config first. We use 'make oldconfig' because this can be safely
-# done even when the package does not support defconfigs.
+# full .config first.
 $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
 	$$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \
 		$$($(2)_KCONFIG_MAKE) $$($(2)_KCONFIG_DEFCONFIG), \
 		cp $$($(2)_KCONFIG_FILE) $$(@))
 	$$(Q)support/kconfig/merge_config.sh -m -O $$(@D) \
 		$$(@) $$($(2)_KCONFIG_FRAGMENT_FILES)
-	$$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig
+	$$($(2)_REGEN_DOT_CONFIG)
 
 # If _KCONFIG_FILE or _KCONFIG_FRAGMENT_FILES exists, this dependency is
 # already implied, but if we only have a _KCONFIG_DEFCONFIG we have to add
@@ -82,7 +99,7 @@  $$($(2)_DIR)/.config: | $(1)-patch
 # The exact rules are specified by the package .mk file.
 define $(2)_FIXUP_DOT_CONFIG
 	$$($(2)_KCONFIG_FIXUP_CMDS)
-	$$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig
+	$$($(2)_REGEN_DOT_CONFIG)
 	$$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done
 endef