Re: [gentoo-dev] Proposal for flag-o-matic.eclass (append-ldflags)

[Mike Frysinger]

On Tuesday 09 December 2008 14:14:05 Donnie Berkholz wrote:
> On 10:33 Mon 08 Dec     , Jeremy Olexa wrote:
> > Hello,
> > I am seeking a positive code review on the following change to
> > flag-o-matic.eclass, diff is below (reasons are below that):
> >
> > %% cvs diff
> > Index: flag-o-matic.eclass
> > RCS file: /var/cvsroot/gentoo-x86/eclass/flag-o-matic.eclass,v
> > retrieving revision 1.126
> > diff -u -r1.126 flag-o-matic.eclass
> > --- flag-o-matic.eclass 3 Nov 2008 05:52:39 -0000       1.126
> > +++ flag-o-matic.eclass 25 Nov 2008 18:36:04 -0000
> > @@ -417,7 +417,8 @@
> >
> >        x=""
> >        for x in "$@" ; do
> > -               test-flag-${comp} "${x}" && flags="${flags}${flags:+
> > }${x}" +               test-flag-${comp} "${x}" &&
> > flags="${flags}${flags:+ }${x}" || \ +                       ewarn
> > "removing ${x} because ${comp} rejected it" done
> >
> >        echo "${flags}"
> > @@ -656,7 +657,7 @@
> >                        ewarn "Appending a library link instruction
> > (${flag}); libraries to link to should not be passed through LDFLAGS"
> >        done
> >
> > -       export LDFLAGS="${LDFLAGS} $*"
> > +       export LDFLAGS="${LDFLAGS} $(test-flags "$@")"
> >        return 0
> >  }
>
> I like the consistency with other flags:
>
> comet $ grep FLAGS.*test-fl /usr/portage/eclass/flag-o-matic.eclass
> 	export CFLAGS=$(test-flags-CC ${CFLAGS})
> 	export CXXFLAGS=$(test-flags-CXX ${CXXFLAGS})
> 	export FFLAGS=$(test-flags-F77 ${FFLAGS})
> 	export FCFLAGS=$(test-flags-FC ${FCFLAGS})

your grep ignores the bigger picture.  this is only in the strip-unsupported-
flag function.  so while we should flesh it out to include CPPFLAGS/LDFLAGS, 
it doesnt really address the original question.
-mike