All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling
@ 2018-01-20 23:44 Enrico Jorns
  2018-01-31  9:49 ` Enrico Joerns
  2018-04-03 22:20 ` [PATCH] " Richard Purdie
  0 siblings, 2 replies; 7+ messages in thread
From: Enrico Jorns @ 2018-01-20 23:44 UTC (permalink / raw)
  To: openembedded-core; +Cc: uol, Enrico Jorns

As introduced by a56fb90dc3805494eeaf04c60538425e8d52efc5 ('base.bbclass
wipe ${S} before unpacking source') the base.bbclass uses a python
anonymous function to set the 'do_unpack' varflag 'cleandirs' to either
'${S}' or '${S}/patches' depending on equality of '${S}' and '${WORKDIR}'.

Not that this only differs from the way almost all other recipes set or
modify a tasks 'cleandirs' flag, it also has a significant impact on the
kernel.bbclass (and possibly further ones) and causes incorrect
behavior for rebuilds triggered by source modification, e.g. by a change
of the defconfig file for a kernel build.

The kernel.bbclass tries to extend do_unpack[cleandirs]:

| do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} ${STAGING_KERNEL_BUILDDIR}"

As python anonymous functions are evaluated at the very end of recipe
parsing, the d.setVarFlag('do_unpack', 'cleandirs', '${S}') statement in
base.bbclass will overwrite every modification to cleandirs that is done
as shown for the kernel class above.

As a result of this, a change to a kernels 'defconfig' will lead to an
updated defconfig file in ${WORKDIR}, but as ${B} never gets cleaned and
${B}/.config still exists, it will not be copied to ${B}/.config and
thus not find its way in the build kernel.

This is a severe issue for the kernel development and build process!

This patch changes setting of the cleandirs varflag in base.bbclass to
a simple variable assignment as almost all other recipes do it. This now
again allows overwriting or appending the varflag with common methods
such as done in kernel.bbclass.

