From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail1.windriver.com ([147.11.146.13]) by linuxtogo.org with esmtp (Exim 4.72) (envelope-from ) id 1SyxH3-0002U0-JT for openembedded-core@lists.openembedded.org; Wed, 08 Aug 2012 05:49:34 +0200 Received: from ALA-HCA.corp.ad.wrs.com (ala-hca [147.11.189.40]) by mail1.windriver.com (8.14.5/8.14.3) with ESMTP id q783bhv8013721 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Tue, 7 Aug 2012 20:37:43 -0700 (PDT) Received: from localhost (128.224.163.138) by ALA-HCA.corp.ad.wrs.com (147.11.189.50) with Microsoft SMTP Server id 14.2.309.2; Tue, 7 Aug 2012 20:37:43 -0700 Date: Wed, 8 Aug 2012 11:37:42 +0800 From: Liang Li To: Richard Purdie Message-ID: <20120808033742.GA19078@localhost> References: <1344008589-3660-1-git-send-email-liang.li@windriver.com> <1344348160.9756.255.camel@ted> MIME-Version: 1.0 In-Reply-To: <1344348160.9756.255.camel@ted> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: openembedded-core@lists.openembedded.org, darren.hart@intel.com Subject: Re: [PATCH] perf: pass STAGING_INCDIR(sysroot) to perf X-BeenThere: openembedded-core@lists.openembedded.org X-Mailman-Version: 2.1.11 Precedence: list Reply-To: Liang Li , Patches and discussions about the oe-core layer List-Id: Patches and discussions about the oe-core layer List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Aug 2012 03:49:34 -0000 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline On 2012-08-07 22:02, Richard Purdie 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? :) > > Signed-off-by: Liang Li > > --- > > 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