Re: [gentoo-dev] [PATCH 2/2] Use new multilib flags in autotools-multilib.

[Alexis Ballier]

On Sat, 26 Jan 2013 13:11:41 +0100
Michał Górny <mgorny@g.o> wrote:

> On Wed, 23 Jan 2013 21:40:13 -0300
> Alexis Ballier <aballier@g.o> wrote:
>
> > On Thu, 24 Jan 2013 00:23:57 +0100
> > Michał Górny <mgorny@g.o> wrote:
> >
> > > This is mostly a proof-of-concept. If approved, I will work on
> > > moving the code into a separate eclass, possibly named
> > > 'multilib-build' ;). ---
> > >  gx86/eclass/autotools-multilib.eclass | 24
> > > +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3
> > > deletions(-)
> > >
> > > diff --git a/gx86/eclass/autotools-multilib.eclass
> > > b/gx86/eclass/autotools-multilib.eclass index 7c8697a..eef7bcc
> > > 100644 --- a/gx86/eclass/autotools-multilib.eclass
> > > +++ b/gx86/eclass/autotools-multilib.eclass
> > > @@ -32,7 +32,23 @@ inherit autotools-utils multilib
> > >
> > >  EXPORT_FUNCTIONS src_configure src_compile src_test src_install
> > >
> > > -IUSE=multilib
> > > +# Declare all of them, profiles will control their visibility.
> > > +IUSE='abi_x86_32 abi_x86_64'
> > > +
> > > +# @FUNCTION: _autotools-multilib_get_enabled_abis
> > > +# @DESCRIPTION:
> > > +# Get the list of enabled ABIs. The returned names are suitable
> > > for use +# with multilib.eclass.
> > > +#
> > > +# If multilib is not enabled or not supported, returns an empty
> > > list. +
> > > +	debug-print-function ${FUNCNAME} "${@}"
> > > +
> > > +	if use amd64; then
> > > +		use abi_x86_64 && echo amd64
> > > +		use abi_x86_32 && echo x86
> > > +	fi
> > > +}
> >
> > I would rather iterate over a variable than hardcoding and
> > duplicating it here:
> >
> > MULTILIB_ABIS='abi_x86_32:x86 abi_x86_64:amd64'
> > IUSE=""
> > for i in $MULTILIB_ABIS ; do
> >    IUSE+=" ${i%:*}"
> > done
> >
> > _autotools-multilib_get_enabled_abis() {
> >    for i in $MULTILIB_ABIS ; do
> >       use ${i%:*} && echo ${i#*:}
> >    done
> > }
>
> What are the advantages? I feel like the explicit solution is much
> more readable and obvious at the first glance.

yes it is more readable but IMHO it's better to avoid to have to touch
multiple places when adding a new ABI: you only have to document
that adding a new ABI consists simply in adding it to this list (and
document the useflag) instead of 'add the useflag, add support for it to
function foo and bar, etc.'
your call in the end, but I fear not trying to separate code from data
could make the eclass harder to maintain.

also, it'll make code much shorter when all the exotic ABIs will be
added :)


>
> > (maybe protect it with has_multilib_profile if you wish)
>
> Well, the current code assumes that no flags == non-multilib profile.

in the code you posted you do not seem to take into account amd64
non-multilib then :)

also, IMHO you shouldn't use arch to guess what useflag to check or
not: they'll all be in IUSE in the end and the profiles should be the
one deciding what to mask or not (which you'll have to do anyway), not
the eclass.

Alexis.