Re: [Buildroot] [PATCH 2/4] package/pkg-kconfig: generate generic help

[Arnout Vandecappelle <arnout@mind.be>]

On 24/07/2021 23:19, Yann E. MORIN wrote:
> Currently, as Thomas pointed out [0], the help for kconfig packages is
> not consistently used and handled by the different packages.
> 
> This commit introduces a generic help text for kconfig packages, that is
> based on what the package declares:
> 
>   - the list of kconfig editors it supports;
> 
>   - whether it is possible to save back the configuration (impossible if
>     the package uses an in-tree defconfig file);
> 
>   - whether the package actually supports (loading and saving) defconfig
>     files, by introducing a new variable a package can set if it does
>     not (only busybox is known to be in that case).
> 
> That new help helper is only used if the package does not already define
> its own help, to be consistent with what we do for other _CMDS.
> 
> [0] http://lists.busybox.net/pipermail/buildroot/2021-July/313570.html
> 
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> ---
>  docs/manual/adding-packages-kconfig.txt |  3 +++
>  package/pkg-kconfig.mk                  | 24 ++++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/docs/manual/adding-packages-kconfig.txt b/docs/manual/adding-packages-kconfig.txt
> index 8f40ea6099..a35681775f 100644
> --- a/docs/manual/adding-packages-kconfig.txt
> +++ b/docs/manual/adding-packages-kconfig.txt
> @@ -91,3 +91,6 @@ be set to suit the needs of the package under consideration:
>  * +FOO_KCONFIG_DEPENDENCIES+: the list of packages (most probably, host
>    packages) that need to be built before this package's kconfig is
>    interpreted. Seldom used. By default, empty.
> +
> +* +FOO_KCONFIG_SUPPORTS_DEFCONFIG+: whether the package's kconfig system
> +  supports using defconfig files; few packages do not. By default, 'YES'.
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index 49074f9220..715c3e04ec 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -85,11 +85,35 @@ $(2)_KCONFIG_OPTS ?=
>  $(2)_KCONFIG_FIXUP_CMDS ?=
>  $(2)_KCONFIG_FRAGMENT_FILES ?=
>  $(2)_KCONFIG_DOTCONFIG ?= .config
> +$(2)_KCONFIG_SUPPORTS_DEFCONFIG ?= YES
>  
>  # Register the kconfig dependencies as regular dependencies, so that
>  # they are also accounted for in the generated graphs.
>  $(2)_DEPENDENCIES += $$($(2)_KCONFIG_DEPENDENCIES)
>  
> +# Generate the kconfig-related help: one entry for each editor.
> +# Additionally, if the package is *not* using an in-tree defconfig
> +# name, an entry for updating the package configuration file.
> +ifndef $(2)_HELP_CMDS
> +define $(2)_HELP_CMDS
> +	$$(foreach editor, $$($(2)_KCONFIG_EDITORS), \
> +		@printf '  %-22s - Run %s %s\n' $(1)-$$(editor) $(1) $$(editor)
> +	)
> +	$$(if $$($(2)_KCONFIG_DEFCONFIG),,\

 We should probably also simply remove the -update-config and -update-defconfig
rules when _KCONFIG_DEFCONFIG is set. There's currently a condition for that in
kconfig-package-update-config, but it's better to just not have the option at
all. I may cook a patch for that.

> +		$$(if $$(filter YES,$$($(2)_KCONFIG_SUPPORTS_DEFCONFIG)),\


 Now we have this variable, it would make sense to also make $(1)-savedefconfig
and $(1)-update-defconfig depend on it. I may cook a patch to do exactly that.



 Regards,
 Arnout


> +			@printf '  %-22s - Save the %s configuration as a defconfig file\n' \
> +				$(1)-update-defconfig $(1)
> +			@printf '  %-22s     to %s\n' '' $$($(2)_KCONFIG_FILE)
> +			@printf '  %-22s     (or override with %s_KCONFIG_FILE)\n' '' $(2)
> +		)
> +		@printf '  %-22s - Save the %s configuration as a full .config file\n' \
> +			$(1)-update-config $(1)
> +		@printf '  %-22s     to %s\n' '' $$($(2)_KCONFIG_FILE)
> +		@printf '  %-22s     (or override with %s_KCONFIG_FILE)\n' '' $(2)
> +	)
> +endef
> +endif
> +
>  # Call the generic package infrastructure to generate the necessary
>  # make targets.
>  # Note: this must be done _before_ attempting to use $$($(2)_DIR) in a
> 
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot