On 2012-08-09 08:36, Darren Hart <darren.h...@intel.com> wrote: > On 08/07/2012 08:37 PM, Liang Li wrote: > > On 2012-08-07 22:02, Richard Purdie <richard.pur...@linuxfoundation.org> > > wrote: > >> On Fri, 2012-08-03 at 23:43 +0800, Liang Li wrote: > >>> Via EXTRA_CFLAGS, we can pass the sysroot include directory to perf to > >>> provide slang.h rather than hardcoded host dir in perf's Makefile. > >>> > >>> Pass WERROR=0 to perf's Makefile to avoid warnings being treated > >>> as errors. Warnings are not fatal, and while they will be fixed in the > >>> future, there's no need for them to break the build. > >> > >> No mention of the additional slang dependency is made here? > >> > > > > Forgot mentioned it. Good catch, but the one line change that add > > slang to DEPENDS seems clear enough for everyone, isn't? :) > > Nope, the patch header declares the intent of the patch. >
Can't agree with you anymore. :) But we don't list all changes(even one line straight forward change) in patch header, don't we? :) Thanks, Liang Li > -- > Darren > > > > >>> Signed-off-by: Liang Li <liang...@windriver.com> > >>> --- > >>> meta/recipes-kernel/perf/perf_3.4.bb | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/meta/recipes-kernel/perf/perf_3.4.bb > >>> b/meta/recipes-kernel/perf/perf_3.4.bb > >>> index 505c7b8..537e926 100644 > >>> --- a/meta/recipes-kernel/perf/perf_3.4.bb > >>> +++ b/meta/recipes-kernel/perf/perf_3.4.bb > >>> @@ -24,6 +24,7 @@ DEPENDS = "virtual/kernel \ > >>> ${MLPREFIX}binutils \ > >>> ${TUI_DEPENDS} \ > >>> ${SCRIPTING_DEPENDS} \ > >>> + slang \ > >>> " > >>> > >>> SCRIPTING_RDEPENDS = "${@perf_feature_enabled('perf-scripting', 'perl > >>> perl-modules python', '',d)}" > >>> @@ -63,6 +64,8 @@ EXTRA_OEMAKE = \ > >>> AR="${AR}" \ > >>> prefix=/usr \ > >>> NO_GTK2=1 ${TUI_DEFINES} NO_DWARF=1 ${SCRIPTING_DEFINES} \ > >>> + WERROR=0 \ > >>> + EXTRA_CFLAGS=-I${STAGING_INCDIR} \ > >>> ' > >> > >> This is is not acceptable since the include directory /usr/include/slang > >> is still being looked at and this just "hides" the error. STAGING_INCDIR > >> is on the compilers default search path anyway. > > > > EXTRA_CFLAGS is processed early than the '-I/usr/include/slang', so > > its being used here, to specify a path for slang.h. That indeed > > 'hides' -I/usr/include/slang. And I agree that specify > > -I/usr/include/slang blindly in Makefile should be fixed. And I've > > discussed the fix for kernel tree several days before. However, the > > fix for kernel tree will not conflict than this patch, I would think > > that this patch for perf.bb would fix the issue for us for now, and > > smooth the adoption of future fix for the Makefile. > > > >> > >> So this patch is wrong in several different ways :( > >> > > > > ... I can't reproduce the 'warning -> error' on my host now, but as I > > explained above, this patch is just 'make use of existing mechanism of > > Makefile to specify correct location of slang.h before the warning is > > generated', even though the STAGING_INCDIR is in compilers default > > search path, seems no hurt in case we just make it float top a bit > > to get searched before wrong include directory is discovered, right? > > :) > > > >> I've merged a temporary fix until we get this resolved properly. > >> > > > > I saw that, but the fix that we've discussed several days before seems > > is what we want: > > > > diff --git a/tools/perf/Makefile b/tools/perf/Makefile > > index b7a7a87..3365ad2 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 has /usr/include/slang/slang.h other than > > /usr/include/slang.h > > + SLANG_INC ?= -I/usr/include/slang > > + BASIC_CFLAGS += $(SLANG_INC) > > + > > EXTLIBS += -lnewt -lslang > > LIB_OBJS += $(OUTPUT)ui/setup.o > > LIB_OBJS += $(OUTPUT)ui/browser.o > > > > --- > > > > Then we still need a patch to perf.bb to specify SLANG_INC. Moreover, > > this patch(EXTRA_CFLAGS for perf.bb) could co-exists with patch for > > kernel. :) > > > > Thanks, > > Liang Li > > > >> Cheers, > >> > >> Richard > > > -- > Darren Hart > Intel Open Source Technology Center > Yocto Project - Linux Kernel _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core