This issue affects morty, pyro, rocko and master.

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
---
 meta/classes/base.bbclass | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 912e81e002..2949b074d8 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -152,12 +152,8 @@ python base_do_fetch() {
 addtask unpack after do_fetch
 do_unpack[dirs] = "${WORKDIR}"
 
-python () {
-    if d.getVar('S') != d.getVar('WORKDIR'):
-        d.setVarFlag('do_unpack', 'cleandirs', '${S}')
-    else:
-        d.setVarFlag('do_unpack', 'cleandirs', os.path.join('${S}', 'patches'))
-}
+do_unpack[cleandirs] = "${@d.getVar('S') if d.getVar('S') != d.getVar('WORKDIR') else os.path.join('${S}', 'patches')}"
+
 python base_do_unpack() {
     src_uri = (d.getVar('SRC_URI') or "").split()
     if len(src_uri) == 0:
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling
  2018-01-20 23:44 [PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling Enrico Jorns
@ 2018-01-31  9:49 ` Enrico Joerns
  2018-03-27 10:40   ` [pyro][rocko][PATCH] " Enrico Joerns
  2018-04-03 22:20 ` [PATCH] " Richard Purdie
  1 sibling, 1 reply; 7+ messages in thread
From: Enrico Joerns @ 2018-01-31  9:49 UTC (permalink / raw)
  To: openembedded-core

Hey,

any comments on that? Not using this patch currently disturbs our 
companies Yocto kernel workflow. Thus I wonder if no one else have had 
issues when building kernels from fully custom 'defconfig' files, too.

Regards, Enrico

On 01/21/2018 12:44 AM, Enrico Jorns wrote:
> As introduced by a56fb90dc3805494eeaf04c60538425e8d52efc5 ('base.bbclass
> wipe ${S} before unpacking source') the base.bbclass uses a python
> anonymous function to set the 'do_unpack' varflag 'cleandirs' to either
> '${S}' or '${S}/patches' depending on equality of '${S}' and '${WORKDIR}'.
> 
> Not that this only differs from the way almost all other recipes set or
> modify a tasks 'cleandirs' flag, it also has a significant impact on the
> kernel.bbclass (and possibly further ones) and causes incorrect
> behavior for rebuilds triggered by source modification, e.g. by a change
> of the defconfig file for a kernel build.
> 
> The kernel.bbclass tries to extend do_unpack[cleandirs]:
> 
> | do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} ${STAGING_KERNEL_BUILDDIR}"
> 
> As python anonymous functions are evaluated at the very end of recipe
> parsing, the d.setVarFlag('do_unpack', 'cleandirs', '${S}') statement in
> base.bbclass will overwrite every modification to cleandirs that is done
> as shown for the kernel class above.
> 
> As a result of this, a change to a kernels 'defconfig' will lead to an
> updated defconfig file in ${WORKDIR}, but as ${B} never gets cleaned and
> ${B}/.config still exists, it will not be copied to ${B}/.config and
> thus not find its way in the build kernel.
> 
> This is a severe issue for the kernel development and build process!
> 
> This patch changes setting of the cleandirs varflag in base.bbclass to
> a simple variable assignment as almost all other recipes do it. This now
> again allows overwriting or appending the varflag with common methods
> such as done in kernel.bbclass.
> 
> This issue affects morty, pyro, rocko and master.
> 
> Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
> ---
>   meta/classes/base.bbclass | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
> index 912e81e002..2949b074d8 100644
> --- a/meta/classes/base.bbclass
> +++ b/meta/classes/base.bbclass
> @@ -152,12 +152,8 @@ python base_do_fetch() {
>   addtask unpack after do_fetch
>   do_unpack[dirs] = "${WORKDIR}"
>   
> -python () {
> -    if d.getVar('S') != d.getVar('WORKDIR'):
> -        d.setVarFlag('do_unpack', 'cleandirs', '${S}')
> -    else:
> -        d.setVarFlag('do_unpack', 'cleandirs', os.path.join('${S}', 'patches'))
> -}
> +do_unpack[cleandirs] = "${@d.getVar('S') if d.getVar('S') != d.getVar('WORKDIR') else os.path.join('${S}', 'patches')}"
> +
>   python base_do_unpack() {
>       src_uri = (d.getVar('SRC_URI') or "").split()
>       if len(src_uri) == 0:
> 

-- 
Pengutronix e.K.                           | Enrico Jörns                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5080 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pyro][rocko][PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling
  2018-01-31  9:49 ` Enrico Joerns
@ 2018-03-27 10:40   ` Enrico Joerns
  2018-03-27 14:27     ` akuster808
  0 siblings, 1 reply; 7+ messages in thread
From: Enrico Joerns @ 2018-03-27 10:40 UTC (permalink / raw)
  To: openembedded-core; +Cc: Armin Kuster

Ping!

If there should be severe changes in the recent base / kernel class code 
that might make this useless, this is at least a candidate for backporting!

/Enrico

On 01/31/2018 10:49 AM, Enrico Joerns wrote:
> Hey,
> 
> any comments on that? Not using this patch currently disturbs our 
> companies Yocto kernel workflow. Thus I wonder if no one else have had 
> issues when building kernels from fully custom 'defconfig' files, too.
> 
> Regards, Enrico
> 
> On 01/21/2018 12:44 AM, Enrico Jorns wrote:
>> As introduced by a56fb90dc3805494eeaf04c60538425e8d52efc5 ('base.bbclass
>> wipe ${S} before unpacking source') the base.bbclass uses a python
>> anonymous function to set the 'do_unpack' varflag 'cleandirs' to either
>> '${S}' or '${S}/patches' depending on equality of '${S}' and 
>> '${WORKDIR}'.
>>
>> Not that this only differs from the way almost all other recipes set or
>> modify a tasks 'cleandirs' flag, it also has a significant impact on the
>> kernel.bbclass (and possibly further ones) and causes incorrect
>> behavior for rebuilds triggered by source modification, e.g. by a change
>> of the defconfig file for a kernel build.
>>
>> The kernel.bbclass tries to extend do_unpack[cleandirs]:
>>
>> | do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} 
>> ${STAGING_KERNEL_BUILDDIR}"
>>
>> As python anonymous functions are evaluated at the very end of recipe
>> parsing, the d.setVarFlag('do_unpack', 'cleandirs', '${S}') statement in
>> base.bbclass will overwrite every modification to cleandirs that is done
>> as shown for the kernel class above.
>>
>> As a result of this, a change to a kernels 'defconfig' will lead to an
>> updated defconfig file in ${WORKDIR}, but as ${B} never gets cleaned and
>> ${B}/.config still exists, it will not be copied to ${B}/.config and
>> thus not find its way in the build kernel.
>>
>> This is a severe issue for the kernel development and build process!
>>
>> This patch changes setting of the cleandirs varflag in base.bbclass to
>> a simple variable assignment as almost all other recipes do it. This now
>> again allows overwriting or appending the varflag with common methods
>> such as done in kernel.bbclass.
>>
>> This issue affects morty, pyro, rocko and master.
>>
>> Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
>> ---
>>   meta/classes/base.bbclass | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
>> index 912e81e002..2949b074d8 100644
>> --- a/meta/classes/base.bbclass
>> +++ b/meta/classes/base.bbclass
>> @@ -152,12 +152,8 @@ python base_do_fetch() {
>>   addtask unpack after do_fetch
>>   do_unpack[dirs] = "${WORKDIR}"
>> -python () {
>> -    if d.getVar('S') != d.getVar('WORKDIR'):
>> -        d.setVarFlag('do_unpack', 'cleandirs', '${S}')
>> -    else:
>> -        d.setVarFlag('do_unpack', 'cleandirs', os.path.join('${S}', 
>> 'patches'))
>> -}
>> +do_unpack[cleandirs] = "${@d.getVar('S') if d.getVar('S') != 
>> d.getVar('WORKDIR') else os.path.join('${S}', 'patches')}"
>> +
>>   python base_do_unpack() {
>>       src_uri = (d.getVar('SRC_URI') or "").split()
>>       if len(src_uri) == 0:
>>
> 

-- 
Pengutronix e.K.                           | Enrico Jörns                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5080 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pyro][rocko][PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling
  2018-03-27 10:40   ` [pyro][rocko][PATCH] " Enrico Joerns
@ 2018-03-27 14:27     ` akuster808
  2018-03-28 12:31       ` Enrico Joerns
  0 siblings, 1 reply; 7+ messages in thread
From: akuster808 @ 2018-03-27 14:27 UTC (permalink / raw)
  To: Enrico Joerns, openembedded-core; +Cc: Armin Kuster



On 03/27/2018 03:40 AM, Enrico Joerns wrote:
> Ping!
>
> If there should be severe changes in the recent base / kernel class
> code that might make this useless, this is at least a candidate for
> backporting!

This has to be in master first before  I can back port. Since you have
the setup that shows the problem, maybe you could double check on master
to see if it is also affected.

regards,
Armin
>
> /Enrico
>
> On 01/31/2018 10:49 AM, Enrico Joerns wrote:
>> Hey,
>>
>> any comments on that? Not using this patch currently disturbs our
>> companies Yocto kernel workflow. Thus I wonder if no one else have
>> had issues when building kernels from fully custom 'defconfig' files,
>> too.
>>
>> Regards, Enrico
>>
>> On 01/21/2018 12:44 AM, Enrico Jorns wrote:
>>> As introduced by a56fb90dc3805494eeaf04c60538425e8d52efc5
>>> ('base.bbclass
>>> wipe ${S} before unpacking source') the base.bbclass uses a python
>>> anonymous function to set the 'do_unpack' varflag 'cleandirs' to either
>>> '${S}' or '${S}/patches' depending on equality of '${S}' and
>>> '${WORKDIR}'.
>>>
>>> Not that this only differs from the way almost all other recipes set or
>>> modify a tasks 'cleandirs' flag, it also has a significant impact on
>>> the
>>> kernel.bbclass (and possibly further ones) and causes incorrect
>>> behavior for rebuilds triggered by source modification, e.g. by a
>>> change
>>> of the defconfig file for a kernel build.
>>>
>>> The kernel.bbclass tries to extend do_unpack[cleandirs]:
>>>
>>> | do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B}
>>> ${STAGING_KERNEL_BUILDDIR}"
>>>
>>> As python anonymous functions are evaluated at the very end of recipe
>>> parsing, the d.setVarFlag('do_unpack', 'cleandirs', '${S}')
>>> statement in
>>> base.bbclass will overwrite every modification to cleandirs that is
>>> done
>>> as shown for the kernel class above.
>>>
>>> As a result of this, a change to a kernels 'defconfig' will lead to an
>>> updated defconfig file in ${WORKDIR}, but as ${B} never gets cleaned
>>> and
>>> ${B}/.config still exists, it will not be copied to ${B}/.config and
>>> thus not find its way in the build kernel.
>>>
>>> This is a severe issue for the kernel development and build process!
>>>
>>> This patch changes setting of the cleandirs varflag in base.bbclass to
>>> a simple variable assignment as almost all other recipes do it. This
>>> now
>>> again allows overwriting or appending the varflag with common methods
>>> such as done in kernel.bbclass.
>>>
>>> This issue affects morty, pyro, rocko and master.
>>>
>>> Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
>>> ---
>>>   meta/classes/base.bbclass | 8 ++------
>>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
>>> index 912e81e002..2949b074d8 100644
>>> --- a/meta/classes/base.bbclass
>>> +++ b/meta/classes/base.bbclass
>>> @@ -152,12 +152,8 @@ python base_do_fetch() {
>>>   addtask unpack after do_fetch
>>>   do_unpack[dirs] = "${WORKDIR}"
>>> -python () {
>>> -    if d.getVar('S') != d.getVar('WORKDIR'):
>>> -        d.setVarFlag('do_unpack', 'cleandirs', '${S}')
>>> -    else:
>>> -        d.setVarFlag('do_unpack', 'cleandirs', os.path.join('${S}',
>>> 'patches'))
>>> -}
>>> +do_unpack[cleandirs] = "${@d.getVar('S') if d.getVar('S') !=
>>> d.getVar('WORKDIR') else os.path.join('${S}', 'patches')}"
>>> +
>>>   python base_do_unpack() {
>>>       src_uri = (d.getVar('SRC_URI') or "").split()
>>>       if len(src_uri) == 0:
>>>
>>
>



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pyro][rocko][PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling
  2018-03-27 14:27     ` akuster808
@ 2018-03-28 12:31       ` Enrico Joerns
  0 siblings, 0 replies; 7+ messages in thread
From: Enrico Joerns @ 2018-03-28 12:31 UTC (permalink / raw)
  To: akuster808; +Cc: Armin Kuster, openembedded-core

On 03/27/2018 04:27 PM, akuster808 wrote:
> On 03/27/2018 03:40 AM, Enrico Joerns wrote:
>> Ping!
>>
>> If there should be severe changes in the recent base / kernel class
>> code that might make this useless, this is at least a candidate for
>> backporting!
> 
> This has to be in master first before  I can back port. Since you have
> the setup that shows the problem, maybe you could double check on master
> to see if it is also affected.

The patch was created against master and is still valid for current 
master. Thus, of course master is also affected!

I makes every line containing a `do_unpack[cleandirs] += ...` in a 
recipe being silently ignored (such as in kernel.bbclass).

Regards, Enrico

>> On 01/31/2018 10:49 AM, Enrico Joerns wrote:
>>> Hey,
>>>
>>> any comments on that? Not using this patch currently disturbs our
>>> companies Yocto kernel workflow. Thus I wonder if no one else have
>>> had issues when building kernels from fully custom 'defconfig' files,
>>> too.
>>>
>>> Regards, Enrico
>>>
>>> On 01/21/2018 12:44 AM, Enrico Jorns wrote:
>>>> As introduced by a56fb90dc3805494eeaf04c60538425e8d52efc5
>>>> ('base.bbclass
>>>> wipe ${S} before unpacking source') the base.bbclass uses a python
>>>> anonymous function to set the 'do_unpack' varflag 'cleandirs' to either
>>>> '${S}' or '${S}/patches' depending on equality of '${S}' and
>>>> '${WORKDIR}'.
>>>>
>>>> Not that this only differs from the way almost all other recipes set or
>>>> modify a tasks 'cleandirs' flag, it also has a significant impact on
>>>> the
>>>> kernel.bbclass (and possibly further ones) and causes incorrect
>>>> behavior for rebuilds triggered by source modification, e.g. by a
>>>> change
>>>> of the defconfig file for a kernel build.
>>>>
>>>> The kernel.bbclass tries to extend do_unpack[cleandirs]:
>>>>
>>>> | do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B}
>>>> ${STAGING_KERNEL_BUILDDIR}"
>>>>
>>>> As python anonymous functions are evaluated at the very end of recipe
>>>> parsing, the d.setVarFlag('do_unpack', 'cleandirs', '${S}')
>>>> statement in
>>>> base.bbclass will overwrite every modification to cleandirs that is
>>>> done
>>>> as shown for the kernel class above.
>>>>
>>>> As a result of this, a change to a kernels 'defconfig' will lead to an
>>>> updated defconfig file in ${WORKDIR}, but as ${B} never gets cleaned
>>>> and
>>>> ${B}/.config still exists, it will not be copied to ${B}/.config and
>>>> thus not find its way in the build kernel.
>>>>
>>>> This is a severe issue for the kernel development and build process!
>>>>
>>>> This patch changes setting of the cleandirs varflag in base.bbclass to
>>>> a simple variable assignment as almost all other recipes do it. This
>>>> now
>>>> again allows overwriting or appending the varflag with common methods
>>>> such as done in kernel.bbclass.
>>>>
>>>> This issue affects morty, pyro, rocko and master.
>>>>
>>>> Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
>>>> ---
>>>>    meta/classes/base.bbclass | 8 ++------
>>>>    1 file changed, 2 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
>>>> index 912e81e002..2949b074d8 100644
>>>> --- a/meta/classes/base.bbclass
>>>> +++ b/meta/classes/base.bbclass
>>>> @@ -152,12 +152,8 @@ python base_do_fetch() {
>>>>    addtask unpack after do_fetch
>>>>    do_unpack[dirs] = "${WORKDIR}"
>>>> -python () {
>>>> -    if d.getVar('S') != d.getVar('WORKDIR'):
>>>> -        d.setVarFlag('do_unpack', 'cleandirs', '${S}')
>>>> -    else:
>>>> -        d.setVarFlag('do_unpack', 'cleandirs', os.path.join('${S}',
>>>> 'patches'))
>>>> -}
>>>> +do_unpack[cleandirs] = "${@d.getVar('S') if d.getVar('S') !=
>>>> d.getVar('WORKDIR') else os.path.join('${S}', 'patches')}"
>>>> +
>>>>    python base_do_unpack() {
>>>>        src_uri = (d.getVar('SRC_URI') or "").split()
>>>>        if len(src_uri) == 0:
>>>>
>>>
>>
> 
> 

-- 
Pengutronix e.K.                           | Enrico Jörns                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5080 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling
  2018-01-20 23:44 [PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling Enrico Jorns
  2018-01-31  9:49 ` Enrico Joerns
@ 2018-04-03 22:20 ` Richard Purdie
  2018-04-04  0:54   ` Paul Eggleton
  1 sibling, 1 reply; 7+ messages in thread
From: Richard Purdie @ 2018-04-03 22:20 UTC (permalink / raw)
  To: Enrico Jorns, openembedded-core; +Cc: uol, Eggleton, Paul

On Sun, 2018-01-21 at 00:44 +0100, Enrico Jorns wrote:
> As introduced by a56fb90dc3805494eeaf04c60538425e8d52efc5 ('base.bbclass
> wipe ${S} before unpacking source') the base.bbclass uses a python
> anonymous function to set the 'do_unpack' varflag 'cleandirs' to either
> '${S}' or '${S}/patches' depending on equality of '${S}' and '${WORKDIR}'.
> 
> Not that this only differs from the way almost all other recipes set or
> modify a tasks 'cleandirs' flag, it also has a significant impact on the
> kernel.bbclass (and possibly further ones) and causes incorrect
> behavior for rebuilds triggered by source modification, e.g. by a change
> of the defconfig file for a kernel build.
> 
> The kernel.bbclass tries to extend do_unpack[cleandirs]:
> 
> > 
> > do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} ${STAGING_KERNEL_BUILDDIR}"
> As python anonymous functions are evaluated at the very end of recipe
> parsing, the d.setVarFlag('do_unpack', 'cleandirs', '${S}') statement in
> base.bbclass will overwrite every modification to cleandirs that is done
> as shown for the kernel class above.
> 
> As a result of this, a change to a kernels 'defconfig' will lead to an
> updated defconfig file in ${WORKDIR}, but as ${B} never gets cleaned and
> ${B}/.config still exists, it will not be copied to ${B}/.config and
> thus not find its way in the build kernel.
> 
> This is a severe issue for the kernel development and build process!
> 
> This patch changes setting of the cleandirs varflag in base.bbclass to
> a simple variable assignment as almost all other recipes do it. This now
> again allows overwriting or appending the varflag with common methods
> such as done in kernel.bbclass.
> 
> This issue affects morty, pyro, rocko and master.
> 
> Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
> ---
>  meta/classes/base.bbclass | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
> index 912e81e002..2949b074d8 100644
> --- a/meta/classes/base.bbclass
> +++ b/meta/classes/base.bbclass
> @@ -152,12 +152,8 @@ python base_do_fetch() {
>  addtask unpack after do_fetch
>  do_unpack[dirs] = "${WORKDIR}"
>  
> -python () {
> -    if d.getVar('S') != d.getVar('WORKDIR'):
> -        d.setVarFlag('do_unpack', 'cleandirs', '${S}')
> -    else:
> -        d.setVarFlag('do_unpack', 'cleandirs', os.path.join('${S}', 'patches'))
> -}
> +do_unpack[cleandirs] = "${@d.getVar('S') if d.getVar('S') != d.getVar('WORKDIR') else os.path.join('${S}', 'patches')}"
> +
>  python base_do_unpack() {
>      src_uri = (d.getVar('SRC_URI') or "").split()
>      if len(src_uri) == 0:

I have tried to add this and it breaks oe-selftest devtool tests:

https://autobuilder.yocto.io/builders/nightly-oe-selftest/builds/964/steps/Running%20oe-selftest/logs/stdio

You can probably reproduce by running:

oe-selftest -r devtool.DevtoolTests

I have no idea why its doing that but it can't merge until we get to
the bottom of why this is happening. I suspect this is why we've never
merged the patch but we've never quite tracked down the patch causing
the regressions and reported back until now, sorry.

One alternative to you patch may be to use d.appendVarFlag in the
anonymous python. I've not tested if that causes any problem with
devtool.

Cheers,

Richard





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling
  2018-04-03 22:20 ` [PATCH] " Richard Purdie
@ 2018-04-04  0:54   ` Paul Eggleton
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Eggleton @ 2018-04-04  0:54 UTC (permalink / raw)
  To: Enrico Jorns; +Cc: uol, openembedded-core

On Wednesday, 4 April 2018 10:20:45 AM NZST Richard Purdie wrote:
> On Sun, 2018-01-21 at 00:44 +0100, Enrico Jorns wrote:
> > As introduced by a56fb90dc3805494eeaf04c60538425e8d52efc5 ('base.bbclass
> > wipe ${S} before unpacking source') the base.bbclass uses a python
> > anonymous function to set the 'do_unpack' varflag 'cleandirs' to either
> > '${S}' or '${S}/patches' depending on equality of '${S}' and '${WORKDIR}'.
> > 
> > Not that this only differs from the way almost all other recipes set or
> > modify a tasks 'cleandirs' flag, it also has a significant impact on the
> > kernel.bbclass (and possibly further ones) and causes incorrect
> > behavior for rebuilds triggered by source modification, e.g. by a change
> > of the defconfig file for a kernel build.
> > 
> > The kernel.bbclass tries to extend do_unpack[cleandirs]:
> > 
> > > 
> > > do_unpack[cleandirs] += " ${S} ${STAGING_KERNEL_DIR} ${B} $
{STAGING_KERNEL_BUILDDIR}"
> > As python anonymous functions are evaluated at the very end of recipe
> > parsing, the d.setVarFlag('do_unpack', 'cleandirs', '${S}') statement in
> > base.bbclass will overwrite every modification to cleandirs that is done
> > as shown for the kernel class above.
> > 
> > As a result of this, a change to a kernels 'defconfig' will lead to an
> > updated defconfig file in ${WORKDIR}, but as ${B} never gets cleaned and
> > ${B}/.config still exists, it will not be copied to ${B}/.config and
> > thus not find its way in the build kernel.
> > 
> > This is a severe issue for the kernel development and build process!
> > 
> > This patch changes setting of the cleandirs varflag in base.bbclass to
> > a simple variable assignment as almost all other recipes do it. This now
> > again allows overwriting or appending the varflag with common methods
> > such as done in kernel.bbclass.
> > 
> > This issue affects morty, pyro, rocko and master.
> > 
> > Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
> > ---
> >  meta/classes/base.bbclass | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
> > index 912e81e002..2949b074d8 100644
> > --- a/meta/classes/base.bbclass
> > +++ b/meta/classes/base.bbclass
> > @@ -152,12 +152,8 @@ python base_do_fetch() {
> >  addtask unpack after do_fetch
> >  do_unpack[dirs] = "${WORKDIR}"
> >  
> > -python () {
> > -    if d.getVar('S') != d.getVar('WORKDIR'):
> > -        d.setVarFlag('do_unpack', 'cleandirs', '${S}')
> > -    else:
> > -        d.setVarFlag('do_unpack', 'cleandirs', os.path.join('${S}', 
'patches'))
> > -}
> > +do_unpack[cleandirs] = "${@d.getVar('S') if d.getVar('S') != 
d.getVar('WORKDIR') else os.path.join('${S}', 'patches')}"
> > +
> >  python base_do_unpack() {
> >      src_uri = (d.getVar('SRC_URI') or "").split()
> >      if len(src_uri) == 0:
> 
> I have tried to add this and it breaks oe-selftest devtool tests:
> 
> https://autobuilder.yocto.io/builders/nightly-oe-selftest/builds/964/steps/
Running%20oe-selftest/logs/stdio
> 
> You can probably reproduce by running:
> 
> oe-selftest -r devtool.DevtoolTests
> 
> I have no idea why its doing that but it can't merge until we get to
> the bottom of why this is happening. I suspect this is why we've never
> merged the patch but we've never quite tracked down the patch causing
> the regressions and reported back until now, sorry.
> 
> One alternative to you patch may be to use d.appendVarFlag in the
> anonymous python. I've not tested if that causes any problem with
> devtool.

So the issue is that devtool uses the externalsrc class, and the code in that 
class tries to ensure that whatever cleandirs is set to doesn't result in the 
user's source tree being trashed. It does so in a manner that avoids 
prematurely expanding the value, but unfortunately it does not currently 
handle expressions such as the ones this patch is setting, and as a result the 
potentially user modified source tree does in fact get deleted, which needless 
to say is quite bad :(

I am testing a fix locally which we should apply regardless of whether or not 
this patch goes in, since it will improve the safety of people's source trees 
with externalsrc. However before I can send it I'm still dealing with another 
possibly unrelated (not yet fully investigated) issue that prevents the 
devtool.DevtoolTests.test_devtool_buildclean oe-selftest test from passing.

Cheers,
Paul

-- 
Paul Eggleton
Intel Open Source Technology Centre




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-04-04  0:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-20 23:44 [PATCH] base.bbclass: fix do_unpack[cleandirs] varflag handling Enrico Jorns
2018-01-31  9:49 ` Enrico Joerns
2018-03-27 10:40   ` [pyro][rocko][PATCH] " Enrico Joerns
2018-03-27 14:27     ` akuster808
2018-03-28 12:31       ` Enrico Joerns
2018-04-03 22:20 ` [PATCH] " Richard Purdie
2018-04-04  0:54   ` Paul Eggleton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.