Skip to contentSkip to conversation

package/sngrep: add libgcrypt missing dependencies

PATCH New Romain Naour 3 participants 12 messages May 20, 2017 – May 25, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 package/sngrep: add libgcrypt missing dependencies

Even if libgcrypt is selected at Kconfig level, it's missing from Makefile level.

Fixes:
http://autobuild.buildroot.net/results/36c/36c28e7e49b31ed2398a67db2e9b08e3aad5a3ee

---
 package/sngrep/sngrep.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/sngrep/sngrep.mk +1 -1
index 74b05b1..7ae271f 100644
@@ -23,7 +23,7 @@ SNGREP_DEPENDENCIES += openssl
 SNGREP_CONF_OPTS += --with-openssl --without-gnutls
 # gnutls support also requires libgcrypt
 else ifeq ($(BR2_PACKAGE_GNUTLS)$(BR2_PACKAGE_LIBGCRYPT),yy)
-SNGREP_DEPENDENCIES += gnutls
+SNGREP_DEPENDENCIES += gnutls libgcrypt
 SNGREP_CONF_OPTS += --with-gnutls --without-openssl
 else
 SNGREP_CONF_OPTS += --without-gnutls --without-openssl
Signature · 1 line
2.9.4
#1 lore raw

PATCH 2/2 package/sngrep: fix static build with gnutls

Fixes:
http://autobuild.buildroot.net/results/f7f/f7fb42d3742f6f01000a0d181e0c785640284405

---
 ...re.ac-switch-to-pkg-config-to-find-gnutls.patch | 60 +++++++++++++++++++
 ...e-libgcrypt-config-to-link-with-gpg-error.patch | 69 ++++++++++++++++++++++
 package/sngrep/sngrep.mk                           |  3 +-
 3 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch
 create mode 100644 package/sngrep/0004-use-libgcrypt-config-to-link-with-gpg-error.patch
