[Buildroot] [PATCH v4 0/3] core/pkg-kconfig: Fix .config generation

[Buildroot] [PATCH v4 0/3] core/pkg-kconfig: Fix .config generation

[Romain Izard]

To use an existing file as .config for a project, it needs to be
validated with 'make oldconfig'. If the file does not match all the
configuration entries of the project, the user will be prompted to
select the values for the new entries.

Historically, 'yes "" | make oldconfig' was used to silently select the
default values for new entries. But this does not work well with the
Linux kernel, as it will not select the correct option when configuring
tristate choice entries.

The Linux kernel has added a 'oldnoconfig' target, since renamed
'olddefconfig', that will work correctly, but this target does not exist
in all projects that rely on the kconfig workflow.

Update buildroot to use 'oldnoconfig' when available, and fallback to
the default method otherwise.

Changelog:
v1: creation as a package option
v2: try 'make oldnoconfig' before 'yes "" | make oldconfig' and remove
    the option
v3: Remove useless $(call ...), and factorize the make command
v4: Fix typo in the commit message

Romain Izard (3):
  core/pkg-kconfig: Factorize make commands
  core/pkg-kconfig: Do not use $(call ...) without parameters
  core/pkg-kconfig: Use oldnoconfig when possible

 package/pkg-kconfig.mk | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

-- 
2.5.0

[Buildroot] [PATCH v4 1/3] core/pkg-kconfig: Factorize make commands

[Romain Izard]

Use a variable to factorize the calls to make, as we use the same set
of options multiple times.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
Changelog:
v3: new
v4: fix commit message

 package/pkg-kconfig.mk | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index 94ec3cc..cb177bb 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -57,6 +57,9 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
 		fi; \
 	done
 
+$(2)_KCONFIG_MAKE = \
+	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS)
+
 # 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
@@ -64,13 +67,11 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
 # done even when the package does not support defconfigs.
 $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
 	$$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \
-		$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
-			$$($(2)_KCONFIG_OPTS) $$($(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)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
-		$$($(2)_KCONFIG_OPTS) oldconfig
+	$$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig
 
 # 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
@@ -81,8 +82,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)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
-		$$($(2)_KCONFIG_OPTS) oldconfig
+	$$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig
 	$$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done
 endef
 
-- 
2.5.0

[Buildroot] [PATCH v4 1/3] core/pkg-kconfig: Factorize make commands

[Yann E. MORIN]

Romain, All,

On 2016-04-18 11:23 +0200, Romain Izard spake thusly:
> Use a variable to factorize the calls to make, as we use the same set
> of options multiple times.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

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

Regards,
Yann E. MORIN.

> ---
> Changelog:
> v3: new
> v4: fix commit message
> 
>  package/pkg-kconfig.mk | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index 94ec3cc..cb177bb 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -57,6 +57,9 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
>  		fi; \
>  	done
>  
> +$(2)_KCONFIG_MAKE = \
> +	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS)
> +
>  # 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
> @@ -64,13 +67,11 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
>  # done even when the package does not support defconfigs.
>  $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
>  	$$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \
> -		$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
> -			$$($(2)_KCONFIG_OPTS) $$($(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)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
> -		$$($(2)_KCONFIG_OPTS) oldconfig
> +	$$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig
>  
>  # 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
> @@ -81,8 +82,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)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \
> -		$$($(2)_KCONFIG_OPTS) oldconfig
> +	$$(Q)yes "" | $$($(2)_KCONFIG_MAKE) oldconfig
>  	$$(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 v4 1/3] core/pkg-kconfig: Factorize make commands

[Thomas Petazzoni]

Hello,

On Mon, 18 Apr 2016 11:23:01 +0200, Romain Izard wrote:
> Use a variable to factorize the calls to make, as we use the same set
> of options multiple times.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
> Changelog:
> v3: new
> v4: fix commit message

Applied to master, thanks.

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

[Buildroot] [PATCH v4 2/3] core/pkg-kconfig: Do not use $(call ...) without parameters

[Romain Izard]

When there are no parameters, simply use $(var) instead of $(call var)

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
Changelog:
v3: new
v4: no change

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

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index cb177bb..4033915 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -87,7 +87,7 @@ define $(2)_FIXUP_DOT_CONFIG
 endef
 
 $$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config
-	$$(call $(2)_FIXUP_DOT_CONFIG)
+	$$($(2)_FIXUP_DOT_CONFIG)
 
 # Before running configure, the configuration file should be present and fixed
 $$($(2)_TARGET_CONFIGURE): $$($(2)_DIR)/.stamp_kconfig_fixup_done
@@ -139,7 +139,7 @@ $$($(2)_DIR)/.kconfig_editor_%: $$($(2)_DIR)/.stamp_kconfig_fixup_done
 		$$($(2)_KCONFIG_OPTS) $$(*)
 	rm -f $$($(2)_DIR)/.stamp_{kconfig_fixup_done,configured,built}
 	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
-	$$(call $(2)_FIXUP_DOT_CONFIG)
+	$$($(2)_FIXUP_DOT_CONFIG)
 
 # Saving back the configuration
 #
-- 
2.5.0

[Buildroot] [PATCH v4 2/3] core/pkg-kconfig: Do not use $(call ...) without parameters

[Yann E. MORIN]

Romain, All,

On 2016-04-18 11:23 +0200, Romain Izard spake thusly:
> When there are no parameters, simply use $(var) instead of $(call var)
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

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

Regards,
Yann E. MORIN.

> ---
> Changelog:
> v3: new
> v4: no change
> 
>  package/pkg-kconfig.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index cb177bb..4033915 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -87,7 +87,7 @@ define $(2)_FIXUP_DOT_CONFIG
>  endef
>  
>  $$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config
> -	$$(call $(2)_FIXUP_DOT_CONFIG)
> +	$$($(2)_FIXUP_DOT_CONFIG)
>  
>  # Before running configure, the configuration file should be present and fixed
>  $$($(2)_TARGET_CONFIGURE): $$($(2)_DIR)/.stamp_kconfig_fixup_done
> @@ -139,7 +139,7 @@ $$($(2)_DIR)/.kconfig_editor_%: $$($(2)_DIR)/.stamp_kconfig_fixup_done
>  		$$($(2)_KCONFIG_OPTS) $$(*)
>  	rm -f $$($(2)_DIR)/.stamp_{kconfig_fixup_done,configured,built}
>  	rm -f $$($(2)_DIR)/.stamp_{target,staging,images}_installed
> -	$$(call $(2)_FIXUP_DOT_CONFIG)
> +	$$($(2)_FIXUP_DOT_CONFIG)
>  
>  # Saving back the configuration
>  #
> -- 
> 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 v4 2/3] core/pkg-kconfig: Do not use $(call ...) without parameters

[Thomas Petazzoni]

Hello,

On Mon, 18 Apr 2016 11:23:02 +0200, Romain Izard wrote:
> When there are no parameters, simply use $(var) instead of $(call var)
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
> Changelog:
> v3: new
> v4: no change

Applied to master, thanks.

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

[Buildroot] [PATCH v4 3/3] core/pkg-kconfig: Use oldnoconfig when possible

[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.

Try to use 'oldnoconfig' by default, using 'oldconfig' as a fallback

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
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

 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..e04cfd5 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -60,18 +60,28 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
 $(2)_KCONFIG_MAKE = \
 	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS)
 
+# 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
+	$$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \
+		(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 +92,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 v4 3/3] core/pkg-kconfig: Use oldnoconfig when possible

[Yann E. MORIN]

Romain, All,

On 2016-04-18 11:23 +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.
> 
> Try to use 'oldnoconfig' by default, using 'oldconfig' as a fallback
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Usually, reviewd-by tags are staged after the signed-off-by tag, as in a
temporal mark: first signed-off-by, then reviewed.

> ---
> 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
> 
>  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..e04cfd5 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -60,18 +60,28 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
>  $(2)_KCONFIG_MAKE = \
>  	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS)
>  
> +# 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
> +	$$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \
> +		(yes "" | $$($(2)_KCONFIG_MAKE) oldconfig)

What I don't much like here, is that we conflate two things:
  - a non-zero return code because of a missing oldnoconfig,
  - a non-zero return code because of an error found by olnoconfig.

What I would have loved however, would be a way to detect whether
oldnoconfig (better, olddefconfig) was supported before calling it.
Maybe something along the lines of:

    if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \
        $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \
    elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \
        $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \
    else
        (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \
    fi
        
Yes, this is more code, less compact, but it is more systematic. At the
very least, it would not fallback to the yes|oldconfig trick in case
old{def,no}config do exist but fail, which we do want to catch.

BTW, the make trick above is highly (as in: almost identical) from the
bash-autocompletion package.

Regards,
Yann E. MORIN.

> +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 +92,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 v4 3/3] core/pkg-kconfig: Use oldnoconfig when possible

[Yann E. MORIN]

Romain, All,

On 2016-04-18 23:08 +0200, Yann E. MORIN spake thusly:
> On 2016-04-18 11:23 +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.
> > 
> > Try to use 'oldnoconfig' by default, using 'oldconfig' as a fallback
> > 
> > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> 
> Usually, reviewd-by tags are staged after the signed-off-by tag, as in a
> temporal mark: first signed-off-by, then reviewed.
> 
> > ---
> > 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
> > 
> >  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..e04cfd5 100644
> > --- a/package/pkg-kconfig.mk
> > +++ b/package/pkg-kconfig.mk
> > @@ -60,18 +60,28 @@ $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES): | $(1)-patch
> >  $(2)_KCONFIG_MAKE = \
> >  	$$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) $$($(2)_KCONFIG_OPTS)
> >  
> > +# 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
> > +	$$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \
> > +		(yes "" | $$($(2)_KCONFIG_MAKE) oldconfig)
> 
> What I don't much like here, is that we conflate two things:
>   - a non-zero return code because of a missing oldnoconfig,
>   - a non-zero return code because of an error found by olnoconfig.
> 
> What I would have loved however, would be a way to detect whether
> oldnoconfig (better, olddefconfig) was supported before calling it.
> Maybe something along the lines of:
> 
>     if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \

Hmmm, incomplete (missing colon). Should have been:

    ... |grep -E '^olddefconfig:' >/dev/null

>         $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \
>     elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \

Ditto, with oldnoconfig...

Regards,
Yann E. MORIN.

>         $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \
>     else
>         (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \
>     fi
>         
> Yes, this is more code, less compact, but it is more systematic. At the
> very least, it would not fallback to the yes|oldconfig trick in case
> old{def,no}config do exist but fail, which we do want to catch.
> 
> BTW, the make trick above is highly (as in: almost identical) from the
> bash-autocompletion package.
> 
> Regards,
> Yann E. MORIN.
> 
> > +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 +92,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 v4 3/3] core/pkg-kconfig: Use oldnoconfig when possible

[Romain Izard]

Hi Yann,

2016-04-18 23:08 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
> Romain, All,
>
> On 2016-04-18 11:23 +0200, Romain Izard spake thusly:
>
>> +# 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
>> +     $$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \
>> +             (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig)
>
> What I don't much like here, is that we conflate two things:
>   - a non-zero return code because of a missing oldnoconfig,
>   - a non-zero return code because of an error found by olnoconfig.
>
> What I would have loved however, would be a way to detect whether
> oldnoconfig (better, olddefconfig) was supported before calling it.
> Maybe something along the lines of:
>
>     if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \
>         $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \
>     elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \
>         $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \
>     else
>         (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \
>     fi
>
> Yes, this is more code, less compact, but it is more systematic. At
> the very least, it would not fallback to the yes|oldconfig trick in
> case old{def,no}config do exist but fail, which we do want to catch.
>
> BTW, the make trick above is highly (as in: almost identical) from the
> bash-autocompletion package.

I see the idea, but the specific implementation is not working: the
kernel's Makefile targets are dynamic, and the targets we want to check
are only present when the target we try to build matches %config.

I will try to find a syntax that works for all projects that use the
kconfig-package infrastructure.

Best regards,
-- 
Romain Izard

[Buildroot] [PATCH v4 3/3] core/pkg-kconfig: Use oldnoconfig when possible

[Yann E. MORIN]

Romain, All,

On 2016-04-19 11:11 +0200, Romain Izard spake thusly:
> 2016-04-18 23:08 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
> > Romain, All,
> >
> > On 2016-04-18 11:23 +0200, Romain Izard spake thusly:
> >
> >> +# 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
> >> +     $$(Q)$$($(2)_KCONFIG_MAKE) oldnoconfig || \
> >> +             (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig)
> >
> > What I don't much like here, is that we conflate two things:
> >   - a non-zero return code because of a missing oldnoconfig,
> >   - a non-zero return code because of an error found by olnoconfig.
> >
> > What I would have loved however, would be a way to detect whether
> > oldnoconfig (better, olddefconfig) was supported before calling it.
> > Maybe something along the lines of:
> >
> >     if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \
> >         $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \
> >     elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \
> >         $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \
> >     else
> >         (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \
> >     fi
> >
> > Yes, this is more code, less compact, but it is more systematic. At
> > the very least, it would not fallback to the yes|oldconfig trick in
> > case old{def,no}config do exist but fail, which we do want to catch.
> >
> > BTW, the make trick above is highly (as in: almost identical) from the
> > bash-autocompletion package.
> 
> I see the idea, but the specific implementation is not working: the
> kernel's Makefile targets are dynamic, and the targets we want to check
> are only present when the target we try to build matches %config.

I just checked, and it works on the current master on the kernel tree:

    $ make -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig:'
    olddefconfig: scripts/kconfig/conf

    $ make -pqn .DEFAULT 2>/dev/null |grep -E '^oldnoconfig:'
    oldnoconfig: olddefconfig

Regards,
Yann E. MORIN.

> I will try to find a syntax that works for all projects that use the
> kconfig-package infrastructure.
> 
> Best regards,
> -- 
> Romain Izard

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 v4 3/3] core/pkg-kconfig: Use oldnoconfig when possible

[Yann E. MORIN]

Romain, All,

On 2016-04-19 19:40 +0200, Yann E. MORIN spake thusly:
> On 2016-04-19 11:11 +0200, Romain Izard spake thusly:
> > 2016-04-18 23:08 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
[--SNIP--]
> > > What I would have loved however, would be a way to detect whether
> > > oldnoconfig (better, olddefconfig) was supported before calling it.
> > > Maybe something along the lines of:
> > >
> > >     if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \
> > >         $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \
> > >     elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \
> > >         $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \
> > >     else
> > >         (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \
> > >     fi
> > >
> > > Yes, this is more code, less compact, but it is more systematic. At
> > > the very least, it would not fallback to the yes|oldconfig trick in
> > > case old{def,no}config do exist but fail, which we do want to catch.
> > >
> > > BTW, the make trick above is highly (as in: almost identical) from the
> > > bash-autocompletion package.
> > 
> > I see the idea, but the specific implementation is not working: the
> > kernel's Makefile targets are dynamic, and the targets we want to check
> > are only present when the target we try to build matches %config.
> 
> I just checked, and it works on the current master on the kernel tree:
> 
>     $ make -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig:'
>     olddefconfig: scripts/kconfig/conf
> 
>     $ make -pqn .DEFAULT 2>/dev/null |grep -E '^oldnoconfig:'
>     oldnoconfig: olddefconfig

I also checked on v3.6 (which is missing olddefconfig) and v3.7 (which
introduced olddefconfig), and the tests above still work: oldnconfig is
found, olddefconfig is not.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 v4 3/3] core/pkg-kconfig: Use oldnoconfig when possible

[Romain Izard]

2016-04-19 19:58 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
> Romain, All,
>
> On 2016-04-19 19:40 +0200, Yann E. MORIN spake thusly:
>> On 2016-04-19 11:11 +0200, Romain Izard spake thusly:
>> > 2016-04-18 23:08 GMT+02:00 Yann E. MORIN <yann.morin.1998@free.fr>:
> [--SNIP--]
>> > > What I would have loved however, would be a way to detect whether
>> > > oldnoconfig (better, olddefconfig) was supported before calling it.
>> > > Maybe something along the lines of:
>> > >
>> > >     if [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig'`" ]; then \
>> > >         $$($(2)_KCONFIG_MAKE) olddefconfig || exit 1; \
>> > >     elif [ -n "`$$($(2)_KCONFIG_MAKE) -pqn .DEFAULT |grep -E '^oldnoconfig'`" ]; then \
>> > >         $$($(2)_KCONFIG_MAKE) oldnoconfig || exit 1; \
>> > >     else
>> > >         (yes "" | $$($(2)_KCONFIG_MAKE) oldconfig) || exit 1; \
>> > >     fi
>> > >
>> > > Yes, this is more code, less compact, but it is more systematic. At
>> > > the very least, it would not fallback to the yes|oldconfig trick in
>> > > case old{def,no}config do exist but fail, which we do want to catch.
>> > >
>> > > BTW, the make trick above is highly (as in: almost identical) from the
>> > > bash-autocompletion package.
>> >
>> > I see the idea, but the specific implementation is not working: the
>> > kernel's Makefile targets are dynamic, and the targets we want to check
>> > are only present when the target we try to build matches %config.
>>
>> I just checked, and it works on the current master on the kernel tree:
>>
>>     $ make -pqn .DEFAULT 2>/dev/null |grep -E '^olddefconfig:'
>>     olddefconfig: scripts/kconfig/conf
>>
>>     $ make -pqn .DEFAULT 2>/dev/null |grep -E '^oldnoconfig:'
>>     oldnoconfig: olddefconfig
>
> I also checked on v3.6 (which is missing olddefconfig) and v3.7 (which
> introduced olddefconfig), and the tests above still work: oldnconfig is
> found, olddefconfig is not.
>
When I tried it yesterday it did not work, but it works today. I must have done
something wrong when testing it.

Best regards,
-- 
Romain Izard