Patchwork [CONSOLIDATED,PULL,29/43] package.bbclass: Ensure kernel modules get stripped

login
register
mail settings
Submitter Saul Wold
Date April 20, 2012, 4:45 p.m.
Message ID <086fa62c043e3cd4b9bc8d2377507ed842a3097b.1334940120.git.sgw@linux.intel.com>
Download mbox | patch
Permalink /patch/26215/
State New
Headers show

Comments

Saul Wold - April 20, 2012, 4:45 p.m.
From: Richard Purdie <richard.purdie@linuxfoundation.org>

Kernel modules are not marked as executable but we do expect to strip them.
This patch adds in missing code to ensure we do this. Without this images
are getting sigificantly bloated in size.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/classes/package.bbclass |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
Darren Hart - April 23, 2012, 3:47 p.m.
On 04/20/2012 09:45 AM, Saul Wold wrote:
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> 
> Kernel modules are not marked as executable but we do expect to strip them.
> This patch adds in missing code to ensure we do this. Without this images
> are getting sigificantly bloated in size.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/classes/package.bbclass |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index 99c945d..71bd3a6 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -870,6 +870,14 @@ python split_and_strip_files () {
>  				elf_file = int(file_list[file][5:])
>  				#bb.note("Strip %s" % file)
>  				runstrip(file, elf_file, d)
> +
> +
> +	if (d.getVar('INHIBIT_PACKAGE_STRIP', True) != '1'):	<- white space at end

Note: Whitespace at end of line.

I understand it's common practice with bitbake recipes to compare to '1'
as a string. However, this isn't documented in the usae of
INHIBIT_PACKAGE_STRIP, and it seems reasonable that someone might try
setting "True" or "yes" or some other common affirmative label.

Scott, can we update the ref manual glossary to indicate that assigning
to the string "1" is the way to set this to true?


> +		for root, dirs, files in os.walk(dvar):
> +			for f in files:
> +				if not f.endswith(".ko"):
> +					continue
> +				runstrip(os.path.join(root, f), None, d)

Not a big deal, but you can drop the "not" and the "continue" and only
runtstrip if the file ends with ".ko" since there isn't anything else
done in the loop.

>  	#
>  	# End of strip
>  	#
Richard Purdie - April 24, 2012, 10:40 a.m.
On Mon, 2012-04-23 at 08:47 -0700, Darren Hart wrote:
> 
> On 04/20/2012 09:45 AM, Saul Wold wrote:
> > From: Richard Purdie <richard.purdie@linuxfoundation.org>
> > 
> > Kernel modules are not marked as executable but we do expect to strip them.
> > This patch adds in missing code to ensure we do this. Without this images
> > are getting sigificantly bloated in size.
> > 
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> >  meta/classes/package.bbclass |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> > index 99c945d..71bd3a6 100644
> > --- a/meta/classes/package.bbclass
> > +++ b/meta/classes/package.bbclass
> > @@ -870,6 +870,14 @@ python split_and_strip_files () {
> >  				elf_file = int(file_list[file][5:])
> >  				#bb.note("Strip %s" % file)
> >  				runstrip(file, elf_file, d)
> > +
> > +
> > +	if (d.getVar('INHIBIT_PACKAGE_STRIP', True) != '1'):	<- white space at end
> 
> Note: Whitespace at end of line.
> 
> I understand it's common practice with bitbake recipes to compare to '1'
> as a string. However, this isn't documented in the usae of
> INHIBIT_PACKAGE_STRIP, and it seems reasonable that someone might try
> setting "True" or "yes" or some other common affirmative label.
> 
> Scott, can we update the ref manual glossary to indicate that assigning
> to the string "1" is the way to set this to true?

I think we need to go over all these "1" tests that change them to be
conditional on the variable set/unset. I did just copy this some another
location within package.bbclass which is fine for 1.2 but needs
revisiting.

> > +		for root, dirs, files in os.walk(dvar):
> > +			for f in files:
> > +				if not f.endswith(".ko"):
> > +					continue
> > +				runstrip(os.path.join(root, f), None, d)
> 
> Not a big deal, but you can drop the "not" and the "continue" and only
> runtstrip if the file ends with ".ko" since there isn't anything else
> done in the loop.

I just have a dislike of functions that look like a set of steps :). I
also suspect the conditions might change here in the future.

Cheers,

Richard
Dominik - May 8, 2012, 12:43 p.m.
> Kernel modules are not marked as executable but we do expect to strip them.
> This patch adds in missing code to ensure we do this. Without this images
> are getting sigificantly bloated in size.
> 


This patch causes problems with TI's dsplink and linuxutils as posted here 
http://comments.gmane.org/gmane.linux.embedded.yocto.meta-ti/340.

Patch

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 99c945d..71bd3a6 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -870,6 +870,14 @@  python split_and_strip_files () {
 				elf_file = int(file_list[file][5:])
 				#bb.note("Strip %s" % file)
 				runstrip(file, elf_file, d)
+
+
+	if (d.getVar('INHIBIT_PACKAGE_STRIP', True) != '1'):	
+		for root, dirs, files in os.walk(dvar):
+			for f in files:
+				if not f.endswith(".ko"):
+					continue
+				runstrip(os.path.join(root, f), None, d)
 	#
 	# End of strip
 	#