Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] package/collectd: add config option for lua
@ 2020-01-27 17:28 Tom Marcuzzi
  2020-01-30 22:26 ` Peter Korsgaard
  2020-02-02 22:56 ` Thomas Petazzoni
  0 siblings, 2 replies; 6+ messages in thread
From: Tom Marcuzzi @ 2020-01-27 17:28 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Tom Marcuzzi <tom.marcuzzi@orolia.com>
---
 package/collectd/Config.in   | 7 +++++++
 package/collectd/collectd.mk | 9 ++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/package/collectd/Config.in b/package/collectd/Config.in
index 402de219f4..303b5dc01b 100644
--- a/package/collectd/Config.in
+++ b/package/collectd/Config.in
@@ -63,6 +63,13 @@ config BR2_PACKAGE_COLLECTD_LOGSTASH
 	help
 	  Writes log messages formatted as logstash JSON events.
 
+config BR2_PACKAGE_COLLECTD_LUA
+	bool "lua"
+	select BR2_PACKAGE_LUA
+	help
+	  Embeds a Lua interpreter into collectd and provides an
+	  interface to collectd's plugin system.
+
 config BR2_PACKAGE_COLLECTD_NOTIFY_EMAIL
 	bool "notify_email"
 	depends on !BR2_STATIC_LIBS # libesmtp
diff --git a/package/collectd/collectd.mk b/package/collectd/collectd.mk
index 0cd86adbd4..9258977613 100644
--- a/package/collectd/collectd.mk
+++ b/package/collectd/collectd.mk
@@ -89,6 +89,7 @@ COLLECTD_CONF_OPTS += \
 	$(if $(BR2_PACKAGE_COLLECTD_LOAD),--enable-load,--disable-load) \
 	$(if $(BR2_PACKAGE_COLLECTD_LOGFILE),--enable-logfile,--disable-logfile) \
 	$(if $(BR2_PACKAGE_COLLECTD_LOGSTASH),--enable-log_logstash,--disable-log_logstash) \
+	$(if $(BR2_PACKAGE_COLLECTD_LUA),--enable-lua,--disable-lua) \
 	$(if $(BR2_PACKAGE_COLLECTD_LVM),--enable-lvm,--disable-lvm) \
 	$(if $(BR2_PACKAGE_COLLECTD_MD),--enable-md,--disable-md) \
 	$(if $(BR2_PACKAGE_COLLECTD_MEMCACHEC),--enable-memcachec,--disable-memcachec) \
@@ -162,6 +163,7 @@ COLLECTD_DEPENDENCIES = \
 	$(if $(BR2_PACKAGE_COLLECTD_GRPC),grpc) \
 	$(if $(BR2_PACKAGE_COLLECTD_IPTABLES),iptables) \
 	$(if $(BR2_PACKAGE_COLLECTD_LOGSTASH),yajl) \
+	$(if $(BR2_PACKAGE_COLLECTD_LUA),lua) \
 	$(if $(BR2_PACKAGE_COLLECTD_LVM),lvm2) \
 	$(if $(BR2_PACKAGE_COLLECTD_MEMCACHEC),libmemcached) \
 	$(if $(BR2_PACKAGE_COLLECTD_MODBUS),libmodbus) \
@@ -213,13 +215,6 @@ else
 COLLECTD_CONF_OPTS += --with-libgcrypt=no
 endif
 
-ifeq ($(BR2_PACKAGE_LUA),y)
-COLLECTD_DEPENDENCIES += lua
-COLLECTD_CONF_OPTS += --enable-lua
-else
-COLLECTD_CONF_OPTS += --disable-lua
-endif
-
 define COLLECTD_INSTALL_TARGET_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) DESTDIR=$(TARGET_DIR) -C $(@D) install
 	rm -f $(TARGET_DIR)/usr/bin/collectd-nagios
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] package/collectd: add config option for lua
  2020-01-27 17:28 [Buildroot] [PATCH] package/collectd: add config option for lua Tom Marcuzzi
