Skip to content

gnutls: load libunistring-optional gnulib module - #21007

Merged
nmav merged 1 commit into
openwrt:masterfrom
micmac1:gnutls-unistring-optional
May 18, 2023
Merged

gnutls: load libunistring-optional gnulib module#21007
nmav merged 1 commit into
openwrt:masterfrom
micmac1:gnutls-unistring-optional

Conversation

@micmac1

@micmac1 micmac1 commented May 13, 2023

Copy link
Copy Markdown
Contributor

Since a few days staging_dir/host/share/aclocal/ contains new m4 files (libunistring-base.m4, libunistring-optional.m4, libunistring.m4 etc.) that get applied during autoreconf. This changes the libunistring setup enough that we run into problem (builds fail).

Load the libunistring-optional gnulib module in configure.ac to be able to add/use the new configure switch "--with-included-libunistring".

This is at most a workaround. This should be followed up with gnutls upstream to clean this up.

Maintainer: @nmav
Compile tested:sdk ath79
Run tested: N/A

Description:
This is regarding the gnutls build failure reported in issue #20969. It was suspected that this is related to gcc-13, but as it turns out it's just a broken interaction between gnutls and updated autoreconf macros.

Since a few days staging_dir/host/share/aclocal/ contains new m4 files
(libunistring-base.m4, libunistring-optional.m4, libunistring.m4 etc.)
that get applied during autoreconf. This changes the libunistring setup
enough that we run into problem (builds fail).

Load the libunistring-optional gnulib module in configure.ac to be able
to add/use the new configure switch "--with-included-libunistring".

This is at most a workaround. This should be followed up with gnutls
upstream to clean this up.

Signed-off-by: Sebastian Kemper <sebastian_ml@gmx.net>
@mcprat

mcprat commented May 13, 2023

Copy link
Copy Markdown
Contributor

I would recommend first trying to get rid of autoreconf for gnutls build, or secondly try to use the autogen.sh script with gnulib source, and if neither of those things are possible then this would be good

@egorenar

Copy link
Copy Markdown
Contributor

thanks! tested right now and it fixed the problem for me.

@1715173329

1715173329 commented May 14, 2023

Copy link
Copy Markdown
Member
diff --git a/libs/gnutls/Makefile b/libs/gnutls/Makefile
index dc94591c2..262df231e 100644
--- a/libs/gnutls/Makefile
+++ b/libs/gnutls/Makefile
@@ -98,7 +98,9 @@ endef
 define Package/libgnutls
 $(call Package/gnutls/Default)
   TITLE+= (library)
-  DEPENDS+= +libnettle +!LIBNETTLE_MINI:libgmp +GNUTLS_EXT_LIBTASN1:libtasn1 +GNUTLS_PKCS11:p11-kit +GNUTLS_CRYPTODEV:kmod-cryptodev +libatomic
+  DEPENDS+= +libnettle +!LIBNETTLE_MINI:libgmp +GNUTLS_EXT_LIBTASN1:libtasn1 \
+    +GNUTLS_PKCS11:p11-kit +GNUTLS_CRYPTODEV:kmod-cryptodev +libatomic \
+    +libunistring
 endef
 
 define Package/libgnutls-dane
@@ -131,7 +133,7 @@ CONFIGURE_ARGS+= \
 	--enable-local-libopts \
 	--without-idn \
 	--with-default-trust-store-dir=/etc/ssl/certs/ \
-	--with-included-unistring \
+	--with-libunistring-prefix=$(STAGING_DIR) \
 	--with-librt-prefix="$(LIBRT_ROOT_DIR)/" \
 	--with-pic \
 	--with-system-priority-file="" \

what about linking to our own libunistring package?

@mcprat

mcprat commented May 16, 2023

Copy link
Copy Markdown
Contributor

see the existing config --with-included-unistring which I believe was supposed to be "libunistring" instead of "unistring" but it is a typo or maybe something changed...

now that #12606 is merged you can try to use the "prefix" config instead of the "included" config in the same way that PR does and see if that works. If you do try it, first make sure that the libunistring package is built first, and to do that you probably need to delete the packages feed Makefile for libunistring because it is now a duplicate with the one in openwrt.git now.

@mcprat

mcprat commented May 16, 2023

Copy link
Copy Markdown
Contributor

and I'm just repeating what @1715173329 already suggested haha

@nmav

nmav commented May 16, 2023

Copy link
Copy Markdown
Contributor

@micmac1 thank you, it looks like a workaround that addresses the build issue. Have you tried the alternative recommended in the comments above to remove the PKG_FIXUP autoreconf step?

@micmac1

micmac1 commented May 16, 2023

Copy link
Copy Markdown
Contributor Author

1715173329's suggestion doesn't build for me.

@mpratt14 "--with-included-unistring" is added by gnutls' configure.ac, unrelated to any m4 files.

@nmav autoreconf was added in commit 9a1ece3 to fix a build issue on Arch. I don't have Arch so can't say if it can be dropped or not.

@mcprat

mcprat commented May 16, 2023

Copy link
Copy Markdown
Contributor

ok, what if you just link to local libunistring without changing anything else?

see post in the forum
https://forum.openwrt.org/t/build-failure-gnutls-3-7-8-latest-snapshot/159426/14

but instead of deleting the config option try to change it to "with--prefix="

./configure --help after autoreconf runs is useful to see if that config option is available

@micmac1

micmac1 commented May 16, 2023

Copy link
Copy Markdown
Contributor Author

The --with-libunistring-prefix=... option doesn't work for me. Builds fail with it. If one leaves it out and drops --with-included-unistring gnutls actually links to external libunistring. So exactly like the forum post @mpratt14 mentioned.

But in my opinion doing this would cause some users grief.

This is the size of the ipks when gnutls links against external libunistring:

762K libgnutls_3.8.0-2_mips_24kc.ipk
625K libunistring_1.1-1_mips_24kc.ipk

This is the size of gnutls using the included libunistring:

817K libgnutls_3.8.0-2_mips_24kc.ipk

So linking external libunistring adds 570K. That's a big difference on small devices.

@mcprat

mcprat commented May 16, 2023

Copy link
Copy Markdown
Contributor

I'm fine with either way, just wanted to see if that worked

@1715173329

Copy link
Copy Markdown
Member

that worked for me though.

so is this ready to go? anyway let's fix the build issue firstly

@micmac1

micmac1 commented May 17, 2023

Copy link
Copy Markdown
Contributor Author

I tried 1715173329's again and this time it worked. I must've copied something wrongly earlier.

Anyway, I have nothing else to add. I don't have write access in this repo so whether to merge or not is not up to me.

@1715173329

Copy link
Copy Markdown
Member

@nmav are you okay with it?

@oskarirauta

Copy link
Copy Markdown
Contributor

I can verify that this fixes the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants