Bug 186707 - [patch] devel/qt4-corelib: fix iconv detection
Summary: [patch] devel/qt4-corelib: fix iconv detection
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-kde (group)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-12 21:50 UTC by Tijl Coosemans
Modified: 2014-03-23 20:00 UTC (History)
0 users

See Also:


Attachments
qt4-corelib.patch (1.28 KB, patch)
2014-02-12 21:50 UTC, Tijl Coosemans
no flags Details | Diff
qt.patch (3.91 KB, patch)
2014-03-20 13:41 UTC, Tijl Coosemans
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tijl Coosemans freebsd_committer freebsd_triage 2014-02-12 21:50:00 UTC
On FreeBSD 10 with converters/libiconv installed qt4-corelib uses libiconv
instead of libc iconv.  This is because the configure test doesn't respect
CXXFLAGS (which contains -DLIBICONV_PLUG).

The attached patch makes all compile tests respect CXXFLAGS.
Comment 1 Edwin Groothuis freebsd_committer freebsd_triage 2014-02-12 21:50:08 UTC
Responsible Changed
From-To: freebsd-ports-bugs->kde

Over to maintainer (via the GNATS Auto Assign Tool)
Comment 2 Max Brazhnikov freebsd_committer freebsd_triage 2014-03-04 06:45:58 UTC
Hi Tijl,

Is it enough to patch single qt4-corelib port? Shouldn't the patch be shared among other Qt4 ports?

Max
Comment 3 Tijl Coosemans freebsd_committer freebsd_triage 2014-03-04 22:12:09 UTC
On Tue, 04 Mar 2014 06:45:58 +0000 Max Brazhnikov wrote:
> Hi Tijl,
> 
> Is it enough to patch single qt4-corelib port? Shouldn't the patch be
> shared among other Qt4 ports?
> 
> Max

You mean to let other Qt ports respect CXXFLAGS during configure or
to fix iconv issues?
Comment 4 Max Brazhnikov freebsd_committer freebsd_triage 2014-03-05 06:08:43 UTC
On Tue, 04 Mar 2014 23:12:09 +0100 Tijl Coosemans wrote:
> On Tue, 04 Mar 2014 06:45:58 +0000 Max Brazhnikov wrote:
> > Hi Tijl,
> > 
> > Is it enough to patch single qt4-corelib port? Shouldn't the patch be
> > shared among other Qt4 ports?
> > 
> > Max
> 
> You mean to let other Qt ports respect CXXFLAGS during configure or
> to fix iconv issues?

All Qt ports are built from the same tarball, other qt4 ports could pick
up ports libiconv again, if the fix is not global.
Comment 5 Tijl Coosemans freebsd_committer freebsd_triage 2014-03-06 22:12:06 UTC
On Wed, 05 Mar 2014 06:08:43 +0000 Max Brazhnikov wrote:
> On Tue, 04 Mar 2014 23:12:09 +0100 Tijl Coosemans wrote:
>> On Tue, 04 Mar 2014 06:45:58 +0000 Max Brazhnikov wrote:
>>> Is it enough to patch single qt4-corelib port? Shouldn't the patch be
>>> shared among other Qt4 ports?
>> 
>> You mean to let other Qt ports respect CXXFLAGS during configure or
>> to fix iconv issues?
> 
> All Qt ports are built from the same tarball, other qt4 ports could pick
> up ports libiconv again, if the fix is not global.

I grepped the full tarball for iconv\.h and only these cases came up:

config.tests/unix/gnu-libiconv/gnu-libiconv.cpp:#include <iconv.h>
config.tests/unix/iconv/iconv.cpp:#include <iconv.h>
src/corelib/codecs/qiconvcodec_p.h:#include <iconv.h>

The first two are configure tests and the last one is a private header
from corelib.  That header is included from these two .cpp files:

src/corelib/codecs/qtextcodec.cpp:#  include "qiconvcodec_p.h"
src/corelib/codecs/qiconvcodec.cpp:#include "qiconvcodec_p.h"

These essentially wrap the iconv API, so I think the use of iconv is
really restricted to corelib and for me the patch is good enough.
It would be nice to fix all qt4 ports so they all respect CXXFLAGS
during configure (build phase is already ok), but I'm not sure where
in bsd.qt.mk a REINPLACE_CMD or something can be added to accomplish
this.
Comment 6 Max Brazhnikov freebsd_committer freebsd_triage 2014-03-09 16:37:51 UTC
On Thu, 06 Mar 2014 23:12:06 +0100 Tijl Coosemans wrote:
> On Wed, 05 Mar 2014 06:08:43 +0000 Max Brazhnikov wrote:
> > On Tue, 04 Mar 2014 23:12:09 +0100 Tijl Coosemans wrote:
> >> On Tue, 04 Mar 2014 06:45:58 +0000 Max Brazhnikov wrote:
> >>> Is it enough to patch single qt4-corelib port? Shouldn't the patch be
> >>> shared among other Qt4 ports?
> >> 
> >> You mean to let other Qt ports respect CXXFLAGS during configure or
> >> to fix iconv issues?
> > 
> > All Qt ports are built from the same tarball, other qt4 ports could pick
> > up ports libiconv again, if the fix is not global.
> 
> I grepped the full tarball for iconv\.h and only these cases came up:
> 
> config.tests/unix/gnu-libiconv/gnu-libiconv.cpp:#include <iconv.h>
> config.tests/unix/iconv/iconv.cpp:#include <iconv.h>
> src/corelib/codecs/qiconvcodec_p.h:#include <iconv.h>
> 
> The first two are configure tests and the last one is a private header
> from corelib.  That header is included from these two .cpp files:
> 
> src/corelib/codecs/qtextcodec.cpp:#  include "qiconvcodec_p.h"
> src/corelib/codecs/qiconvcodec.cpp:#include "qiconvcodec_p.h"
> 
> These essentially wrap the iconv API, so I think the use of iconv is
> really restricted to corelib and for me the patch is good enough.

ok.

> It would be nice to fix all qt4 ports so they all respect CXXFLAGS
> during configure (build phase is already ok), but I'm not sure where
> in bsd.qt.mk a REINPLACE_CMD or something can be added to accomplish
> this.

You can use common EXTRA_PATCHES defined in bsd.qt.mk for Qt ports,
the patches are stored in devel/qt4.
Comment 7 Tijl Coosemans freebsd_committer freebsd_triage 2014-03-20 13:41:19 UTC
On Sun, 09 Mar 2014 16:37:51 +0000 Max Brazhnikov wrote:
> On Thu, 06 Mar 2014 23:12:06 +0100 Tijl Coosemans wrote:
>> It would be nice to fix all qt4 ports so they all respect CXXFLAGS
>> during configure (build phase is already ok), but I'm not sure where
>> in bsd.qt.mk a REINPLACE_CMD or something can be added to accomplish
>> this.
> 
> You can use common EXTRA_PATCHES defined in bsd.qt.mk for Qt ports,
> the patches are stored in devel/qt4.

I've attached a patch that uses this.  It also applies the patch to
qt5.  qt5-core doesn't actually need iconv so the patch removes that
dependency.  There's this in src/corelib/codecs/codecs.pri:

contains(QT_CONFIG,icu) {
...
} else {
...  use iconv ...
}

And the port Makefile adds "icu" to QT_CONFIG.

Tested on redports:
qt4: https://redports.org/buildarchive/20140319203417-94556/
qt5: https://redports.org/buildarchive/20140319203451-88592/
Comment 8 Max Brazhnikov freebsd_committer freebsd_triage 2014-03-23 19:02:46 UTC
On Thu, 20 Mar 2014 14:41:19 +0100 Tijl Coosemans wrote:
> On Sun, 09 Mar 2014 16:37:51 +0000 Max Brazhnikov wrote:
> > On Thu, 06 Mar 2014 23:12:06 +0100 Tijl Coosemans wrote:
> >> It would be nice to fix all qt4 ports so they all respect CXXFLAGS
> >> during configure (build phase is already ok), but I'm not sure where
> >> in bsd.qt.mk a REINPLACE_CMD or something can be added to accomplish
> >> this.
> > 
> > You can use common EXTRA_PATCHES defined in bsd.qt.mk for Qt ports,
> > the patches are stored in devel/qt4.
> 
> I've attached a patch that uses this.  It also applies the patch to
> qt5.  qt5-core doesn't actually need iconv so the patch removes that
> dependency.  There's this in src/corelib/codecs/codecs.pri:
> 
> contains(QT_CONFIG,icu) {
> ....
> } else {
> ....  use iconv ...
> }
> 
> And the port Makefile adds "icu" to QT_CONFIG.

Then iconv can be explicitly disabled in qt5-core, please replace
-no-feature-iconv with -no-iconv and commit with my approval.
 
> Tested on redports:
> qt4: https://redports.org/buildarchive/20140319203417-94556/
> qt5: https://redports.org/buildarchive/20140319203451-88592/

Thanks a lot for dipping into the problem and fixing it!

Max
Comment 9 dfilter service freebsd_committer freebsd_triage 2014-03-23 19:58:39 UTC
Author: tijl
Date: Sun Mar 23 19:58:33 2014
New Revision: 348886
URL: http://svnweb.freebsd.org/changeset/ports/348886
QAT: https://qat.redports.org/buildarchive/r348886/

Log:
  - Make Qt4 and Qt5 respect CXXFLAGS during configure so they pick up
    -DLIBICONV_PLUG from USES=iconv and always use libc iconv when it is
    available.
  - Remove the iconv dependency from Qt5.  It uses icu instead.
  
  PR:		ports/186707
  Approved by:	kde (makc)

Added:
  head/devel/qt4/files/extrapatch-config.tests-unix-compile.test   (contents, props changed)
  head/devel/qt5/files/extrapatch-config.tests-unix-compile.test   (contents, props changed)
Modified:
  head/Mk/bsd.qt.mk
  head/devel/qt4-corelib/Makefile
  head/devel/qt5-core/Makefile

Modified: head/Mk/bsd.qt.mk
==============================================================================
--- head/Mk/bsd.qt.mk	Sun Mar 23 19:50:59 2014	(r348885)
+++ head/Mk/bsd.qt.mk	Sun Mar 23 19:58:33 2014	(r348886)
@@ -166,7 +166,8 @@ CONFIGURE_ARGS+=-verbose
 . endif
 
 . if ${QT_DIST} == "base" || ${_QT_VERSION:M4*}
-EXTRA_PATCHES?=	${.CURDIR:H:H}/devel/${_QT_RELNAME}/files/extrapatch-configure
+EXTRA_PATCHES?=	${.CURDIR:H:H}/devel/${_QT_RELNAME}/files/extrapatch-configure \
+		${.CURDIR:H:H}/devel/${_QT_RELNAME}/files/extrapatch-config.tests-unix-compile.test
 .  if ${_QT_VERSION:M5*}
 EXTRA_PATCHES+=	${.CURDIR:H:H}/devel/qt5-core/files/extrapatch-src__corelib__tools__qdatetime.cpp
 .  endif

Modified: head/devel/qt4-corelib/Makefile
==============================================================================
--- head/devel/qt4-corelib/Makefile	Sun Mar 23 19:50:59 2014	(r348885)
+++ head/devel/qt4-corelib/Makefile	Sun Mar 23 19:58:33 2014	(r348886)
@@ -3,7 +3,7 @@
 
 PORTNAME=	corelib
 DISTVERSION=	${QT4_VERSION}
-PORTREVISION=	2
+PORTREVISION=	3
 CATEGORIES=	devel
 PKGNAMEPREFIX=	qt4-
 

Added: head/devel/qt4/files/extrapatch-config.tests-unix-compile.test
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/devel/qt4/files/extrapatch-config.tests-unix-compile.test	Sun Mar 23 19:58:33 2014	(r348886)
@@ -0,0 +1,11 @@
+--- config.tests/unix/compile.test.orig
++++ config.tests/unix/compile.test
+@@ -13,7 +13,7 @@
+ shift 7
+ LFLAGS="$SYSROOT_FLAG"
+ INCLUDEPATH=""
+-CXXFLAGS="$SYSROOT_FLAG"
++CXXFLAGS="$CXXFLAGS $SYSROOT_FLAG"
+ MAC_ARCH_CXXFLAGS=""
+ MAC_ARCH_LFLAGS=""
+ while [ "$#" -gt 0 ]; do

Modified: head/devel/qt5-core/Makefile
==============================================================================
--- head/devel/qt5-core/Makefile	Sun Mar 23 19:50:59 2014	(r348885)
+++ head/devel/qt5-core/Makefile	Sun Mar 23 19:58:33 2014	(r348886)
@@ -2,6 +2,7 @@
 
 PORTNAME=	core
 DISTVERSION=	${QT5_VERSION}
+PORTREVISION=	1
 CATEGORIES=	devel
 PKGNAMEPREFIX=	qt5-
 
@@ -14,14 +15,13 @@ LIB_DEPENDS=	libicui18n.so:${PORTSDIR}/d
 USE_GNOME=	glib20
 USE_QT5=	qmake_build buildtools_build
 QT_DIST=	base
-USES=		iconv
 HAS_CONFIGURE=	yes
 # Disable (almost) everything to install minimal qconfig.h.
 # -no-feature-* adds QT_NO_* (for features which have no switch or
 # that need to be detected).
 CONFIGURE_ARGS=	-no-accessibility -no-gif -no-libpng -no-libjpeg \
 		-no-openssl -no-gui -no-widgets -no-cups \
-		-no-feature-iconv -no-dbus -no-xcb -no-opengl \
+		-no-iconv -no-dbus -no-xcb -no-opengl \
 		-no-feature-glib -no-feature-alsa \
 		-no-feature-concurrent -no-feature-evdev \
 		-no-fontconfig -no-freetype \
@@ -34,17 +34,11 @@ USE_LDCONFIG=	${PREFIX}/${QT_LIBDIR_REL}
 BUILD_WRKSRC=	${WRKSRC}/src/corelib
 INSTALL_WRKSRC=	${BUILD_WRKSRC}
 
-QT_DEFINES=	GLIB ICONV
+QT_DEFINES=	GLIB
 QT_CONFIG=	glib icu
 
 .include <bsd.port.pre.mk>
 
-.if ${ICONV_PREFIX} == "/usr"
-QT_CONFIG+=	sun-libiconv
-.else
-QT_CONFIG+=	gnu-libiconv
-.endif
-
 post-install:
 	${INSTALL_DATA} ${WRKSRC}/src/3rdparty/harfbuzz/src/harfbuzz*.h \
 		${STAGEDIR}${PREFIX}/${QT_INCDIR_REL}/QtCore/${QT5_VERSION:C/-.*//}/QtCore/private

Added: head/devel/qt5/files/extrapatch-config.tests-unix-compile.test
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/devel/qt5/files/extrapatch-config.tests-unix-compile.test	Sun Mar 23 19:58:33 2014	(r348886)
@@ -0,0 +1,11 @@
+--- config.tests/unix/compile.test.orig
++++ config.tests/unix/compile.test
+@@ -13,7 +13,7 @@
+ shift 7
+ LFLAGS="$SYSROOT_FLAG"
+ INCLUDEPATH=""
+-CXXFLAGS="$SYSROOT_FLAG"
++CXXFLAGS="$CXXFLAGS $SYSROOT_FLAG"
+ MAC_ARCH_CXXFLAGS=""
+ MAC_ARCH_LFLAGS=""
+ while [ "$#" -gt 0 ]; do
_______________________________________________
svn-ports-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-ports-all
To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"
Comment 10 Tijl Coosemans freebsd_committer freebsd_triage 2014-03-23 19:59:01 UTC
State Changed
From-To: open->closed

Committed in r348886.