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