[Buildroot] [PATCH] netsnmp: add option to enable debugging code

[Buildroot] [PATCH] netsnmp: add option to enable debugging code

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 package/netsnmp/Config.in  |    9 +++++++++
 package/netsnmp/netsnmp.mk |    6 +++++-
 2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/package/netsnmp/Config.in b/package/netsnmp/Config.in
index 431be2d..108ab83 100644
--- a/package/netsnmp/Config.in
+++ b/package/netsnmp/Config.in
@@ -14,3 +14,12 @@ config BR2_PACKAGE_NETSNMP_ENABLE_MIBS
 	  The net-snmp package contains a selection of MIB files.
 	  Say yes if you want those MIB files installed on the target
 	  and enable the code that parses the MIB files.
+
+config BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING
+	bool "Enable debugging code"
+	depends on BR2_PACKAGE_NETSNMP
+	help
+	  By default net-snmp is compiled without debugging support
+	  (--disable-debugging). Enable this options if you need debugging
+	  support, including the ability to log with DEBUGMSG(),
+	  DEBUGMSGTL() and companion macros.
diff --git a/package/netsnmp/netsnmp.mk b/package/netsnmp/netsnmp.mk
index 959e39f..448e3fd 100644
--- a/package/netsnmp/netsnmp.mk
+++ b/package/netsnmp/netsnmp.mk
@@ -12,7 +12,7 @@ NETSNMP_CONF_ENV = ac_cv_NETSNMP_CAN_USE_SYSCTL=yes
 NETSNMP_CONF_OPT = --with-persistent-directory=/var/lib/snmp --disable-static \
 		--with-defaults --enable-mini-agent --without-rpm \
 		--with-logfile=none --without-kmem-usage $(DISABLE_IPV6) \
-		--enable-as-needed --disable-debugging --without-perl-modules \
+		--enable-as-needed --without-perl-modules \
 		--disable-embedded-perl --disable-perl-cc-checks \
 		--disable-scripts --with-default-snmp-version="1" \
 		--enable-silent-libtool --enable-mfd-rewrites \
@@ -55,6 +55,10 @@ define NETSNMP_REMOVE_MIBS_IPV6
 endef
 endif
 
+ifneq ($(BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING),y)
+	NETSNMP_CONF_OPT += --disable-debugging
+endif
+
 define NETSNMP_INSTALL_TARGET_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
 		DESTDIR=$(TARGET_DIR) install
-- 
1.7.5.4

[Buildroot] [PATCH] netsnmp: add option to enable debugging code

[Arnout Vandecappelle]

On Thursday 15 March 2012 22:04:27 Luca Ceresoli wrote:
> +ifneq ($(BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING),y)
> +       NETSNMP_CONF_OPT += --disable-debugging
> +endif

 Peter, is it correct that you prefer to have both the positive and the
negative option in such a situation?  I.e.

ifeq ($(BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING),y)
       NETSNMP_CONF_OPT += --enable-debugging
else
       NETSNMP_CONF_OPT += --disable-debugging
endif

or alternatively

NETSNMP_CONF_OPT += --$(if $(BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING),en,dis)able-debugging

 Otherwise,
Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


 Regards,
 Arnout

-- 
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH] netsnmp: add option to enable debugging code

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 Arnout> On Thursday 15 March 2012 22:04:27 Luca Ceresoli wrote:
 >> +ifneq ($(BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING),y)
 >> +       NETSNMP_CONF_OPT += --disable-debugging
 >> +endif

 Arnout>  Peter, is it correct that you prefer to have both the positive
 Arnout> and the negative option in such a situation?  I.e.

 Arnout> ifeq ($(BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING),y)
 Arnout>        NETSNMP_CONF_OPT += --enable-debugging
 Arnout> else
 Arnout>        NETSNMP_CONF_OPT += --disable-debugging
 Arnout> endif

I don't feel strongly about it, but the 2nd approach means you don't
need to look at configure.ac to check if it defaults to --enable (and it
won't break if the default is ever changed).

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] netsnmp: add option to enable debugging code

[Peter Korsgaard]

>>>>> "Luca" == Luca Ceresoli <luca@lucaceresoli.net> writes:

 Luca> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] netsnmp: add option to enable debugging code

[Luca Ceresoli]

Peter Korsgaard wrote:
>>>>>> "Arnout" == Arnout Vandecappelle<arnout@mind.be>  writes:
>   Arnout>  On Thursday 15 March 2012 22:04:27 Luca Ceresoli wrote:
>   >>  +ifneq ($(BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING),y)
>   >>  +       NETSNMP_CONF_OPT += --disable-debugging
>   >>  +endif
>
>   Arnout>   Peter, is it correct that you prefer to have both the positive
>   Arnout>  and the negative option in such a situation?  I.e.
>
>   Arnout>  ifeq ($(BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING),y)
>   Arnout>         NETSNMP_CONF_OPT += --enable-debugging
>   Arnout>  else
>   Arnout>         NETSNMP_CONF_OPT += --disable-debugging
>   Arnout>  endif
>
> I don't feel strongly about it, but the 2nd approach means you don't
> need to look at configure.ac to check if it defaults to --enable (and it
> won't break if the default is ever changed).
>

Arnout's version is definitely more robust, although less concise and more
boring to write.

But there are many other examples in the current Buildroot code without the
else part. So I think we should have a guideline to follow coherently.

It would be great to have a macro to generate all of this boilerplate code,
both robust and concise. I would love to be able to write something like:

   $(call ac-arg-endisable,BR2_PACKAGE_NETSNMP_ENABLE_DEBUGGING,debugging)

It's even on my TODO list, which is the first step to many great things!

Luca