package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch new file +60 -0
new file mode 100644
index 0000000..67b21a2
@@ -0,0 +1,60 @@
+From b0428b0279b03d88520b79b651f692d9aea65f8c Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+Date: Sat, 20 May 2017 15:00:16 +0200
+Subject: [PATCH] configure.ac: switch to pkg-config to find gnutls.
+
+Like for openssl in patch [1] and for the same reason,
+use pkg-config to find gnutls.
+
+gnutls can be linked with :
+    -lintl -lgmp -lunistring -lhogweed -lnettle -ltasn1 -lz
+
+Fixes:
+http://autobuild.buildroot.net/results/f7f/f7fb42d3742f6f01000a0d181e0c785640284405
+
+Upstream: pending [2]
+
+[1] 2563b016ae16cb8cd39d7181917699ae82772296
+[2] https://github.com/irontec/sngrep/pull/188
+
+Signed-off-by: Romain Naour <[email protected]>
+---
+ configure.ac    | 5 +++++
+ src/Makefile.am | 2 ++
+ 2 files changed, 7 insertions(+)
+
+diff --git a/configure.ac b/configure.ac
+index 0255ad2..09271b2 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -106,9 +106,14 @@ AC_ARG_WITH([gnutls],
+ )
+ 
+ AS_IF([test "x$WITH_GNUTLS" == "xyes"], [
++
++    m4_ifdef([PKG_CHECK_MODULES], [
++        PKG_CHECK_MODULES([LIBGNUTLS], [gnutls])
++    ], [
+ 	AC_CHECK_LIB([gnutls], [gnutls_init], [], [
+ 	    AC_MSG_ERROR([ You need to have gnutls installed to compile sngrep])
+ 	])
++    ])
+ 
+ 	AC_CHECK_LIB([gcrypt], [gcry_md_map_name], [], [
+ 	    AC_MSG_ERROR([ You need to have libgcrypt installed to compile sngrep])
+diff --git a/src/Makefile.am b/src/Makefile.am
+index 3a471b7..e5b34fe 100644
+--- a/src/Makefile.am
++++ b/src/Makefile.am
+@@ -8,6 +8,8 @@ sngrep_SOURCES+=capture_eep.c
+ endif
+ if WITH_GNUTLS
+ sngrep_SOURCES+=capture_gnutls.c
++sngrep_CFLAGS+=$(LIBGNUTLS_CFLAGS)
++sngrep_LDADD+=$(LIBGNUTLS_LIBS)
+ endif
+ if WITH_OPENSSL
+ sngrep_SOURCES+=capture_openssl.c
+-- 
+2.9.4
+
package/sngrep/0004-use-libgcrypt-config-to-link-with-gpg-error.patch new file +69 -0

69 lines not on this page · raw message

package/sngrep/sngrep.mk +2 -1
index 7ae271f..809c5db 100644
@@ -12,7 +12,8 @@ SNGREP_AUTORECONF = YES
 SNGREP_DEPENDENCIES = libpcap ncurses host-pkgconf
 
 SNGREP_CONF_ENV += \
-	$(if $(BR2_STATIC_LIBS),LIBS="`$(STAGING_DIR)/usr/bin/pcap-config --static --libs`")
+	$(if $(BR2_STATIC_LIBS),LIBS="`$(STAGING_DIR)/usr/bin/pcap-config --static --libs`") \
+	LIBGCRYPT_CONFIG=$(STAGING_DIR)/usr/bin/libgcrypt-config
 
 # our ncurses wchar support is not properly detected
 SNGREP_CONF_OPTS += --disable-unicode
Signature · 1 line
2.9.4
#2 lore raw

PATCH 2/2 package/sngrep: fix static build with gnutls

Hello,

package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch line 4

Quoted 10 lines · Romain Naour
package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch new file +4 -0
new file mode 100644
index 0000000..67b21a2
@@ -0,0 +1,60 @@
+From b0428b0279b03d88520b79b651f692d9aea65f8c Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+Date: Sat, 20 May 2017 15:00:16 +0200
+Subject: [PATCH] configure.ac: switch to pkg-config to find gnutls.

This patch looks good, but the description could use some improvement.

package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch line 10

Quoted 5 lines
package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch new file +5 -0
+Like for openssl in patch [1] and for the same reason,
+use pkg-config to find gnutls.
+
+gnutls can be linked with :
+    -lintl -lgmp -lunistring -lhogweed -lnettle -ltasn1 -lz

And ?

package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch line 12

Quoted 3 lines
package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch new file +3 -0
++	AC_PATH_PROG([LIBGCRYPT_CONFIG],[libgcrypt-config],[no])
++	if test "x${LIBGCRYPT_CONFIG}" = "xno"; then
++	    AC_MSG_FAILURE([libgcrypt-config not found in PATH])

Use AC_MSG_ERROR() here.

package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch line 22

Quoted 10 lines
package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch new file +10 -0
++	fi
++	AC_CHECK_LIB(
++		[gcrypt],
++		[gcry_md_map_name],
++		[LIBGCRYPT_CFLAGS="`${LIBGCRYPT_CONFIG} --cflags`"
++		LIBGCRYPT_LIBS="`${LIBGCRYPT_CONFIG} --libs`"
++		],
++		[AC_MSG_ERROR([ You need to have libgcrypt installed to compile sngrep])],
++		[`${LIBGCRYPT_CONFIG} --libs --cflags`]
++		)

I don't understand what you're doing here. What about instead:

- If LIBGCRYPT_CONFIG != no, use it to assign LIBGCRYPT_{CFLAGS,LIBS}

- If LIBGCRYPT_CONFIG == no, fall back to AC_CHECK_LIB

What do you think ?

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#3 lore raw

PATCH 2/2 package/sngrep: fix static build with gnutls

Le 23/05/2017 ? 16:27, Thomas Petazzoni a ?crit :

Quoted 42 lines

Hello,

package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch line 4

Quoted 10 lines · Romain Naour
package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch new file +4 -0
new file mode 100644
index 0000000..67b21a2
@@ -0,0 +1,60 @@
+From b0428b0279b03d88520b79b651f692d9aea65f8c Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+Date: Sat, 20 May 2017 15:00:16 +0200
+Subject: [PATCH] configure.ac: switch to pkg-config to find gnutls.

This patch looks good, but the description could use some improvement.

package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch line 10

Quoted 5 lines
package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch new file +5 -0
+Like for openssl in patch [1] and for the same reason,
+use pkg-config to find gnutls.
+
+gnutls can be linked with :
+    -lintl -lgmp -lunistring -lhogweed -lnettle -ltasn1 -lz

And ?

package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch line 12

Quoted 3 lines
package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch new file +3 -0
++	AC_PATH_PROG([LIBGCRYPT_CONFIG],[libgcrypt-config],[no])
++	if test "x${LIBGCRYPT_CONFIG}" = "xno"; then
++	    AC_MSG_FAILURE([libgcrypt-config not found in PATH])

Use AC_MSG_ERROR() here.

package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch line 22

Quoted 10 lines
package/sngrep/0003-configure.ac-switch-to-pkg-config-to-find-gnutls.patch new file +10 -0
++	fi
++	AC_CHECK_LIB(
++		[gcrypt],
++		[gcry_md_map_name],
++		[LIBGCRYPT_CFLAGS="`${LIBGCRYPT_CONFIG} --cflags`"
++		LIBGCRYPT_LIBS="`${LIBGCRYPT_CONFIG} --libs`"
++		],
++		[AC_MSG_ERROR([ You need to have libgcrypt installed to compile sngrep])],
++		[`${LIBGCRYPT_CONFIG} --libs --cflags`]
++		)
Quoted 7 lines

I don't understand what you're doing here. What about instead:

- If LIBGCRYPT_CONFIG != no, use it to assign LIBGCRYPT_{CFLAGS,LIBS}

- If LIBGCRYPT_CONFIG == no, fall back to AC_CHECK_LIB

What do you think ?

I think this patch is not necessary anymore since gnutls shouldn't be linked statically, see [1].

The code use __attribute__((constructor)) and __attribute__((destructor)) to call constructor/desctructor when a shared library is loaded. Constructor/desctructor are not used when a static library is used (except when if -Wl,--whole-archive -lgnutls -Wno-whole-archive is used, not tested).

Even if gnutls initialization (_gnutls_global_init()) may be called manually, the maintainer said it's not supported.

So we should add !BR2_STATIC_LIBS for gnutls package ?
Doing so, it will avoid all static linking issues with gnutls (taskd, ffmpeg, sngrep...)

Best regards,
Romain

[1] https://gitlab.com/gnutls/gnutls/issues/203

Quoted 5 lines

Thanks,

Thomas

#4 lore raw

PATCH 2/2 package/sngrep: fix static build with gnutls

Hello,

Quoted 8 lines · Romain Naour

I think this patch is not necessary anymore since gnutls shouldn't be linked statically, see [1].

The code use __attribute__((constructor)) and __attribute__((destructor)) to call constructor/desctructor when a shared library is loaded. Constructor/desctructor are not used when a static library is used (except when if -Wl,--whole-archive -lgnutls -Wno-whole-archive is used, not tested).

Quoted 6 lines

Even if gnutls initialization (_gnutls_global_init()) may be called manually, the maintainer said it's not supported.

So we should add !BR2_STATIC_LIBS for gnutls package ?
Doing so, it will avoid all static linking issues with gnutls (taskd, ffmpeg, sngrep...)

Yes, I believe we should add !BR2_STATIC_LIBS, of course propagated to all reverse dependencies (but there are not too many of them).

A side question is: is there a way to detect/error out when some code is using this constructor/destructor mechanism in a way that doesn't work for static linking? It would be useful to check if other packages are affected.

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#5 lore raw

PATCH 2/2 package/sngrep: fix static build with gnutls

Hi Thomas,

Le 24/05/2017 ? 08:47, Thomas Petazzoni a ?crit :

Quoted 15 lines

Hello,

Quoted 11 lines · Romain Naour

I think this patch is not necessary anymore since gnutls shouldn't be linked statically, see [1].

The code use __attribute__((constructor)) and __attribute__((destructor)) to call constructor/desctructor when a shared library is loaded. Constructor/desctructor are not used when a static library is used (except when if -Wl,--whole-archive -lgnutls -Wno-whole-archive is used, not tested).

Even if gnutls initialization (_gnutls_global_init()) may be called manually, the maintainer said it's not supported.

Quoted 6 lines
Quoted 3 lines

So we should add !BR2_STATIC_LIBS for gnutls package ?
Doing so, it will avoid all static linking issues with gnutls (taskd, ffmpeg, sngrep...)

Yes, I believe we should add !BR2_STATIC_LIBS, of course propagated to all reverse dependencies (but there are not too many of them).

Ok, I'll do it.

Quoted 4 lines

A side question is: is there a way to detect/error out when some code is using this constructor/destructor mechanism in a way that doesn't work for static linking? It would be useful to check if other packages are affected.

I don't know, I don't think so...

Now the gnutls script warn about static linking:
https://gitlab.com/gnutls/gnutls/commit/6b748886799f88ddee9721dba4fc4d52854832ae

Best regards,
Romain

Quoted 5 lines

Thanks!

Thomas

#6 lore raw

PATCH 2/2 package/sngrep: fix static build with gnutls

Am Wed, 24 May 2017 23:54:03 +0200 schrieb Romain Naour:

Quoted 4 lines
Quoted 2 lines

Yes, I believe we should add !BR2_STATIC_LIBS, of course propagated to all reverse dependencies (but there are not too many of them).

Ok, I'll do it.

Hi,

patch done: http://patchwork.ozlabs.org/patch/766903/

Regards, Bernd

#7 lore raw

PATCH 2/2 package/sngrep: fix static build with gnutls

Hi Bernd,

Le 25/05/2017 ? 11:02, Bernd Kuhls a ?crit :

Quoted 10 lines

Am Wed, 24 May 2017 23:54:03 +0200 schrieb Romain Naour:

Quoted 4 lines
Quoted 2 lines

Yes, I believe we should add !BR2_STATIC_LIBS, of course propagated to all reverse dependencies (but there are not too many of them).

Ok, I'll do it.

Hi,

patch done: http://patchwork.ozlabs.org/patch/766903/

Thanks for the help!
It was too late to finish my own patch yesterday...

Your patch is almost good but some option comments are missing.

Also, I suggest to add --disable-static in GNUTLS_CONF_OPTS unconditionally to avoid linking with libgnutls.a even if libgnutls.so is present (BR2_SHARED_STATIC_LIBS case).

I'll resend an updated version, I'm keeping you as original author.

Best regards,
Romain

Quoted 8 lines

Regards, Bernd

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

#8 lore raw

PATCH 1/2 package/sngrep: add libgcrypt missing dependencies

In reply to Romain Naour

Hello,

Quoted 5 lines · Romain Naour

Even if libgcrypt is selected at Kconfig level, it's missing from Makefile level.

Fixes:
http://autobuild.buildroot.net/results/36c/36c28e7e49b31ed2398a67db2e9b08e3aad5a3ee

Could you explain how this build failure apparently related to ncurses has something to do with the gnutls/libgcrypt issue ?

The patch itself looks good, but the relation with this particular build failure isn't really clear. Could you clarify?

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#9 lore raw

PATCH 1/2 package/sngrep: add libgcrypt missing dependencies

Le 23/05/2017 ? 16:19, Thomas Petazzoni a ?crit :

Quoted 6 lines

Hello,

Quoted 3 lines · Romain Naour

Even if libgcrypt is selected at Kconfig level, it's missing from Makefile level.

Quoted 8 lines
Quoted 2 lines

Fixes:
http://autobuild.buildroot.net/results/36c/36c28e7e49b31ed2398a67db2e9b08e3aad5a3ee

Could you explain how this build failure apparently related to ncurses has something to do with the gnutls/libgcrypt issue ?

The patch itself looks good, but the relation with this particular build failure isn't really clear. Could you clarify?

Sorry,

I was looking for this kind of build failure with a glibc based system:

checking for ncurses.h... yes
checking for initscr in -lncurses... yes
checking for new_panel in -lpanel... yes
checking for new_form in -lform... yes
checking for new_item in -lmenu... yes
checking for gnutls_init in -lgnutls... yes
checking for gcry_md_map_name in -lgcrypt... no
configure: error: You need to have libgcrypt installed to compile sngrep

I just read what I wanted to read in the autobuilder reference...

It's weird that the autobuilder didn't catch it.

Best regards,
Romain

Quoted 5 lines

Thanks!

Thomas

#10 lore raw

PATCH 1/2 package/sngrep: add libgcrypt missing dependencies

Hello,

Quoted 11 lines · Romain Naour

I was looking for this kind of build failure with a glibc based system:

checking for ncurses.h... yes
checking for initscr in -lncurses... yes
checking for new_panel in -lpanel... yes
checking for new_form in -lform... yes
checking for new_item in -lmenu... yes
checking for gnutls_init in -lgnutls... yes
checking for gcry_md_map_name in -lgcrypt... no
configure: error: You need to have libgcrypt installed to compile sngrep

Quoted 3 lines

I just read what I wanted to read in the autobuilder reference...

It's weird that the autobuilder didn't catch it.

So in other words, there are no autobuilder reference for the build failure you are fixing? If so, could you add in the commit log a minimal defconfig that exhibits the issue?

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#11 lore raw

PATCH 1/2 package/sngrep: add libgcrypt missing dependencies

Hi Thomas,

Le 24/05/2017 ? 08:44, Thomas Petazzoni a ?crit :

Quoted 15 lines

Hello,

Quoted 11 lines · Romain Naour

I was looking for this kind of build failure with a glibc based system:

checking for ncurses.h... yes
checking for initscr in -lncurses... yes
checking for new_panel in -lpanel... yes
checking for new_form in -lform... yes
checking for new_item in -lmenu... yes
checking for gnutls_init in -lgnutls... yes
checking for gcry_md_map_name in -lgcrypt... no
configure: error: You need to have libgcrypt installed to compile sngrep

Quoted 7 lines
Quoted 3 lines

I just read what I wanted to read in the autobuilder reference...

It's weird that the autobuilder didn't catch it.

So in other words, there are no autobuilder reference for the build failure you are fixing? If so, could you add in the commit log a minimal defconfig that exhibits the issue?

Exactly...

Here the minimal defconfig:

BR2_arm=y
BR2_cortex_a9=y
BR2_ARM_ENABLE_NEON=y
BR2_ARM_ENABLE_VFP=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_PACKAGE_GNUTLS=y
BR2_PACKAGE_LIBGCRYPT=y
BR2_PACKAGE_SNGREP=y

Best regards,
Romain

Quoted 5 lines

Thanks!

Thomas