Created attachment 361606 [details, diff] gdk-pixbuf-2.28.2.ebuild.patch Dependency of GTK+. The gdk-pixbuf-query-loaders-32 bit is copied out of emul-linux-x86-gtklibs (with appropriate changes for native multilib).
Can't we just use program-suffix instead of moving bins around ?
>Can't we just use program-suffix instead of moving bins around ? I thought about that, but that would install all of the binaries (gdk-pixbuf-csource, gdk-pixbuf-pixdata, gdk-pixbuf-query-loaders) when the only one we actually need a 32-bit version of is query-loaders. We could rm the unneeded ones, but as long as we're modifying the install anyway, it seemed more concise to just skip program-suffix.
Due to the recent discussions on multilib@gentoo.org, should the binary be named ${CHOST}-gdk-* ? And multilib_build_binaries should be used to decide if all binaries should be build.
Created attachment 366592 [details, diff] gdk-pixbuf-2.28.2.ebuild.patch Thanks for the review. I've updated the patch to use mutilib_build_binaries, and it will now use a ${CHOST}- prefix on non-native ABI binaries -- is that what you intended? (I'm not on multilib@gentoo.org and I can't find a list archive offhand.)
We should wait until Michał's latest patch to wrap binaries gets merged and use the functions provided therein.
MULTILIB_CHOST_TOOLS from multilib-build.eclass can now be used.
Created attachment 367984 [details, diff] gdk-pixbuf-2.28.2.ebuild.patch Patch updated. It works fine for me, but I'm not sure whether or how MULTILIB_CHOST_TOOLS would interact with --program-prefix in the multilib_build_binaries && ! multilib_is_native_abi case. Will the eclass deal with that properly?
Please don't hardcode lib32 in pkg_postinst(), instead do something like this: do_update() { local GDK_PIXBUF_UPDATE_BIN=/usr/bin/${CHOST}-gdk-pixbuf-query-loaders gnome2_gdk_pixbuf_update } multilib_foreach_abi do_update This way, all ABIs get updated and the correct libdir is used for each ABI (for instance, I have LIBDIR_x86=lib, and *no* lib32 dir on my system).
Created attachment 367986 [details, diff] gdk-pixbuf-2.28.2.ebuild.patch Done.
Comment on attachment 367986 [details, diff] gdk-pixbuf-2.28.2.ebuild.patch >--- gdk-pixbuf-2.28.2.ebuild.orig 2013-09-04 07:01:36 +0900 >+++ gdk-pixbuf-2.28.2.ebuild 2014-01-17 17:24:52 +0900 >@@ -4,7 +4,7 @@ >-src_configure() { >+multilib_src_configure() { >+ local program_prefix="" >+ if multilib_build_binaries && ! multilib_is_native_abi; then >+ program_prefix="--program-prefix=${CHOST}-" >+ fi What does it do? Aren't you basically mixing eclass renaming with some custom renaming? > # png always on to display icons >+ ECONF_SOURCE="${S}" \ > econf \ >+ ${program_prefix} \ > $(usex debug --enable-debug=yes "") \ > $(use_with jpeg libjpeg) \ > $(use_with jpeg2k libjasper) \ > $(use_with tiff libtiff) \ >- $(use_enable introspection) \ >+ $(multilib_is_native_abi \ >+ && use_enable introspection \ >+ || echo --disable-introspection) \ multilib_build_binaries here. You should never use multilib_is_native_abi. >+do_update() { >+ local GDK_PIXBUF_UPDATE_BIN=/usr/bin/${CHOST}-gdk-pixbuf-query-loaders >+ gnome2_gdk_pixbuf_update >+} Can't we move this logic to the eclass? I don't think it'd hurt anything if it had a search logic similar to autoconf.
Created attachment 368014 [details, diff] gdk-pixbuf-2.28.2.ebuild.patch >What does it do? Aren't you basically mixing eclass renaming with some custom >renaming? This is to handle binaries other than gdk-pixbuf-query-loaders (which is always installed for all ABIs). Isn't program-prefix necessary when building binaries for non-native ABIs? Or do you just assume a separate prefix for each ABI in that case? >multilib_build_binaries here. You should never use multilib_is_native_abi. Done. >Can't we move this logic to the eclass? I don't think it'd hurt anything if it >had a search logic similar to autoconf. SGTM. Should I put a patch here or open a separate bug for it?
(In reply to Andrew Church from comment #11) > >What does it do? Aren't you basically mixing eclass renaming with some custom >renaming? > > This is to handle binaries other than gdk-pixbuf-query-loaders (which is > always installed for all ABIs). Isn't program-prefix necessary when > building binaries for non-native ABIs? Or do you just assume a separate > prefix for each ABI in that case? Does it make sense to wrap all of them? I'd rather you use the eclass renaming since it keeps basename working. > >Can't we move this logic to the eclass? I don't think it'd hurt anything if it >had a search logic similar to autoconf. > > SGTM. Should I put a patch here or open a separate bug for it? Wait for gnome@'s reply.
-src_configure() { +multilib_src_configure() { + local program_prefix="" + if multilib_build_binaries && ! multilib_is_native_abi; then + program_prefix="--program-prefix=${CHOST}-" + fi This part does make no sense at all and means your assignment will never be done. multilib_is_native_abi and multilib_build_binaries by default return the same.
>Does it make sense to wrap all of them? I'd rather you use the eclass renaming >since it keeps basename working. Not really, since only gdk-pixbuf-query-loaders is needed for all ABIs. E.g. on multilib amd64, we still need i686-...-query-loaders to generate the loader cache for 32-bit programs, but the other binaries only need amd64 versions. >This part does make no sense at all and means your assignment will never be >done. multilib_is_native_abi and multilib_build_binaries by default return the >same. Then how does one tell whether building for a non-native ABI?
(In reply to Andrew Church from comment #14) > >This part does make no sense at all and means your assignment will never be >done. multilib_is_native_abi and multilib_build_binaries by default return the >same. > > Then how does one tell whether building for a non-native ABI? multilib_build_binaries can be used the same way as previously multilib_is_default_abi, so just check for a false of multilib_build_binaries. As a short information: multilib_build_binaries is the same as multilib_is_native_abi, but it can be overriden for external use-cases like multilb-portage. After initial introduction, where it was only planned for conditional building of binaries, it has been shown that it is instead a complete replacement for multilib_is_native_abi, so you should always use multilib_build_binaries instead of multilib_is_native_abi.
>As a short information: multilib_build_binaries is the same as >multilib_is_native_abi, but it can be overriden for external use-cases like >multilb-portage. After initial introduction, where it was only planned for >conditional building of binaries, it has been shown that it is instead a >complete replacement for multilib_is_native_abi, so you should always use >multilib_build_binaries instead of multilib_is_native_abi. Hmm, okay... I think the is_native_abi name is clearer, but whatever (: In that case I'll just drop the program-prefix setting since there won't be multiple ABI versions of non-wrapped binaries.
Created attachment 368032 [details, diff] gdk-pixbuf-2.28.2.ebuild.patch
Looks ok to me.
ping @ gnome!
Sorry but adding multilib support is not our #1 priority right now. We will process these reports as soon as possible but our focus is on stabilizing Gnome 3.8 and 3.10 and stop being months late in Gnome releases. Thanks for your understanding.
Anyway, if people from multilib has time to review the patches and they consider them ok, no problem with going ahead and committing on a new revision as done already with glib (for example) and other gnome maintained stuff
As said above the patch looks good to me. @mgorny: any final comments?
Could you update the patch for the latest ebuild, please? It does no longer apply cleanly.
Created attachment 370008 [details, diff] gdk-pixbuf-2.30.4.ebuild.patch Here you go.
Well, it's missing a proper blocker against emul-linux-x86-gtklibs. Other than that, looks fine. @gnome: though I'd honestly prefer having some kind of 'type -p' check for ${CHOST}-gdk-pixbuf-query-loaders.
Did you want me to add the emul-linux blocker? I figured that would have to be synced with an update to emul-linux-x86-gtklibs so I left it out.
+*gdk-pixbuf-2.30.5-r1 (01 Mar 2014) + + 01 Mar 2014; Michał Górny <mgorny@gentoo.org> +gdk-pixbuf-2.30.5-r1.ebuild: + Introduce multilib support, bug #488998. +*emul-linux-x86-gtklibs-20131008-r3 (01 Mar 2014) + + 01 Mar 2014; Michał Górny <mgorny@gentoo.org> + +emul-linux-x86-gtklibs-20131008-r3.ebuild, +files/remove-native-20131008-r3: + Add gdk-pixbuf.
Well, let's mark it FIXED to have dep-graph updated.
In 2.30.6-r1 pkg_postrm(), why are you using "rm -f "${EROOT}"usr/lib*/${PN}-2.0/2.10.0/loaders.cache" instead of "multilib_foreach_abi rm -f "${EROOT}"usr/$(get_libdir)/${PN}-2.0/2.10.0/loaders.cache"? It shouldn't cause problems for users with sane profiles, but it looks strange.
(In reply to Alexandre Rostovtsev from comment #29) > In 2.30.6-r1 pkg_postrm(), why are you using "rm -f > "${EROOT}"usr/lib*/${PN}-2.0/2.10.0/loaders.cache" instead of > "multilib_foreach_abi rm -f > "${EROOT}"usr/$(get_libdir)/${PN}-2.0/2.10.0/loaders.cache"? > > It shouldn't cause problems for users with sane profiles, but it looks > strange. For proper $(get_libdir) expansion, it would have to go to a sub-function. I doubt it makes real sense but feel free to change that.
