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?

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

+ libdeps[$soname]+=" ${soversion}-${soarch}" + fi + else + libdeps[$soname]="${soversion}-${soarch}" fi - fi - done + done + done < <(find "$pkgdir" -type f -perm -u+x) + fi + + local libdepends v + for d in "${depends[@]}"; do + case "$d" in + *.so) + if [[ ${libdeps[$d]} ]]; then + for v in ${libdeps[$d]}; do + libdepends+=("$d=$v") + done + else + warning "$(gettext "Library listed in %s is not required by any files: %s")" "'depends'" "$d" + libdepends+=("$d") + fi + ;; + *) + libdepends+=("$d") + ;; + esac done + + printf '%s\n' "${libdepends[@]}" }

+ find_libprovides() { - local libprovides missing + local p libprovides missing for p in "${provides[@]}"; do missing=0 case "$p" in @@ -1246,38 +1290,20 @@ write_pkginfo() { printf "size = %s\n" "$size" printf "arch = %s\n" "$pkgarch"

- [[ $license ]] && printf "license = %s\n" "${license[@]}" - [[ $replaces ]] && printf "replaces = %s\n" "${replaces[@]}" - [[ $groups ]] && printf "group = %s\n" "${groups[@]}" - [[ $optdepends ]] && printf "optdepend = %s\n" "${optdepends[@]//+([[:space:]])/ }" - [[ $conflicts ]] && printf "conflict = %s\n" "${conflicts[@]}" - mapfile -t provides < <(find_libprovides) - [[ $provides ]] && printf "provides = %s\n" "${provides[@]}" - - [[ $backup ]] && printf "backup = %s\n" "${backup[@]}" - + mapfile -t depends < <(find_libdepends) + + [[ $license ]] && printf "license = %s\n" "${license[@]}" + [[ $replaces ]] && printf "replaces = %s\n" "${replaces[@]}" + [[ $groups ]] && printf "group = %s\n" "${groups[@]}" + [[ $conflicts ]] && printf "conflict = %s\n" "${conflicts[@]}" + [[ $provides ]] && printf "provides = %s\n" "${provides[@]}" + [[ $backup ]] && printf "backup = %s\n" "${backup[@]}" + [[ $depends ]] && printf "depend = %s\n" "${depends[@]}" + [[ $optdepends ]] && printf "optdepend = %s\n" "${optdepends[@]//+([[:space:]])/ }" + [[ $makedepends ]] && printf "makedepend = %s\n" "${makedepends[@]}"

local it - mapfile -t libdepends < <(find_libdepends) - depends+=("${libdepends[@]}") - - for it in "${depends[@]}"; do - if [[ $it = *.so ]]; then - # check if the entry has been found by find_libdepends - # if not, it's unneeded; tell the user so he can remove it - printf -v re '(^|\s)%s=.*' "$it" - if [[ ! $libdepends =~ $re ]]; then - error "$(gettext "Cannot find library listed in %s: %s")" "'depends'" "$it" - return 1 - fi - else - printf "depend = %s\n" "$it" - fi - done - - [[ $makedepends ]] && printf "makedepend = %s\n" "${makedepends[@]}" - for it in "${packaging_options[@]}"; do check_option "$it" "y" case $? in -- 1.7.10.2