Gentoo Archives: gentoo-dev

From: "Michał Górny" <mgorny@g.o>
To: gentoo-dev@l.g.o
Cc: dberkholz@g.o
Subject: Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.
Date: Mon, 12 Sep 2011 21:57:03
Message-Id: 20110912235827.43dfb959@pomiocik.lan
In Reply to: Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs. by Donnie Berkholz
On Mon, 12 Sep 2011 16:00:20 -0500
Donnie Berkholz <dberkholz@g.o> wrote:

> > # @FUNCTION: remove_libtool_files > > -# @USAGE: [all|none] > > +# @USAGE: [all|only-not-required|none] > > Is there a way to document the arguments of eclass functions? You > added the name of the arg but didn't describe its purpose or why > anyone would want to use it. > > On a semantic note, that argument name (only-not-required) doesn't > make sense to me. I might do something more helpful like > pkgconfig-duplicates instead.
I thinked about 'as-needed' or sth like that. Maybe the new argument should be (temporarily) not public instead?
> > + if [[ "$1" == 'only-not-required' ]]; then > > This is way more quoting than you need within double brackets.
It's nice visual quoting, just to match the others.
> > local f > > for f in $(find "${D}" -type f -name '*.la'); do > > # Keep only .la files with shouldnotlink=yes - > > likely plugins local shouldnotlink=$(sed -ne > > '/^shouldnotlink=yes$/p' "${f}") if [[ "$1" == 'all' || -z > > ${shouldnotlink} ]]; then > > + if [[ "$1" == 'only-not-required' ]]; then > > Is there a case where one of those arguments might be $2 but you'd > still want to run this?
Er? What are you referring to?
> I feel like that shouldnotlink thing is really confusing the logic, > because there's multiple nested tests for different values of $1 in > here instead of just testing the args once at the top and setting > variables.
As mentioned earlier, the code needs to be refactored. First things first, then we'll rewrite it to be nice and clean. I don't really want to waste time doing this if we would need to rewrite it for more logic in the future.
> > + # remove .la files only when .pc > > files provide the libs > > + # already or they don't give any > > information > > + ! has $(basename "${f}") > > ${pc_libs} \ > > + && [[ -n "$(sed -n > > \ > > The comment says "or" but I see an "and" here.
Because everything's negated here. Boolean magic :D. -- Best regards, Michał Górny

Attachments

File name MIME type
signature.asc application/pgp-signature

Replies