[pacman-dev] [PATCH 4/5] makepkg: add soprovides support

[Dan McGee]

On Wed, Jan 19, 2011 at 11:13 AM, Florian Pritz <bluewind at xssn.at> wrote:

<Descriptive commit message usually goes here>

What does this do? Why? What is the generated format? This stuff needs
to be here in permanent history so someone patching bugs 2 years from
now can figure out the what and why.

> Support-by: brain0 <thomas at archlinux.org>
> Support-by: GNU\caustic <Christoph.Schied at uni-ulm.de>
Since the real names of these guys are so obvious, I'd prefer those
are there rather than nicks.

>
> Signed-off-by: Florian Pritz <bluewind at xssn.at>
> ---
>  scripts/makepkg.sh.in |   38 +++++++++++++++++++++++++++++++++++++-
>  1 files changed, 37 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 6ebfac0..cf7dbb9 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -917,6 +917,28 @@ tidy_install() {
>        fi
>  }
>
> +find_soprovides() {
> +       local soprovides
> +       find $pkgdir -type f -name \*.so\* | while read filename
$pkgdir needs quotes, just like everywhere else it is used.

> +       do
> +    if readelf -h "$filename" 2>/dev/null | grep -q '.*Type:.*DYN (Shared object file).*'; then
> +                       soarch="$(objdump -a "$filename" 2>/dev/null | \
> +                               sed -rn 's/.* file format elf[0-9]+-(.*)$/\1/p' | tr - _)_$(uname -s)"
> +      sofile=$(readelf -d "$filename" 2>/dev/null | sed -nr 's/.*Library soname: \[(.*)\].*/\1/p')
> +                       [ -z "$sofile" ] && sofile="$(basename "$filename")"
> +
> +                       soname=$(sed -rn 's/(.*)\.so.*/\1.so/p' <<< "$sofile")
> +                       soversion=$(sed -rn 's/.*\.so\.(.*)/\1/p' <<< "$sofile")
Please give examples in a resubmit of what is coming out of here- this
is not super easy to follow. I'm also a bit concerned about:
* the uses of sed (is -r portable? We use -n already, but not -r)
* the inconsistencies between `sed -rn` and `sed -nr`
* running this in any non-C, non-en locale
* introducing dependencies on readelf and objdump. At least elsewhere
in the strip code, we use `file` to locate shared libraries. And you
can get the SONAME bit out of objdump -p, which would at least limit
this to one tool.

> +                       if in_array "${soname}" ${provides[@]}; then
> +                               if ! in_array "${soname}=${soversion}-${soarch}" ${soprovides[@]}; then
> +                                       echo "${soname}=${soversion}-${soarch}"
> +                                       soprovides=(${soprovides[@]} "${soname}=${soversion}-${soarch}")
What's the reasoning behind the ${soarch} append? I suppose it might
help with multilib, but something about this just doesn't seem right.
It is most definitely not a valid pkgver (dash) or pkgrel (not a
number).

> +                               fi
> +                       fi
> +    fi
> +       done
> +}
> +
>  write_pkginfo() {
>        local builddate=$(date -u "+%s")
>        if [[ -n $PACKAGER ]]; then
> @@ -950,10 +972,24 @@ write_pkginfo() {
>        [[ $depends ]]    && printf "depend = %s\n"    "${depends[@]}"
>        [[ $optdepends ]] && printf "optdepend = %s\n" "${optdepends[@]}"
>        [[ $conflicts ]]  && printf "conflict = %s\n"  "${conflicts[@]}"
> -       [[ $provides ]]   && printf "provides = %s\n"  "${provides[@]}"
>        [[ $backup ]]     && printf "backup = %s\n"    "${backup[@]}"
>
>        local it
> +
> +       soprovides=$(find_soprovides)
> +       provides=("${provides[@]}" ${soprovides})
> +
> +       for it in "${provides[@]}"; do
> +               if grep -q ".*\.so$" <<< "$it"; then
> +                       if ! grep -q "\(^\|\s\)${it}=.*" <<< $soprovides; then
> +                               error "$(gettext "Can't find library listed in \$provides: %s")" "$it"
Do we really need to add 18 more forks to makepkg and use grep here?
Bash has regexes, so it would prevent the cryptic use of <<<.
I don't even know why we are doing this check, can you elaborate?
> +                               return 1
> +                       fi
> +               else
> +                       echo "provides = $it"
> +               fi
> +       done
> +
>        for it in "${packaging_options[@]}"; do
>                local ret="$(check_option $it)"
>                if [[ $ret != "?" ]]; then
> --
> 1.7.3.5