Patchwork [1/1] package.bbclass: needs_ldconfig from linux_so is needed in global namespace

login
register
mail settings
Submitter Andrei Gherzan
Date April 6, 2012, 1:57 p.m.
Message ID <4803097570d9e27eb3931388eb76fe6d3d00591a.1333720467.git.andrei@gherzan.ro>
Download mbox | patch
Permalink /patch/25277/
State New
Headers show

Comments

Andrei Gherzan - April 6, 2012, 1:57 p.m.
"The suite of statements in a function definition executes with a local namespace
that is different from the global namespace. This means that all variables created
within a function are local to that function. When the suite finishes, these
working variables are discarded."

In this way the needs_ldconfig variable in linux_so never gets True in the statements
below this function. As global statement is generally discouraged, a return value
would be a clean and fast way to solve this issue.

[YOCTO #2205]

Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
---
 meta/classes/package.bbclass |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
Richard Purdie - April 10, 2012, 9:02 a.m.
On Fri, 2012-04-06 at 16:57 +0300, Andrei Gherzan wrote:
> "The suite of statements in a function definition executes with a local namespace
> that is different from the global namespace. This means that all variables created
> within a function are local to that function. When the suite finishes, these
> working variables are discarded."
> 
> In this way the needs_ldconfig variable in linux_so never gets True in the statements
> below this function. As global statement is generally discouraged, a return value
> would be a clean and fast way to solve this issue.
> 
> [YOCTO #2205]
> 
> Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
> ---
>  meta/classes/package.bbclass |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index d35667a..c98e8fa 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -1263,6 +1263,7 @@ python package_do_shlibs() {
>  	lf = bb.utils.lockfile(d.expand("${PACKAGELOCK}"))
>  
>  	def linux_so(root, path, file):
> +		needs_ldconfig = False
>  		cmd = d.getVar('OBJDUMP', True) + " -p " + pipes.quote(os.path.join(root, file)) + " 2>/dev/null"
>  		cmd = "PATH=\"%s\" %s" % (d.getVar('PATH', True), cmd)
>  		fd = os.popen(cmd)
> @@ -1283,6 +1284,7 @@ python package_do_shlibs() {
>  					needs_ldconfig = True
>  				if snap_symlinks and (file != this_soname):
>  					renames.append((os.path.join(root, file), os.path.join(root, this_soname)))
> +		return needs_ldconfig
>  
>  	def darwin_so(root, path, file):
>  		fullpath = os.path.join(root, file)
> @@ -1382,7 +1384,7 @@ python package_do_shlibs() {
>  				if targetos == "darwin" or targetos == "darwin8":
>  					darwin_so(root, dirs, file)
>  				elif os.access(path, os.X_OK) or lib_re.match(file):
> -					linux_so(root, dirs, file)
> +					needs_ldconfig = linux_so(root, dirs, file)

Shouldn't this be something like:

ldconfig = linux_so(root, dirs, file)
needs_ldconfig = needs_ldconfig or ldconfig

or can this only get called once?

Cheers,

Richard
Andrei Gherzan - April 10, 2012, 3:34 p.m.
On Tue, Apr 10, 2012 at 12:02, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Fri, 2012-04-06 at 16:57 +0300, Andrei Gherzan wrote:
> > "The suite of statements in a function definition executes with a local
> namespace
> > that is different from the global namespace. This means that all
> variables created
> > within a function are local to that function. When the suite finishes,
> these
> > working variables are discarded."
> >
> > In this way the needs_ldconfig variable in linux_so never gets True in
> the statements
> > below this function. As global statement is generally discouraged, a
> return value
> > would be a clean and fast way to solve this issue.
> >
> > [YOCTO #2205]
> >
> > Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
> > ---
> >  meta/classes/package.bbclass |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> > index d35667a..c98e8fa 100644
> > --- a/meta/classes/package.bbclass
> > +++ b/meta/classes/package.bbclass
> > @@ -1263,6 +1263,7 @@ python package_do_shlibs() {
> >       lf = bb.utils.lockfile(d.expand("${PACKAGELOCK}"))
> >
> >       def linux_so(root, path, file):
> > +             needs_ldconfig = False
> >               cmd = d.getVar('OBJDUMP', True) + " -p " +
> pipes.quote(os.path.join(root, file)) + " 2>/dev/null"
> >               cmd = "PATH=\"%s\" %s" % (d.getVar('PATH', True), cmd)
> >               fd = os.popen(cmd)
> > @@ -1283,6 +1284,7 @@ python package_do_shlibs() {
> >                                       needs_ldconfig = True
> >                               if snap_symlinks and (file != this_soname):
> >                                       renames.append((os.path.join(root,
> file), os.path.join(root, this_soname)))
> > +             return needs_ldconfig
> >
> >       def darwin_so(root, path, file):
> >               fullpath = os.path.join(root, file)
> > @@ -1382,7 +1384,7 @@ python package_do_shlibs() {
> >                               if targetos == "darwin" or targetos ==
> "darwin8":
> >                                       darwin_so(root, dirs, file)
> >                               elif os.access(path, os.X_OK) or
> lib_re.match(file):
> > -                                     linux_so(root, dirs, file)
> > +                                     needs_ldconfig = linux_so(root,
> dirs, file)
>
> Shouldn't this be something like:
>
> ldconfig = linux_so(root, dirs, file)
> needs_ldconfig = needs_ldconfig or ldconfig
>
> or can this only get called once?
>
>
As i recall this is called once but if you want i can amend with your hint.

@g
Richard Purdie - April 11, 2012, 11:57 a.m.
On Tue, 2012-04-10 at 18:34 +0300, Andrei Gherzan wrote:
> On Tue, Apr 10, 2012 at 12:02, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>         On Fri, 2012-04-06 at 16:57 +0300, Andrei Gherzan wrote:
>         > "The suite of statements in a function definition executes
>         with a local namespace
>         > that is different from the global namespace. This means that
>         all variables created
>         > within a function are local to that function. When the suite
>         finishes, these
>         > working variables are discarded."
>         >
>         > In this way the needs_ldconfig variable in linux_so never
>         gets True in the statements
>         > below this function. As global statement is generally
>         discouraged, a return value
>         > would be a clean and fast way to solve this issue.
>         >
>         > [YOCTO #2205]
>         >
>         > Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
>         > ---
>         >  meta/classes/package.bbclass |    4 +++-
>         >  1 files changed, 3 insertions(+), 1 deletions(-)
>         >
>         > diff --git a/meta/classes/package.bbclass
>         b/meta/classes/package.bbclass
>         > index d35667a..c98e8fa 100644
>         > --- a/meta/classes/package.bbclass
>         > +++ b/meta/classes/package.bbclass
>         > @@ -1263,6 +1263,7 @@ python package_do_shlibs() {
>         >       lf = bb.utils.lockfile(d.expand("${PACKAGELOCK}"))
>         >
>         >       def linux_so(root, path, file):
>         > +             needs_ldconfig = False
>         >               cmd = d.getVar('OBJDUMP', True) + " -p " +
>         pipes.quote(os.path.join(root, file)) + " 2>/dev/null"
>         >               cmd = "PATH=\"%s\" %s" % (d.getVar('PATH',
>         True), cmd)
>         >               fd = os.popen(cmd)
>         > @@ -1283,6 +1284,7 @@ python package_do_shlibs() {
>         >                                       needs_ldconfig = True
>         >                               if snap_symlinks and (file !=
>         this_soname):
>         >
>         renames.append((os.path.join(root, file), os.path.join(root,
>         this_soname)))
>         > +             return needs_ldconfig
>         >
>         >       def darwin_so(root, path, file):
>         >               fullpath = os.path.join(root, file)
>         > @@ -1382,7 +1384,7 @@ python package_do_shlibs() {
>         >                               if targetos == "darwin" or
>         targetos == "darwin8":
>         >                                       darwin_so(root, dirs,
>         file)
>         >                               elif os.access(path, os.X_OK)
>         or lib_re.match(file):
>         > -                                     linux_so(root, dirs,
>         file)
>         > +                                     needs_ldconfig =
>         linux_so(root, dirs, file)
>         
>         
>         Shouldn't this be something like:
>         
>         ldconfig = linux_so(root, dirs, file)
>         needs_ldconfig = needs_ldconfig or ldconfig
>         
>         or can this only get called once?
>         
>  
> As i recall this is called once but if you want i can amend with your
> hint.

I've checked and it is called multiple times as I suspected. It needs
the fix I described. I've gone ahead, tweaked and merged this I'd like
this fixed in the release.

Cheers,

Richard

Patch

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index d35667a..c98e8fa 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -1263,6 +1263,7 @@  python package_do_shlibs() {
 	lf = bb.utils.lockfile(d.expand("${PACKAGELOCK}"))
 
 	def linux_so(root, path, file):
+		needs_ldconfig = False
 		cmd = d.getVar('OBJDUMP', True) + " -p " + pipes.quote(os.path.join(root, file)) + " 2>/dev/null"
 		cmd = "PATH=\"%s\" %s" % (d.getVar('PATH', True), cmd)
 		fd = os.popen(cmd)
@@ -1283,6 +1284,7 @@  python package_do_shlibs() {
 					needs_ldconfig = True
 				if snap_symlinks and (file != this_soname):
 					renames.append((os.path.join(root, file), os.path.join(root, this_soname)))
+		return needs_ldconfig
 
 	def darwin_so(root, path, file):
 		fullpath = os.path.join(root, file)
@@ -1382,7 +1384,7 @@  python package_do_shlibs() {
 				if targetos == "darwin" or targetos == "darwin8":
 					darwin_so(root, dirs, file)
 				elif os.access(path, os.X_OK) or lib_re.match(file):
-					linux_so(root, dirs, file)
+					needs_ldconfig = linux_so(root, dirs, file)
 		for (old, new) in renames:
 		    	bb.note("Renaming %s to %s" % (old, new))
 			os.rename(old, new)