On Wed, 30 May 2012 17:19:49 -0400 Mike Frysinger <vap...@gentoo.org> wrote:
> On Monday 28 May 2012 03:58:56 Michał Górny wrote:
> > +# @USAGE: [all]
>
> this is incorrect. the usage is:
> <all | files to remove>
No, it's perfectly valid. Moreover, it even explains what the function
actually does rather than your imagination.
But if we're not interested in keeping it compatible, I'd probably
start with making it '--all'.
> > + local arg
> > + for arg in $(find "${D}" -name '*.pc' -exec \
> > + sed -n -e 's;^Libs:;;p' {}
> > +); do
> > + [[ ${arg} == -l* ]] &&
> > pc_libs+=(lib${arg#-l}.la)
> > + done
>
> let sed do the processing:
> pc_libs=(
> $(find "${D}" -name '*.pc' \
> -exec sed -n -e '/^Libs:/{s|^[^:]*:||;s: :\n:g;p}' {}
> + | \ sed -n '/^-l/{s:^-l:lib:;s:$:.la:;p}')
> )
Nope. My poor eyes are too weak to disband all those magical characters.
> however, i'd point out that parsing these files directly doesn't
> actually work. some .pc files use variables in the filename which
> isn't expanded by using sed. thus your only recourse is to let
> pkg-config expand things for you.
Right. I'll have to think about this a little.
> although, since we don't call die or anything, we can pipeline it to
> speed things up a bit:
> pc_libs=( $(
> tpc="${T}/.pc"
> find "${D}" -name '*.pc' -type f | \
> while read pc ; do
> sed -e '/^Requires:/d' "${pc}" > "${tpc}"
> $(tc-getPKG_CONFIG) --libs "${tpc}"
> done | tr ' ' '\n' | sort -u | \
> sed -n '/^-l/{s:^-l:lib:;s:$:.la:;p}'
> rm -f "${tpc}"
> ) )
Could you remind me, please, what performance-critical use of this
function does justify making it so harsh?
> > + local archivefile=${f/%.la/.a}
>
> remove the suffix and it'll be faster i think:
> local archivefile="${f%.la}.a"
Hmm, it's indeed inconsistent.
> > + [[ "${f}" != "${archivefile}" ]] || die 'regex
> > sanity check failed'
>
> no need to quote the ${f}, but eh
Yep.
> > + rm -f "${archivefile}" || die
>
> `rm -f` almost never fails. in the edge cases where it does, you've
> got bigger problems.
And that problem is good enough to die here.
> > + elif has "$(basename "${f}")" "${pc_libs[@]}"; then
>
> use ${f##*/} rather than `basename`
Hmm, yes. I'd split it out to a sep var as I see you used it already
more than once.
> > + removing='covered by .pc'
> > + elif [[ ! $(sed -n -e \
> > +
> > "s/^\(dependency_libs\|inherited_linker_flags\)='\(.*\)'$/\2/p"
> > \
> > + "${f}") ]]; then removing='no libs & flags'
>
> unwrap that body, and use -r rather than escaping the (|) chars
Will do.
Expect a new version in next 12hrs.
--
Best regards,
Michał Górny
signature.asc
Description: PGP signature
