19 Jan
2011
19 Jan
'11
6:54 p.m.
On Wed, Jan 19, 2011 at 11:13 AM, Florian Pritz <bluewind@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@archlinux.org>
> Support-by: GNU\caustic <Christoph.Schied@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@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