FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Gentoo > Gentoo Development

 
 
LinkBack Thread Tools
 
Old 05-28-2012, 09:03 AM
Pacho Ramos
 
Default Move remove_libtool_files() from autotools-utils for wider use.

El lun, 28-05-2012 a las 09:58 +0200, Michał Górny escribió:
> As autotools-utils exports phase functions, it will be better if
> remove_libtool_files() functions would be somewhere else.
> ---
> eutils.eclass | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ +++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/eutils.eclass b/eutils.eclass
> index c88ef35..fb92256 100644
> --- a/eutils.eclass
> +++ b/eutils.eclass
> @@ -1330,6 +1330,74 @@ makeopts_jobs() {
> echo ${jobs:-1}
> }
>
> +# @FUNCTION: remove_libtool_files
> +# @USAGE: [all]
> +# @DESCRIPTION:
> +# Determines unnecessary libtool files (.la) and libtool static archives (.a),
> +# and removes them from installation image.
> +#
> +# To unconditionally remove all libtool files, pass 'all' as an argument.
> +# Otherwise, libtool archives required for static linking will be preserved.
> +remove_libtool_files() {
> + debug-print-function ${FUNCNAME} "$@"
> + local removing_all
> + [[ ${#} -le 1 ]] || die "Invalid number of args to ${FUNCNAME}()"
> + if [[ ${#} -eq 1 ]]; then
> + case "${1}" in
> + all)
> + removing_all=1
> + ;;
> + *)
> + die "Invalid argument to ${FUNCNAME}(): ${1}"
> + esac
> + fi
> +
> + local pc_libs=()
> + if [[ ! ${removing_all} ]]; then
> + 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
> + fi
> +
> + local f
> + find "${D}" -type f -name '*.la' -print0 | while read -r -d ' f; do
> + local shouldnotlink=$(sed -ne '/^shouldnotlink=yes$/p' "${f}")
> + local archivefile=${f/%.la/.a}
> + [[ "${f}" != "${archivefile}" ]] || die 'regex sanity check failed'
> +
> + # Remove static libs we're not supposed to link against.
> + if [[ ${shouldnotlink} ]]; then
> + einfo "Removing unnecessary ${archivefile#${D%/}}"
> + rm -f "${archivefile}" || die
> + # The .la file may be used by a module loader, so avoid removing it
> + # unless explicitly requested.
> + [[ ${removing_all} ]] || continue
> + fi
> +
> + # Remove .la files when:
> + # - user explicitly wants us to remove all .la files,
> + # - respective static archive doesn't exist,
> + # - they are covered by a .pc file already,
> + # - they don't provide any new information (no libs & no flags).
> + local removing
> + if [[ ${removing_all} ]]; then removing='forced'
> + elif [[ ! -f ${archivefile} ]]; then removing='no static archive'
> + elif has "$(basename "${f}")" "${pc_libs[@]}"; then
> + removing='covered by .pc'
> + elif [[ ! $(sed -n -e
> + "s/^(dependency_libs|inherited_linker_flags)='(.*)'$/2/p"
> + "${f}") ]]; then removing='no libs & flags'
> + fi
> +
> + if [[ ${removing} ]]; then
> + einfo "Removing unnecessary ${f#${D%/}} (${removing})"
> + rm -f "${f}" || die
> + fi
> + done
> +}
> +
> check_license() { die "you no longer need this as portage supports ACCEPT_LICENSE itself"; }
>
> fi

+1

This was the main reason for me still doing manually cleaning over using
this function
 
Old 05-29-2012, 01:50 PM
Steven J Long
 
Default Move remove_libtool_files() from autotools-utils for wider use.

Michał Górny wrote:

> + find "${D}" -type f -name '*.la' -print0 | while read -r -d ' f;
..
> + rm -f "${f}" || die
..
> + done

Don't pipe to read like that; it means the final command is in a subshell
and "die is /not/ guaranteed to work correctly if called from a subshell
environment."[1]

More seriously, the script doesn't actually get the correct filenames,
despite being written to handle any filename.
eg:
$ touch $' foo bar
'
$ while read -r -d ' f; do echo "'$f'"; done < <(find . -type f -print0)
'./ foo bar'

You do it like this:

while read -rd '; do
f=$REPLY;
..
done < <(find "$D" -type f -name '*.la' -print0)

eg:
$ while read -rd '; do f=$REPLY; echo "'$f'"; done < <(find . -type f -
print0)
'./ foo bar
'

Or use: while IFS= read -rd ' f; do .. if you prefer.
See: help read # in a terminal.

It's called 'Process Substitution' if anyone wants to read about it in
man bash. The classic example with find is to get the list in an array:
arr=()
while read -rd '; do
arr+=("$REPLY")
done < <(find "$dir" -type f .. -print0)

(perhaps conditionally though that's usually better done within find
which can later be handled on a per-file basis, or passed to:
foo "${arr[@]}"

..or if you just want to know whether there is a matching file:
if read -rd ' < <(find . -type f -print0); then
something matched
else nothing did
fi

They're both things I came up with a few years ago when I was learning
from #bash, which you are in dire need of, based on reading git-2.eclass.

[1] http://dev.gentoo.org/~ulm/pms/head/pms.html#x1-12600011.3.3
(11.3.3.6)
--
#friendly-coders -- We're friendly, but we're not /that/ friendly ;-)
 
Old 05-29-2012, 02:26 PM
Steven J Long
 
Default Move remove_libtool_files() from autotools-utils for wider use.

Steven J Long wrote:
> More seriously, the script doesn't actually get the correct filenames,
> despite being written to handle any filename.

This doesn't actually apply in this case with -name '*.la' but it still
looks wrong, and implies one doesn't really grok the usage. *shrug*

--
#friendly-coders -- We're friendly, but we're not /that/ friendly ;-)
 
Old 05-30-2012, 05:16 PM
Michał Górny
 
Default Move remove_libtool_files() from autotools-utils for wider use.

On Tue, 29 May 2012 14:50:19 +0100
Steven J Long <slong@rathaus.eclipse.co.uk> wrote:

> Michał Górny wrote:
>
> > + find "${D}" -type f -name '*.la' -print0 | while read -r -d '
> > f;
> ..
> > + rm -f "${f}" || die
> ..
> > + done
>
> Don't pipe to read like that; it means the final command is in a
> subshell and "die is /not/ guaranteed to work correctly if called
> from a subshell environment."[1]

Didn't we actually change that in the past? I think I'm recalling
something like that...

> More seriously, the script doesn't actually get the correct
> filenames, despite being written to handle any filename.
> eg:
> $ touch $' foo bar
'
> $ while read -r -d ' f; do echo "'$f'"; done < <(find . -type f
> -print0) './ foo bar'
>
> You do it like this:
>
> while read -rd '; do
> f=$REPLY;
> ..
> done < <(find "$D" -type f -name '*.la' -print0)

Thanks.

--
Best regards,
Michał Górny
 
Old 05-30-2012, 09:19 PM
Mike Frysinger
 
Default Move remove_libtool_files() from autotools-utils for wider use.

On Monday 28 May 2012 03:58:56 Michał Górny wrote:
> +# @FUNCTION: remove_libtool_files

"preen_libtool_files" might be better. after all, you aren't just removing
them all.

> +# @USAGE: [all]

this is incorrect. the usage is:
<all | files to remove>

> + [[ ${#} -le 1 ]] || die "Invalid number of args to ${FUNCNAME}()"
> + if [[ ${#} -eq 1 ]]; then
> + case "${1}" in
> + all)
> + removing_all=1
> + ;;
> + *)
> + die "Invalid argument to ${FUNCNAME}(): ${1}"
> + esac
> + fi

personally i don't think the internal @ $ 0-9 # variables should get braces
unless absolutely necessary

the *) case is missing a ";;" ... yes, it's not required in the last case
statement, but that's easy to bite people, so i prefer to make it explicit

> + if [[ ! ${removing_all} ]]; then

i know you don't like specifying -z/-n but instead relying on the implicit -n,
but i find it less clear, especially for people who aren't aware of the exact
semantics

> + 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: :
:g;p}' {} + |
sed -n '/^-l/{s:^-l:lib:;s:$:.la:;p}')
)

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.

$ grep '^Libs:.*-l[^ ]*[$]' /usr/lib64/pkgconfig/*.pc
/usr/lib64/pkgconfig/apr-1.pc:Libs: -L${libdir} -lapr-${APR_MAJOR_VERSION} ...
/usr/lib64/pkgconfig/gtk+-2.0.pc:Libs: -L${libdir} -lgtk-${target}-2.0

so maybe something like:
local pc
while read -rd ' pc ; do
sed -e '/^Requires:/d' "${pc}" > "${T}/${pc##*/}"
pc_libs+=(
$($(tc-getPKG_CONFIG) --libs "${T}"/${pc##*/}" |
tr ' ' '
' |
sed -n '/^-l/{s:^-l:lib:;s:$:.la:;p}')
)
done < <(find "${D}" -name '*.pc' -type f -print0)
pc_libs=( $(printf '%s
' "${pc_libs[@]}") | sort -u )

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 ' ' '
' | sort -u |
sed -n '/^-l/{s:^-l:lib:;s:$:.la:;p}'
rm -f "${tpc}"
) )

> + local archivefile=${f/%.la/.a}

remove the suffix and it'll be faster i think:
local archivefile="${f%.la}.a"

> + [[ "${f}" != "${archivefile}" ]] || die 'regex sanity check failed'

no need to quote the ${f}, but eh

> + rm -f "${archivefile}" || die

`rm -f` almost never fails. in the edge cases where it does, you've got
bigger problems.

> + if [[ ${removing_all} ]]; then removing='forced'
> + elif [[ ! -f ${archivefile} ]]; then removing='no static archive'

unwrap these

> + elif has "$(basename "${f}")" "${pc_libs[@]}"; then

use ${f##*/} rather than `basename`

> + 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
-mike
 
Old 05-31-2012, 05:46 AM
Michał Górny
 
Default Move remove_libtool_files() from autotools-utils for wider use.

On Wed, 30 May 2012 17:19:49 -0400
Mike Frysinger <vapier@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: :
: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 ' ' '
' | 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
 
Old 05-31-2012, 06:09 AM
Mike Frysinger
 
Default Move remove_libtool_files() from autotools-utils for wider use.

On Thursday 31 May 2012 01:46:41 Michał Górny wrote:
> On Wed, 30 May 2012 17:19:49 -0400 Mike Frysinger 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.

why are you so angry all the time ? try being less confrontational for once.

going from the usage:
remove_libtool_files [all]

that means this may be called in only two ways:
1) remove_libtool_files
2) remove_libtool_files all

yet, if you read the actual code, you'll see:
+ [[ ${#} -le 1 ]] || die "Invalid number of args to ${FUNCNAME}()"
+ if [[ ${#} -eq 1 ]]; then
+ ...
+ fi

that means if more than 1 argument is passed, no error is thrown. i thought
you were intending to parse $@ further on because of it (hence the suggestion
of updating the @USAGE), but it looks merely like your arg parsing is
incorrect and needs fixing. probably easiest by doing:
case $#:$1 in
0:') ;;
1:all) removing_all=1 ;;
*) die "invalid usage" ;;
esac

> > 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 ' ' '
' | 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?

looks perfectly fine to me, and it has the bonus of working

> > > + 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.

more like the system at large is going to be falling over independently
-mike
 
Old 05-31-2012, 11:40 AM
Michał Górny
 
Default Move remove_libtool_files() from autotools-utils for wider use.

On Thu, 31 May 2012 02:09:11 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> yet, if you read the actual code, you'll see:
> + [[ ${#} -le 1 ]] || die "Invalid number of args to
> ${FUNCNAME}()"
> + if [[ ${#} -eq 1 ]]; then
> + ...
> + fi
>
> that means if more than 1 argument is passed, no error is thrown.

The exact opposite. If more than a single argument is passed, error is
thrown.

> thought you were intending to parse $@ further on because of it
> (hence the suggestion of updating the @USAGE), but it looks merely
> like your arg parsing is incorrect and needs fixing. probably
> easiest by doing: case $#:$1 in
> 0:') ;;
> 1:all) removing_all=1 ;;
> *) die "invalid usage" ;;
> esac

Just a little reverse logic in spirit of makefiles. But the case
variant would be probably more readable indeed.

--
Best regards,
Michał Górny
 

Thread Tools




All times are GMT. The time now is 09:40 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org