Re: [Buildroot] [PATCH v2 1/1] package/pkg-utils: prevent kconfig_enable_opt from changing =m to =y

[Arnout Vandecappelle <arnout@mind.be>]

On 17/05/2022 12:36, Christian Stewart via buildroot wrote:
> The KCONFIG_ENABLE_OPT is intended to enable a required kernel configuration
> option when a package requires it.
> 
> However, this will often override an existing enabled module with `=m` with `=y`
> which overrides the module to be built-in instead of separate.
> 
> This is undesirable behavior; we often want these as `=m` and not `=y` to reduce
> the size of the kernel image.
> 
> This patch changes KCONFIG_MUNGE_DOT_CONFIG to prevent changing `=m` to `=y`.

  Good catch!

  Patch is a bit over-complicated, but I don't know how to do better.

> 
> Signed-off-by: Christian Stewart <christian@paral.in>
> 
> ---
> 
> v1 -> v2:
> 
>   - fix indentation spacing: use tab instead of spaces
>   - simplify by using $(1) instead of two separate variables
> 
> Signed-off-by: Christian Stewart <christian@paral.in>
> ---
>   package/pkg-utils.mk | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index 7d1aea7710..218d6ea9b0 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -21,8 +21,14 @@ KCONFIG_DOT_CONFIG = $(strip \
>   )
>   
>   # KCONFIG_MUNGE_DOT_CONFIG (option, newline [, file])
> +# If setting to =y and the option is already set to =m, ignore.
>   define KCONFIG_MUNGE_DOT_CONFIG
> -	$(SED) "/\\<$(strip $(1))\\>/d" $(call KCONFIG_DOT_CONFIG,$(3))
> +	if [[ "$(lastword $(subst =, ,$(strip $(2))))" == "y" ]]; then \

  This part we could avoid by adding a 4th option and setting it only on 
KCONFIG_ENABLE_OPT.

> +		if grep -q "$(strip $(1))=m" $(call KCONFIG_DOT_CONFIG,$(3)); then \
> +			exit 0; \

  This is a kind of hard-to-follow control structure - better turn around the 
condition and do the replacement inside it. So, combined with the above:

	if $(if $(4),grep -q ...,true); then \
		sed ...; \
		echo ...; \
	fi



  Regards,
  Arnout

> +		fi; \
> +	fi; \
> +	$(SED) "/\\<$(strip $(1))\\>/d" $(call KCONFIG_DOT_CONFIG,$(3)); \
>   	echo '$(strip $(2))' >> $(call KCONFIG_DOT_CONFIG,$(3))
>   endef
>   
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot