Skip to contentSkip to conversation

php: fix wrong -L and -I paths added by iconv tests

PATCH New Max Filippov 3 participants 4 messages Apr 12, 2014 – Apr 20, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 php: fix wrong -L and -I paths added by iconv tests

The commit f2a2c4cce "php: fix iconv related build failure" have removed logic that used to detect the correct PHP_ICONV_PREFIX with an assumption that "ICONV_DIR is fine". Currently the ICONV_DIR is only fine when BR2_PACKAGE_LIBICONV is set to 'y', otherwise php is configured with plain --with-iconv. This results in empty PHP_ICONV_PREFIX and -L/lib and -I/include added to CFLAGS and LDFLAGS respectively, which leads to build failures when e.g. /lib contains files with the same names as the toolchain sysroot needed for linking.

Fix that by always specifying --with-iconv=$(STAGING_DIR)/usr when iconv is configured for PHP.

Cc Gustavo Zacarias <[email protected]>
Cc Peter Korsgaard <[email protected]>
---
 package/php/php.mk |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
package/php/php.mk +1 -3
index ba1200b..bb86829 100644
@@ -108,11 +108,9 @@ ifeq ($(BR2_PACKAGE_PHP_EXT_GETTEXT),y)
 endif
 
 ifeq ($(BR2_PACKAGE_PHP_EXT_ICONV),y)
-ifeq ($(BR2_PACKAGE_LIBICONV),y)
 	PHP_CONF_OPT += --with-iconv=$(STAGING_DIR)/usr
+ifeq ($(BR2_PACKAGE_LIBICONV),y)
 	PHP_DEPENDENCIES += libiconv
-else
-	PHP_CONF_OPT += --with-iconv
 endif
 endif
 
Signature · 1 line
1.7.7.6
#1 lore raw

PATCH 1/1 php: fix wrong -L and -I paths added by iconv tests

Dear Max Filippov,

Quoted 12 lines · Max Filippov

The commit f2a2c4cce "php: fix iconv related build failure" have removed logic that used to detect the correct PHP_ICONV_PREFIX with an assumption that "ICONV_DIR is fine". Currently the ICONV_DIR is only fine when BR2_PACKAGE_LIBICONV is set to 'y', otherwise php is configured with plain --with-iconv. This results in empty PHP_ICONV_PREFIX and -L/lib and -I/include added to CFLAGS and LDFLAGS respectively, which leads to build failures when e.g. /lib contains files with the same names as the toolchain sysroot needed for linking.

Fix that by always specifying --with-iconv=$(STAGING_DIR)/usr when iconv is configured for PHP.

Quoted 8 lines
Cc Gustavo Zacarias <[email protected]>
Cc Peter Korsgaard <[email protected]>
---
 package/php/php.mk |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

I remember trying this possible fix the other day before replying to you, and it didn't work: the iconv detection was failing. I'll reproduce and get back to you with more details.

Thanks!

Thomas

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

PATCH 1/1 php: fix wrong -L and -I paths added by iconv tests

Quoted 3 lines · Thomas Petazzoni

I remember trying this possible fix the other day before replying to you, and it didn't work: the iconv detection was failing. I'll reproduce and get back to you with more details.

The fatal combination is basically uclibc with locale support where libiconv isn't built and php is built with iconv support but not xmlrpc (which has it's own iconv option, yeah, that's php for you). I haven't delved too much into a proper fix at the moment but that'll probably won't work in all the iconv scenarios around.
A possibility might be doing AUTORECONF and just fixing PHP_SETUP_ICONV in aclocal/acinclude.m4, in the past AUTORECONFing php was broken but that might have changed in recent versions.
Regards.

#3 lore raw

PATCH 1/1 php: fix wrong -L and -I paths added by iconv tests

In reply to Max Filippov

Dear Max Filippov,

Quoted 12 lines · Max Filippov

The commit f2a2c4cce "php: fix iconv related build failure" have removed logic that used to detect the correct PHP_ICONV_PREFIX with an assumption that "ICONV_DIR is fine". Currently the ICONV_DIR is only fine when BR2_PACKAGE_LIBICONV is set to 'y', otherwise php is configured with plain --with-iconv. This results in empty PHP_ICONV_PREFIX and -L/lib and -I/include added to CFLAGS and LDFLAGS respectively, which leads to build failures when e.g. /lib contains files with the same names as the toolchain sysroot needed for linking.

Fix that by always specifying --with-iconv=$(STAGING_DIR)/usr when iconv is configured for PHP.

Quoted 8 lines
Cc Gustavo Zacarias <[email protected]>
Cc Peter Korsgaard <[email protected]>
---
 package/php/php.mk |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

Unfortunately, this doesn't solve the problem, so I've marked this patch as Rejected for now. More investigation will be needed.

Thanks,

Thomas

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