Re: [Buildroot] [PATCH 2/3] package/watchdogd: convert Config.in options, int -> bool

["Yann E. MORIN" <yann.morin.1998@free.fr>]

On 2024-01-15 08:41 +0100, Joachim Wiberg spake thusly:
> Enabling the optional system monitor plugins have changed syntax
> upstream.  The `--with-foo=SECONDS` is now `--with-foo`.
> 
> This patch converts the menuconfig options from int to bool by renaming
> the config options and adding legacy option conversion support.

This change should be done as part of the version bump. Indeed, if the
version bump was applied but not this path (see below why), the .mk
would not set the proper options at build time.

So this really belongs to the version bump, and I was about to merge the
two together, when...

    $ make check-package
    package/watchdogd/Config.in:32: BR2_PACKAGE_WATCHDOGD_GENERIC_POLL referenced but not defined
    package/watchdogd/Config.in:38: BR2_PACKAGE_WATCHDOGD_LOADAVG_POLL referenced but not defined
    package/watchdogd/Config.in:44: BR2_PACKAGE_WATCHDOGD_FILENR_POLL referenced but not defined
    package/watchdogd/Config.in:50: BR2_PACKAGE_WATCHDOGD_MEMINFO_POLL referenced but not defined

...

> Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
> ---
>  package/watchdogd/Config.in    | 36 +++++++++++++++-------------------
>  package/watchdogd/watchdogd.mk | 17 ++++++++--------
>  2 files changed, 25 insertions(+), 28 deletions(-)
> 
> diff --git a/package/watchdogd/Config.in b/package/watchdogd/Config.in
> index ca5933848d..db1bee94c2 100644
> --- a/package/watchdogd/Config.in
> +++ b/package/watchdogd/Config.in
> @@ -27,32 +27,28 @@ config BR2_PACKAGE_WATCHDOGD_TEST_SUITE
>  	  They can be used to verify correct operation of watchdogd and
>  	  the kernel watchdog driver.
>  
> -config BR2_PACKAGE_WATCHDOGD_GENERIC_POLL
> -	int "Generic script monitor poll interval (sec)"
> -	default "300"
> +config BR2_PACKAGE_WATCHDOGD_GENERIC
> +	bool "Generic script monitor"
> +	default y if BR2_PACKAGE_WATCHDOGD_GENERIC_POLL != 0 # legacy

... Indeed BR2_PACKAGE_WATCHDOGD_GENERIC_POLL is the old config option
name. but it is now defined nowhere.

