[Buildroot] [PATCH v5] core/pkg-kconfig: Use olddefconfig when available

[Buildroot] [PATCH v5] core/pkg-kconfig: Use olddefconfig when available

[Romain Izard]

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: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
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

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

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index 4033915..e14b33f 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
 
-- 
2.5.0

[Buildroot] [PATCH v5] core/pkg-kconfig: Use olddefconfig when available

[Thomas Petazzoni]

Hello,

On Tue, 19 Apr 2016 17:07:14 +0200, Romain Izard wrote:

> +$(2)_KCONFIG_RULES = \
> +	$$(shell $$($(2)_KCONFIG_MAKE) -pn config 2>/dev/null | \
> +		sed 's/^\([_0-9a-zA-Z]*config:\).*/\1/ p; d')

Does this actually work? Does this gets expanded/evaluated *before* the
package source code is extracted?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v5] core/pkg-kconfig: Use olddefconfig when available

[Yann E. MORIN]

Romain, All,

On 2016-04-19 17:07 +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: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
> 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
> 
>  package/pkg-kconfig.mk | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index 4033915..e14b33f 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')

NAK, because this will be executed at the time the package rules are
exapnded (i.e. during the call to $(eval $(kconfig-package)), so would
not work for a build from scratch...

(but double-dollar make that assertion difficult to asses... I'd have to
double-check).

Besides...

> +# 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

... this would get expand twice, so the shell construct would be run
twice, whereas doing it all in shell as I suggested would only run the
needed checks: if olddefconfig is supported, the check for oldnoconfig
would no even run; in your case, both tests would be run, always.

But yeah, my proposal had a flaw; even my fix on that proposal had a
flaw: the output of the grep should not be >/dev/null, since we want to
check whether it isempty or not...

Regards,
Yann E. MORIN.

>  # 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
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v5] core/pkg-kconfig: Use olddefconfig when available

[Yann E. MORIN]

Romain, All,

On 2016-04-19 19:45 +0200, Yann E. MORIN spake thusly:
> On 2016-04-19 17:07 +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: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> > ---
> > 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
> > 
> >  package/pkg-kconfig.mk | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> > 
> > diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> > index 4033915..e14b33f 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')
> 
> NAK, because this will be executed at the time the package rules are
> exapnded (i.e. during the call to $(eval $(kconfig-package)), so would
> not work for a build from scratch...
> 
> (but double-dollar make that assertion difficult to asses... I'd have to
> double-check).

OK, so I withdraw that comment: it *does* work. And the second shell
eval is only called when needed! :-)

Still, I'm not too fond of it being written in Makefile. Especially the
part below...

> Besides...
> 
> > +# 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

I am not usually one to argue against writting Makefile code in a
Makefile. However, for this case, I think writting a shell rule is much
more readable.

And I don't see how your solution would be inherently different from
mine: both use 'make -pqn' to get the list of targets and grep/sed the
output to decide if the requested target exists...

By the way, if your sed expression would have moved the colon ':'
outside the ()-expression, you would not have had to repeqt the colon in
the filter match. Also, using extended regexps would allow using
character classes.

    $(2)_KCONFIG_RULES = \
        $$(shell $$($(2)_KCONFIG_MAKE) -pn config 2>/dev/null | \
            sed -r '/^([[:alnum:]]*config):.*/!d; s//\1/;')

Again, I think a shell construct is either to read.

Regards,
Yann E. MORIN.

> >  # 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
> > 
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v5] core/pkg-kconfig: Use olddefconfig when available

[Thomas Petazzoni]

Romain,

On Tue, 19 Apr 2016 17:07:14 +0200, Romain Izard wrote:

> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Where did Arnout gave you his Reviewed-by?

If he did give you his Reviewed-by on a previous version, and you
changed the patch significantly (which you did), then you cannot keep
his Reviewed-by tag because you have no idea if his review still holds.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v5] core/pkg-kconfig: Use olddefconfig when available

[Yann E. MORIN]

Romain, All,

On 2016-04-19 17:07 +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: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

As Thomas said: this was given on a previous iteration of the patch, and
this one is fundamentaly different; the review by arnout no longer
stand on this new version.

However, after some discussion with Thomas on IRC, here is a more
thourough review from me...

> ---
> 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
> 
>  package/pkg-kconfig.mk | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index 4033915..e14b33f 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')

Move the colon ':' outside the ()-expression, and then...

> +# 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)),

... drop the colon here...

> +		$$(Q)$$($(2)_KCONFIG_MAKE) olddefconfig,
> +		$$(if $$(filter oldnoconfig:,$$($(2)_KCONFIG_RULES)),

... and here.

When you fix that, then you can add my:

    Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Thanks! :-)

Regards,
Yann E. MORIN.

> +			$$(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
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v5] core/pkg-kconfig: Use olddefconfig when available

[Arnout Vandecappelle]

On 04/19/16 22:35, Yann E. MORIN wrote:
> Romain, All,
>
> On 2016-04-19 19:45 +0200, Yann E. MORIN spake thusly:
>> On 2016-04-19 17:07 +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: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>>> ---
>>> 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
>>>
>>>   package/pkg-kconfig.mk | 18 ++++++++++++++----
>>>   1 file changed, 14 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
>>> index 4033915..e14b33f 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')
>>
>> NAK, because this will be executed at the time the package rules are
>> exapnded (i.e. during the call to $(eval $(kconfig-package)), so would
>> not work for a build from scratch...
>>
>> (but double-dollar make that assertion difficult to asses... I'd have to
>> double-check).
>
> OK, so I withdraw that comment: it *does* work. And the second shell
> eval is only called when needed! :-)
>
> Still, I'm not too fond of it being written in Makefile. Especially the
> part below...
>
>> Besides...
>>
>>> +# 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
>
> I am not usually one to argue against writting Makefile code in a
> Makefile. However, for this case, I think writting a shell rule is much
> more readable.

  +1 to that.

  And as Thomas said: my Rev-by no longer applies.

  Regards,
  Arnout

>
> And I don't see how your solution would be inherently different from
> mine: both use 'make -pqn' to get the list of targets and grep/sed the
> output to decide if the requested target exists...
>
> By the way, if your sed expression would have moved the colon ':'
> outside the ()-expression, you would not have had to repeqt the colon in
> the filter match. Also, using extended regexps would allow using
> character classes.
>
>      $(2)_KCONFIG_RULES = \
>          $$(shell $$($(2)_KCONFIG_MAKE) -pn config 2>/dev/null | \
>              sed -r '/^([[:alnum:]]*config):.*/!d; s//\1/;')
>
> Again, I think a shell construct is either to read.
>
> Regards,
> Yann E. MORIN.
>
>>>   # 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
>>>
>>
>> --
>> .-----------------.--------------------.------------------.--------------------.
>> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
>> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
>> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
>> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
>> '------------------------------^-------^------------------^--------------------'
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF