silnrsi / teckit

A Text Encoding Conversion toolkit
Other
17 stars 11 forks source link

Mixing of FLAGS (expat_CFLAGS="$CFLAGS") #16

Closed spl closed 5 years ago

spl commented 5 years ago

This issue was reported as bug #338110 for the Gentoo Linux app-text/teckit-2.5.1 package. I don't know if it was ever reported upstream to you, but I thought it would be a good idea to make a note of it here and see what you think.

The report was:

I found a mixing of FLAGS:

/bin/sh ../libtool --tag=CXX   --mode=link x86_64-pc-linux-gnu-g++  -O2 -pipe -march=core2 -frecord-gcc-switches -mssse3 -mcx16 -mmmx -g -Wmissing-include-dirs -Wenum-compare -DNDEBUG  -Wl,-O1 -Wl,--as-needed -Wl,-O1,--hash-style=gnu,--sort-common -o txtconv TxtConv.o ../lib/lib
TECkit.la -lz
libtool: link: x86_64-pc-linux-gnu-g++ -O2 -pipe -march=core2 -frecord-gcc-switches -mssse3 -mcx16 -mmmx -g -Wmissing-include-dirs -Wenum-compare -DNDEBUG -Wl,-O1 -Wl,-O1 -Wl,--hash-style=gnu -Wl,--sort-common -o .libs/txtconv TxtConv.o  -Wl,--as-needed ../lib/.libs/libTECkit.so
 -lz
cc1plus: warning: command line option "-Wimplicit-function-declaration" is valid for C/ObjC but not for C++

The fix is this patch:

No need to pass CFLAGS twice, esp. if they are used to feed g++
Bug #338110

Index: TECkit_2_5_1/configure.ac
===================================================================
--- TECkit_2_5_1.orig/configure.ac
+++ TECkit_2_5_1/configure.ac
@@ -76,7 +76,7 @@ noexpat_CFLAGS="$CFLAGS"
 noexpat_LIBS="$LIBS"
 AC_CHECK_LIB(expat, XML_ExpatVersion)
 AM_CONDITIONAL(SYSTEM_EXPAT, test x$ac_cv_lib_expat_XML_ExpatVersion = xyes)
-expat_CFLAGS="$CFLAGS"
+expat_CFLAGS=""
 expat_LIBS="$LIBS"
 CFLAGS="$noexpat_CFLAGS"
 LIBS="$noexpat_LIBS"

The patch is still being applied in Gentoo's current package, app-text/teckit-2.5.6.

devosb commented 5 years ago

Thanks. I was not aware of the Gentoo bug report. Fixed in https://github.com/silnrsi/teckit/commit/3dc618225215f4dc64bfc24d68dd74282c22ae2c