[PATCH 1/7] common-tasks.rst: remove SRC_URI:append from examples

[PATCH 1/7] common-tasks.rst: remove SRC_URI:append from examples

[Mikko Rapeli]

Using SRC_URI:append without recipe, machine or architecture
specific limitations makes the :append'ed text unremovable
and thus users and custom layers can not change the variable
anymore. This makes it hard to e.g. override SRC_URI completely
in a bbappend to update the full recipe to a newer version.
Thus common, reusable layers which users are meant to re-use and
customize should not use SRC_URI:append but SRC_URI += instead.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 documentation/dev-manual/common-tasks.rst | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/documentation/dev-manual/common-tasks.rst b/documentation/dev-manual/common-tasks.rst
index b08a55331d..3112f9b893 100644
--- a/documentation/dev-manual/common-tasks.rst
+++ b/documentation/dev-manual/common-tasks.rst
@@ -2577,7 +2577,7 @@ chapter of the BitBake User Manual.
 
       S = "${WORKDIR}/postfix-${PV}"
       CFLAGS += "-DNO_ASM"
-      SRC_URI:append = " file://fixup.patch"
+      CFLAGS:append = " --enable-important-feature"
 
 -  *Functions:* Functions provide a series of actions to be performed.
    You usually use functions to override the default implementation of a
@@ -2708,19 +2708,20 @@ in the BitBake User Manual.
    to existing variables. This operator does not add any additional
    space. Also, the operator is applied after all the ``+=``, and ``=+``
    operators have been applied and after all ``=`` assignments have
-   occurred.
+   occurred. This means that if ``:append`` is used, that text can not be
+   removed.
 
    The following example shows the space being explicitly added to the
    start to ensure the appended value is not merged with the existing
    value::
 
-      SRC_URI:append = " file://fix-makefile.patch"
+      CFLAGS:append = " --enable-important-feature"
 
    You can also use
    the ``:append`` operator with overrides, which results in the actions
    only being performed for the specified target or machine::
 
-      SRC_URI:append:sh4 = " file://fix-makefile.patch"
+      CFLAGS:append:sh4 = " --enable-important-sh4-specific-feature"
 
 -  *Prepending (:prepend):* Use the ``:prepend`` operator to prepend
    values to existing variables. This operator does not add any
-- 
2.17.1

[PATCH 2/7] kernel-dev/common.rst: remove SRC_URI:append from examples

[Mikko Rapeli]

It's better to use SRC_URI += to append patches etc. If anything
is added via :append, that can no longer be removed at all.
If common, re-usable layers use SRC_URI:append, then users can
not change those patches or SRC_URI entries without completely
replacing the recipe with a copy in their own layer. Thus
+= is better.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 documentation/kernel-dev/common.rst | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/documentation/kernel-dev/common.rst b/documentation/kernel-dev/common.rst
index 16ef6453bd..fb8d7cd029 100644
--- a/documentation/kernel-dev/common.rst
+++ b/documentation/kernel-dev/common.rst
@@ -360,9 +360,9 @@ home directory:
 
       FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:"
 
-      SRC_URI:append = " file://patch-file-one.patch"
-      SRC_URI:append = " file://patch-file-two.patch"
-      SRC_URI:append = " file://patch-file-three.patch"
+      SRC_URI += "file://patch-file-one.patch"
+      SRC_URI += "file://patch-file-two.patch"
+      SRC_URI += "file://patch-file-three.patch"
 
    The :term:`FILESEXTRAPATHS` and :term:`SRC_URI` statements
    enable the OpenEmbedded build system to find patch files. For more
@@ -1002,7 +1002,7 @@ Section.
    contents::
 
       FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:"
-      SRC_URI:append = " file://0001-calibrate.c-Added-some-printk-statements.patch"
+      SRC_URI += "file://0001-calibrate.c-Added-some-printk-statements.patch"
 
    The :term:`FILESEXTRAPATHS` and :term:`SRC_URI` statements
    enable the OpenEmbedded build system to find the patch file.
@@ -1875,7 +1875,7 @@ build.
 2. *Add the Feature File to SRC_URI:* Add the ``.scc`` file to the
    recipe's :term:`SRC_URI` statement::
 
-      SRC_URI:append = " file://test.scc"
+      SRC_URI += "file://test.scc"
 
    The leading space before the path is important as the path is
    appended to the existing path.
@@ -1884,7 +1884,7 @@ build.
    :term:`KERNEL_FEATURES` statement to specify the feature as a kernel
    feature::
 
-      KERNEL_FEATURES:append = " test.scc"
+      KERNEL_FEATURES += "test.scc"
 
    The OpenEmbedded build
    system processes the kernel feature when it builds the kernel.
-- 
2.17.1

[PATCH 3/7] u-boot: switch from append to += in SRC_URI

[Mikko Rapeli]

+= allows custom layers to change the SRC_URI e.g. when
updating the whole recipe to newer u-boot version.
With :append, there is no way to change the variable
from a bbappend.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta/recipes-bsp/u-boot/u-boot_2022.07.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-bsp/u-boot/u-boot_2022.07.bb b/meta/recipes-bsp/u-boot/u-boot_2022.07.bb
index 0d2464d74b..1ae575790c 100644
--- a/meta/recipes-bsp/u-boot/u-boot_2022.07.bb
+++ b/meta/recipes-bsp/u-boot/u-boot_2022.07.bb
@@ -1,7 +1,7 @@
 require u-boot-common.inc
 require u-boot.inc
 
-SRC_URI:append = " file://0001-riscv32-Use-double-float-ABI-for-rv32.patch \
+SRC_URI +=       " file://0001-riscv32-Use-double-float-ABI-for-rv32.patch \
                    file://0001-riscv-fix-build-with-binutils-2.38.patch \
                  "
 
-- 
2.17.1

[PATCH 4/7] glibc-tests: use += instead of :append

[Mikko Rapeli]

:append can not be modified in bbappends and thus += is
better in re-usable, generic layers and recipes.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta/recipes-core/glibc/glibc-tests_2.36.bb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/recipes-core/glibc/glibc-tests_2.36.bb b/meta/recipes-core/glibc/glibc-tests_2.36.bb
index aca9675ebb..c71c0831c6 100644
--- a/meta/recipes-core/glibc/glibc-tests_2.36.bb
+++ b/meta/recipes-core/glibc/glibc-tests_2.36.bb
@@ -4,7 +4,7 @@ require glibc-tests.inc
 inherit ptest features_check
 REQUIRED_DISTRO_FEATURES = "ptest"
 
-SRC_URI:append = " \
+SRC_URI += "\
 	file://run-ptest \
 "
 
@@ -29,7 +29,7 @@ python __anonymous() {
 RPROVIDES:${PN} = "${PN}"
 RRECOMMENDS:${PN} = ""
 RDEPENDS:${PN} = " glibc sed"
-DEPENDS:append = " sed"
+DEPENDS += "sed"
 
 export oe_srcdir="${exec_prefix}/src/debug/glibc/${PV}/"
 
-- 
2.17.1

[PATCH 5/7] go-native: switch from SRC_URI:append to SRC_URI +=

[Mikko Rapeli]

The :append can not be removed if needed in other layers.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta/recipes-devtools/go/go-native_1.19.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-devtools/go/go-native_1.19.bb b/meta/recipes-devtools/go/go-native_1.19.bb
index 76c0ab73a6..ddf25b2c9b 100644
--- a/meta/recipes-devtools/go/go-native_1.19.bb
+++ b/meta/recipes-devtools/go/go-native_1.19.bb
@@ -5,7 +5,7 @@ require go-${PV}.inc
 
 inherit native
 
-SRC_URI:append = " https://dl.google.com/go/go1.4-bootstrap-20171003.tar.gz;name=bootstrap;subdir=go1.4"
+SRC_URI += "https://dl.google.com/go/go1.4-bootstrap-20171003.tar.gz;name=bootstrap;subdir=go1.4"
 SRC_URI[bootstrap.sha256sum] = "f4ff5b5eb3a3cae1c993723f3eab519c5bae18866b5e5f96fe1102f0cb5c3e52"
 
 export GOOS = "${BUILD_GOOS}"
-- 
2.17.1

[PATCH 6/7] python3-rfc3986-validator: switch from SRC_URI:append to SRC_URI +=

[Mikko Rapeli]

The :append can not be removed via bbappends if needed. Thus it's better
for open source layers to use += append if possible.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta/recipes-devtools/python/python3-rfc3986-validator_0.1.1.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-devtools/python/python3-rfc3986-validator_0.1.1.bb b/meta/recipes-devtools/python/python3-rfc3986-validator_0.1.1.bb
index 4abd181acf..e374979cb4 100644
--- a/meta/recipes-devtools/python/python3-rfc3986-validator_0.1.1.bb
+++ b/meta/recipes-devtools/python/python3-rfc3986-validator_0.1.1.bb
@@ -13,7 +13,7 @@ UPSTREAM_CHECK_REGEX = "/rfc3986-validator/(?P<pver>(\d+[\.\-_]*)+)/"
 
 inherit pypi setuptools3
 
-SRC_URI:append = " \
+SRC_URI += "\
     file://0001-setup.py-move-pytest-runner-to-test_requirements.patch \
 "
 
-- 
2.17.1

[PATCH 7/7] linux-libc-headers: switch from SRC_URI:append to SRC_URI +=

[Mikko Rapeli]

The :append can not be removed via bbappends in custom layers so it's
better to use += appends when ever possible.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 .../linux-libc-headers/linux-libc-headers_5.19.bb               | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-kernel/linux-libc-headers/linux-libc-headers_5.19.bb b/meta/recipes-kernel/linux-libc-headers/linux-libc-headers_5.19.bb
index 528e1d3379..3557526d14 100644
--- a/meta/recipes-kernel/linux-libc-headers/linux-libc-headers_5.19.bb
+++ b/meta/recipes-kernel/linux-libc-headers/linux-libc-headers_5.19.bb
@@ -7,7 +7,7 @@ SRC_URI:append:libc-musl = "\
     file://0001-include-linux-stddef.h-in-swab.h-uapi-header.patch \
    "
 
-SRC_URI:append = "\
+SRC_URI += "\
     file://0001-scripts-Use-fixed-input-and-output-files-instead-of-.patch \
     file://0001-kbuild-install_headers.sh-Strip-_UAPI-from-if-define.patch \
 "
-- 
2.17.1

Re: [docs] [PATCH 1/7] common-tasks.rst: remove SRC_URI:append from examples

[Quentin Schulz]

Hi Mikko,

On 9/12/22 09:32, Mikko Rapeli wrote:
> Using SRC_URI:append without recipe, machine or architecture
> specific limitations makes the :append'ed text unremovable
> and thus users and custom layers can not change the variable
> anymore. This makes it hard to e.g. override SRC_URI completely
> in a bbappend to update the full recipe to a newer version.
> Thus common, reusable layers which users are meant to re-use and
> customize should not use SRC_URI:append but SRC_URI += instead.
> 
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> ---
>   documentation/dev-manual/common-tasks.rst | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/documentation/dev-manual/common-tasks.rst b/documentation/dev-manual/common-tasks.rst
> index b08a55331d..3112f9b893 100644
> --- a/documentation/dev-manual/common-tasks.rst
> +++ b/documentation/dev-manual/common-tasks.rst
> @@ -2577,7 +2577,7 @@ chapter of the BitBake User Manual.
>   
>         S = "${WORKDIR}/postfix-${PV}"
>         CFLAGS += "-DNO_ASM"
> -      SRC_URI:append = " file://fixup.patch"
> +      CFLAGS:append = " --enable-important-feature"
>   
>   -  *Functions:* Functions provide a series of actions to be performed.
>      You usually use functions to override the default implementation of a
> @@ -2708,19 +2708,20 @@ in the BitBake User Manual.
>      to existing variables. This operator does not add any additional
>      space. Also, the operator is applied after all the ``+=``, and ``=+``
>      operators have been applied and after all ``=`` assignments have
> -   occurred.
> +   occurred. This means that if ``:append`` is used, that text can not be
> +   removed.
>   

Pedantic: it can, with :remove. One should try really hard to not use it 
though.

The rest is fine.

Cheers,
Quentin

Re: [OE-core] [docs] [PATCH 1/7] common-tasks.rst: remove SRC_URI:append from examples

[Michael Opdenacker]

Mikko, thanks for the patch series.
Quentin, thanks for the review.

Indeed, I think that the description of the commit should be improved 
too. See below.

On 9/19/22 16:04, Quentin Schulz via lists.openembedded.org wrote:
> Hi Mikko,
>
> On 9/12/22 09:32, Mikko Rapeli wrote:
>> Using SRC_URI:append without recipe, machine or architecture
>> specific limitations makes the :append'ed text unremovable
>> and thus users and custom layers can not change the variable
>> anymore. This makes it hard to e.g. override SRC_URI completely
>> in a bbappend to update the full recipe to a newer version.
>> Thus common, reusable layers which users are meant to re-use and
>> customize should not use SRC_URI:append but SRC_URI += instead.


What the following text instead?

Using SRC_URI:append without recipe, machine or architecture specific 
limitations makes the :append'ed text more difficult to override than if 
the "+=" operator was used. This makes it hard for example to override 
SRC_URI completely in a bbappend to update the full recipe to a newer 
version. Thus common, reusable layers which users are meant to re-use 
and customize should not use SRC_URI:append but SRC_URI += instead.

>>
>> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
>> ---
>>   documentation/dev-manual/common-tasks.rst | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/documentation/dev-manual/common-tasks.rst 
>> b/documentation/dev-manual/common-tasks.rst
>> index b08a55331d..3112f9b893 100644
>> --- a/documentation/dev-manual/common-tasks.rst
>> +++ b/documentation/dev-manual/common-tasks.rst
>> @@ -2577,7 +2577,7 @@ chapter of the BitBake User Manual.
>>           S = "${WORKDIR}/postfix-${PV}"
>>         CFLAGS += "-DNO_ASM"
>> -      SRC_URI:append = " file://fixup.patch"
>> +      CFLAGS:append = " --enable-important-feature"


Didn't you mean
+     SRC_URI += "file://fixup.patch"

instead here?

>>     -  *Functions:* Functions provide a series of actions to be 
>> performed.
>>      You usually use functions to override the default implementation 
>> of a
>> @@ -2708,19 +2708,20 @@ in the BitBake User Manual.
>>      to existing variables. This operator does not add any additional
>>      space. Also, the operator is applied after all the ``+=``, and 
>> ``=+``
>>      operators have been applied and after all ``=`` assignments have
>> -   occurred.
>> +   occurred. This means that if ``:append`` is used, that text can 
>> not be
>> +   removed.
>
> Pedantic: it can, with :remove. One should try really hard to not use 
> it though.


What about...

"This means that if ``:append`` is used in a recipe, it cannot only be 
overridden by another layer using the  special ``:remove`` operator, 
which in turn will prevent further layers from adding it back."

I'm not very happy with this wording, because ":remove" hasn't been 
introduced at this stage in this section. Maybe we should add a link 
to https://docs.yoctoproject.org/bitbake/bitbake-user-manual/bitbake-user-manual-metadata.html#conditional-syntax-overrides

Any thoughts?

Cheers
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [OE-core] [docs] [PATCH 1/7] common-tasks.rst: remove SRC_URI:append from examples

[Mikko Rapeli]

Hi,

On Mon, 19 Sept 2022 at 23:35, Michael Opdenacker
<michael.opdenacker@bootlin.com> wrote:
>
> Mikko, thanks for the patch series.
> Quentin, thanks for the review.
>
> Indeed, I think that the description of the commit should be improved
> too. See below.
>
> On 9/19/22 16:04, Quentin Schulz via lists.openembedded.org wrote:
> > Hi Mikko,
> >
> > On 9/12/22 09:32, Mikko Rapeli wrote:
> >> Using SRC_URI:append without recipe, machine or architecture
> >> specific limitations makes the :append'ed text unremovable
> >> and thus users and custom layers can not change the variable
> >> anymore. This makes it hard to e.g. override SRC_URI completely
> >> in a bbappend to update the full recipe to a newer version.
> >> Thus common, reusable layers which users are meant to re-use and
> >> customize should not use SRC_URI:append but SRC_URI += instead.
>
>
> What the following text instead?
>
> Using SRC_URI:append without recipe, machine or architecture specific
> limitations makes the :append'ed text more difficult to override than if
> the "+=" operator was used. This makes it hard for example to override
> SRC_URI completely in a bbappend to update the full recipe to a newer
> version. Thus common, reusable layers which users are meant to re-use
> and customize should not use SRC_URI:append but SRC_URI += instead.

Ok, will send a v2 with this.

> >>
> >> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> >> ---
> >>   documentation/dev-manual/common-tasks.rst | 9 +++++----
> >>   1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/documentation/dev-manual/common-tasks.rst
> >> b/documentation/dev-manual/common-tasks.rst
> >> index b08a55331d..3112f9b893 100644
> >> --- a/documentation/dev-manual/common-tasks.rst
> >> +++ b/documentation/dev-manual/common-tasks.rst
> >> @@ -2577,7 +2577,7 @@ chapter of the BitBake User Manual.
> >>           S = "${WORKDIR}/postfix-${PV}"
> >>         CFLAGS += "-DNO_ASM"
> >> -      SRC_URI:append = " file://fixup.patch"
> >> +      CFLAGS:append = " --enable-important-feature"
>
>
> Didn't you mean
> +     SRC_URI += "file://fixup.patch"
>
> instead here?

No. The sample code shows how to use variables so it needs an :append example
but SRC_URI is not good use for :append's. Hence I changed it to CFLAGS:append.

> >>     -  *Functions:* Functions provide a series of actions to be
> >> performed.
> >>      You usually use functions to override the default implementation
> >> of a
> >> @@ -2708,19 +2708,20 @@ in the BitBake User Manual.
> >>      to existing variables. This operator does not add any additional
> >>      space. Also, the operator is applied after all the ``+=``, and
> >> ``=+``
> >>      operators have been applied and after all ``=`` assignments have
> >> -   occurred.
> >> +   occurred. This means that if ``:append`` is used, that text can
> >> not be
> >> +   removed.
> >
> > Pedantic: it can, with :remove. One should try really hard to not use
> > it though.
>
>
> What about...
>
> "This means that if ``:append`` is used in a recipe, it cannot only be
> overridden by another layer using the  special ``:remove`` operator,
> which in turn will prevent further layers from adding it back."
>
> I'm not very happy with this wording, because ":remove" hasn't been
> introduced at this stage in this section. Maybe we should add a link
> to https://docs.yoctoproject.org/bitbake/bitbake-user-manual/bitbake-user-manual-metadata.html#conditional-syntax-overrides
>
> Any thoughts?

I'll send v2 with this version.

I'd like to get rid of all SRC_URI:append's in documentation
since that bad practice already ended in various recipes in poky master
and kirkstone branches.

Cheers,
-Mikko