When we want to handle legacy symbols, we add the old symbols to the
top-level Config.in.legacy file. For boolweans, it is trivial, but for
int (or strings) we need a little trick, like (adapt the help text as
appropriate, I'm mostly making the reason up):

    Config.in.legacy:

    config BR2_PACKAGE_WATCHDOGD_GENERIC_POLL
        int
        default 0
        help
          The BR2_PACKAGE_WATCHDOGD_GENERIC_POLL value is now runtime
          configurable, and only the generic feature is configurable
          with BR2_PACKAGE_WATCHDOGD_GENERIC.

    config BR2_PACKAGE_WATCHDOGD_GENERIC_POLL_WRAP
        bool
        default y if BR2_PACKAGE_WATCHDOGD_GENERIC_POLL != 0
        select BR2_LEGACY

And then in the package Config.in, what you did is OK.

Care to fix up, squash patches 1 and 2, and respin, please?

Regards,
Yann E. MORIN.

>  	help
> -	  Poll interval for generic script monitor, in seconds.  A value
> -	  of zero (0) disables the monitor.
> +	  Enable generic script monitor.
>  
> -config BR2_PACKAGE_WATCHDOGD_LOADAVG_POLL
> -	int "CPU load average monitor poll interval (sec)"
> -	default "300"
> +config BR2_PACKAGE_WATCHDOGD_LOADAVG
> +	bool "CPU load average monitor"
> +	default y if BR2_PACKAGE_WATCHDOGD_LOADAVG_POLL != 0 # legacy
>  	help
> -	  Poll interval for CPU load average monitor, in seconds.  A
> -	  value of zero (0) disables the monitor.
> +	  Enable CPU load average monitor.
>  
> -config BR2_PACKAGE_WATCHDOGD_FILENR_POLL
> -	int "File descriptor leak monitor poll interval (sec)"
> -	default "300"
> +config BR2_PACKAGE_WATCHDOGD_FILENR
> +	bool "File descriptor leak monitor"
> +	default y if BR2_PACKAGE_WATCHDOGD_FILENR_POLL != 0 # legacy
>  	help
> -	  Poll interval for file descriptor leak monitor, in seconds.  A
> -	  value of zero (0) disables the monitor.
> +	  Enable file descriptor leak monitor.
>  
> -config BR2_PACKAGE_WATCHDOGD_MEMINFO_POLL
> -	int "Memory leak monitor poll interval (sec)"
> -	default "300"
> +config BR2_PACKAGE_WATCHDOGD_MEMINFO
> +	bool "Memory leak monitor"
> +	default y if BR2_PACKAGE_WATCHDOGD_MEMINFO_POLL != 0 # legacy
>  	help
> -	  Poll interval for memory leak monitor, in seconds.  A value of
> -	  zero (0) disables the monitor.
> +	  Enable memory leak monitor.
>  
>  endif
> diff --git a/package/watchdogd/watchdogd.mk b/package/watchdogd/watchdogd.mk
> index d140039540..9af71dbfaa 100644
> --- a/package/watchdogd/watchdogd.mk
> +++ b/package/watchdogd/watchdogd.mk
> @@ -20,28 +20,29 @@ else
>  WATCHDOGD_CONF_OPTS += --enable-builtin-tests
>  endif
>  
> -ifeq ($(BR2_PACKAGE_WATCHDOGD_GENERIC_POLL),0)
> +ifeq ($(BR2_PACKAGE_WATCHDOGD_GENERIC),y)
>  WATCHDOGD_CONF_OPTS += --without-generic
>  else
> -WATCHDOGD_CONF_OPTS += --with-generic=$(BR2_PACKAGE_WATCHDOGD_GENERIC_POLL)
> +WATCHDOGD_CONF_OPTS += --with-generic
>  endif
>  
> -ifeq ($(BR2_PACKAGE_WATCHDOGD_LOADAVG_POLL),0)
> +ifeq ($(BR2_PACKAGE_WATCHDOGD_LOADAVG),y)
>  WATCHDOGD_CONF_OPTS += --without-loadavg
>  else
> -WATCHDOGD_CONF_OPTS += --with-loadavg=$(BR2_PACKAGE_WATCHDOGD_LOADAVG_POLL)
> +WATCHDOGD_CONF_OPTS += --with-loadavg
>  endif
>  
> -ifeq ($(BR2_PACKAGE_WATCHDOGD_FILENR_POLL),0)
> +ifeq ($(BR2_PACKAGE_WATCHDOGD_FILENR),y)
>  WATCHDOGD_CONF_OPTS += --without-filenr
>  else
> -WATCHDOGD_CONF_OPTS += --with-filenr=$(BR2_PACKAGE_WATCHDOGD_FILENR_POLL)
> +WATCHDOGD_CONF_OPTS += --with-filenr
>  endif
>  
> -ifeq ($(BR2_PACKAGE_WATCHDOGD_MEMINFO_POLL),0)
> +ifeq ($(BR2_PACKAGE_WATCHDOGD_MEMINFO),y)
>  WATCHDOGD_CONF_OPTS += --without-meminfo
>  else
> -WATCHDOGD_CONF_OPTS += --with-meminfo=$(BR2_PACKAGE_WATCHDOGD_MEMINFO_POLL)
> +WATCHDOGD_CONF_OPTS += --with-meminfo
> +endif
>  endif
>  
>  define WATCHDOGD_INSTALL_INIT_SYSV
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot