Skip to contentSkip to conversation

package/harfbuzz: disable test programs

PATCH New Romain Naour 2 participants 5 messages May 12, 2015 – May 19, 2015

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/2 package/harfbuzz: disable test programs

Workaround a build issue by disabling all test
binaries since it forget to link with -lstd++.

Add a new configure option to disable test programs in a upstreamable way.

Fixes:
http://autobuild.buildroot.net/results/210/2107f9dfb39eeb6559fb4271c7af8b39aef521ca/

---
v2: reword commit log

---
 .../0001-autotools-add-disable-tests-option.patch  | 57 ++++++++++++++++++++++
 package/harfbuzz/harfbuzz.mk                       |  6 ++-
 2 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 package/harfbuzz/0001-autotools-add-disable-tests-option.patch
package/harfbuzz/0001-autotools-add-disable-tests-option.patch new file +57 -0
new file mode 100644
index 0000000..59a7b5a
@@ -0,0 +1,57 @@
+From 4641f22b2297fdbf4206ccaea971868301887aa7 Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+Date: Fri, 8 May 2015 15:58:51 +0200
+Subject: [PATCH] autotools: add --disable-tests option
+
+Disable all test programs in order to only build harfbuzz
+librarie and speed up the build time.
+
+Signed-off-by: Romain Naour <[email protected]>
+---
+ Makefile.am  |  6 +++++-
+ configure.ac | 12 ++++++++++++
+ 2 files changed, 17 insertions(+), 1 deletion(-)
+
+diff --git a/Makefile.am b/Makefile.am
+index 47aeb97..0f8ee46 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -4,7 +4,11 @@ NULL =
+ 
+ ACLOCAL_AMFLAGS = -I m4
+ 
+-SUBDIRS = src util test docs
++SUBDIRS = src util docs
++
++if WITH_TESTS
++SUBDIRS += test
++endif
+ 
+ EXTRA_DIST = \
+ 	autogen.sh \
+diff --git a/configure.ac b/configure.ac
+index 5baad1f..b15f31e 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -408,6 +408,18 @@ fi
+ 
+ dnl ===========================================================================
+ 
++AC_ARG_ENABLE([tests],
++  AS_HELP_STRING([--enable-tests], [build tests [default=yes]]),
++  [], enable_tests=yes
++)
++have_tests=yes
++if test "$enable_tests" = "no"; then
++  have_tests="no"
++fi
++AM_CONDITIONAL(WITH_TESTS, [test "$have_tests" = "yes"])
++
++dnl ===========================================================================
++
+ AC_CONFIG_FILES([
+ Makefile
+ src/Makefile
+-- 
+1.9.3
+
package/harfbuzz/harfbuzz.mk +4 -2
index 9cfd613..3f0b028 100644
@@ -10,7 +10,8 @@ HARFBUZZ_SOURCE = harfbuzz-$(HARFBUZZ_VERSION).tar.bz2
 HARFBUZZ_LICENSE = MIT, ISC (ucdn library)
 HARFBUZZ_LICENSE_FILES = COPYING src/hb-ucdn/COPYING
 HARFBUZZ_INSTALL_STAGING = YES
-HARFBUZZ_CONF_OPTS = --without-coretext --without-uniscribe --without-graphite2
+HARFBUZZ_CONF_OPTS = --disable-tests --without-coretext --without-uniscribe \
+	--without-graphite2
 
 # freetype & glib2 support required by host-pango
 HOST_HARFBUZZ_DEPENDENCIES = \
@@ -23,7 +24,8 @@ HOST_HARFBUZZ_CONF_OPTS = \
 	--with-cairo=no \
 	--with-icu=no \
 	--with-freetype=yes \
-	--with-glib=yes
+	--with-glib=yes \
+	--disable-tests
 
 # beta libtool version
 HARFBUZZ_AUTORECONF = YES
Signature · 1 line
1.9.3
#1 lore raw

PATCH v2 2/2 Revert "harfbuzz: fix static linking of test programs when toolchain has threads support"

harfbuzz version was updated since this change and
it's no not useful anymore to add -pthread in LDFLAGS (fixed upstream).

Also, test programs are not build anymore since --disable-tests was used in harfbuzz.mk

This reverts commit 3d03017f53c2566ed32a0ae76d79eeb1ef4a7feb.

---
v2: reword commit log
add Sob line

---
 package/harfbuzz/harfbuzz.mk | 5 -----
 1 file changed, 5 deletions(-)
package/harfbuzz/harfbuzz.mk +0 -5
index 3f0b028..c5182a4 100644
@@ -30,11 +30,6 @@ HOST_HARFBUZZ_CONF_OPTS = \
 # beta libtool version
 HARFBUZZ_AUTORECONF = YES
 
-ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
-# forgets to link test programs with -pthread breaking static link
-HARFBUZZ_CONF_ENV = LDFLAGS="$(TARGET_LDFLAGS) -pthread"
-endif
-
 ifeq ($(BR2_PACKAGE_CAIRO),y)
 HARFBUZZ_DEPENDENCIES += cairo
 HARFBUZZ_CONF_OPTS += --with-cairo=yes
Signature · 1 line
1.9.3
#2 lore raw

PATCH v2 1/2 package/harfbuzz: disable test programs

In reply to Romain Naour

Dear Romain Naour,

Quoted 10 lines · Romain Naour

Workaround a build issue by disabling all test
binaries since it forget to link with -lstd++.

Add a new configure option to disable test programs in a upstreamable way.

Fixes:
http://autobuild.buildroot.net/results/210/2107f9dfb39eeb6559fb4271c7af8b39aef521ca/

I was almost going to apply this one, but I'm not entirely happy in fact. I really works around the real problem, and does not fix it:
imagine a real program now links statically to harfbuzz using harfbuzz pkg-config file. This pkg-config file does not advertise the dependency on libstdc++ in its Libs.private, so the link would also fail, just like it currently fails for the test programs.

Unfortunately, fixing the .pc file would not fix the internal harfbuzz test programs, since they don't use pkg-config to get the link flags.

To be honest, I am not sure what is the correct automake way of fixing this problem. Maybe:

-LDADD = $(top_builddir)/src/libharfbuzz.la $(GLIB_LIBS) +LDADD = $(top_builddir)/src/libharfbuzz.la $(GLIB_LIBS) -lstdc++

in test/api/Makefile.am ?

Best regards,

Thomas

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

PATCH v2 1/2 package/harfbuzz: disable test programs

Hi Thomas,

Le 18/05/2015 23:03, Thomas Petazzoni a ?crit :

Quoted 12 lines

Dear Romain Naour,

Quoted 9 lines · Romain Naour

Workaround a build issue by disabling all test
binaries since it forget to link with -lstd++.

Add a new configure option to disable test programs in a upstreamable way.

Fixes:
http://autobuild.buildroot.net/results/210/2107f9dfb39eeb6559fb4271c7af8b39aef521ca/

Quoted 8 lines
Quoted 1 line

I was almost going to apply this one, but I'm not entirely happy in fact. I really works around the real problem, and does not fix it:
imagine a real program now links statically to harfbuzz using harfbuzz pkg-config file. This pkg-config file does not advertise the dependency on libstdc++ in its Libs.private, so the link would also fail, just like it currently fails for the test programs.

-lstdc++ is only needed by libharfbuzz-icu.la which use the icu package. So maybe -lstdc++ is missing in Libs.private from icu-uc.pc ?

Quoted 11 lines

Unfortunately, fixing the .pc file would not fix the internal harfbuzz test programs, since they don't use pkg-config to get the link flags.

To be honest, I am not sure what is the correct automake way of fixing this problem. Maybe:

(quoted diff) +1 -1
-LDADD = $(top_builddir)/src/libharfbuzz.la $(GLIB_LIBS)
+LDADD = $(top_builddir)/src/libharfbuzz.la $(GLIB_LIBS) -lstdc++

in test/api/Makefile.am ?

You would said ;)
test_unicode_LDADD += $(top_builddir)/src/libharfbuzz-icu.la -lstdc++

With that fixed, Harbuzz build fine.

Thanks for the review.

Best regards,
Romain

Quoted 5 lines

Best regards,

Thomas

#4 lore raw

PATCH v2 1/2 package/harfbuzz: disable test programs

Romain,

Quoted 9 lines · Romain Naour
Quoted 6 lines

I was almost going to apply this one, but I'm not entirely happy in fact. I really works around the real problem, and does not fix it:
imagine a real program now links statically to harfbuzz using harfbuzz pkg-config file. This pkg-config file does not advertise the dependency on libstdc++ in its Libs.private, so the link would also fail, just like it currently fails for the test programs.

-lstdc++ is only needed by libharfbuzz-icu.la which use the icu package. So maybe -lstdc++ is missing in Libs.private from icu-uc.pc ?

Yes, might be. It's weird: harfbuzz uses .cc as the extension for its source files, but it seems to be only pure C, not C++. And to add to the confusion, it also uses AC_PROG_CXX in its configure.ac, which would also indicate it's in C++.

But obviously, icu is implemented in C++, so the icu-uc pkg-config file should have the corresponding -lstdc++ Libs.private flag.

Quoted 4 lines

You would said ;)
test_unicode_LDADD += $(top_builddir)/src/libharfbuzz-icu.la -lstdc++

With that fixed, Harbuzz build fine.

Sounds good to me.

Thanks!

Thomas

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