@ 2020-01-30 22:26 ` Peter Korsgaard
  2020-01-31  9:14   ` Tom Marcuzzi
  2020-02-02 22:56 ` Thomas Petazzoni
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2020-01-30 22:26 UTC (permalink / raw)
  To: buildroot

>>>>> "Tom" == Tom Marcuzzi <tom.marcuzzi@orolia.com> writes:

 > Signed-off-by: Tom Marcuzzi <tom.marcuzzi@orolia.com>

Why? What is wrong with the automatic dependency?


 > ---
 >  package/collectd/Config.in   | 7 +++++++
 >  package/collectd/collectd.mk | 9 ++-------
 >  2 files changed, 9 insertions(+), 7 deletions(-)

 > diff --git a/package/collectd/Config.in b/package/collectd/Config.in
 > index 402de219f4..303b5dc01b 100644
 > --- a/package/collectd/Config.in
 > +++ b/package/collectd/Config.in
 > @@ -63,6 +63,13 @@ config BR2_PACKAGE_COLLECTD_LOGSTASH
 >  	help
 >  	  Writes log messages formatted as logstash JSON events.
 
 > +config BR2_PACKAGE_COLLECTD_LUA
 > +	bool "lua"
 > +	select BR2_PACKAGE_LUA
 > +	help
 > +	  Embeds a Lua interpreter into collectd and provides an
 > +	  interface to collectd's plugin system.
 > +
 >  config BR2_PACKAGE_COLLECTD_NOTIFY_EMAIL
 >  	bool "notify_email"
 >  	depends on !BR2_STATIC_LIBS # libesmtp
 > diff --git a/package/collectd/collectd.mk b/package/collectd/collectd.mk
 > index 0cd86adbd4..9258977613 100644
 > --- a/package/collectd/collectd.mk
 > +++ b/package/collectd/collectd.mk
 > @@ -89,6 +89,7 @@ COLLECTD_CONF_OPTS += \
 >  	$(if $(BR2_PACKAGE_COLLECTD_LOAD),--enable-load,--disable-load) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_LOGFILE),--enable-logfile,--disable-logfile) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_LOGSTASH),--enable-log_logstash,--disable-log_logstash) \
 > +	$(if $(BR2_PACKAGE_COLLECTD_LUA),--enable-lua,--disable-lua) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_LVM),--enable-lvm,--disable-lvm) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_MD),--enable-md,--disable-md) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_MEMCACHEC),--enable-memcachec,--disable-memcachec) \
 > @@ -162,6 +163,7 @@ COLLECTD_DEPENDENCIES = \
 >  	$(if $(BR2_PACKAGE_COLLECTD_GRPC),grpc) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_IPTABLES),iptables) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_LOGSTASH),yajl) \
 > +	$(if $(BR2_PACKAGE_COLLECTD_LUA),lua) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_LVM),lvm2) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_MEMCACHEC),libmemcached) \
 >  	$(if $(BR2_PACKAGE_COLLECTD_MODBUS),libmodbus) \
 > @@ -213,13 +215,6 @@ else
 >  COLLECTD_CONF_OPTS += --with-libgcrypt=no
 >  endif
 
 > -ifeq ($(BR2_PACKAGE_LUA),y)
 > -COLLECTD_DEPENDENCIES += lua
 > -COLLECTD_CONF_OPTS += --enable-lua
 > -else
 > -COLLECTD_CONF_OPTS += --disable-lua
 > -endif
 > -
 >  define COLLECTD_INSTALL_TARGET_CMDS
 >  	$(TARGET_MAKE_ENV) $(MAKE) DESTDIR=$(TARGET_DIR) -C $(@D) install
 >  	rm -f $(TARGET_DIR)/usr/bin/collectd-nagios
 > -- 
 > 2.17.1

 > _______________________________________________
 > buildroot mailing list
 > buildroot at busybox.net
 > http://lists.busybox.net/mailman/listinfo/buildroot

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] package/collectd: add config option for lua
  2020-01-30 22:26 ` Peter Korsgaard
@ 2020-01-31  9:14   ` Tom Marcuzzi
  2020-01-31  9:30     ` Peter Korsgaard
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Marcuzzi @ 2020-01-31  9:14 UTC (permalink / raw)
  To: buildroot

Hi Peter,

On 1/30/20 11:26 PM, Peter Korsgaard wrote:
>>>>>> "Tom" == Tom Marcuzzi <tom.marcuzzi@orolia.com> writes:
> 
>   > Signed-off-by: Tom Marcuzzi <tom.marcuzzi@orolia.com>
> 
> Why? What is wrong with the automatic dependency?
>  

