Re: [discussion] perf: specify SLANG_INC dir for perf

[Liang Li <liang.li@windriver.com>]

On 2012-08-20 22:48, Bruce Ashfield <bruce.ashfield@windriver.com> wrote:
> 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 ?
> 

No problem, the patch for kernel.bbclass:

commit 60a0b06
Author: Liang Li <liang.li@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@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
+	sed -i 's#-I/usr/include/slang#-I=/usr/include/slang#g' $kerneldir/tools/perf/Makefile
 }
 
 PACKAGE_PREPROCESS_FUNCS += "kernel_package_preprocess"

---

The patch for kernel tree:

commit 6b72896
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>

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
+		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.