[pacman-dev] .so provides/depends patches review
Hi, Sorry this has taken me a while... so long in fact that I do not have the original email stored anymore to reply to. But here goes my comments on the patches for soname provides/depends: http://mailman.archlinux.org/pipermail/pacman-dev/2010-February/010431.html My comments are currently only on functionality. I'll get to code after that is fully sorted. Firstly, it works well and does as advertised. So no issues there... However: 1) If I put a bad soname provides in the array (e.g. provides="readline.so") it just silently ignores it. No error and nothing put in the .PKGINFO. There needs to be an error (or at minimum a warning). The same goes for depends (although these are mostly caught by the initial dependency checking in makepkg). 2) Building readline on i686 with provides="libreadline.so", in the .PKGINFO I get: provides = libreadline.so=6-elf32_i386 My concern about that is that the library is not i386 but is i686. I may be prepared to accept that though as I guess x86 is x86 really... But what additional information is the elf32 providing? I could perhaps understand the operating system from the host triplet. If anything, my preferences are: libreadline.so=6-i686_linux (best) libreadline.so=6-i686 / libreadline.so=6-i386_linux libreadline.so=6-i386 (worst) Allan
On 12.04.2010 17:24, Allan McRae wrote:
1) If I put a bad soname provides in the array (e.g. provides="readline.so") it just silently ignores it. No error and nothing put in the .PKGINFO. There needs to be an error (or at minimum a warning). The same goes for depends (although these are mostly caught by the initial dependency checking in makepkg).
Implemented an error.
2) Building readline on i686 with provides="libreadline.so", in the .PKGINFO I get: provides = libreadline.so=6-elf32_i386 My concern about that is that the library is not i386 but is i686. I may be prepared to accept that though as I guess x86 is x86 really... But what additional information is the elf32 providing? I could perhaps understand the operating system from the host triplet. If anything, my preferences are: libreadline.so=6-i686_linux (best) libreadline.so=6-i686 / libreadline.so=6-i386_linux libreadline.so=6-i386 (worst)
Telling apart i368 and i686 libs seems to be only possible if you scan them for instructions that only work on i686 and not on i368, but that's way to complicated. ELF doesn't save this information. It now generates "libreadline.so=6-i386_linux".
Support-by: brain0 <thomas@archlinux.org> Support-by: GNU\caustic <Christoph.Schied@uni-ulm.de> Signed-off-by: Florian Pritz <bluewind@xssn.at> --- scripts/makepkg.sh.in | 34 +++++++++++++++++++++++++++++++++- 1 files changed, 33 insertions(+), 1 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 131519f..584c427 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -869,6 +869,28 @@ tidy_install() { fi } +find_soprovides() { + local soprovides + find $pkgdir -type f -name \*.so\* | while read filename + 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") + if in_array "${soname}" ${provides[@]}; then + if ! in_array "${soname}=${soversion}-${soarch}" ${soprovides[@]}; then + echo "${soname}=${soversion}-${soarch}" + soprovides=(${soprovides[@]} "${soname}=${soversion}-${soarch}") + fi + fi + fi + done +} + write_pkginfo() { local builddate=$(date -u "+%s") if [[ -n $PACKAGER ]]; then @@ -898,6 +920,9 @@ write_pkginfo() { echo "force = true" >> .PKGINFO fi + soprovides=$(find_soprovides) + provides=("${provides[@]}" ${soprovides}) + local it for it in "${license[@]}"; do echo "license = $it" >>.PKGINFO @@ -918,7 +943,14 @@ write_pkginfo() { echo "conflict = $it" >>.PKGINFO done for it in "${provides[@]}"; do - echo "provides = $it" >>.PKGINFO + if grep -q ".*\.so$" <<< "$it"; then + if ! grep -q "\(^\|\s\)${it}=.*" <<< $soprovides; then + error "$(gettext "Can't find library listed in \$provides: %s")" "$it" + return 1 + fi + else + echo "provides = $it" >>.PKGINFO + fi done for it in "${backup[@]}"; do echo "backup = $it" >>.PKGINFO -- 1.7.0.4
Support-by: brain0 <thomas@archlinux.org> Support-by: GNU\caustic <Christoph.Schied@uni-ulm.de> Signed-off-by: Florian Pritz <bluewind@xssn.at> --- scripts/makepkg.sh.in | 32 +++++++++++++++++++++++++++++++- 1 files changed, 31 insertions(+), 1 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 584c427..6e04618 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -869,6 +869,27 @@ tidy_install() { fi } +find_sodepends() { + local sodepends + find $pkgdir -type f | while read filename + do + soarch="$(objdump -a "$filename" 2>/dev/null | \ + sed -rn 's/.* file format elf[0-9]+-(.*)$/\1/p' | tr - _)_$(uname -s)" + [ -n "$soarch" ] || continue + for sofile in $(readelf -d "$filename" 2> /dev/null | sed -nr 's/.*Shared library: \[(.*)\].*/\1/p') + do + soname=$(sed -rn 's/(.*)\.so.*/\1.so/p' <<< "$sofile") + soversion=$(sed -rn 's/.*\.so\.(.*)/\1/p' <<< "$sofile") + if in_array "${soname}" ${depends[@]}; then + if ! in_array "${soname}=${soversion}-${soarch}" ${sodepends[@]}; then + echo "${soname}=${soversion}-${soarch}" + sodepends=(${sodepends[@]} "${soname}=${soversion}-${soarch}") + fi + fi + done + done +} + find_soprovides() { local soprovides find $pkgdir -type f -name \*.so\* | while read filename @@ -921,6 +942,8 @@ write_pkginfo() { fi soprovides=$(find_soprovides) + sodepends=$(find_sodepends) + depends=("${depends[@]}" ${sodepends}) provides=("${provides[@]}" ${soprovides}) local it @@ -934,7 +957,14 @@ write_pkginfo() { echo "group = $it" >>.PKGINFO done for it in "${depends[@]}"; do - echo "depend = $it" >>.PKGINFO + if grep -q ".*\.so$" <<< "$it"; then + if ! grep -q "\(^\|\s\)${it}=.*" <<< $sodepends; then + error "$(gettext "Can't find library listed in \$depends: %s")" "$it" + return 1 + fi + else + echo "depend = $it" >>.PKGINFO + fi done for it in "${optdepends[@]}"; do echo "optdepend = $it" >>.PKGINFO -- 1.7.0.4
Hi,
Sorry this has taken me a while... so long in fact that I do not have the original email stored anymore to reply to.
But here goes my comments on the patches for soname provides/depends: http://mailman.archlinux.org/pipermail/pacman-dev/2010-February/010431.html
My comments are currently only on functionality. I'll get to code after that is fully sorted.
Firstly, it works well and does as advertised. So no issues there... However:
1) If I put a bad soname provides in the array (e.g. provides="readline.so") it just silently ignores it. No error and nothing put in the .PKGINFO. There needs to be an error (or at minimum a warning). The same goes for depends (although these are mostly caught by the initial dependency checking in makepkg).
2) Building readline on i686 with provides="libreadline.so", in the .PKGINFO I get: provides = libreadline.so=6-elf32_i386 My concern about that is that the library is not i386 but is i686. I may be prepared to accept that though as I guess x86 is x86 really... But what additional information is the elf32 providing? I could perhaps understand the operating system from the host triplet. If anything, my preferences are: libreadline.so=6-i686_linux (best) libreadline.so=6-i686 / libreadline.so=6-i386_linux libreadline.so=6-i386 (worst)
Allan
Would it (make sense|be possible) to generalize this beyond .so files so that it can accommodate other uses in the future? The first thing that comes to mind is that this might be useful for packages which provide modules for interpreted languages as well (e.g. CPAN distributions for Perl) but there are almost certainly other valid uses. As I've understood it, this is really just a way to provide finer dependency control and it seems that it should not be limited to particular file types. Sorry if this is already the intention but from what I've read of the discussion it seems that only compiled (C and C++) shared libraries have been considered. I've admittedly only followed the discussion loosely and have not read through the code itself so sorry again if I've misunderstood something. Xyne
On 13.04.2010 20:43, Xyne wrote:
Would it (make sense|be possible) to generalize this beyond .so files so that it can accommodate other uses in the future? The first thing that comes to mind is that this might be useful for packages which provide modules for interpreted languages as well (e.g. CPAN distributions for Perl) but there are almost certainly other valid uses.
Sure that's possible and would make sense, but I don't really know how to do it exactly. Maybe you could do something with an array that holds the names of the functions you want to call to clean, but I think the part with the warnings (where it writes the dependency into .PKGINFO) would become a big mess if you want to keep them useful.
As I've understood it, this is really just a way to provide finer dependency control and it seems that it should not be limited to particular file types. Sorry if this is already the intention but from what I've read of the discussion it seems that only compiled (C and C++) shared libraries have been considered.
Yes, my patch only works for ELF binaries (C, C++, haskell and maybe others). -- Florian Pritz -- {flo,bluewind}@server-speed.net
participants (4)
-
Allan McRae
-
Florian Pritz
-
Florian Pritz
-
Xyne