It is not that there is anything wrong with the automatic dependency, 
but in my humble opinion adding the lua option may be a more logical way 
of doing in regards to the other collectd plugins, since every 
implemented plugin has a configuration option. This gives more 
flexibility and is more explicit since the user can choose himself 
whether he wants the plugin or not, which is not the case with the 
automatic dependency.

>   > ---
>   >  package/collectd/Config.in   | 7 +++++++
>   >  package/collectd/collectd.mk | 9 ++-------
>   >  2 files changed, 9 insertions(+), 7 deletions(-)
> 
>   > diff --git a/package/collectd/Config.in b/package/collectd/Config.in
>   > index 402de219f4..303b5dc01b 100644
>   > --- a/package/collectd/Config.in
>   > +++ b/package/collectd/Config.in
>   > @@ -63,6 +63,13 @@ config BR2_PACKAGE_COLLECTD_LOGSTASH
>   >      help
>   >        Writes log messages formatted as logstash JSON events.
> 
>   > +config BR2_PACKAGE_COLLECTD_LUA
>   > +    bool "lua"
>   > +    select BR2_PACKAGE_LUA
>   > +    help
>   > +      Embeds a Lua interpreter into collectd and provides an
>   > +      interface to collectd's plugin system.
>   > +
>   >  config BR2_PACKAGE_COLLECTD_NOTIFY_EMAIL
>   >      bool "notify_email"
>   >      depends on !BR2_STATIC_LIBS # libesmtp
>   > diff --git a/package/collectd/collectd.mk b/package/collectd/collectd.mk
>   > index 0cd86adbd4..9258977613 100644
>   > --- a/package/collectd/collectd.mk
>   > +++ b/package/collectd/collectd.mk
>   > @@ -89,6 +89,7 @@ COLLECTD_CONF_OPTS += \
>   >      $(if $(BR2_PACKAGE_COLLECTD_LOAD),--enable-load,--disable-load) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_LOGFILE),--enable-logfile,--disable-logfile) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_LOGSTASH),--enable-log_logstash,--disable-log_logstash) \
>   > +    $(if $(BR2_PACKAGE_COLLECTD_LUA),--enable-lua,--disable-lua) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_LVM),--enable-lvm,--disable-lvm) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_MD),--enable-md,--disable-md) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_MEMCACHEC),--enable-memcachec,--disable-memcachec) \
>   > @@ -162,6 +163,7 @@ COLLECTD_DEPENDENCIES = \
>   >      $(if $(BR2_PACKAGE_COLLECTD_GRPC),grpc) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_IPTABLES),iptables) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_LOGSTASH),yajl) \
>   > +    $(if $(BR2_PACKAGE_COLLECTD_LUA),lua) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_LVM),lvm2) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_MEMCACHEC),libmemcached) \
>   >      $(if $(BR2_PACKAGE_COLLECTD_MODBUS),libmodbus) \
>   > @@ -213,13 +215,6 @@ else
>   >  COLLECTD_CONF_OPTS += --with-libgcrypt=no
>   >  endif
> 
>   > -ifeq ($(BR2_PACKAGE_LUA),y)
>   > -COLLECTD_DEPENDENCIES += lua
>   > -COLLECTD_CONF_OPTS += --enable-lua
>   > -else
>   > -COLLECTD_CONF_OPTS += --disable-lua
>   > -endif
>   > -
>   >  define COLLECTD_INSTALL_TARGET_CMDS
>   >      $(TARGET_MAKE_ENV) $(MAKE) DESTDIR=$(TARGET_DIR) -C $(@D) install
>   >      rm -f $(TARGET_DIR)/usr/bin/collectd-nagios
>   > --
>   > 2.17.1
> 
>   > _______________________________________________
>   > buildroot mailing list
>   > buildroot at busybox.net
>   > http://lists.busybox.net/mailman/listinfo/buildroot
> 
> --
> Bye, Peter Korsgaard
> ATTENTION: This email came from an external source.
> Do not open attachments or click on links from unknown senders or unexpected emails.
> 

Regards,

Tom Marcuzzi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] package/collectd: add config option for lua
  2020-01-31  9:14   ` Tom Marcuzzi
@ 2020-01-31  9:30     ` Peter Korsgaard
  2020-01-31  9:50       ` Tom Marcuzzi
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2020-01-31  9:30 UTC (permalink / raw)
  To: buildroot

>>>>> "Tom" == Tom Marcuzzi <tom.marcuzzi@orolia.com> writes:

 > Hi Peter,
 > On 1/30/20 11:26 PM, Peter Korsgaard wrote:
 >>>>>>> "Tom" == Tom Marcuzzi <tom.marcuzzi@orolia.com> writes:
 >> 
 >> > Signed-off-by: Tom Marcuzzi <tom.marcuzzi@orolia.com>
 >> 
 >> Why? What is wrong with the automatic dependency?
 >> 

 > It is not that there is anything wrong with the automatic dependency, 
 > but in my humble opinion adding the lua option may be a more logical way 
 > of doing in regards to the other collectd plugins, since every 
 > implemented plugin has a configuration option. This gives more 
 > flexibility and is more explicit since the user can choose himself 
 > whether he wants the plugin or not, which is not the case with the 
 > automatic dependency.

But if your system has collectd and lua enabled, what would the reason
be to not enable lua support in collectd? How much extra size does this
add to collectd?

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] package/collectd: add config option for lua
  2020-01-31  9:30     ` Peter Korsgaard
@ 2020-01-31  9:50       ` Tom Marcuzzi
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Marcuzzi @ 2020-01-31  9:50 UTC (permalink / raw)
  To: buildroot

On 1/31/20 10:30 AM, Peter Korsgaard wrote:
>>>>>> "Tom" == Tom Marcuzzi <tom.marcuzzi@orolia.com> writes:
> 
>   > Hi Peter,
>   > On 1/30/20 11:26 PM, Peter Korsgaard wrote:
>   >>>>>>> "Tom" == Tom Marcuzzi <tom.marcuzzi@orolia.com> writes:
>   >>
>   >> > Signed-off-by: Tom Marcuzzi <tom.marcuzzi@orolia.com>
>   >>
>   >> Why? What is wrong with the automatic dependency?
>   >>
> 
>   > It is not that there is anything wrong with the automatic dependency,
>   > but in my humble opinion adding the lua option may be a more logical way
>   > of doing in regards to the other collectd plugins, since every
>   > implemented plugin has a configuration option. This gives more
>   > flexibility and is more explicit since the user can choose himself
>   > whether he wants the plugin or not, which is not the case with the
>   > automatic dependency.
> 
> But if your system has collectd and lua enabled, what would the reason
> be to not enable lua support in collectd? How much extra size does this
> add to collectd?

It adds on my system around 18KB. I agree that the reason not to enable 
lua support may be unclear. My patch is more about consistency, in 
regard to what has been done before for the previous plugins, which are 
not enabled by default.

Regards,

Tom Marcuzzi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] package/collectd: add config option for lua
  2020-01-27 17:28 [Buildroot] [PATCH] package/collectd: add config option for lua Tom Marcuzzi
  2020-01-30 22:26 ` Peter Korsgaard
@ 2020-02-02 22:56 ` Thomas Petazzoni
  1 sibling, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2020-02-02 22:56 UTC (permalink / raw)
  To: buildroot

On Mon, 27 Jan 2020 17:28:15 +0000
Tom Marcuzzi <tom.marcuzzi@orolia.com> wrote:

> Signed-off-by: Tom Marcuzzi <tom.marcuzzi@orolia.com>
> ---
>  package/collectd/Config.in   | 7 +++++++
>  package/collectd/collectd.mk | 9 ++-------
>  2 files changed, 9 insertions(+), 7 deletions(-)

I've applied, after extending the commit log. I discussed this with
Peter Korsgaard, and while we normally prefer automatic dependencies,
collected indeed has lots of sub-options for its different features, so
it kind of made sense to do the same for Lua.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-02 22:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-27 17:28 [Buildroot] [PATCH] package/collectd: add config option for lua Tom Marcuzzi
2020-01-30 22:26 ` Peter Korsgaard
2020-01-31  9:14   ` Tom Marcuzzi
2020-01-31  9:30     ` Peter Korsgaard
2020-01-31  9:50       ` Tom Marcuzzi
2020-02-02 22:56 ` Thomas Petazzoni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox