mbox series

[0/1] Fix dependency handling bug of :remove operation

Message ID 20230918090522.659869-1-insu0.park@gmail.com
Headers show
Series Fix dependency handling bug of :remove operation | expand

Message

Insu Park Sept. 18, 2023, 9:05 a.m. UTC
Unlike the meta layers under the Yocto projects, custom layers frequently use following style
of overriding to disable something default in the base layer without touching it:
    bb in the base layer:
        OPTIONS = "foo bar"
    bbappend in the custom layer:
        OPTIONS:remove = "${@bb.utils.contains('CUSTOM_FEATURES', 'no-foo', 'foo', '', d)}"

The problem is that the value of "CUSTOM_FEATURES" is not shown in the task signature.
Here is a test example.

test-remove-operation.bb:
    TEST_VAR        =  "v1 ${@bb.utils.contains('TEST_FEAT1', 'foo', 'v1foo', '', d)}"
    TEST_VAR:append = " v2 ${@bb.utils.contains('TEST_FEAT2', 'bar', 'v2bar', '', d)}"
    TEST_VAR:remove =  "v3 ${@bb.utils.contains('TEST_FEAT3', 'disable-v1', 'v1', '', d)}"

local.conf:
    TEST_FEAT1 = "foo"
    TEST_FEAT2 = "foo"
    TEST_FEAT3 = "foo"

Signature dump (Bug):
    Variable TEST_VAR value is v1 ${@bb.utils.contains('TEST_FEAT1', 'foo', 'v1foo', '', d)} v2 ${@bb.utils.contains('TEST_FEAT2', 'bar', 'v2bar', '', d)}
    TEST_FEAT1{foo} = Set
    TEST_FEAT2{bar} = Unset
    _remove of v3 ${@bb.utils.contains('TEST_FEAT3', 'disable-v1', 'v1', '', d)}

Note that the signature dump above doesn't take the value of TEST_FEAT3 into account.
Therefore, even if the local.conf changes the TEST_FEAT3 value to "disable-v1",
the two different configs produce exactly same signature, so the recipe won't be re-built,
even worse, its sstate-cache output is not deterministic (depend on the config it first met).

With the fixed code, the new signature recognizes the TEST_FEAT3, and the problem solved.
Signature dump (Fixed):
    (Same as above)
    TEST_FEAT3{disable-v1} = Unset

Insu Park (1):
  bitbake: data: Add missing dependency handling of :remove operation

 lib/bb/data.py | 1 +
 1 file changed, 1 insertion(+)

Comments

Richard Purdie Sept. 18, 2023, 10:02 a.m. UTC | #1
On Mon, 2023-09-18 at 18:05 +0900, Insu Park wrote:
> Unlike the meta layers under the Yocto projects, custom layers frequently use following style
> of overriding to disable something default in the base layer without touching it:
>     bb in the base layer:
>         OPTIONS = "foo bar"
>     bbappend in the custom layer:
>         OPTIONS:remove = "${@bb.utils.contains('CUSTOM_FEATURES', 'no-foo', 'foo', '', d)}"
> 
> The problem is that the value of "CUSTOM_FEATURES" is not shown in the task signature.
> Here is a test example.
> 
> test-remove-operation.bb:
>     TEST_VAR        =  "v1 ${@bb.utils.contains('TEST_FEAT1', 'foo', 'v1foo', '', d)}"
>     TEST_VAR:append = " v2 ${@bb.utils.contains('TEST_FEAT2', 'bar', 'v2bar', '', d)}"
>     TEST_VAR:remove =  "v3 ${@bb.utils.contains('TEST_FEAT3', 'disable-v1', 'v1', '', d)}"
> 
> local.conf:
>     TEST_FEAT1 = "foo"
>     TEST_FEAT2 = "foo"
>     TEST_FEAT3 = "foo"
> 
> Signature dump (Bug):
>     Variable TEST_VAR value is v1 ${@bb.utils.contains('TEST_FEAT1', 'foo', 'v1foo', '', d)} v2 ${@bb.utils.contains('TEST_FEAT2', 'bar', 'v2bar', '', d)}
>     TEST_FEAT1{foo} = Set
>     TEST_FEAT2{bar} = Unset
>     _remove of v3 ${@bb.utils.contains('TEST_FEAT3', 'disable-v1', 'v1', '', d)}
> 
> Note that the signature dump above doesn't take the value of TEST_FEAT3 into account.
> Therefore, even if the local.conf changes the TEST_FEAT3 value to "disable-v1",
> the two different configs produce exactly same signature, so the recipe won't be re-built,
> even worse, its sstate-cache output is not deterministic (depend on the config it first met).
> 
> With the fixed code, the new signature recognizes the TEST_FEAT3, and the problem solved.
> Signature dump (Fixed):
>     (Same as above)
>     TEST_FEAT3{disable-v1} = Unset

This looks reasonable to fix but before we do, I'd like to ensure that
there are tests for it added to lib/bb/tests/ (run with bitbake-
selftest). Could you have a look and see if you could add them please?

This is to ensure we don't regress this corner case in future.

Thanks,

Richard
Insu Park Sept. 19, 2023, 12:42 a.m. UTC | #2
On Mon, Sep 18, 2023 at 7:02 PM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> This looks reasonable to fix but before we do, I'd like to ensure that
> there are tests for it added to lib/bb/tests/ (run with bitbake-
> selftest). Could you have a look and see if you could add them please?
>
> This is to ensure we don't regress this corner case in future.
>
> Thanks,
>
> Richard
>

All right. I will post v2 soon.

Thanks,
Insu