[OE-core] [PATCH] utils.bbclass: Avoid recursive symlink in oe_soinstall

[Andre McCurdy]

On Thu, Feb 27, 2020 at 3:42 AM Eugeny Shcheglov
<eugenyshcheglov at gmail.com> wrote:
>
> >This patch look OK but it might be nice to also clean up this error
> message (and the Example comment a few lines above it) to drop the 3
> levels of version number. There should only be 2, ie the fully
> versioned filename would be libfoo.so.1.2 instead of libfoo.so.1.2.3
>
> Yeah, sound reasonable. According to the tLDP (http://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html)
> there can be be 3 levels, but last one is optional. Quote from tLDP: "The real name adds to the soname a period, a minor
> number, another period, and the release number. The last period and release number are optional".

In terms of creating symlinks there are only two levels to the version
number: .so is a symlink to .so.<major> and .so.<major> is a symlink
to a file called .so.<major>.<everythingelse> (and soname matches
.so.<major>). You can define the structure of <everythingelse> to be
just a minor number, a minor number and a release number, or something
else it still works fine from the point of view of this function. The
common case is for <everythingelse> to just be the minor release
number and I think it's clearer if the the comments and error messages
for this function reflect that.

> So yes, correct fully versioned name should contain at least two numbers
>
> >"oe_soinstall: $libname soname is equal to fully versioned real (file) name. The soname should include the major version only (e.g.libfoo.so.1 instead of libfoo.so.1.2)"
>
> Good point. But what if library real name is libfoo.so.1 and soname is libfoo.so.1 too? In my opinion there is two ways:

That's a case where you only need one symlink (ie .so is a symlink to
a file called .so.<major> and soname matches .so.<major>) and it
doesn't seem to have been considered when this function was originally
written. I agree it's a valid use case though (and I guess trying to
fix that is the original motivation behind sending this patch?).

Ideally the function should be able to distinguish between needing to
create a single symlink and needing to create two symlinks but the
soname is incorrect... and give a meaningful error in the second case.

> 1) Add one more check: parse $sonamelink and throw bbwarn if soname is incorrect. "Incorrect" in this case means that
> soname contain more than one version (libfoo.so.1.2)
>
> 2) Mention both possible problems in bbwarn. Like: " $libname soname is equal to fully versioned real (file)
> name. The soname should include the major version only (e.g. libfoo.so.1 instead of libfoo.so.1.2) or minor version
> should be added to library real name (e.g. libfoo.so.1.2 instead of libfoo.so.1) ; "
>
> First one is definitely clearer. So I'm going to implement it and send version 2 of patch
>
> On Wed, Feb 26, 2020 at 10:08 PM Andre McCurdy <armccurdy at gmail.com> wrote:
>>
>> On Tue, Feb 25, 2020 at 6:17 AM Yevhenii Shchehlov
>> <eugenyshcheglov at gmail.com> wrote:
>> >
>> > This patch fixes an issue when oe_soinstall function creates
>> > non-functional recursive symlinks in case library soname is equal
>> > to library real (file) name.
>> >
>> > Signed-off-by: Yevhenii Shchehlov <eugenyshcheglov at gmail.com>
>> > ---
>> >  meta/classes/utils.bbclass | 8 ++++++--
>> >  1 file changed, 6 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/meta/classes/utils.bbclass b/meta/classes/utils.bbclass
>> > index cd3d05709e..dea824f54f 100644
>> > --- a/meta/classes/utils.bbclass
>> > +++ b/meta/classes/utils.bbclass
>> > @@ -25,7 +25,7 @@ oe_soinstall() {
>> >         libname=`basename $1`
>> >         case "$libname" in
>> >             *.so)
>> > -               bbfatal "oe_soinstall: Shared library must haved versioned filename (e.g. libfoo.so.1.2.3)"
>> > +               bbfatal "oe_soinstall: Shared library must haved versioned filename (e.g. libfoo.so.1.2.3 instead of libfoo.so)"
>>
>> This patch look OK but it might be nice to also clean up this error
>> message (and the Example comment a few lines above it) to drop the 3
>> levels of version number. There should only be 2, ie the fully
>> versioned filename would be libfoo.so.1.2 instead of libfoo.so.1.2.3
>>
>> >                 ;;
>> >         esac
>> >         install -m 755 $1 $2/$libname
>> > @@ -33,8 +33,12 @@ oe_soinstall() {
>> >         if [ -z $sonamelink ]; then
>> >                 bbfatal "oe_soinstall: $libname is missing ELF tag 'SONAME'."
>> >         fi
>> > +       if [ "$sonamelink" == "$libname" ]; then
>> > +               bbwarn "oe_soinstall: $libname soname is equal to real (file) name. Minor version should be added to a real name (e.g. libfoo.so.1.2 instead of libfoo.so.1)"
>>
>> It might be clearer to say:
>>
>> "oe_soinstall: $libname soname is equal to fully versioned real (file)
>> name. The soname should include the major version only (e.g.
>> libfoo.so.1 instead of libfoo.so.1.2)"
>>
>> > +       else
>> > +               ln -sf $libname $2/$sonamelink
>> > +       fi
>> >         solink=`echo $libname | sed -e 's/\.so\..*/.so/'`
>> > -       ln -sf $libname $2/$sonamelink
>> >         ln -sf $libname $2/$solink
>> >  }
>> >
>> > --
>> > 2.25.1
>> >
>> > --
>> > _______________________________________________
>> > Openembedded-core mailing list
>> > Openembedded-core at lists.openembedded.org
>> > http://lists.openembedded.org/mailman/listinfo/openembedded-core