Bug 480422 – dev-libs/icu-51.2-r1 - add multilib support

• Product: Gentoo Linux
• Status: RESOLVED FIXED

[Description Mike Lothian 2013-08-09 19:02:04 UTC]

New ebuild

Reproducible: Always

[Comment 1 Mike Lothian 2013-08-09 19:03:07 UTC]

Created attachment 355534 [details]
New icu ebuild

Adds 32bit support and creates icu-config.abi for additional abi's

[Comment 2 Jeroen Roovers (RETIRED) 2013-08-11 12:54:48 UTC]

Comment on attachment 355534 [details]
New icu ebuild

--- icu-51.2.ebuild     2013-07-28 22:34:15.670697106 +0200
+++ -   2013-08-11 14:54:38.177658692 +0200
@@ -4,7 +4,7 @@
 
 EAPI=5

-inherit eutils toolchain-funcs base autotools
+inherit eutils toolchain-funcs base autotools multilib-minimal
 
 DESCRIPTION="International Components for Unicode"
 HOMEPAGE="http://www.icu-project.org/"
@@ -53,9 +53,11 @@
                -e 's:icudefs.mk:icudefs.mk Doxyfile:' \
                configure.in || die
        eautoreconf
+
+       multilib_copy_sources
 }

-src_configure() {
+multilib_src_configure() {
        local cross_opts

        # bootstrap for cross compilation
@@ -81,7 +83,7 @@
                ${cross_opts}
 }

-src_compile() {
+multilib_src_compile() {
        default
        
        if use doc; then
@@ -90,7 +92,7 @@
        fi
 }

-src_test() {
+multilib_src_test() {
        # INTLTEST_OPTS: intltest options
        #   -e: Exhaustive testing
        #   -l: Reporting of memory leaks
@@ -104,10 +106,23 @@
        emake -j1 VERBOSE="1" check
 }

-src_install() {
+multilib_src_install() {
        default
        
        dohtml ../readme.html

        use doc && dohtml -p api -r doc/html/
+
+       if multilib_is_native_abi; then
+               # Move files back.
+               if path_exists -o "${ED}"/tmp/icu-config.*; then
+                       mv "${ED}"/tmp/icu-config.* "${ED}"/usr/bin || die
+               fi
+       else
+               # Preserve ABI-variant of icu-config,
+               # then drop all the executables
+               mkdir -p "${ED}"/tmp || die
+               mv "${ED}"/usr/bin/icu-config "${ED}"/tmp/icu-config.${ABI} || die
+               rm -r "${ED}"/usr/bin || die
+       fi
 }

[Comment 3 Pacho Ramos 2013-09-05 19:52:53 UTC]

(In reply to Jeroen Roovers from comment #2)
[...]
>        use doc && dohtml -p api -r doc/html/
> +
> +       if multilib_is_native_abi; then
> +               # Move files back.
> +               if path_exists -o "${ED}"/tmp/icu-config.*; then
> +                       mv "${ED}"/tmp/icu-config.* "${ED}"/usr/bin || die
> +               fi
> +       else
> +               # Preserve ABI-variant of icu-config,
> +               # then drop all the executables
> +               mkdir -p "${ED}"/tmp || die
> +               mv "${ED}"/usr/bin/icu-config "${ED}"/tmp/icu-config.${ABI}
> || die
> +               rm -r "${ED}"/usr/bin || die
> +       fi
>  }

I am a bit unsure about if this part could be handled in a different way :/

[Comment 4 Alexis Ballier 2013-09-11 16:56:23 UTC]

Comment on attachment 355534 [details]
New icu ebuild

>src_prepare() {
[...]
>	multilib_copy_sources

out of tree builds seem to work. don't they?

>multilib_src_configure() {
>	local cross_opts
>
>	# bootstrap for cross compilation
>	if tc-is-cross-compiler; then
>		CFLAGS="" CXXFLAGS="" ASFLAGS="" LDFLAGS="" \
>		CC="$(tc-getBUILD_CC)" CXX="$(tc-getBUILD_CXX)" AR="$(tc-getBUILD_AR)" \
>		RANLIB="$(tc-getBUILD_RANLIB)" LD="$(tc-getBUILD_LD)" \
>		./configure --disable-renaming --disable-debug \
>			--disable-samples --enable-static || die
>		emake
>		mkdir -p "${WORKDIR}/host/"
>		cp -a {bin,lib,config,tools} "${WORKDIR}/host/"
>		emake clean
>
>		cross_opts="--with-cross-build=${WORKDIR}/host"
>	fi

this belongs to src_configure and not to multilib_src_configure: we want to bootstrap it once for the whole package, not once for each abi

also, if we are going for out of tree builds this should be simplified / fixed too.

>
>	econf \
>		--disable-renaming \
>		$(use_enable debug) \
>		$(use_enable examples samples) \

what does --enable-samples do ? those are likely binaries that are useless for non native ABIs

>		$(use_enable static-libs static) \
>		${cross_opts}
>}
>
>multilib_src_compile() {
>	default
>
>	if use doc; then
>		doxygen -u Doxyfile || die
>		doxygen Doxyfile || die
>	fi
>}

do not build the documentation for non native ABIs either: it will be overwritten anyway


>
>multilib_src_test() {
>	# INTLTEST_OPTS: intltest options
>	#   -e: Exhaustive testing
>	#   -l: Reporting of memory leaks
>	#   -v: Increased verbosity
>	# IOTEST_OPTS: iotest options
>	#   -e: Exhaustive testing
>	#   -v: Increased verbosity
>	# CINTLTST_OPTS: cintltst options
>	#   -e: Exhaustive testing
>	#   -v: Increased verbosity
>	emake -j1 VERBOSE="1" check
>}
>
>multilib_src_install() {
>	default
>
>	dohtml ../readme.html
>
>	use doc && dohtml -p api -r doc/html/

these dohtml must be in src_install not multilib_

>	if multilib_is_native_abi; then
>		# Move files back.
>		if path_exists -o "${ED}"/tmp/icu-config.*; then
>			mv "${ED}"/tmp/icu-config.* "${ED}"/usr/bin || die
>		fi
>	else
>		# Preserve ABI-variant of icu-config,
>		# then drop all the executables
>		mkdir -p "${ED}"/tmp || die
>		mv "${ED}"/usr/bin/icu-config "${ED}"/tmp/icu-config.${ABI} || die
>		rm -r "${ED}"/usr/bin || die
>	fi
>}

I'd rather not do that. We will have to patch every single package using icu-config.
If we really have to do some renaming, this code should be in the eclasses just like header wrapping.

I'd rather either wrap it like abiwrapper from multilib-portage imported into multilib eclasses, or even better: just don't do anything.

Does multilib libxml2 fails if it is using native abi's icu-config ? Can libxml2 be patched to use pkgconfig instead?

[Comment 5 Andrew Church 2013-10-21 14:11:52 UTC]

Created attachment 361526 [details, diff]
icu-51.2.ebuild.patch

FWIW, this trivial multilib patch is enough for harfbuzz to build (see bug 488864), though harfbuzz uses pkg-config in preference to icu-config so the issue with the latter doesn't arise.

[Comment 6 Michał Górny 2013-12-20 09:25:16 UTC]

(In reply to Pacho Ramos from comment #3)
> (In reply to Jeroen Roovers from comment #2)
> [...]
> >        use doc && dohtml -p api -r doc/html/
> > +
> > +       if multilib_is_native_abi; then
> > +               # Move files back.
> > +               if path_exists -o "${ED}"/tmp/icu-config.*; then
> > +                       mv "${ED}"/tmp/icu-config.* "${ED}"/usr/bin || die
> > +               fi
> > +       else
> > +               # Preserve ABI-variant of icu-config,
> > +               # then drop all the executables
> > +               mkdir -p "${ED}"/tmp || die
> > +               mv "${ED}"/usr/bin/icu-config "${ED}"/tmp/icu-config.${ABI}
> > || die
> > +               rm -r "${ED}"/usr/bin || die
> > +       fi
> >  }
> 
> I am a bit unsure about if this part could be handled in a different way :/

icu seems to have pkg-config files, so if they cover everything icu-config does, we just install those and expect multilib-aware software to use pkg-config.

[Comment 7 Michał Górny 2013-12-22 14:58:38 UTC]

I'll work on this.

[Comment 8 Michał Górny 2013-12-22 16:20:23 UTC]

Created attachment 365890 [details, diff]
Proposed ebuild

I've removed the base.eclass inherit we're hating. Works well. The cross-compile part seems to work too.

@maintainer: ok to commit this?

[Comment 9 Michał Górny 2013-12-26 11:12:53 UTC]

+*icu-51.2-r2 (26 Dec 2013)
+
+  26 Dec 2013; Michał Górny <[e-mail]> +icu-51.2-r2.ebuild:
+  Introduce multilib support wrt bug #480422. Acked by scarabeus.