On 12-08-21 01:08 AM, Liang Li wrote:
On 2012-08-20 22:48, Bruce Ashfield<[email protected]>  wrote:
On 12-08-17 09:05 AM, Liang Li wrote:
On 2012-08-17 21:01, Liang Li<[email protected]>   wrote:
On 2012-08-17 18:53, Richard Purdie<[email protected]>   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<[email protected]>
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<[email protected]>

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<[email protected]>
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<[email protected]>

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.


_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core

Reply via email to