[PATCH 0/1] kernelsrc.bbclass: Move virtual/kernel:do_shared_workdir to do_configure

[PATCH 0/1] kernelsrc.bbclass: Move virtual/kernel:do_shared_workdir to do_configure

[liezhi.yang]

From: Robert Yang <liezhi.yang@windriver.com>

The following changes since commit 20a64bc696033b4d67294058b6c4a2e8f29f3aaa:

  gsettings-desktop-schemas: update 46.1 -> 47.1 (2024-09-20 15:56:27 +0100)

are available in the Git repository at:

  https://github.com/robertlinux/yocto rbt/kernel
  https://github.com/robertlinux/yocto/tree/rbt/kernel

Robert Yang (1):
  kernelsrc.bbclass: Move virtual/kernel:do_shared_workdir to
    do_configure

 meta/classes-recipe/kernelsrc.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.25.1

[PATCH 1/1] kernelsrc.bbclass: Move virtual/kernel:do_shared_workdir to do_configure

[liezhi.yang]

From: Robert Yang <liezhi.yang@windriver.com>

The do_shared_workdir populates build artifacts to
work-shared/${MACHINE}/kernel-build-artifacts, which isn't useful for do_patch,
make do_patch depend on it will make a lot of recipes compiling which makes
world's do_patch very slow, e.g.:

Download sources locally:
$ bitbake world --runall=fetch

* Before the patch:
$ rm -fr sstate-cache/ tmp/; time bitbake world --runall=patch
real    23m31.739s
user    0m25.086s
sys     0m4.630s

* Now
$ rm -fr sstate-cache/ tmp/; time bitbake world --runall=patch
real    12m25.650s
user    0m35.641s
sys     0m5.699s

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 meta/classes-recipe/kernelsrc.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes-recipe/kernelsrc.bbclass b/meta/classes-recipe/kernelsrc.bbclass
index ecb02dc9edc..f23a1e6bff2 100644
--- a/meta/classes-recipe/kernelsrc.bbclass
+++ b/meta/classes-recipe/kernelsrc.bbclass
@@ -7,8 +7,8 @@
 S = "${STAGING_KERNEL_DIR}"
 deltask do_fetch
 deltask do_unpack
-do_patch[depends] += "virtual/kernel:do_shared_workdir"
 do_patch[noexec] = "1"
+do_configure[depends] += "virtual/kernel:do_shared_workdir"
 do_package[depends] += "virtual/kernel:do_populate_sysroot"
 KERNEL_VERSION = "${@get_kernelversion_file("${STAGING_KERNEL_BUILDDIR}")}"
 LOCAL_VERSION = "${@get_kernellocalversion_file("${STAGING_KERNEL_BUILDDIR}")}"
-- 
2.25.1

Re: [OE-core] [PATCH 1/1] kernelsrc.bbclass: Move virtual/kernel:do_shared_workdir to do_configure

[Bruce Ashfield]

On Mon, Sep 23, 2024 at 1:34 AM Robert Yang via lists.openembedded.org
<liezhi.yang=windriver.com@lists.openembedded.org> wrote:
>
> From: Robert Yang <liezhi.yang@windriver.com>
>
> The do_shared_workdir populates build artifacts to
> work-shared/${MACHINE}/kernel-build-artifacts, which isn't useful for do_patch,
> make do_patch depend on it will make a lot of recipes compiling which makes
> world's do_patch very slow, e.g.:
>
> Download sources locally:
> $ bitbake world --runall=fetch
>
> * Before the patch:
> $ rm -fr sstate-cache/ tmp/; time bitbake world --runall=patch
> real    23m31.739s
> user    0m25.086s
> sys     0m4.630s
>
> * Now
> $ rm -fr sstate-cache/ tmp/; time bitbake world --runall=patch
> real    12m25.650s
> user    0m35.641s
> sys     0m5.699s

The motivation for why this is a priority should be included in
the commit message. It helps us decide if any issues this may
trigger are worth the risk.

I assume you are doing some sort of source archiving, or
licensing and all you care about is the patched source
for this, and not any potentially generated files from the
build(s) ?

See below for why I ask.

>
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  meta/classes-recipe/kernelsrc.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/classes-recipe/kernelsrc.bbclass b/meta/classes-recipe/kernelsrc.bbclass
> index ecb02dc9edc..f23a1e6bff2 100644
> --- a/meta/classes-recipe/kernelsrc.bbclass
> +++ b/meta/classes-recipe/kernelsrc.bbclass
> @@ -7,8 +7,8 @@
>  S = "${STAGING_KERNEL_DIR}"
>  deltask do_fetch
>  deltask do_unpack
> -do_patch[depends] += "virtual/kernel:do_shared_workdir"
>  do_patch[noexec] = "1"
> +do_configure[depends] += "virtual/kernel:do_shared_workdir"

From what I can see (bitbake expansion and dependency experts
can feel free to correct the below):

There's already an issue with the KERNEL_VERSION / LOCAL_VERSION
variables, in that they depend on STAGING_KERNEL_BUILDDIR, which
is populated from the do_shared_workdir. We are moving that population
later / to a different task with this patch, so those variables are now going to
be defined at a different point in the build. There's potential risk in that.

Looking at the routines get_kernel* that the variables are using, they
gracefully return None if the file doesn't exist, so the question is .. would
an impacted user of the variables notice a potential issue ?

One wonders if they shouldn't just propagate the exception to the caller
so it could be dealt with, and harder to ignore.

So while it does speed up do_patch, there is a behaviour change in
when the variables are going to be valid/defined.

Bruce

>  do_package[depends] += "virtual/kernel:do_populate_sysroot"
>  KERNEL_VERSION = "${@get_kernelversion_file("${STAGING_KERNEL_BUILDDIR}")}"
>  LOCAL_VERSION = "${@get_kernellocalversion_file("${STAGING_KERNEL_BUILDDIR}")}"
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#204794): https://lists.openembedded.org/g/openembedded-core/message/204794
> Mute This Topic: https://lists.openembedded.org/mt/108603063/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@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

Re: [OE-core] [PATCH 1/1] kernelsrc.bbclass: Move virtual/kernel:do_shared_workdir to do_configure

[Robert Yang]

Hi Bruce,

Sorry for the late reply, I just came back from holiday,

On 10/1/24 22:42, Bruce Ashfield wrote:
> On Mon, Sep 23, 2024 at 1:34 AM Robert Yang via lists.openembedded.org
> <liezhi.yang=windriver.com@lists.openembedded.org> wrote:
>>
>> From: Robert Yang <liezhi.yang@windriver.com>
>>
>> The do_shared_workdir populates build artifacts to
>> work-shared/${MACHINE}/kernel-build-artifacts, which isn't useful for do_patch,
>> make do_patch depend on it will make a lot of recipes compiling which makes
>> world's do_patch very slow, e.g.:
>>
>> Download sources locally:
>> $ bitbake world --runall=fetch
>>
>> * Before the patch:
>> $ rm -fr sstate-cache/ tmp/; time bitbake world --runall=patch
>> real    23m31.739s
>> user    0m25.086s
>> sys     0m4.630s
>>
>> * Now
>> $ rm -fr sstate-cache/ tmp/; time bitbake world --runall=patch
>> real    12m25.650s
>> user    0m35.641s
>> sys     0m5.699s
> 
> The motivation for why this is a priority should be included in
> the commit message. It helps us decide if any issues this may
> trigger are worth the risk.
> 
> I assume you are doing some sort of source archiving, or
> licensing and all you care about is the patched source
> for this, and not any potentially generated files from the
> build(s) ?

Yes, I'm trying to collect the patched sources for ip scanning, and there are a
lot of do_compile tasks are running when run --runall=patch which looks strange.

> 
> See below for why I ask.
> 
>>
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>> ---
>>   meta/classes-recipe/kernelsrc.bbclass | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/meta/classes-recipe/kernelsrc.bbclass b/meta/classes-recipe/kernelsrc.bbclass
>> index ecb02dc9edc..f23a1e6bff2 100644
>> --- a/meta/classes-recipe/kernelsrc.bbclass
>> +++ b/meta/classes-recipe/kernelsrc.bbclass
>> @@ -7,8 +7,8 @@
>>   S = "${STAGING_KERNEL_DIR}"
>>   deltask do_fetch
>>   deltask do_unpack
>> -do_patch[depends] += "virtual/kernel:do_shared_workdir"
>>   do_patch[noexec] = "1"
>> +do_configure[depends] += "virtual/kernel:do_shared_workdir"
> 
>>From what I can see (bitbake expansion and dependency experts
> can feel free to correct the below):
> 
> There's already an issue with the KERNEL_VERSION / LOCAL_VERSION
> variables, in that they depend on STAGING_KERNEL_BUILDDIR, which
> is populated from the do_shared_workdir. We are moving that population
> later / to a different task with this patch, so those variables are now going to
> be defined at a different point in the build. There's potential risk in that.
> 
> Looking at the routines get_kernel* that the variables are using, they
> gracefully return None if the file doesn't exist, so the question is .. would
> an impacted user of the variables notice a potential issue ?

Maybe we should make get_kernel_* report errors rather than None?

// Robert

> 
> One wonders if they shouldn't just propagate the exception to the caller
> so it could be dealt with, and harder to ignore.
> 
> So while it does speed up do_patch, there is a behaviour change in
> when the variables are going to be valid/defined.
> 
> Bruce
> 
>>   do_package[depends] += "virtual/kernel:do_populate_sysroot"
>>   KERNEL_VERSION = "${@get_kernelversion_file("${STAGING_KERNEL_BUILDDIR}")}"
>>   LOCAL_VERSION = "${@get_kernellocalversion_file("${STAGING_KERNEL_BUILDDIR}")}"
>> --
>> 2.25.1
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#204794): https://lists.openembedded.org/g/openembedded-core/message/204794
>> Mute This Topic: https://lists.openembedded.org/mt/108603063/1050810
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
> 
> 

