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 <allan@archlinux.org> ---
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