Re: [pacman-dev] [PATCH] makepkg: libprovides: don't provide both versioned and unversioned sonames

2020-06-25 Thread Allan McRae
On 15/5/20 3:05 am, Eli Schwartz wrote:
> If multiple files match the pattern libfoo.so*, we want to check each of
> them and see if they are shared libraries, and if so, if they have
> versions attached.
> 
> But some packages can have both shared libraries and random files which
> match the filename pattern. This is true at least for files in
> /usr/share/gdb/auto-load/, which must match the filename they are paired
> with, followed by "-gdb.py" (or some other gdb scripting ext), but
> definitely don't contain a shared library. In this case, we don't want
> to double-report the library in the generated provides.
> 
> It's also possible (probably) for a package to provide a versioned as
> well as an unversioned shared library, but in such cases a single
> provides entry is sufficient to cover both cases (and the libdepends
> for the depending package would contain an unversioned dependency).
> 
> Solve this by keeping track of whether we have added a versioned soname
> provides already, and then only adding a maximum of one unversioned
> provides *iff* there isn't a versioned one yet.
> 
> Signed-off-by: Eli Schwartz 
> ---


This patch is fine, but I don't like "newp" as a variable name.  It will
not help me understand this in a years time.

How about versioned_provides=() instead?

> 
> Notes:
> Currently we only have gdb/auto-load/ scripts for shared libraries in:
> - gcc
> - glib2
> - gstreamer
> - efl
> 
> In the first 3 cases, libprovides is entirely useless anyway.
> 
> libstdc++ is perpetually stable (backwards-compatible ABI) just like 
> glibc.

Except when it did get an soname bump...  Those were the days!

> glib2 actually does use libprovides, despite that it makes no sense for 
> glib2
> to libprovide anything, as the glib2 developers have this thing where they
> always use a soname of "0" and bump ABI versions by changing 
> libglib-2.0.so to
> libglib-3.0.so -- as a result if you want to track dependencies you need 
> to
> change to a different soname manually, and might as well just depend on 
> the
> package itself (which already has a version in the pkgname).
> 
> (gstreamer is in the same boat as glib2 because it is part of the same 
> ecosystem.)
> 
> efl doesn't currently libprovide "libeo.so", but if it did, then the issue
> would exist and be fixed by this patch. And more generally, any project 
> wishing
> to implement gdb/auto-load/ scripts could have this issue, so it makes 
> sense to
> make it work nicely.
> 
>  scripts/makepkg.sh.in | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index d1416d15..3f746b77 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -516,9 +516,10 @@ find_libdepends() {
>  
>  
>  find_libprovides() {
> - local p libprovides missing
> + local p newp libprovides missing
>   for p in "${provides[@]}"; do
>   missing=0
> + newp=()
>   case "$p" in
>   *.so)
>   mapfile -t filename < <(find "$pkgdir" -type f 
> -name $p\*)
> @@ -531,7 +532,6 @@ find_libprovides() {
>   local sofile=$(LC_ALL=C 
> readelf -d "$fn" 2>/dev/null | sed -n 's/.*Library soname: \[\(.*\)\].*/\1/p')
>   if [[ -z "$sofile" ]]; 
> then
>   warning 
> "$(gettext "Library listed in %s is not versioned: %s")" "'provides'" "$p"
> - 
> libprovides+=("$p")
>   continue
>   fi
>  
> @@ -541,25 +541,25 @@ find_libprovides() {
>   # extract the library 
> major version
>   local 
> soversion="${sofile##*\.so\.}"
>  
> - 
> libprovides+=("${p}=${soversion}-${soarch}")
> + 
> newp+=("${p}=${soversion}-${soarch}")
>   else
>   warning "$(gettext 
> "Library listed in %s is not a shared object: %s")" "'provides'" "$p"
> - libprovides+=("$p")
>   fi
>   done
>   else
> - libprovides+=("$p")
>   missing=1
>   fi
>   ;;
> - *)
> - libprovides+=("$p")
> - 

Re: [pacman-dev] [PATCH] makepkg: libprovides: don't provide both versioned and unversioned sonames

2020-09-22 Thread Eli Schwartz
On 6/26/20 2:09 AM, Allan McRae wrote:
> This patch is fine, but I don't like "newp" as a variable name.  It will
> not help me understand this in a years time.
> 
> How about versioned_provides=() instead?

Yes, that sounds fine to me. Do you want to do a simple replacement
while pulling the patch, or should I resubmit it?

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature