Patchwork [discussion] perf: specify SLANG_INC dir for perf

On 2012-08-17 18:53, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> On Fri, 2012-08-17 at 18:00 +0800, Liang Li wrote:
> > I am totally confused, you mentioned 'general kernel do_install', I
> > assume it's oe-core kernel.bbclass concept. Then you mentioned 'get
> > the fix upstream in the mainline kernel', how could that happen?
> > 
> > We are discussing about the solution to 'fix the compile warning to
> > error' stuff that triggered by the '-I/usr/include/slang', right?
> 
> Yes.
> 
> >  We do not necessarily have to change recipe to fix it since the issue
> > is not introduced by the recipe, the hard coded '-I/usr/include/slang'
> > in the Makefile cause the issue, we can fix the root cause by kernel
> > patch(other than just comment the line out). I see your previous patch
> > to kernel, by comment out the '-I/usr/include/slang' line in the
> > Makefile, is the same behavior, but we won't have the change(comment
> > out -I.. in Makefile) upstream to mainline, right?
> 
> I am suggesting that firstly, someone send a patch to the mainline
> kernel which changes -I/usr/include/slang to -I=/usr/include/slang in
> that Makefile.
> 
> Secondly, I'm suggesting that we add a line to kernel_do_install() in
> kernel.bbclass which does a sed on the Makefile as installed into
> $kerneldir which changes -I/usr/include/slang to -I=/usr/include/slang.
> 
> We can then drop the patch I added to the linux-yocto kernels.
> 
> This is all that should be needed, it should fix all the issues people
> have reported in a way that is acceptable to everyone.
> 

Ah, I see what you mean now. But we have push acceptable kernel patch
to linux-yocto kernel first, and propose it to mainline kernel in the
meantime. With proper kernel patch in linux-yocto kernel(s), we does
not have to do the second above, right? :)

And I see what you mean here, let me paste the revised change to
kernel here:

commit 5236988
Author: Liang Li <liang.li@windriver.com>
Date:   Wed Aug 1 14:31:24 2012 +0800

    perf: add SLANG_INC for slang.h
    
    Previously we hard code '-I/usr/include/slang' to CFLAGS to works with
    some hosts that has /usr/include/slang/slang.h other than
    /usr/include/slang.h like Fedora. This will cause compiling warnings
    in some cases.
    
    We could downgrade the priority of the default hard coded path, and
    provide user a chance to specify correct location of slang.h then user
    could specify SLANG_INC to avoid compile warnings like the
    '/usr/include/slang' is not exists etc.
    
    Signed-off-by: Liang Li <liang.li@windriver.com>


---

Thanks for clarification.

Regards,
		Liang Li

> Cheers,
> 
> Richard
>

On 2012-08-17 21:01, Liang Li <liang.li@windriver.com> wrote:
> On 2012-08-17 18:53, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> > On Fri, 2012-08-17 at 18:00 +0800, Liang Li wrote:
> > > I am totally confused, you mentioned 'general kernel do_install', I
> > > assume it's oe-core kernel.bbclass concept. Then you mentioned 'get
> > > the fix upstream in the mainline kernel', how could that happen?
> > > 
> > > We are discussing about the solution to 'fix the compile warning to
> > > error' stuff that triggered by the '-I/usr/include/slang', right?
> > 
> > Yes.
> > 
> > >  We do not necessarily have to change recipe to fix it since the issue
> > > is not introduced by the recipe, the hard coded '-I/usr/include/slang'
> > > in the Makefile cause the issue, we can fix the root cause by kernel
> > > patch(other than just comment the line out). I see your previous patch
> > > to kernel, by comment out the '-I/usr/include/slang' line in the
> > > Makefile, is the same behavior, but we won't have the change(comment
> > > out -I.. in Makefile) upstream to mainline, right?
> > 
> > I am suggesting that firstly, someone send a patch to the mainline
> > kernel which changes -I/usr/include/slang to -I=/usr/include/slang in
> > that Makefile.
> > 
> > Secondly, I'm suggesting that we add a line to kernel_do_install() in
> > kernel.bbclass which does a sed on the Makefile as installed into
> > $kerneldir which changes -I/usr/include/slang to -I=/usr/include/slang.
> > 
> > We can then drop the patch I added to the linux-yocto kernels.
> > 
> > This is all that should be needed, it should fix all the issues people
> > have reported in a way that is acceptable to everyone.
> > 
> 
> Ah, I see what you mean now. But we have push acceptable kernel patch

Sorry, I mean 'we can ...' instead of 'we have ...', just typo.

On 08/17/2012 06:01 AM, Liang Li wrote:
> On 2012-08-17 18:53, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
>> On Fri, 2012-08-17 at 18:00 +0800, Liang Li wrote:
>>> I am totally confused, you mentioned 'general kernel do_install', I
>>> assume it's oe-core kernel.bbclass concept. Then you mentioned 'get
>>> the fix upstream in the mainline kernel', how could that happen?
>>>
>>> We are discussing about the solution to 'fix the compile warning to
>>> error' stuff that triggered by the '-I/usr/include/slang', right?
>>
>> Yes.
>>
>>>  We do not necessarily have to change recipe to fix it since the issue
>>> is not introduced by the recipe, the hard coded '-I/usr/include/slang'
>>> in the Makefile cause the issue, we can fix the root cause by kernel
>>> patch(other than just comment the line out). I see your previous patch
>>> to kernel, by comment out the '-I/usr/include/slang' line in the
>>> Makefile, is the same behavior, but we won't have the change(comment
>>> out -I.. in Makefile) upstream to mainline, right?
>>
>> I am suggesting that firstly, someone send a patch to the mainline
>> kernel which changes -I/usr/include/slang to -I=/usr/include/slang in
>> that Makefile.
>>
>> Secondly, I'm suggesting that we add a line to kernel_do_install() in
>> kernel.bbclass which does a sed on the Makefile as installed into
>> $kerneldir which changes -I/usr/include/slang to -I=/usr/include/slang.
>>
>> We can then drop the patch I added to the linux-yocto kernels.
>>
>> This is all that should be needed, it should fix all the issues people
>> have reported in a way that is acceptable to everyone.
>>
> 
> Ah, I see what you mean now. But we have push acceptable kernel patch
> to linux-yocto kernel first, and propose it to mainline kernel in the
> meantime. With proper kernel patch in linux-yocto kernel(s), we does
> not have to do the second above, right? :)

No, the kernel.bbclass supports more than the linux-yocto kernels. It
needs to be generally useful with 2.6+kernels. The sed patch for the
kernel.bbclass is still necessary.

On 12-08-17 09:05 AM, Liang Li wrote:
> On 2012-08-17 21:01, Liang Li<liang.li@windriver.com>  wrote:
>> On 2012-08-17 18:53, Richard Purdie<richard.purdie@linuxfoundation.org>  wrote:
>>> On Fri, 2012-08-17 at 18:00 +0800, Liang Li wrote:
>>>> I am totally confused, you mentioned 'general kernel do_install', I
>>>> assume it's oe-core kernel.bbclass concept. Then you mentioned 'get
>>>> the fix upstream in the mainline kernel', how could that happen?
>>>>
>>>> We are discussing about the solution to 'fix the compile warning to
>>>> error' stuff that triggered by the '-I/usr/include/slang', right?
>>>
>>> Yes.
>>>
>>>>   We do not necessarily have to change recipe to fix it since the issue
>>>> is not introduced by the recipe, the hard coded '-I/usr/include/slang'
>>>> in the Makefile cause the issue, we can fix the root cause by kernel
>>>> patch(other than just comment the line out). I see your previous patch
>>>> to kernel, by comment out the '-I/usr/include/slang' line in the
>>>> Makefile, is the same behavior, but we won't have the change(comment
>>>> out -I.. in Makefile) upstream to mainline, right?
>>>
>>> I am suggesting that firstly, someone send a patch to the mainline
>>> kernel which changes -I/usr/include/slang to -I=/usr/include/slang in
>>> that Makefile.
>>>
>>> Secondly, I'm suggesting that we add a line to kernel_do_install() in
>>> kernel.bbclass which does a sed on the Makefile as installed into
>>> $kerneldir which changes -I/usr/include/slang to -I=/usr/include/slang.
>>>
>>> We can then drop the patch I added to the linux-yocto kernels.
>>>
>>> This is all that should be needed, it should fix all the issues people
>>> have reported in a way that is acceptable to everyone.
>>>
>>
>> Ah, I see what you mean now. But we have push acceptable kernel patch
>

One final (I hope) follow up on this.

Liang: were you going to put together (and test) the 'sed fix' for
kernel.bbclass ?

I have my own set of issues that are consuming my time now, and I want
to ensure that this doesn't fall through the cracks, since we need a
full/real fix for this as soon as possible.

Cheers,

Bruce


> Sorry, I mean 'we can ...' instead of 'we have ...', just typo.