[PATCH] kernel.bbclass: Allow ${S} to be overridden

[PATCH] kernel.bbclass: Allow ${S} to be overridden

[Paul Barker]

For kernel recipes which fetch sources from an archive instead of git, it's
necessary to point ${S} at the extracted sources. In kernel.bbclass there is
already logic in base_do_unpack_append() to link ${KERNEL_STAGING_DIR} to ${S}
if these aren't the same, so we just need to be able to override ${S}.

Signed-off-by: Paul Barker <paul@paulbarker.me.uk>
---
 meta/classes/kernel.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 25a153c..a7d467b 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -3,7 +3,7 @@ inherit linux-kernel-base kernel-module-split
 PROVIDES += "virtual/kernel"
 DEPENDS += "virtual/${TARGET_PREFIX}binutils virtual/${TARGET_PREFIX}gcc kmod-native depmodwrapper-cross bc-native lzop-native"
 
-S = "${STAGING_KERNEL_DIR}"
+S ?= "${STAGING_KERNEL_DIR}"
 B = "${WORKDIR}/build"
 KBUILD_OUTPUT = "${B}"
 OE_TERMINAL_EXPORTS += "KBUILD_OUTPUT"
-- 
2.1.4

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Paul Barker]

On Sat, 22 Oct 2016 19:38:00 +0100
Paul Barker <paul@paulbarker.me.uk> wrote:

> For kernel recipes which fetch sources from an archive instead of git, it's
> necessary to point ${S} at the extracted sources. In kernel.bbclass there is
> already logic in base_do_unpack_append() to link ${KERNEL_STAGING_DIR} to ${S}
> if these aren't the same, so we just need to be able to override ${S}.
> 
> Signed-off-by: Paul Barker <paul@paulbarker.me.uk>
> ---
>  meta/classes/kernel.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index 25a153c..a7d467b 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -3,7 +3,7 @@ inherit linux-kernel-base kernel-module-split
>  PROVIDES += "virtual/kernel"
>  DEPENDS += "virtual/${TARGET_PREFIX}binutils virtual/${TARGET_PREFIX}gcc kmod-native depmodwrapper-cross bc-native lzop-native"
>  
> -S = "${STAGING_KERNEL_DIR}"
> +S ?= "${STAGING_KERNEL_DIR}"
>  B = "${WORKDIR}/build"
>  KBUILD_OUTPUT = "${B}"
>  OE_TERMINAL_EXPORTS += "KBUILD_OUTPUT"

Ping on this. Anyone had a chance to look at it?

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 287 bytes --]

On 5 November 2016 at 10:14, Paul Barker <paul@paulbarker.me.uk> wrote:

> Ping on this. Anyone had a chance to look at it?
>

I tend to want to let Bruce have a look at kernel patches, but this does
seem sufficiently obvious that I've just merge it to my staging branch.

Ross

[-- Attachment #2: Type: text/html, Size: 686 bytes --]

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Bruce Ashfield]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

On Tue, Nov 8, 2016 at 8:04 PM, Burton, Ross <ross.burton@intel.com> wrote:

>
> On 5 November 2016 at 10:14, Paul Barker <paul@paulbarker.me.uk> wrote:
>
>> Ping on this. Anyone had a chance to look at it?
>>
>
> I tend to want to let Bruce have a look at kernel patches, but this does
> seem sufficiently obvious that I've just merge it to my staging branch.
>

It managed to slip by me. Sorry about that.

I can ack this patch, since no defaults change .. there's no risk to
existing users.

Bruce


>
> Ross
>
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>


-- 
"Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end"

[-- Attachment #2: Type: text/html, Size: 2275 bytes --]

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]

On 9 November 2016 at 02:23, Bruce Ashfield <bruce.ashfield@gmail.com>
wrote:

> I can ack this patch, since no defaults change .. there's no risk to
> existing users.
>

Saying that of course doomed the patch:

ERROR: hello-mod-0.1-r0 do_make_scripts: Function failed: do_make_scripts
(log file is located at
/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-qa-skeleton/build/build/tmp/work/qemux86_64-poky-linux/hello-mod/0.1-r0/temp/log.do_make_scripts.14071)
ERROR: Logfile of failure stored in:
/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-qa-skeleton/build/build/tmp/work/qemux86_64-poky-linux/hello-mod/0.1-r0/temp/log.do_make_scripts.14071
Log data follows:
| DEBUG: Executing shell function do_make_scripts
| make: Entering directory
'/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-qa-skeleton/build/build/tmp/work-shared/qemux86-64/kernel-source'
| make: *** No rule to make target 'scripts'.  Stop.
| make: Leaving directory
'/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-qa-skeleton/build/build/tmp/work-shared/qemux86-64/kernel-source'
| WARNING:
/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-qa-skeleton/build/build/tmp/work/qemux86_64-poky-linux/hello-mod/0.1-r0/temp/run.do_make_scripts.14071:1
exit 2 from 'make CC="x86_64-poky-linux-gcc  -fuse-ld=bfd"
LD="x86_64-poky-linux-ld.bfd " AR="x86_64-poky-linux-ar " -C
/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-qa-skeleton/build/build/tmp/work-shared/qemux86-64/kernel-source
O=/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-qa-skeleton/build/build/tmp/work-shared/qemux86-64/kernel-build-artifacts
scripts'
| ERROR: Function failed: do_make_scripts (log file is located at
/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-qa-skeleton/build/build/tmp/work/qemux86_64-poky-linux/hello-mod/0.1-r0/temp/log.do_make_scripts.14071)
NOTE: recipe hello-mod-0.1-r0: task do_make_scripts: Failed

The autobuilder builds hello-mod from meta-skeleton, which inherits module
and sets S=${WORKDIR}.

Ross

[-- Attachment #2: Type: text/html, Size: 2543 bytes --]

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

On 9 November 2016 at 10:04, Burton, Ross <ross.burton@intel.com> wrote:

> Saying that of course doomed the patch:
>

Double doomed, perf can't build either.

ERROR: perf-1.0-r9 do_configure: Function failed: do_configure (log file is
located at
/data/poky-master/tmp-glibc/work/intel_corei7_64-poky-linux/perf/1.0-r9/temp/log.do_configure.23322)
ERROR: Logfile of failure stored in:
/data/poky-master/tmp-glibc/work/intel_corei7_64-poky-linux/perf/1.0-r9/temp/log.do_configure.23322
Log data follows:
| DEBUG: Executing python function sysroot_cleansstate
| DEBUG: Python function sysroot_cleansstate finished
| DEBUG: Executing shell function do_configure
| sed: can't read
/data/poky-master/tmp-glibc/work-shared/intel-corei7-64/kernel-source/tools/perf/Makefile*:
No such file or directory
| WARNING: exit code 2 from a shell command.
| ERROR: Function failed: do_configure (log file is located at
/data/poky-master/tmp-glibc/work/intel_corei7_64-poky-linux/perf/1.0-r9/temp/log.do_configure.23322)

Ross

[-- Attachment #2: Type: text/html, Size: 1800 bytes --]

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Bruce Ashfield]

[-- Attachment #1: Type: text/plain, Size: 2577 bytes --]

On Wed, Nov 9, 2016 at 5:04 AM, Burton, Ross <ross.burton@intel.com> wrote:

>
> On 9 November 2016 at 02:23, Bruce Ashfield <bruce.ashfield@gmail.com>
> wrote:
>
>> I can ack this patch, since no defaults change .. there's no risk to
>> existing users.
>>
>
> Saying that of course doomed the patch:
>

interesting.

Paul: obviously you were building with this in place, and my local test did
work here
as well .. so any idea to the difference ?

Let me know if you want any help looking into it.

Cheers,

Bruce


>
> ERROR: hello-mod-0.1-r0 do_make_scripts: Function failed: do_make_scripts
> (log file is located at /home/pokybuild/yocto-autobuilder/yocto-worker/
> nightly-qa-skeleton/build/build/tmp/work/qemux86_64-
> poky-linux/hello-mod/0.1-r0/temp/log.do_make_scripts.14071)
> ERROR: Logfile of failure stored in: /home/pokybuild/yocto-
> autobuilder/yocto-worker/nightly-qa-skeleton/build/
> build/tmp/work/qemux86_64-poky-linux/hello-mod/0.1-r0/
> temp/log.do_make_scripts.14071
> Log data follows:
> | DEBUG: Executing shell function do_make_scripts
> | make: Entering directory '/home/pokybuild/yocto-
> autobuilder/yocto-worker/nightly-qa-skeleton/build/
> build/tmp/work-shared/qemux86-64/kernel-source'
> | make: *** No rule to make target 'scripts'.  Stop.
> | make: Leaving directory '/home/pokybuild/yocto-autobuilder/yocto-worker/
> nightly-qa-skeleton/build/build/tmp/work-shared/qemux86-64/kernel-source'
> | WARNING: /home/pokybuild/yocto-autobuilder/yocto-worker/
> nightly-qa-skeleton/build/build/tmp/work/qemux86_64-
> poky-linux/hello-mod/0.1-r0/temp/run.do_make_scripts.14071:1 exit 2 from
> 'make CC="x86_64-poky-linux-gcc  -fuse-ld=bfd" LD="x86_64-poky-linux-ld.bfd
> " AR="x86_64-poky-linux-ar " -C /home/pokybuild/yocto-
> autobuilder/yocto-worker/nightly-qa-skeleton/build/
> build/tmp/work-shared/qemux86-64/kernel-source O=/home/pokybuild/yocto-
> autobuilder/yocto-worker/nightly-qa-skeleton/build/
> build/tmp/work-shared/qemux86-64/kernel-build-artifacts scripts'
> | ERROR: Function failed: do_make_scripts (log file is located at
> /home/pokybuild/yocto-autobuilder/yocto-worker/nightly-qa-skeleton/build/
> build/tmp/work/qemux86_64-poky-linux/hello-mod/0.1-r0/
> temp/log.do_make_scripts.14071)
> NOTE: recipe hello-mod-0.1-r0: task do_make_scripts: Failed
>
> The autobuilder builds hello-mod from meta-skeleton, which inherits module
> and sets S=${WORKDIR}.
>
> Ross
>



-- 
"Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end"

[-- Attachment #2: Type: text/html, Size: 3891 bytes --]

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Paul Barker]

On Wed, 9 Nov 2016 08:09:09 -0500
Bruce Ashfield <bruce.ashfield@gmail.com> wrote:

> On Wed, Nov 9, 2016 at 5:04 AM, Burton, Ross <ross.burton@intel.com> wrote:
> 
> >
> > On 9 November 2016 at 02:23, Bruce Ashfield <bruce.ashfield@gmail.com>
> > wrote:
> >
> >> I can ack this patch, since no defaults change .. there's no risk to
> >> existing users.
> >>
> >
> > Saying that of course doomed the patch:
> >
> 
> interesting.
> 
> Paul: obviously you were building with this in place, and my local test did
> work here
> as well .. so any idea to the difference ?
> 
> Let me know if you want any help looking into it.
> 
> Cheers,
> 
> Bruce
> 

The reason for this patch is that I'm using a tarball for the kernel
sources in the new meta-arduino layer. That tarball doesn't get
extracted to ${STAGING_KERNEL_DIR} and so it needs to be copied over to
there. Looking at kernel.bbclass, it looks like the logic to do this is
present:


    # Old style kernels may set ${S} = ${WORKDIR}/git for example
    # We need to move these over to STAGING_KERNEL_DIR. We can't just
    # create the symlink in advance as the git fetcher can't cope with
    # the symlink.
    do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} ${STAGING_KERNEL_BUILDDIR}"
    do_clean[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} ${STAGING_KERNEL_BUILDDIR}"
    base_do_unpack_append () {
        s = d.getVar("S", True)
        if s[-1] == '/':
            # drop trailing slash, so that os.symlink(kernsrc, s) doesn't use s as directory name and fail
            s=s[:-1]
        kernsrc = d.getVar("STAGING_KERNEL_DIR", True)
        if s != kernsrc:
            bb.utils.mkdirhier(kernsrc)
            bb.utils.remove(kernsrc, recurse=True)
            if d.getVar("EXTERNALSRC", True):
                # With EXTERNALSRC S will not be wiped so we can symlink to it
                os.symlink(s, kernsrc)
            else:
                import shutil
                shutil.move(s, kernsrc)
                os.symlink(kernsrc, s)
    }

The problem is we can't set S to anything unless we can override it.

I've now looked into this futher and it seems to be working for some
kernel recipes but not others. The issue may be that we have an
assignment for S in bitbake.conf:

    S = "${WORKDIR}/${BP}"

That's obviously overriding the 'S ?= ...' assignment when my patch is
applied where S isn't also explicitly assigned in the recipe, some
other class or include file.

I'm not sure on how precedence is determined if a variable is assigned
using '=' multiple times.

I'll have another look into it and see where I get. I have a workaround
in place for the meta-arduino layer for now so it doesn't need an
urgent fix.

However, if it turns out we can't reliably override S in a kernel recipe
then should we still have that comment and base_do_unpack_append() in
kernel.bbclass?

Thanks,
Paul

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Andre McCurdy]

On Wed, Nov 9, 2016 at 10:08 AM, Paul Barker <paul@paulbarker.me.uk> wrote:
> On Wed, 9 Nov 2016 08:09:09 -0500
> Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
>
>> On Wed, Nov 9, 2016 at 5:04 AM, Burton, Ross <ross.burton@intel.com> wrote:
>>
>> >
>> > On 9 November 2016 at 02:23, Bruce Ashfield <bruce.ashfield@gmail.com>
>> > wrote:
>> >
>> >> I can ack this patch, since no defaults change .. there's no risk to
>> >> existing users.
>> >>
>> >
>> > Saying that of course doomed the patch:
>> >
>>
>> interesting.
>>
>> Paul: obviously you were building with this in place, and my local test did
>> work here
>> as well .. so any idea to the difference ?
>>
>> Let me know if you want any help looking into it.
>>
>> Cheers,
>>
>> Bruce
>>
>
> The reason for this patch is that I'm using a tarball for the kernel
> sources in the new meta-arduino layer. That tarball doesn't get
> extracted to ${STAGING_KERNEL_DIR} and so it needs to be copied over to
> there. Looking at kernel.bbclass, it looks like the logic to do this is
> present:
>
>
>     # Old style kernels may set ${S} = ${WORKDIR}/git for example
>     # We need to move these over to STAGING_KERNEL_DIR. We can't just
>     # create the symlink in advance as the git fetcher can't cope with
>     # the symlink.
>     do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} ${STAGING_KERNEL_BUILDDIR}"
>     do_clean[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} ${STAGING_KERNEL_BUILDDIR}"
>     base_do_unpack_append () {
>         s = d.getVar("S", True)
>         if s[-1] == '/':
>             # drop trailing slash, so that os.symlink(kernsrc, s) doesn't use s as directory name and fail
>             s=s[:-1]
>         kernsrc = d.getVar("STAGING_KERNEL_DIR", True)
>         if s != kernsrc:
>             bb.utils.mkdirhier(kernsrc)
>             bb.utils.remove(kernsrc, recurse=True)
>             if d.getVar("EXTERNALSRC", True):
>                 # With EXTERNALSRC S will not be wiped so we can symlink to it
>                 os.symlink(s, kernsrc)
>             else:
>                 import shutil
>                 shutil.move(s, kernsrc)
>                 os.symlink(kernsrc, s)
>     }
>
> The problem is we can't set S to anything unless we can override it.
>
> I've now looked into this futher and it seems to be working for some
> kernel recipes but not others. The issue may be that we have an
> assignment for S in bitbake.conf:
>
>     S = "${WORKDIR}/${BP}"
>
> That's obviously overriding the 'S ?= ...' assignment when my patch is
> applied where S isn't also explicitly assigned in the recipe, some
> other class or include file.
>
> I'm not sure on how precedence is determined if a variable is assigned
> using '=' multiple times.
>
> I'll have another look into it and see where I get. I have a workaround
> in place for the meta-arduino layer for now so it doesn't need an
> urgent fix.
>
> However, if it turns out we can't reliably override S in a kernel recipe
> then should we still have that comment and base_do_unpack_append() in
> kernel.bbclass?

The solution I use (learned the hard way) is to ensure that "S = ..."
comes after "inherit kernel" in the kernel recipe.

> Thanks,
> Paul
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Paul Barker]

On Wed, 9 Nov 2016 15:42:29 -0800
Andre McCurdy <armccurdy@gmail.com> wrote:

> On Wed, Nov 9, 2016 at 10:08 AM, Paul Barker <paul@paulbarker.me.uk> wrote:
> >
> > The reason for this patch is that I'm using a tarball for the kernel
> > sources in the new meta-arduino layer. That tarball doesn't get
> > extracted to ${STAGING_KERNEL_DIR} and so it needs to be copied over to
> > there. Looking at kernel.bbclass, it looks like the logic to do this is
> > present:
> >
> >
> >     # Old style kernels may set ${S} = ${WORKDIR}/git for example
> >     # We need to move these over to STAGING_KERNEL_DIR. We can't just
> >     # create the symlink in advance as the git fetcher can't cope with
> >     # the symlink.
> >     do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} ${STAGING_KERNEL_BUILDDIR}"
> >     do_clean[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} ${STAGING_KERNEL_BUILDDIR}"
> >     base_do_unpack_append () {
> >         s = d.getVar("S", True)
> >         if s[-1] == '/':
> >             # drop trailing slash, so that os.symlink(kernsrc, s) doesn't use s as directory name and fail
> >             s=s[:-1]
> >         kernsrc = d.getVar("STAGING_KERNEL_DIR", True)
> >         if s != kernsrc:
> >             bb.utils.mkdirhier(kernsrc)
> >             bb.utils.remove(kernsrc, recurse=True)
> >             if d.getVar("EXTERNALSRC", True):
> >                 # With EXTERNALSRC S will not be wiped so we can symlink to it
> >                 os.symlink(s, kernsrc)
> >             else:
> >                 import shutil
> >                 shutil.move(s, kernsrc)
> >                 os.symlink(kernsrc, s)
> >     }
> >
> > The problem is we can't set S to anything unless we can override it.
> >
> > I've now looked into this futher and it seems to be working for some
> > kernel recipes but not others. The issue may be that we have an
> > assignment for S in bitbake.conf:
> >
> >     S = "${WORKDIR}/${BP}"
> >
> > That's obviously overriding the 'S ?= ...' assignment when my patch is
> > applied where S isn't also explicitly assigned in the recipe, some
> > other class or include file.
> >
> > I'm not sure on how precedence is determined if a variable is assigned
> > using '=' multiple times.
> >
> > I'll have another look into it and see where I get. I have a workaround
> > in place for the meta-arduino layer for now so it doesn't need an
> > urgent fix.
> >
> > However, if it turns out we can't reliably override S in a kernel recipe
> > then should we still have that comment and base_do_unpack_append() in
> > kernel.bbclass?
> 
> The solution I use (learned the hard way) is to ensure that "S = ..."
> comes after "inherit kernel" in the kernel recipe.
> 

Ok, yea, this works for me.

Is this expected? S is set to ${STAGING_KERNEL_DIR} at the start of
kernel.bbclass. The base_do_unpack_append() function is defined later
in kernel.bbclass. If S is set to ${WORKDIR}/${BP} only after
kernel.bbclass is inherited, then that's after base_do_unpack_append()
has been parsed.

I've re-read the bitbake user manual [1] but I can't clearly understand
why this works. Is it to do with when the _append is applied?

Is there any way we can make this more obvious or is this just the
nature of bitbake?

[1]: http://www.yoctoproject.org/docs/2.2/bitbake-user-manual/bitbake-user-manual.html#basic-syntax

Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden

[Patrick Ohly]

On Fri, 2016-11-11 at 19:39 +0000, Paul Barker wrote:
> On Wed, 9 Nov 2016 15:42:29 -0800
> Andre McCurdy <armccurdy@gmail.com> wrote:
> 
> > The solution I use (learned the hard way) is to ensure that "S = ..."
> > comes after "inherit kernel" in the kernel recipe.
> > 
> 
> Ok, yea, this works for me.
> 
> Is this expected? S is set to ${STAGING_KERNEL_DIR} at the start of
> kernel.bbclass. The base_do_unpack_append() function is defined later
> in kernel.bbclass. If S is set to ${WORKDIR}/${BP} only after
> kernel.bbclass is inherited, then that's after base_do_unpack_append()
> has been parsed.

Parsing itself does not expand variable references (*), so it is
possible to use a variable that gets changed later on and use the value
that the variable eventually ends up having.

(*) There's one exception - the := assignment expands based on the
currently defined variables at the time of parsing the assignment.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.