diff mbox

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

Message ID 1397299022-31655-1-git-send-email-jcmvbkbc@gmail.com
State Rejected
Headers show

Commit Message

Max Filippov April 12, 2014, 10:37 a.m. UTC
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.

Fixes: http://autobuild.buildroot.net/results/959/959b77fa2c1f13b1958b234803437e09734e882e/

Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
Cc: Peter Korsgaard <peter@korsgaard.com>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 package/php/php.mk |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

Comments

Thomas Petazzoni April 12, 2014, 11:14 a.m. UTC | #1
Dear Max Filippov,

On Sat, 12 Apr 2014 14:37:02 +0400, Max Filippov wrote:
> 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.
> 
> Fixes: http://autobuild.buildroot.net/results/959/959b77fa2c1f13b1958b234803437e09734e882e/
> 
> Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  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
Gustavo Zacarias April 12, 2014, 11:38 a.m. UTC | #2
On 04/12/2014 08:14 AM, Thomas Petazzoni wrote:
> 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.
Thomas Petazzoni April 20, 2014, 5:05 p.m. UTC | #3
Dear Max Filippov,

On Sat, 12 Apr 2014 14:37:02 +0400, Max Filippov wrote:
> 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.
> 
> Fixes: http://autobuild.buildroot.net/results/959/959b77fa2c1f13b1958b234803437e09734e882e/
> 
> Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  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
diff mbox

Patch

diff --git a/package/php/php.mk b/package/php/php.mk
index ba1200b..bb86829 100644
--- a/package/php/php.mk
+++ b/package/php/php.mk
@@ -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