| Message ID | 20200514170530.2575612-1-eschwartz@archlinux.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [pacman-dev] makepkg: libprovides: don't provide both versioned and unversioned sonames | expand |
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 <eschwartz@archlinux.org> > --- 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") > - ;; > esac > > if (( missing )); then > warning "$(gettext "Cannot find library listed in %s: %s")" "'provides'" "$p" > fi > + if (( ${#newp[@]} > 0 )); then > + libprovides+=("${newp[@]}") > + else > + libprovides+=("$p") > + fi > done > > (( ${#libprovides[@]} )) && printf '%s\n' "${libprovides[@]}" >
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?
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") - ;; esac if (( missing )); then warning "$(gettext "Cannot find library listed in %s: %s")" "'provides'" "$p" fi + if (( ${#newp[@]} > 0 )); then + libprovides+=("${newp[@]}") + else + libprovides+=("$p") + fi done (( ${#libprovides[@]} )) && printf '%s\n' "${libprovides[@]}"
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 <eschwartz@archlinux.org> --- 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. 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(-)