| Submitter | Liang Li |
|---|---|
| Date | Aug. 3, 2012, 3:43 p.m. |
| Message ID | <1344008589-3660-1-git-send-email-liang.li@windriver.com> |
| Download | mbox | patch |
| Permalink | /patch/33745/ |
| State | New |
| Headers | show |
Comments
Adding Richard. Richard: did this patch not fix the problem for you ? .. it fixes it here, and should replace the patch that you made earlier today .. this may have slipped through the cracks. I've never seen the perf slang failure, so I'm hesitant to take this patch, apply it and drop yours. Is there anyway that someone seeing this problem can test this patch ? Cheers, Bruce On Fri, Aug 3, 2012 at 11:43 AM, Liang Li <liang.li@windriver.com> 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. > > Signed-off-by: Liang Li <liang.li@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} \ > ' > > do_compile() { > -- > 1.7.12.rc0.22.gcdd159b > > > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
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? > Signed-off-by: Liang Li <liang.li@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. So this patch is wrong in several different ways :( I've merged a temporary fix until we get this resolved properly. Cheers, Richard
On 12-08-07 10:02 AM, 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? > >> Signed-off-by: Liang Li<liang.li@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. But this makes it first, which is what we were going for. For an external toolchain, this also might not be true. > > So this patch is wrong in several different ways :( > > I've merged a temporary fix until we get this resolved properly. I think this is a point of view thing, since from here, this is a better temporary fix than commenting out the path in the perf Makefile, which will break standalone tree cases if I merge the patch into the tree, and I don't want to go to a milestone freeze with an out of tree patch. So I'd ask again that we reconsider this patch, since what's in tree has issues with other use cases. Cheers, Bruce > > Cheers, > > Richard >
On Tue, 2012-08-07 at 08:56 -0400, Bruce Ashfield wrote: > Adding Richard. > > Richard: did this patch not fix the problem for you ? .. it fixes it here, and > should replace the patch that you made earlier today .. this may have slipped > through the cracks. > > I've never seen the perf slang failure, so I'm hesitant to take this > patch, apply > it and drop yours. > > Is there anyway that someone seeing this problem can test this patch ? Reproducing the problem is easy "sudo mkdir /usr/include/slang", then build perf. Cheers, Richard
On Tue, Aug 7, 2012 at 10:12 AM, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > On Tue, 2012-08-07 at 08:56 -0400, Bruce Ashfield wrote: >> Adding Richard. >> >> Richard: did this patch not fix the problem for you ? .. it fixes it here, and >> should replace the patch that you made earlier today .. this may have slipped >> through the cracks. >> >> I've never seen the perf slang failure, so I'm hesitant to take this >> patch, apply >> it and drop yours. >> >> Is there anyway that someone seeing this problem can test this patch ? > > Reproducing the problem is easy "sudo mkdir /usr/include/slang", then > build perf. That's fine .. but if I take the existing patch, and drop the kernel temp one, would you take the change ? I'd rather not spend my time reproducing and fixing something to generate a patch that won't be taken :) Cheers, Bruce > > Cheers, > > Richard > > >
On Tue, 2012-08-07 at 10:07 -0400, Bruce Ashfield wrote: > On 12-08-07 10:02 AM, 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? > > > >> Signed-off-by: Liang Li<liang.li@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. > > But this makes it first, which is what we were going for. For > an external toolchain, this also might not be true. You have an external toolchain which does not search the sysroot usr/include directory? Really? ;-) I accept we won't search usr/include/slang but we don't install slang in that directory anyway so its irrelevant and that isn't the path you're adding. > > So this patch is wrong in several different ways :( > > > > I've merged a temporary fix until we get this resolved properly. > > I think this is a point of view thing, since from here, this is a > better temporary fix than commenting out the path in the perf > Makefile, which will break standalone tree cases if I merge the patch > into the tree, and I don't want to go to a milestone freeze with > an out of tree patch. I doubt it will actually break anything. > So I'd ask again that we reconsider this patch, since what's in tree > has issues with other use cases. No, I'm flatly refusing it, sorry. Cheers, Richard
On Tue, 2012-08-07 at 10:19 -0400, Bruce Ashfield wrote: > On Tue, Aug 7, 2012 at 10:12 AM, Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > On Tue, 2012-08-07 at 08:56 -0400, Bruce Ashfield wrote: > >> Adding Richard. > >> > >> Richard: did this patch not fix the problem for you ? .. it fixes it here, and > >> should replace the patch that you made earlier today .. this may have slipped > >> through the cracks. > >> > >> I've never seen the perf slang failure, so I'm hesitant to take this > >> patch, apply > >> it and drop yours. > >> > >> Is there anyway that someone seeing this problem can test this patch ? > > > > Reproducing the problem is easy "sudo mkdir /usr/include/slang", then > > build perf. > > That's fine .. but if I take the existing patch, and drop the kernel > temp one, would > you take the change ? I'd rather not spend my time reproducing and fixing > something to generate a patch that won't be taken :) Which patch is "the existing patch"? To be really clear, I'm not hacking around this issue, I do not want /usr/include/slang in the compiler search path at all, ever. Cheers, Richard
On 12-08-07 10:22 AM, Richard Purdie wrote: > On Tue, 2012-08-07 at 10:07 -0400, Bruce Ashfield wrote: >> On 12-08-07 10:02 AM, 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? >>> >>>> Signed-off-by: Liang Li<liang.li@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. >> >> But this makes it first, which is what we were going for. For >> an external toolchain, this also might not be true. > > You have an external toolchain which does not search the sysroot > usr/include directory? Really? ;-) I do. Trust me on this. I'm not talking about Wind River, I'm not talking about oe-core or yocto here. > > I accept we won't search usr/include/slang but we don't install slang in > that directory anyway so its irrelevant and that isn't the path you're > adding. > >>> So this patch is wrong in several different ways :( >>> >>> I've merged a temporary fix until we get this resolved properly. >> >> I think this is a point of view thing, since from here, this is a >> better temporary fix than commenting out the path in the perf >> Makefile, which will break standalone tree cases if I merge the patch >> into the tree, and I don't want to go to a milestone freeze with >> an out of tree patch. > > I doubt it will actually break anything. It will, and does if I merge the patch to the kernel tree where it belongs. That is my point. > >> So I'd ask again that we reconsider this patch, since what's in tree >> has issues with other use cases. > > No, I'm flatly refusing it, sorry. That's your prerogative, but I dislike the kernel patch that you did with the same flatness. I'm not trying to raise a fuss here, but you have to see the other point of view on this and acknowledge that the existing change is just as unpalatable to me, as the recipe space one is to you. In the end, I don't really care, but I just don't like the optics of an unsigned, out of tree patch, that I'm not going to merge to the kernel, and one that will have to sit where it is for M3. Cheers, Bruce > > Cheers, > > Richard >
On Tue, 2012-08-07 at 10:29 -0400, Bruce Ashfield wrote: > On 12-08-07 10:22 AM, Richard Purdie wrote: > > On Tue, 2012-08-07 at 10:07 -0400, Bruce Ashfield wrote: > >> On 12-08-07 10:02 AM, 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? > >>> > >>>> Signed-off-by: Liang Li<liang.li@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. > >> > >> But this makes it first, which is what we were going for. For > >> an external toolchain, this also might not be true. > > > > You have an external toolchain which does not search the sysroot > > usr/include directory? Really? ;-) > > I do. Trust me on this. I'm not talking about Wind River, I'm > not talking about oe-core or yocto here. Well, for such a toolchain, you need to pass STAGING_INCDIR into the CFLAGS anyway and are likely doing so at a different higher level. I don't see how this patch changes anything. > > I accept we won't search usr/include/slang but we don't install slang in > > that directory anyway so its irrelevant and that isn't the path you're > > adding. > > > >>> So this patch is wrong in several different ways :( > >>> > >>> I've merged a temporary fix until we get this resolved properly. > >> > >> I think this is a point of view thing, since from here, this is a > >> better temporary fix than commenting out the path in the perf > >> Makefile, which will break standalone tree cases if I merge the patch > >> into the tree, and I don't want to go to a milestone freeze with > >> an out of tree patch. > > > > I doubt it will actually break anything. > > It will, and does if I merge the patch to the kernel tree where > it belongs. That is my point. Can you please explain how it breaks? Previously all you've said it "it might break". Now you say it does break but not how. As I've explained, I can't see how it would do that as we should never be looking in /usr/include/slang/. I'm going to guess that this isn't an external toolchain problem and is if you build the kernel standalone against your native system with no cross compilation involved? Its likely a fedora system rather than ubuntu/debian derived? It would be nice to not have to guess at what the problem is though... > >> So I'd ask again that we reconsider this patch, since what's in tree > >> has issues with other use cases. > > > > No, I'm flatly refusing it, sorry. > > That's your prerogative, but I dislike the kernel patch that you > did with the same flatness. > > I'm not trying to raise a fuss here, but you have to see the > other point of view on this and acknowledge that the existing change > is just as unpalatable to me, as the recipe space one is to you. Well, I'm trying to illustrate how and why its a bad idea, you were just talking about hypothetical issues with no details. If it does break something, fine but at least please give some indication of what/how/why and then we can discuss it. > In the end, I don't really care, but I just don't like the optics > of an unsigned, out of tree patch, that I'm not going to merge > to the kernel, and one that will have to sit where it is for M3. We had the autobuilder showing read for 5 days with radio silence and admittedly some confusion over a patch status. What am I meant to do given I'd really like to take the vacation I have planned this week? Anyhow, lets see if we can resolve this. Can you answer my previous question about whether changing this to -I=/usr/include/slang would work? Cheers, Richard
On 12-08-07 10:44 AM, Richard Purdie wrote: > On Tue, 2012-08-07 at 10:29 -0400, Bruce Ashfield wrote: >> On 12-08-07 10:22 AM, Richard Purdie wrote: >>> On Tue, 2012-08-07 at 10:07 -0400, Bruce Ashfield wrote: >>>> On 12-08-07 10:02 AM, 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? >>>>> >>>>>> Signed-off-by: Liang Li<liang.li@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. >>>> >>>> But this makes it first, which is what we were going for. For >>>> an external toolchain, this also might not be true. >>> >>> You have an external toolchain which does not search the sysroot >>> usr/include directory? Really? ;-) >> >> I do. Trust me on this. I'm not talking about Wind River, I'm >> not talking about oe-core or yocto here. > > Well, for such a toolchain, you need to pass STAGING_INCDIR into the > CFLAGS anyway and are likely doing so at a different higher level. I > don't see how this patch changes anything. I wasn't clear, I'm building the tree (and tools) standalone using both a hand generated cross toolchain and the native toolchain. Neither uses a sysroot, so if I merged that patch into the tree, then they wouldn't find slang and the build breaks. > >>> I accept we won't search usr/include/slang but we don't install slang in >>> that directory anyway so its irrelevant and that isn't the path you're >>> adding. >>> >>>>> So this patch is wrong in several different ways :( >>>>> >>>>> I've merged a temporary fix until we get this resolved properly. >>>> >>>> I think this is a point of view thing, since from here, this is a >>>> better temporary fix than commenting out the path in the perf >>>> Makefile, which will break standalone tree cases if I merge the patch >>>> into the tree, and I don't want to go to a milestone freeze with >>>> an out of tree patch. >>> >>> I doubt it will actually break anything. >> >> It will, and does if I merge the patch to the kernel tree where >> it belongs. That is my point. > > Can you please explain how it breaks? Previously all you've said it "it > might break". Now you say it does break but not how. > > As I've explained, I can't see how it would do that as we should never > be looking in /usr/include/slang/. > > I'm going to guess that this isn't an external toolchain problem and is > if you build the kernel standalone against your native system with no > cross compilation involved? Its likely a fedora system rather than > ubuntu/debian derived? > > It would be nice to not have to guess at what the problem is though... Sorry about that, 400 unread email have me moving quickly this morning and not taking enough time to put all the detail into what I am writing. I added that above. And yes, it is some older RHEL/Suse boxes and other ones that are derived from non debian systems that give me pain. > >>>> So I'd ask again that we reconsider this patch, since what's in tree >>>> has issues with other use cases. >>> >>> No, I'm flatly refusing it, sorry. >> >> That's your prerogative, but I dislike the kernel patch that you >> did with the same flatness. >> >> I'm not trying to raise a fuss here, but you have to see the >> other point of view on this and acknowledge that the existing change >> is just as unpalatable to me, as the recipe space one is to you. > > Well, I'm trying to illustrate how and why its a bad idea, you were just > talking about hypothetical issues with no details. If it does break > something, fine but at least please give some indication of what/how/why > and then we can discuss it. > >> In the end, I don't really care, but I just don't like the optics >> of an unsigned, out of tree patch, that I'm not going to merge >> to the kernel, and one that will have to sit where it is for M3. > > We had the autobuilder showing read for 5 days with radio silence and > admittedly some confusion over a patch status. What am I meant to do > given I'd really like to take the vacation I have planned this week? Sorry, I thought when Liang sent the patch the issue was resolved :( We at least did. That's how it goes sometimes. > > Anyhow, lets see if we can resolve this. Can you answer my previous > question about whether changing this to -I=/usr/include/slang would > work? It should, but there's a distinct lack of precedent for sysroot style include paths in the kernel tools (behind the times), but in the end, if it fixes the oe-core case, and doesn't break standalone builds (the toolchain just has to no error on the syntax), it should be fine. I've already asked that a full kernel fix be generated, but I don't think it will make the deadline for your vacation, or M3, so merging a sysroot based include is probably the reasonable middle ground that I'll end on. Cheers, Bruce > > Cheers, > > Richard > > > >
On Tue, 2012-08-07 at 11:26 -0400, Bruce Ashfield wrote: > I wasn't clear, I'm building the tree (and tools) standalone using > both a hand generated cross toolchain and the native toolchain. Neither > uses a sysroot, so if I merged that patch into the tree, then they > wouldn't find slang and the build breaks. I'm not sure you want that hand generated cross toolchain finding slang in that location, but anyhow... :) > > It would be nice to not have to guess at what the problem is though... > > Sorry about that, 400 unread email have me moving quickly this morning > and not taking enough time to put all the detail into what I am > writing. I added that above. And yes, it is some older RHEL/Suse boxes > and other ones that are derived from non debian systems that give me > pain. Ok, I understand the problem now. > > We had the autobuilder showing read for 5 days with radio silence and > > admittedly some confusion over a patch status. What am I meant to do > > given I'd really like to take the vacation I have planned this week? > > Sorry, I thought when Liang sent the patch the issue was resolved :( > We at least did. That's how it goes sometimes. Right, some of it is misunderstanding and bad timing etc. but I needed to do something. I agree its suboptimal but its a better position for OE-Core than we were in and we can recover from it. > > Anyhow, lets see if we can resolve this. Can you answer my previous > > question about whether changing this to -I=/usr/include/slang would > > work? > > It should, but there's a distinct lack of precedent for sysroot style > include paths in the kernel tools (behind the times), but in the end, > if it fixes the oe-core case, and doesn't break standalone builds > (the toolchain just has to no error on the syntax), it should be fine. > > I've already asked that a full kernel fix be generated, but I don't think > it will make the deadline for your vacation, or M3, so merging a sysroot > based include is probably the reasonable middle ground that I'll end > on. Lets try the sysroot prefix (=) approach then. The kernel is sparse on these since it never actually has to deal with sysroots (or userspace) often and is still learning in that area. I suspect this should be the fix upstream might take. Cheers, Richard
On Tue, Aug 7, 2012 at 11:41 AM, Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > On Tue, 2012-08-07 at 11:26 -0400, Bruce Ashfield wrote: >> I wasn't clear, I'm building the tree (and tools) standalone using >> both a hand generated cross toolchain and the native toolchain. Neither >> uses a sysroot, so if I merged that patch into the tree, then they >> wouldn't find slang and the build breaks. > > I'm not sure you want that hand generated cross toolchain finding slang > in that location, but anyhow... :) > >> > It would be nice to not have to guess at what the problem is though... >> >> Sorry about that, 400 unread email have me moving quickly this morning >> and not taking enough time to put all the detail into what I am >> writing. I added that above. And yes, it is some older RHEL/Suse boxes >> and other ones that are derived from non debian systems that give me >> pain. > > Ok, I understand the problem now. > >> > We had the autobuilder showing read for 5 days with radio silence and >> > admittedly some confusion over a patch status. What am I meant to do >> > given I'd really like to take the vacation I have planned this week? >> >> Sorry, I thought when Liang sent the patch the issue was resolved :( >> We at least did. That's how it goes sometimes. > > Right, some of it is misunderstanding and bad timing etc. but I needed > to do something. I agree its suboptimal but its a better position for > OE-Core than we were in and we can recover from it. > >> > Anyhow, lets see if we can resolve this. Can you answer my previous >> > question about whether changing this to -I=/usr/include/slang would >> > work? >> >> It should, but there's a distinct lack of precedent for sysroot style >> include paths in the kernel tools (behind the times), but in the end, >> if it fixes the oe-core case, and doesn't break standalone builds >> (the toolchain just has to no error on the syntax), it should be fine. >> >> I've already asked that a full kernel fix be generated, but I don't think >> it will make the deadline for your vacation, or M3, so merging a sysroot >> based include is probably the reasonable middle ground that I'll end >> on. > > Lets try the sysroot prefix (=) approach then. The kernel is sparse on > these since it never actually has to deal with sysroots (or userspace) > often and is still learning in that area. I suspect this should be the > fix upstream might take. Agreed. And to close the loop, I'd prefer to leave what you merged as-is for today, and by next week, have another pull request with an in tree and upstream submitted patch to replace it. Does that work for you ? Bruce > > Cheers, > > Richard > > > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
On Tue, 2012-08-07 at 11:54 -0400, Bruce Ashfield wrote: > On Tue, Aug 7, 2012 at 11:41 AM, Richard Purdie > > Lets try the sysroot prefix (=) approach then. The kernel is sparse on > > these since it never actually has to deal with sysroots (or userspace) > > often and is still learning in that area. I suspect this should be the > > fix upstream might take. > > Agreed. And to close the loop, I'd prefer to leave what you merged as-is for > today, and by next week, have another pull request with an in tree and upstream > submitted patch to replace it. > > Does that work for you ? Yes, that works, thanks. Richard
Patch
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} \ ' do_compile() {
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. Signed-off-by: Liang Li <liang.li@windriver.com> --- meta/recipes-kernel/perf/perf_3.4.bb | 3 +++ 1 file changed, 3 insertions(+)