Re: [OE-core] [PATCH 6/6] perf: drop 'include' copy

["Andrey Zhizhikin" <andrey.z@gmail.com>]

On Tue, May 26, 2020 at 2:31 PM Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
>
> On Tue, May 26, 2020 at 1:44 AM Andrey Zhizhikin <andrey.z@gmail.com> wrote:
> >
> > On Mon, Oct 21, 2019 at 10:58 PM Bruce Ashfield
> > <bruce.ashfield@gmail.com> wrote:
> > >
> > > On Mon, Oct 21, 2019 at 4:24 PM Martin Jansa <martin.jansa@gmail.com> wrote:
> > > >
> > > > On Mon, Oct 21, 2019 at 04:16:18PM -0400, bruce.ashfield@gmail.com wrote:
> > > > > From: Bruce Ashfield <bruce.ashfield@gmail.com>
> > > > >
> > > > > The copy of the kernel's top level include directory is not
> > > > > required to build perf. We have both the linux-libc-headers and
> > > > > perf's captured/copied headers for what it requires.
> > > > >
> > > > > The copy of the kernel's headers is leading us to multiple smaller
> > > > > fixes to ensure that the various .h files are in sync. We can
> > > > > remove the copy and all of the sync checks, and perf still builds
> > > > > and executes correctly.
> > > >
> > > > Maybe reorder this before "[OE-core] [PATCH 3/6] perf: fix v5.4+ builds"
> > > > as it removes most of what was possibly incorrectly added there (see 2nd
> > > > review)
> > > >
> > >
> > > I left this here on purpose, since it is only RFC. The other patches
> > > as they appear in the series are required to build against 5.4+, but
> > > this is optional.
> >
> > Look like this patch causes issues with building perf for 4.4.y kernel...
> >
> > I've recently started to upgrade some of BSPs from zeus to dunfell,
> > and kernel 4.4.y fails to build with this patch applied. When I
> > applied changes from Martin in commit
> > [f3515d2d2545cae6b88fe5e86d081f4ef6133bf6] - build went through.
> >
> > I have now the bbappend in the layer, which sets the PERF_SRC to add
> > the "include" folder back but I don't think this is a clean approach.
> >
> > Should this be reverted, or those who has a "dunfell+4.4.y kernel"
> > setup need to use appends to override the PERF_SRC?
>
> If you revert it, you might be trading warnings and breakages for
> newer kernels to fix 4.4. That's the issue with perf, since it builds
> right from the kernel source, it has to serve many different versions
> (and we juggle breaking/fixing that from time to time).

I got the same conclusion that healing one can hurt the other... perf
is a gentle beast, I had my own history with it already.

>
> So any revert would have to then build all of the different kernel
> versions and perf to make sure that we don't trade one breakage for
> another .. and being LTS we need to be extra careful. Since we don't
> have an active 4.4 kernel in that branch, it would have to be ad-hoc
> testing to confirm.

Kernel 4.4 is also LTS and has (had) a very long time span, so I
believe there are some people out there who might still have it in
their Products (industrial applications are pretty conservative guys).
I have to admit that Yocto 3.1 + Kernel 4.4 is a rather strange
combination, but it also has a valid reason to exist.

I guess reverting this patch is not really an option, but this topic
deserves then a bit of documentation somewhere so people can look this
up and have a quick fix for themselves. The question is: where to put
this information?

Honestly, it didn't take me long to find the commit that solved the
build, it took me rather long to figure out why I didn't see changes
from it on the [dunfell]...

>
> What exactly was the build error that you saw ?

Exactly what JaMa had and fixed with his commit, namely a multitude of:
<bitops> error: #include nested too deeply

and some of:

<snip>/perf/1.0-r9/perf-1.0/tools/include/linux/list.h:5:10: fatal
error: ../../../include/linux/list.h: No such file or directory

>
> Bruce
>
>
>
> Bruce
>
> >
> > >
> > > Bruce
> > >
> > > > > Signed-off-by: Bruce Ashfield <bruce.ashfield@gmail.com>
> > > > > ---
> > > > >  meta/recipes-kernel/perf/perf.bb | 9 ---------
> > > > >  1 file changed, 9 deletions(-)
> > > > >
> > > > > diff --git a/meta/recipes-kernel/perf/perf.bb b/meta/recipes-kernel/perf/perf.bb
> > > > > index 191305969c..5f0ba7c180 100644
> > > > > --- a/meta/recipes-kernel/perf/perf.bb
> > > > > +++ b/meta/recipes-kernel/perf/perf.bb
> > > > > @@ -106,7 +106,6 @@ EXTRA_OEMAKE += "\
> > > > >  EXTRA_OEMAKE_append_task-configure = " JOBS=1"
> > > > >
> > > > >  PERF_SRC ?= "Makefile \
> > > > > -             include \
> > > > >               tools/arch \
> > > > >               tools/build \
> > > > >               tools/include \
> > > > > @@ -248,14 +247,6 @@ do_configure_prepend () {
> > > > >      # so we copy it from the sysroot unistd.h to the perf unistd.h
> > > > >      install -D -m0644 ${STAGING_INCDIR}/asm-generic/unistd.h ${S}/tools/include/uapi/asm-generic/unistd.h
> > > > >      install -D -m0644 ${STAGING_INCDIR}/asm-generic/unistd.h ${S}/include/uapi/asm-generic/unistd.h
> > > > > -
> > > > > -    # bits.h can have the same issue as unistd.h, so we make the tools variant take precedence
> > > > > -    [ -e ${S}/tools/include/linux/bits.h ] && install -D -m0644 ${S}/tools/include/linux/bits.h ${S}/include/linux/bits.h
> > > > > -
> > > > > -    [ -e ${S}/tools/perf/util/include/linux/ctype.h ] && install -D -m0644 ${S}/include/linux/ctype.h ${S}/tools/perf/util/include/linux/ctype.h
> > > > > -
> > > > > -    [ -e ${S}/include/uapi/linux/kvm.h ] && install -D -m0644 ${S}/include/uapi/linux/kvm.h  ${S}/tools/include/uapi/linux/kvm.h
> > > > > -    [ -e ${S}/include/uapi/linux/sched.h ] && install -D -m0644 ${S}/include/uapi/linux/sched.h  ${S}/tools/include/uapi/linux/sched.h
> > > > >  }
> > > > >
> > > > >  python do_package_prepend() {
> > > > > --
> > > > > 2.19.1
> > > > >
> > > > > --
> > > > > _______________________________________________
> > > > > Openembedded-core mailing list
> > > > > Openembedded-core@lists.openembedded.org
> > > > > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> > > >
> > > > --
> > > > Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
> > >
> > >
> > >
> > > --
> > > - Thou shalt not follow the NULL pointer, for chaos and madness await
> > > thee at its end
> > > - "Use the force Harry" - Gandalf, Star Trek II
> > > --
> > > _______________________________________________
> > > Openembedded-core mailing list
> > > Openembedded-core@lists.openembedded.org
> > > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> >
> >
> >
> > --
> > Regards,
> > Andrey.
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II

-- 
Regards,
Andrey.