Re: [OE-core] [PATCH 1/1] kernelsrc.bbclass: Move virtual/kernel:do_shared_workdir to do_configure

[Bruce Ashfield]

On Tue, Oct 8, 2024 at 11:18 AM Robert Yang <liezhi.yang@windriver.com> wrote:
>
> Hi Bruce,
>
> Sorry for the late reply, I just came back from holiday,
>
> On 10/1/24 22:42, Bruce Ashfield wrote:
> > On Mon, Sep 23, 2024 at 1:34 AM Robert Yang via lists.openembedded.org
> > <liezhi.yang=windriver.com@lists.openembedded.org> wrote:
> >>
> >> From: Robert Yang <liezhi.yang@windriver.com>
> >>
> >> The do_shared_workdir populates build artifacts to
> >> work-shared/${MACHINE}/kernel-build-artifacts, which isn't useful for do_patch,
> >> make do_patch depend on it will make a lot of recipes compiling which makes
> >> world's do_patch very slow, e.g.:
> >>
> >> Download sources locally:
> >> $ bitbake world --runall=fetch
> >>
> >> * Before the patch:
> >> $ rm -fr sstate-cache/ tmp/; time bitbake world --runall=patch
> >> real    23m31.739s
> >> user    0m25.086s
> >> sys     0m4.630s
> >>
> >> * Now
> >> $ rm -fr sstate-cache/ tmp/; time bitbake world --runall=patch
> >> real    12m25.650s
> >> user    0m35.641s
> >> sys     0m5.699s
> >
> > The motivation for why this is a priority should be included in
> > the commit message. It helps us decide if any issues this may
> > trigger are worth the risk.
> >
> > I assume you are doing some sort of source archiving, or
> > licensing and all you care about is the patched source
> > for this, and not any potentially generated files from the
> > build(s) ?
>
> Yes, I'm trying to collect the patched sources for ip scanning, and there are a
> lot of do_compile tasks are running when run --runall=patch which looks strange.
>
> >
> > See below for why I ask.
> >
> >>
> >> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> >> ---
> >>   meta/classes-recipe/kernelsrc.bbclass | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/meta/classes-recipe/kernelsrc.bbclass b/meta/classes-recipe/kernelsrc.bbclass
> >> index ecb02dc9edc..f23a1e6bff2 100644
> >> --- a/meta/classes-recipe/kernelsrc.bbclass
> >> +++ b/meta/classes-recipe/kernelsrc.bbclass
> >> @@ -7,8 +7,8 @@
> >>   S = "${STAGING_KERNEL_DIR}"
> >>   deltask do_fetch
> >>   deltask do_unpack
> >> -do_patch[depends] += "virtual/kernel:do_shared_workdir"
> >>   do_patch[noexec] = "1"
> >> +do_configure[depends] += "virtual/kernel:do_shared_workdir"
> >
> >>From what I can see (bitbake expansion and dependency experts
> > can feel free to correct the below):
> >
> > There's already an issue with the KERNEL_VERSION / LOCAL_VERSION
> > variables, in that they depend on STAGING_KERNEL_BUILDDIR, which
> > is populated from the do_shared_workdir. We are moving that population
> > later / to a different task with this patch, so those variables are now going to
> > be defined at a different point in the build. There's potential risk in that.
> >
> > Looking at the routines get_kernel* that the variables are using, they
> > gracefully return None if the file doesn't exist, so the question is .. would
> > an impacted user of the variables notice a potential issue ?
>
> Maybe we should make get_kernel_* report errors rather than None?

That was the idea that Ross and I had when I was looking through
the series.

It would definitely let us know if this is an issue, and flag it in the
future if it becomes an issue.

Bruce

>
> // Robert
>
> >
> > One wonders if they shouldn't just propagate the exception to the caller
> > so it could be dealt with, and harder to ignore.
> >
> > So while it does speed up do_patch, there is a behaviour change in
> > when the variables are going to be valid/defined.
> >
> > Bruce
> >
> >>   do_package[depends] += "virtual/kernel:do_populate_sysroot"
> >>   KERNEL_VERSION = "${@get_kernelversion_file("${STAGING_KERNEL_BUILDDIR}")}"
> >>   LOCAL_VERSION = "${@get_kernellocalversion_file("${STAGING_KERNEL_BUILDDIR}")}"
> >> --
> >> 2.25.1
> >>
> >>
> >> -=-=-=-=-=-=-=-=-=-=-=-
> >> Links: You receive all messages sent to this group.
> >> View/Reply Online (#204794): https://lists.openembedded.org/g/openembedded-core/message/204794
> >> Mute This Topic: https://lists.openembedded.org/mt/108603063/1050810
> >> Group Owner: openembedded-core+owner@lists.openembedded.org
> >> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@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