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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

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