Patchwork [RFC] package.bbclass: fix strip and split logic

login
register
mail settings
Submitter Koen Kooi
Date Jan. 21, 2014, 9:47 a.m.
Message ID <1390297632-9966-1-git-send-email-koen.kooi@linaro.org>
Download mbox | patch
Permalink /patch/65311/
State Accepted
Commit afc4431250cfb35dcd6415d1a5c143226c5b8751
Headers show

Comments

Koen Kooi - Jan. 21, 2014, 9:47 a.m.
Marks original commit message and variable documentation state that stripping and splitting are independent of eachother, but package.bbclass ANDs the two INHIBIT flags to see which files can be stripped and/or split.

Original behaviour:

INHIBIT_PACKAGE_STRIP: no strip, no debug split
INHIBIT_PACAKGE_DEBUG_SPLIT: no strip, no debug split

Behaviour after this patch:

INHIBIT_PACKAGE_STRIP: no strip, no debug split
INHIBIT_PACKAGE_DEBUG_SPLIT: strip, no split

Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
---
 meta/classes/package.bbclass | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Richard Purdie - Jan. 21, 2014, 1:57 p.m.
On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
> Marks original commit message and variable documentation state that stripping and splitting are independent of eachother, but package.bbclass ANDs the two INHIBIT flags to see which files can be stripped and/or split.
> 
> Original behaviour:
> 
> INHIBIT_PACKAGE_STRIP: no strip, no debug split
> INHIBIT_PACAKGE_DEBUG_SPLIT: no strip, no debug split
> 
> Behaviour after this patch:
> 
> INHIBIT_PACKAGE_STRIP: no strip, no debug split
> INHIBIT_PACKAGE_DEBUG_SPLIT: strip, no split
> 
> Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
> ---
>  meta/classes/package.bbclass | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

FWIW this resulted in a failure on minnow:

http://autobuilder.yoctoproject.org/main/builders/minnow/builds/15/steps/BuildImages/logs/stdio

So we may have some fixing up to do before this change can be merged...

Cheers,

Richard
Koen Kooi - Jan. 21, 2014, 2:03 p.m.
On 01/21/2014 02:57 PM, Richard Purdie wrote:
> On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
>> Marks original commit message and variable documentation state that stripping and splitting are independent of eachother, but package.bbclass ANDs the two INHIBIT flags to see which files can be stripped and/or split.
>>
>> Original behaviour:
>>
>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
>> INHIBIT_PACAKGE_DEBUG_SPLIT: no strip, no debug split
>>
>> Behaviour after this patch:
>>
>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
>> INHIBIT_PACKAGE_DEBUG_SPLIT: strip, no split
>>
>> Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
>> ---
>>   meta/classes/package.bbclass | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> FWIW this resulted in a failure on minnow:
>
> http://autobuilder.yoctoproject.org/main/builders/minnow/builds/15/steps/BuildImages/logs/stdio
>
> So we may have some fixing up to do before this change can be merged...

I have that QA issue as warning not as error. I guess that's why my 
builds kept working :)

Aside from that, what are your thoughts on this patch?

regards,

Koen
Mark Hatle - Jan. 21, 2014, 3:04 p.m.
On 1/21/14, 8:03 AM, Koen Kooi wrote:
> On 01/21/2014 02:57 PM, Richard Purdie wrote:
>> On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
>>> Marks original commit message and variable documentation state that stripping and splitting are independent of eachother, but package.bbclass ANDs the two INHIBIT flags to see which files can be stripped and/or split.
>>>
>>> Original behaviour:
>>>
>>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
>>> INHIBIT_PACAKGE_DEBUG_SPLIT: no strip, no debug split
>>>
>>> Behaviour after this patch:
>>>
>>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
>>> INHIBIT_PACKAGE_DEBUG_SPLIT: strip, no split

My memory of the original theory was that there would be three different uses:

1 - I want no debug (stripped) software on the target and debug binaries (split)

2 - I want debug (not-stripped/not-split) software on the target

3 - I want no debug (stripped) software on the target, and I don't want debug 
binaries

So based on that, it looks like your change fixes things.  As the original 
implementation didn't allow for #3.

--Mark

>>> Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
>>> ---
>>>    meta/classes/package.bbclass | 3 +--
>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> FWIW this resulted in a failure on minnow:
>>
>> http://autobuilder.yoctoproject.org/main/builders/minnow/builds/15/steps/BuildImages/logs/stdio
>>
>> So we may have some fixing up to do before this change can be merged...
>
> I have that QA issue as warning not as error. I guess that's why my
> builds kept working :)
>
> Aside from that, what are your thoughts on this patch?
>
> regards,
>
> Koen
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
Richard Purdie - Jan. 21, 2014, 3:09 p.m.
On Tue, 2014-01-21 at 15:03 +0100, Koen Kooi wrote:
> On 01/21/2014 02:57 PM, Richard Purdie wrote:
> > On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
> >> Marks original commit message and variable documentation state that stripping and splitting are independent of eachother, but package.bbclass ANDs the two INHIBIT flags to see which files can be stripped and/or split.
> >>
> >> Original behaviour:
> >>
> >> INHIBIT_PACKAGE_STRIP: no strip, no debug split
> >> INHIBIT_PACAKGE_DEBUG_SPLIT: no strip, no debug split
> >>
> >> Behaviour after this patch:
> >>
> >> INHIBIT_PACKAGE_STRIP: no strip, no debug split
> >> INHIBIT_PACKAGE_DEBUG_SPLIT: strip, no split
> >>
> >> Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
> >> ---
> >>   meta/classes/package.bbclass | 3 +--
> >>   1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > FWIW this resulted in a failure on minnow:
> >
> > http://autobuilder.yoctoproject.org/main/builders/minnow/builds/15/steps/BuildImages/logs/stdio
> >
> > So we may have some fixing up to do before this change can be merged...
> 
> I have that QA issue as warning not as error. I guess that's why my 
> builds kept working :)
> 
> Aside from that, what are your thoughts on this patch?

I'm ok with it in principle but I'd like to see known build issues fixed
before it goes in since red autobuilders cause me enough grief
already ;-).

Cheers,

Richard
Koen Kooi - Jan. 23, 2014, 9:34 a.m.
On 01/21/2014 04:09 PM, Richard Purdie wrote:
> On Tue, 2014-01-21 at 15:03 +0100, Koen Kooi wrote:
>> On 01/21/2014 02:57 PM, Richard Purdie wrote:
>>> On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
>>>> Marks original commit message and variable documentation state that stripping and splitting are independent of eachother, but package.bbclass ANDs the two INHIBIT flags to see which files can be stripped and/or split.
>>>>
>>>> Original behaviour:
>>>>
>>>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
>>>> INHIBIT_PACAKGE_DEBUG_SPLIT: no strip, no debug split
>>>>
>>>> Behaviour after this patch:
>>>>
>>>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
>>>> INHIBIT_PACKAGE_DEBUG_SPLIT: strip, no split
>>>>
>>>> Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
>>>> ---
>>>>    meta/classes/package.bbclass | 3 +--
>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> FWIW this resulted in a failure on minnow:
>>>
>>> http://autobuilder.yoctoproject.org/main/builders/minnow/builds/15/steps/BuildImages/logs/stdio
>>>
>>> So we may have some fixing up to do before this change can be merged...
>>
>> I have that QA issue as warning not as error. I guess that's why my
>> builds kept working :)
>>
>> Aside from that, what are your thoughts on this patch?
>
> I'm ok with it in principle but I'd like to see known build issues fixed
> before it goes in since red autobuilders cause me enough grief
> already ;-).

I've changed all occurrences of INHIBIT_PACKAGE_DEBUG_SPLIT to have 
INHIBIT_PACKAGE_STRIP as well in all the layers angstrom has configured. 
I've sent patches to:

meta-intel
meta-initramfs
meta-oe
meta-fsl-arm
meta-android
meta-aurora
meta-linaro-toolchain

Which brings me to my next point:

If you list a mailinglist in your README where you want to have patches 
sent, don't make it automatically reject them. I'm looking at you, 
shr-devel!
Otavio Salvador - Jan. 23, 2014, 11:33 a.m.
Hello,

