| Message ID | 20240115074112.157216-3-troglobit@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | package/watchdogd: version bump and major Config.in rewrite | expand |
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, thank you for taking the time to review my patch! I'll do a respin and send to the list later today. On Sat, 2024-01-27 at 18:18 +0100, Yann E. MORIN wrote: > On 2024-01-15 08:41 +0100, Joachim Wiberg spake thusly: > > [snip] > > 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 > > [snip] > ... Indeed BR2_PACKAGE_WATCHDOGD_GENERIC_POLL is the old config > option name. but it is now defined nowhere. Ouch, didn't think of that, thanks! > 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 Ahaaa, yeah I was hoping to get some guidance on that part. Great! > Care to fix up, squash patches 1 and 2, and respin, please? Absolutely, v2 coming up! /J
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 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
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. 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(-)