On 19/05/12 23:44, Dave Reisner wrote:
> On Sat, May 19, 2012 at 11:22:36PM +1000, Allan McRae wrote:
>> Rewrite the handling of libdepends. The primary advantage are:
>> - Moves functionality from write_pkginfo() to find_libdepends().
>> - The order of the depends array in the PKGBUILD is kept in the package.
>> - An unneeded libdepends is only a warning and not an error. This allows
>> putting a libdepend on a library that is dlopened.
>> - It is now modular so can be extended to library types other than
>> ELF *.so.
>> - Finding the list of libraries a package depends only occurs when a
>> libdepend is specified in the depends array.
>>
>> Signed-off-by: Allan McRae <[email protected]>
>> ---
>
> Without having tested this too thoroughly, this gets a +1 from me.
> There's two small nitpicks left inline.
>
>> scripts/makepkg.sh.in | 126
>> +++++++++++++++++++++++++++++--------------------
>> 1 file changed, 76 insertions(+), 50 deletions(-)
>>
>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>> index 8d018c0..8930ec4 100644
>> --- a/scripts/makepkg.sh.in
>> +++ b/scripts/makepkg.sh.in
>> @@ -1131,32 +1131,76 @@ tidy_install() {
>> }
>>
>> find_libdepends() {
>> - local libdepends
>> - find "$pkgdir" -type f -perm -u+x | while read filename
>> - do
>> - # get architecture of the file; if soarch is empty it's not an
>> ELF binary
>> - soarch=$(LC_ALL=C readelf -h "$filename" 2>/dev/null | sed -n
>> 's/.*Class.*ELF\(32\|64\)/\1/p')
>> - [ -n "$soarch" ] || continue
>> - # process all libraries needed by the binary
>> - for sofile in $(LC_ALL=C readelf -d "$filename" 2>/dev/null |
>> sed -nr 's/.*Shared library: \[(.*)\].*/\1/p')
>> - do
>> - # extract the library name: libfoo.so
>> - soname="${sofile%.so?(+(.+([0-9])))}".so
>> - # extract the major version: 1
>> - soversion="${sofile##*\.so\.}"
>> - if in_array "${soname}" ${depends[@]}; then
>> - if ! in_array
>> "${soname}=${soversion}-${soarch}" ${libdepends[@]}; then
>> - # libfoo.so=1-64
>> - printf "%s"
>> "${soname}=${soversion}-${soarch}"
>> -
>> libdepends+=("${soname}=${soversion}-${soarch}")
>> + local d sodepends;
>> +
>> + sodepends=0;
>> + for d in "${depends[@]}"; do
>> + if [[ $d = *.so ]]; then
>> + sodepends=1;
>> + break;
>> + fi
>> + done
>> +
>> + if (( sodepends == 0 )); then
>> + printf '%s\n' "${depends[@]}"
>> + return;
>> + fi
>> +
>> + local libdeps;
>> + declare -A libdeps;
>> +
>> + if (( sodepends == 1 )); then
>
> You've already checked that $sodepends isn't 0. Can't you save a whole
> level of indenting here?
I was doing that test so allow the libdeps array to be filled by
different methods if someone steps up to implement this on an non-ELF
system. Probably an absolute waste as that will likely never happen...
Removed on my working branch.
>> + local filename soarch sofile soname soversion
>> +
>> + while read filename; do
>> + # get architecture of the file; if soarch is empty it's
>> not an ELF binary
>> + soarch=$(LC_ALL=C readelf -h "$filename" 2>/dev/null |
>> sed -n 's/.*Class.*ELF\(32\|64\)/\1/p')
>> + [[ -n "$soarch" ]] || continue
>> +
>> + # process all libraries needed by the binary
>> + for sofile in $(LC_ALL=C readelf -d "$filename"
>> 2>/dev/null | sed -nr 's/.*Shared library: \[(.*)\].*/\1/p')
>> + do
>> + # extract the library name: libfoo.so
>> + soname="${sofile%.so?(+(.+([0-9])))}".so
>> + # extract the major version: 1
>> + soversion="${sofile##*\.so\.}"
>> +
>> + if [[ ${libdeps[$soname]} ]]; then
>> + if [[ ! ${libdeps[$soname]} =
>> *${soversion}-${soarch}* ]]; then
>
> nitpick: "$foo != $bar" instead of "! $foo = $bar"
Fixed.
Allan
