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

2020-02-27 Thread Andre McCurdy
On Thu, Feb 27, 2020 at 3:42 AM Eugeny Shcheglov
 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. and .so. is a symlink
to a file called .so.. (and soname matches
.so.). You can define the structure of  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  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. and soname matches .so.) 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  wrote:
>>
>> On Tue, Feb 25, 2020 at 6:17 AM Yevhenii Shchehlov
>>  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 
>> > ---
>> >  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 

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

2020-02-27 Thread André Draszik
On Tue, 2020-02-25 at 16:17 +0200, Yevhenii Shchehlov 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 
> ---
>  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)"
>   ;;
>   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

   ^^
== is a bashism, please use only one (1) =


Cheers,
Andre'
  
> + 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)"
> + 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@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


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

2020-02-27 Thread Eugeny Shcheglov
>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". *

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:

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  wrote:

> On Tue, Feb 25, 2020 at 6:17 AM Yevhenii Shchehlov
>  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 
> > ---
> >  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@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


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

2020-02-26 Thread Andre McCurdy
On Tue, Feb 25, 2020 at 6:17 AM Yevhenii Shchehlov
 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 
> ---
>  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@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


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

2020-02-25 Thread Yevhenii Shchehlov
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 
---
 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)"
;;
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)"
+   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@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core