diff mbox

netsnmp: fix libnl include path in configure script

Message ID 1481544668-900-1-git-send-email-julien.floret@6wind.com
State Changes Requested
Headers show

Commit Message

Julien Floret Dec. 12, 2016, 12:11 p.m. UTC
Search for libnl headers in staging directory, not in the host machine
/usr/include/libnl3 folder.

Signed-off-by: Julien Floret <julien.floret@6wind.com>
---
 package/netsnmp/netsnmp.mk | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Thomas Petazzoni Dec. 12, 2016, 9:26 p.m. UTC | #1
Hello,

On Mon, 12 Dec 2016 13:11:08 +0100, Julien Floret wrote:
> Search for libnl headers in staging directory, not in the host machine
> /usr/include/libnl3 folder.
> 
> Signed-off-by: Julien Floret <julien.floret@6wind.com>

Thanks for the patch. However, it's not the right way of fixing the
issue, I'm afraid.

> +define NETSNMP_FIX_LIBNL_INCLUDE_PATH
> +	$(SED) "s,-I/usr/include/libnl3,-I$(STAGING_DIR)/usr/include/libnl3,g" \
> +		$(@D)/configure
> +endef
> +
> +NETSNMP_PRE_CONFIGURE_HOOKS += NETSNMP_FIX_LIBNL_INCLUDE_PATH

The right fix is to modify configure.d/config_os_libs2 to avoid passing
-I/usr/include/libnl3, and then autoreconf the package. To do this, you
have a few options:

 - Remove -I/usr/include/libnl3 entirely, and assume the user will pass
   the appropriate options in CPPFLAGS.

 - Use -I=/usr/include/libnl3, gcc will automatically replace = by the
   compiler sysroot. However, I have no idea whether this also works
   with non-gcc compilers.

 - Change the --with-nl option so that it takes a path to the nl
   library.

Also, if netsnmp optionally uses libnl, then it should optionally
depend on it. Something like:

ifeq ($(BR2_PACKAGE_LIBNL),y)
NETSNMP_DEPENDENCIES += libnl
NETSNMP_CONF_OPTS += --with-nl
else
NETSNMP_CONF_OPTS += --without-nl
endif

Or, if you don't care about libnl support in netsnmp and don't want to
spend time fixing the problem, just add --without-nl to netsnmp
configuration options.

Thanks!

Thomas
Julien Floret Dec. 13, 2016, 2:54 p.m. UTC | #2
Hello Thomas,

> The right fix is to modify configure.d/config_os_libs2 to avoid passing
> -I/usr/include/libnl3, and then autoreconf the package. To do this, you
> have a few options:
>
>  - Remove -I/usr/include/libnl3 entirely, and assume the user will pass
>    the appropriate options in CPPFLAGS.
>
>  - Use -I=/usr/include/libnl3, gcc will automatically replace = by the
>    compiler sysroot. However, I have no idea whether this also works
>    with non-gcc compilers.
>
>  - Change the --with-nl option so that it takes a path to the nl
>    library.

Thanks for your analysis. Indeed, your proposals are much better than
patching the configure script on the fly ;)

In fact, I just saw that the issue has been addressed upstream:
libnl include paths & libs can now be overriden with configure environment
variables:
https://sourceforge.net/p/net-snmp/code/ci/852dcd644cfe4cfc7177649eaec163d6221f2be1/

So if you're ok with this, I suggest to:
- embed the upstream patch in package/netsnmp/
- trigger an autoreconf
- pass the right LIBNL3_{CFLAGS,LIBS} variables in NETSNMP_CONF_ENV.

> Also, if netsnmp optionally uses libnl, then it should optionally
> depend on it. Something like:
>
> ifeq ($(BR2_PACKAGE_LIBNL),y)
> NETSNMP_DEPENDENCIES += libnl
> NETSNMP_CONF_OPTS += --with-nl
> else
> NETSNMP_CONF_OPTS += --without-nl
> endif

OK, I will add this as well.

> Or, if you don't care about libnl support in netsnmp and don't want to
> spend time fixing the problem, just add --without-nl to netsnmp
> configuration options.
Peter Korsgaard Dec. 13, 2016, 3:26 p.m. UTC | #3
>>>>> "Julien" == Julien Floret <julien.floret@6wind.com> writes:

Hi,

 >> - Change the --with-nl option so that it takes a path to the nl
 >> library.

 > Thanks for your analysis. Indeed, your proposals are much better than
 > patching the configure script on the fly ;)

 > In fact, I just saw that the issue has been addressed upstream:
 > libnl include paths & libs can now be overriden with configure environment
 > variables:
 > https://sourceforge.net/p/net-snmp/code/ci/852dcd644cfe4cfc7177649eaec163d6221f2be1/

 > So if you're ok with this, I suggest to:
 > - embed the upstream patch in package/netsnmp/
 > - trigger an autoreconf
 > - pass the right LIBNL3_{CFLAGS,LIBS} variables in NETSNMP_CONF_ENV.

Yes, sounds good, except that the last step shouldn't be needed as
pkg-config (once you add host-pkgconf to NETSNMP_DEPENDENCIES) should
return the correct values.

Thanks!
diff mbox

Patch

diff --git a/package/netsnmp/netsnmp.mk b/package/netsnmp/netsnmp.mk
index 48273b51bcc1..c893ea6189d6 100644
--- a/package/netsnmp/netsnmp.mk
+++ b/package/netsnmp/netsnmp.mk
@@ -93,6 +93,13 @@  else
 NETSNMP_CONF_OPTS += --disable-applications
 endif
 
+define NETSNMP_FIX_LIBNL_INCLUDE_PATH
+	$(SED) "s,-I/usr/include/libnl3,-I$(STAGING_DIR)/usr/include/libnl3,g" \
+		$(@D)/configure
+endef
+
+NETSNMP_PRE_CONFIGURE_HOOKS += NETSNMP_FIX_LIBNL_INCLUDE_PATH
+
 define NETSNMP_REMOVE_BLOAT_MIBS
 	for mib in $(NETSNMP_BLOAT_MIBS); do \
 		rm -f $(TARGET_DIR)/usr/share/snmp/mibs/$$mib-MIB.txt; \