[OE-core] [discussion] perf: specify SLANG_INC dir for perf
Bruce Ashfield
bruce.ashfield at windriver.com
Tue Aug 21 13:07:21 UTC 2012
On 12-08-21 01:08 AM, Liang Li wrote:
> On 2012-08-20 22:48, Bruce Ashfield<bruce.ashfield at windriver.com> wrote:
>> On 12-08-17 09:05 AM, Liang Li wrote:
>>> On 2012-08-17 21:01, Liang Li<liang.li at windriver.com> wrote:
>>>> On 2012-08-17 18:53, Richard Purdie<richard.purdie at 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 ?
>>
>
> No problem, the patch for kernel.bbclass:
>
> commit 60a0b06
> Author: Liang Li<liang.li at windriver.com>
> Date: Tue Aug 21 11:06:01 2012 +0800
>
> kernel.bbclass: fix INC directory for SLANG
>
> The change is intend to fix the hardcoded '-I/usr/include/slang' in
> the Makefile to be able to aware of SYSROOT if its specified.
>
> A planned kernel patch almost did the same change, but the change here
> won't conflict with it so this change could work for all kernels.
>
> Signed-off-by: Liang Li<liang.li at windriver.com>
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 1afb9ab..282194d 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -190,6 +190,9 @@ kernel_do_install() {
> for entry in $bin_files; do
> rm -f $kerneldir/$entry
> done
> +
> + # Fix SLNAG_INC for slang.h
s/SLNAG_INC/SLANG_INC/
> + sed -i 's#-I/usr/include/slang#-I=/usr/include/slang#g' $kerneldir/tools/perf/Makefile
It looks like your baseline for this patch is the denzil branch. We'd
want a version for master, which we could backport as required.
> }
>
> PACKAGE_PREPROCESS_FUNCS += "kernel_package_preprocess"
>
> ---
>
> The patch for kernel tree:
>
> commit 6b72896
> Author: Liang Li<liang.li at 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.
I'd rephrase this slightly as:
CFLAGS was previously hard coded to contain "-I/usr/include/slang" to
work with hosts that have "/usr/include/slang/slang.h" as well as hosts
that have "/usr/include/slang.h". This path can cause compile warnings
in some cases:
<put the warnings here>
.. and indicate that these warnings can actually cause build errors if
WERROR is enabled.
>
> 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.
Another minor rephrase:
To fix this issue, we can use -idirafter to downgrade the priority of the
default hard coded path. We can also make the slang include directory
a variable, to allow the user to specify SLANG_INC and set their own
include location.
>
> Signed-off-by: Liang Li<liang.li at windriver.com>
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index b7a7a87..e403c36 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -496,8 +496,10 @@ else
> msg := $(warning newt not found, disables TUI support. Please install newt-devel or libnewt-dev);
> BASIC_CFLAGS += -DNO_NEWT_SUPPORT
> else
> - # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h
> - BASIC_CFLAGS += -I/usr/include/slang
> + # Some releases like Fedora has /usr/include/slang/slang.h other than /usr/include/slang.h
> + SLANG_INC ?= -idirafter =/usr/include/slang
One more question, have you confirmed that gcc is fine with this being
in sysroot notation ? (I assume it is .. but I need to ask.
Cheers,
Bruce
> + BASIC_CFLAGS += $(SLANG_INC)
> +
> EXTLIBS += -lnewt -lslang
> LIB_OBJS += $(OUTPUT)ui/setup.o
> LIB_OBJS += $(OUTPUT)ui/browser.o
>
> ---
>
> Comments? :)
>
> Thanks,
> Liang Li
>
>> 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.
More information about the Openembedded-core
mailing list