On Thu, Jan 23, 2014 at 7:34 AM, Koen Kooi <koen.kooi@linaro.org> wrote:
> meta-fsl-arm
...

I got this patch and I will merge it next time I merge things there.
Martin Jansa - Jan. 23, 2014, 12:41 p.m.
On Thu, Jan 23, 2014 at 10:34:48AM +0100, Koen Kooi wrote:
> On 01/21/2014 04:09 PM, Richard Purdie wrote:
> > On Tue, 2014-01-21 at 15:03 +0100, Koen Kooi wrote:
> >> On 01/21/2014 02:57 PM, Richard Purdie wrote:
> >>> On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
> >>>> Marks original commit message and variable documentation state that stripping and splitting are independent of eachother, but package.bbclass ANDs the two INHIBIT flags to see which files can be stripped and/or split.
> >>>>
> >>>> Original behaviour:
> >>>>
> >>>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
> >>>> INHIBIT_PACAKGE_DEBUG_SPLIT: no strip, no debug split
> >>>>
> >>>> Behaviour after this patch:
> >>>>
> >>>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
> >>>> INHIBIT_PACKAGE_DEBUG_SPLIT: strip, no split
> >>>>
> >>>> Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
> >>>> ---
> >>>>    meta/classes/package.bbclass | 3 +--
> >>>>    1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> FWIW this resulted in a failure on minnow:
> >>>
> >>> http://autobuilder.yoctoproject.org/main/builders/minnow/builds/15/steps/BuildImages/logs/stdio
> >>>
> >>> So we may have some fixing up to do before this change can be merged...
> >>
> >> I have that QA issue as warning not as error. I guess that's why my
> >> builds kept working :)
> >>
> >> Aside from that, what are your thoughts on this patch?
> >
> > I'm ok with it in principle but I'd like to see known build issues fixed
> > before it goes in since red autobuilders cause me enough grief
> > already ;-).
> 
> I've changed all occurrences of INHIBIT_PACKAGE_DEBUG_SPLIT to have 
> INHIBIT_PACKAGE_STRIP as well in all the layers angstrom has configured. 
> I've sent patches to:
> 
> meta-intel
> meta-initramfs
> meta-oe
> meta-fsl-arm
> meta-android
> meta-aurora
> meta-linaro-toolchain
> 
> Which brings me to my next point:
> 
> If you list a mailinglist in your README where you want to have patches 
> sent, don't make it automatically reject them. I'm looking at you, 
> shr-devel!

/me hides behind shr-devel owner, which is nobody knows who.

but I'll check
Koen Kooi - Feb. 4, 2014, 8:58 a.m.
On 01/21/2014 04:09 PM, Richard Purdie wrote:
> On Tue, 2014-01-21 at 15:03 +0100, Koen Kooi wrote:
>> On 01/21/2014 02:57 PM, Richard Purdie wrote:
>>> On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
>>>> Marks original commit message and variable documentation state that stripping and splitting are independent of eachother, but package.bbclass ANDs the two INHIBIT flags to see which files can be stripped and/or split.
>>>>
>>>> Original behaviour:
>>>>
>>>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
>>>> INHIBIT_PACAKGE_DEBUG_SPLIT: no strip, no debug split
>>>>
>>>> Behaviour after this patch:
>>>>
>>>> INHIBIT_PACKAGE_STRIP: no strip, no debug split
>>>> INHIBIT_PACKAGE_DEBUG_SPLIT: strip, no split
>>>>
>>>> Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
>>>> ---
>>>>    meta/classes/package.bbclass | 3 +--
>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> FWIW this resulted in a failure on minnow:
>>>
>>> http://autobuilder.yoctoproject.org/main/builders/minnow/builds/15/steps/BuildImages/logs/stdio
>>>
>>> So we may have some fixing up to do before this change can be merged...
>>
>> I have that QA issue as warning not as error. I guess that's why my
>> builds kept working :)
>>
>> Aside from that, what are your thoughts on this patch?
>
> I'm ok with it in principle but I'd like to see known build issues fixed
> before it goes in since red autobuilders cause me enough grief
> already ;-).

What's the status on this? I know a fix went into meta-intel (albeit it 
in a legally questionable way, but that's their problem) and the other 
affected layers have received patches for it.
Richard Purdie - Feb. 4, 2014, 9:32 a.m.
On Tue, 2014-02-04 at 09:58 +0100, Koen Kooi wrote:
> On 01/21/2014 04:09 PM, Richard Purdie wrote:
> > On Tue, 2014-01-21 at 15:03 +0100, Koen Kooi wrote:
> >> On 01/21/2014 02:57 PM, Richard Purdie wrote:
> >>> On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
> > I'm ok with it in principle but I'd like to see known build issues fixed
> > before it goes in since red autobuilders cause me enough grief
> > already ;-).
> 
> What's the status on this? I know a fix went into meta-intel (albeit it 
> in a legally questionable way, but that's their problem) and the other 
> affected layers have received patches for it.

The last test build I ran:

http://autobuilder.yoctoproject.org/main/builders/nightly-fsl-arm/builds/10/steps/BuildImages_1/logs/stdio

still shows an error in the fsl-arm layer which I suspect is from this.
I was hoping that could get fixed before it merges, its very much still
in the queue though.

I won't wait forever, equally, maintaining the green builds is proving
to consume a lot of my time and if I can avoid adding more failures, I'd
prefer to do so.

Cheers,

Richard
Paul Eggleton - Feb. 4, 2014, 10:10 a.m.
On Tuesday 04 February 2014 09:58:47 Koen Kooi wrote:
> What's the status on this? I know a fix went into meta-intel (albeit it
> in a legally questionable way, but that's their problem) and the other
> affected layers have received patches for it.

Legally questionable in what way? If you refer to your name being removed from 
the patch, that was rectified before the patch was merged.

Cheers,
Paul
Koen Kooi - Feb. 4, 2014, 11:01 a.m.
On 02/04/2014 11:10 AM, Paul Eggleton wrote:
> On Tuesday 04 February 2014 09:58:47 Koen Kooi wrote:
>> What's the status on this? I know a fix went into meta-intel (albeit it
>> in a legally questionable way, but that's their problem) and the other
>> affected layers have received patches for it.
>
> Legally questionable in what way? If you refer to your name being removed from
> the patch, that was rectified before the patch was merged.

Good, the last I heard was that Tom merged the patches where authorship 
was removed due to "git workflow issues". I just checked meta-intel git 
and it seems to be OK now.

I still don't get how someone can (accidentally or not) add 
--reset-author their git patch workflow, but that's a different discussion.
Otavio Salvador - Feb. 4, 2014, 11:20 a.m.
Hello Richard,

On Tue, Feb 4, 2014 at 7:32 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Tue, 2014-02-04 at 09:58 +0100, Koen Kooi wrote:
>> On 01/21/2014 04:09 PM, Richard Purdie wrote:
>> > On Tue, 2014-01-21 at 15:03 +0100, Koen Kooi wrote:
>> >> On 01/21/2014 02:57 PM, Richard Purdie wrote:
>> >>> On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
>> > I'm ok with it in principle but I'd like to see known build issues fixed
>> > before it goes in since red autobuilders cause me enough grief
>> > already ;-).
>>
>> What's the status on this? I know a fix went into meta-intel (albeit it
>> in a legally questionable way, but that's their problem) and the other
>> affected layers have received patches for it.
>
> The last test build I ran:
>
> http://autobuilder.yoctoproject.org/main/builders/nightly-fsl-arm/builds/10/steps/BuildImages_1/logs/stdio
>
> still shows an error in the fsl-arm layer which I suspect is from this.
> I was hoping that could get fixed before it merges, its very much still
> in the queue though.
>
> I won't wait forever, equally, maintaining the green builds is proving
> to consume a lot of my time and if I can avoid adding more failures, I'd
> prefer to do so.

I asked two fixes for Koen in the patch and I am awaiting this to
apply his patches.
Koen Kooi - Feb. 4, 2014, 11:27 a.m.
On 02/04/2014 12:20 PM, Otavio Salvador wrote:
> Hello Richard,
>
> On Tue, Feb 4, 2014 at 7:32 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> On Tue, 2014-02-04 at 09:58 +0100, Koen Kooi wrote:
>>> On 01/21/2014 04:09 PM, Richard Purdie wrote:
>>>> On Tue, 2014-01-21 at 15:03 +0100, Koen Kooi wrote:
>>>>> On 01/21/2014 02:57 PM, Richard Purdie wrote:
>>>>>> On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
>>>> I'm ok with it in principle but I'd like to see known build issues fixed
>>>> before it goes in since red autobuilders cause me enough grief
>>>> already ;-).
>>>
>>> What's the status on this? I know a fix went into meta-intel (albeit it
>>> in a legally questionable way, but that's their problem) and the other
>>> affected layers have received patches for it.
>>
>> The last test build I ran:
>>
>> http://autobuilder.yoctoproject.org/main/builders/nightly-fsl-arm/builds/10/steps/BuildImages_1/logs/stdio
>>
>> still shows an error in the fsl-arm layer which I suspect is from this.
>> I was hoping that could get fixed before it merges, its very much still
>> in the queue though.
>>
>> I won't wait forever, equally, maintaining the green builds is proving
>> to consume a lot of my time and if I can avoid adding more failures, I'd
>> prefer to do so.
>
> I asked two fixes for Koen in the patch and I am awaiting this to
> apply his patches.

And I already said I'm not planning to jump through more hoops just to 
get this patch into OE-core.
Otavio Salvador - Feb. 4, 2014, 12:12 p.m.
On Tue, Feb 4, 2014 at 9:27 AM, Koen Kooi <koen.kooi@linaro.org> wrote:
> On 02/04/2014 12:20 PM, Otavio Salvador wrote:
>>
>> Hello Richard,
>>
>> On Tue, Feb 4, 2014 at 7:32 AM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>>>
>>> On Tue, 2014-02-04 at 09:58 +0100, Koen Kooi wrote:
>>>>
>>>> On 01/21/2014 04:09 PM, Richard Purdie wrote:
>>>>>
>>>>> On Tue, 2014-01-21 at 15:03 +0100, Koen Kooi wrote:
>>>>>>
>>>>>> On 01/21/2014 02:57 PM, Richard Purdie wrote:
>>>>>>>
>>>>>>> On Tue, 2014-01-21 at 10:47 +0100, Koen Kooi wrote:
>>>>>
>>>>> I'm ok with it in principle but I'd like to see known build issues
>>>>> fixed
>>>>> before it goes in since red autobuilders cause me enough grief
>>>>> already ;-).
>>>>
>>>>
>>>> What's the status on this? I know a fix went into meta-intel (albeit it
>>>> in a legally questionable way, but that's their problem) and the other
>>>> affected layers have received patches for it.
>>>
>>>
>>> The last test build I ran:
>>>
>>>
>>> http://autobuilder.yoctoproject.org/main/builders/nightly-fsl-arm/builds/10/steps/BuildImages_1/logs/stdio
>>>
>>> still shows an error in the fsl-arm layer which I suspect is from this.
>>> I was hoping that could get fixed before it merges, its very much still
>>> in the queue though.
>>>
>>> I won't wait forever, equally, maintaining the green builds is proving
>>> to consume a lot of my time and if I can avoid adding more failures, I'd
>>> prefer to do so.
>>
>> I asked two fixes for Koen in the patch and I am awaiting this to
>> apply his patches.
>
> And I already said I'm not planning to jump through more hoops just to get
> this patch into OE-core.

Sorry Koen but I think as you send the patch you should send the reviewed one.

Patch

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 768047c..fa0b7eb 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -781,8 +781,7 @@  python split_and_strip_files () {
     kernmods = []
     libdir = os.path.abspath(dvar + os.sep + d.getVar("libdir", True))
     baselibdir = os.path.abspath(dvar + os.sep + d.getVar("base_libdir", True))
-    if (d.getVar('INHIBIT_PACKAGE_DEBUG_SPLIT', True) != '1') and \
-            (d.getVar('INHIBIT_PACKAGE_STRIP', True) != '1'):
+    if (d.getVar('INHIBIT_PACKAGE_STRIP', True) != '1'):
         for root, dirs, files in cpath.walk(dvar):
             for f in files:
                 file = os.path.join(root, f)