[PATCH] systemd: change syntax for SRC_URI append

[PATCH] systemd: change syntax for SRC_URI append

[Peter Bergin]

In order to be able to override the git repo used in a bbappend
file the additions of files to SRC_URI needs to be done with :append
instead of +=, otherwise those will not be added.

My use case for this change is to try to build latest
systemd that is located in another repo than the one used
by the recipe (systemd.git instead of systemd-stable.git).
With this change this is possible to do by having SRC_URI, SRCREV
and SRCBRANCH in a bbappend.

Signed-off-by: Peter Bergin <peter@berginkonsult.se>
---
 meta/recipes-core/systemd/systemd_251.4.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-core/systemd/systemd_251.4.bb b/meta/recipes-core/systemd/systemd_251.4.bb
index 87668cadc7..f6c9640823 100644
--- a/meta/recipes-core/systemd/systemd_251.4.bb
+++ b/meta/recipes-core/systemd/systemd_251.4.bb
@@ -14,7 +14,7 @@ inherit useradd pkgconfig meson perlnative update-rc.d update-alternatives qemu
 # that we don't build both udev and systemd in world builds.
 REQUIRED_DISTRO_FEATURES = "systemd"
 
-SRC_URI += " \
+SRC_URI:append = " \
            file://touchscreen.rules \
            file://00-create-volatile.conf \
            ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', 'file://org.freedesktop.hostname1_no_polkit.conf', '', d)} \
-- 
2.34.1

Re: [OE-core] [PATCH] systemd: change syntax for SRC_URI append

[Alexander Kanavin]

On Mon, 31 Oct 2022 at 22:09, Peter Bergin <peter@berginkonsult.se> wrote:
>
> In order to be able to override the git repo used in a bbappend
> file the additions of files to SRC_URI needs to be done with :append
> instead of +=, otherwise those will not be added.
>
> My use case for this change is to try to build latest
> systemd that is located in another repo than the one used
> by the recipe (systemd.git instead of systemd-stable.git).
> With this change this is possible to do by having SRC_URI, SRCREV
> and SRCBRANCH in a bbappend.

Sorry but overriding these three things in a bbappend is a horrible
idea, and should not be done. Copy and tweak the whole recipe please.

Alex

Re: [OE-core] [PATCH] systemd: change syntax for SRC_URI append

[Peter Bergin]

Hi Alex,

On 2022-10-31 22:24, Alexander Kanavin wrote:
> On Mon, 31 Oct 2022 at 22:09, Peter Bergin <peter@berginkonsult.se> wrote:
>> In order to be able to override the git repo used in a bbappend
>> file the additions of files to SRC_URI needs to be done with :append
>> instead of +=, otherwise those will not be added.
>>
>> My use case for this change is to try to build latest
>> systemd that is located in another repo than the one used
>> by the recipe (systemd.git instead of systemd-stable.git).
>> With this change this is possible to do by having SRC_URI, SRCREV
>> and SRCBRANCH in a bbappend.
> Sorry but overriding these three things in a bbappend is a horrible
> idea, and should not be done. Copy and tweak the whole recipe please.

Sure, I can do that. My aim for override those in a bbappend is just for 
development purposes and nothing I plan to use in production or push in 
public. My way of working when I realized this was to build latest 
master of systemd that had a bunch of patches that I would like to test. 
To do that I needed to change git repo to systemd.git. When tested I 
plan to backport relevant patches that I need to add to current systemd 
version in oe-core to my local layer until next systemd release. Then I 
thought this could simplify things for more people. But I'm fine if this 
is not accepted for upstream and I can work around that.

Best regards,
/Peter

Re: [OE-core] [PATCH] systemd: change syntax for SRC_URI append

[Alexander Kanavin]

On Mon, 31 Oct 2022 at 22:32, Peter Bergin <peter@berginkonsult.se> wrote:
> Sure, I can do that. My aim for override those in a bbappend is just for
> development purposes and nothing I plan to use in production or push in
> public. My way of working when I realized this was to build latest
> master of systemd that had a bunch of patches that I would like to test.
> To do that I needed to change git repo to systemd.git. When tested I
> plan to backport relevant patches that I need to add to current systemd
> version in oe-core to my local layer until next systemd release. Then I
> thought this could simplify things for more people. But I'm fine if this
> is not accepted for upstream and I can work around that.

We had a related discussion recently regarding mesa, where there is a
similar desire to 'reuse' the recipe from core as much as possible but
build from a different revision. Typically this would be done by
having some kind of common .inc file (systemd.inc in this case), and
then adding the missing parts in a custom .bb that includes the .inc
across layers (bitbake allows this). No promises about keeping things
in .inc compatible with whatever happens in custom layers though.

Another option is to use the devupstream class, and there are examples
in oe-core for it. Would that work for you?

Alex

Re: [OE-core] [PATCH] systemd: change syntax for SRC_URI append

[Peter Bergin]

On 2022-10-31 22:54, Alexander Kanavin wrote:
> On Mon, 31 Oct 2022 at 22:32, Peter Bergin <peter@berginkonsult.se> wrote:
>> Sure, I can do that. My aim for override those in a bbappend is just for
>> development purposes and nothing I plan to use in production or push in
>> public. My way of working when I realized this was to build latest
>> master of systemd that had a bunch of patches that I would like to test.
>> To do that I needed to change git repo to systemd.git. When tested I
>> plan to backport relevant patches that I need to add to current systemd
>> version in oe-core to my local layer until next systemd release. Then I
>> thought this could simplify things for more people. But I'm fine if this
>> is not accepted for upstream and I can work around that.
> We had a related discussion recently regarding mesa, where there is a
> similar desire to 'reuse' the recipe from core as much as possible but
> build from a different revision. Typically this would be done by
> having some kind of common .inc file (systemd.inc in this case), and
> then adding the missing parts in a custom .bb that includes the .inc
> across layers (bitbake allows this). No promises about keeping things
> in .inc compatible with whatever happens in custom layers though.

This opened my eyes for the way systemd recipes are built up in oe-core. 
It seems a bit the other way around. systemd.inc contains almost only 
version information. systemd_251.4.bb is huge and contains a lot of 
thing that I think can be shared among versions. If it had been the 
other way around it had been easy to have a systemd_git.bb, including 
systemd.inc, in my local layer (or upstream if desired) that build 
systemd main branch. What do you think of that? Is is worth working on 
such a patch? Or are there reasons for that setup?

> Another option is to use the devupstream class, and there are examples
> in oe-core for it. Would that work for you?

I'll manage this but thanks for the tip about devupstream class.

Best regards,
/Peter

Re: [OE-core] [PATCH] systemd: change syntax for SRC_URI append

[Alexander Kanavin]

On Tue, 1 Nov 2022 at 09:12, Peter Bergin <peter@berginkonsult.se> wrote:
> This opened my eyes for the way systemd recipes are built up in oe-core.
> It seems a bit the other way around. systemd.inc contains almost only
> version information. systemd_251.4.bb is huge and contains a lot of
> thing that I think can be shared among versions. If it had been the
> other way around it had been easy to have a systemd_git.bb, including
> systemd.inc, in my local layer (or upstream if desired) that build
> systemd main branch. What do you think of that? Is is worth working on
> such a patch? Or are there reasons for that setup?

I prefer the opposite actually. Where possible, I fold the .inc files
into the main recipe because that makes maintenance easier, and if
someone needs to change that in non-upstreamable manner, they have to
perform a full fork in a private layer. I do not like bits and pieces
of code scattered all over the place and gathered together by bitbake,
that does not help readability.

For systemd, I think the reason for systemd.inc is that something else
in the same folder is using it.

Alex

Re: [OE-core] [PATCH] systemd: change syntax for SRC_URI append

[Martin Jansa]

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

> I do not like bits and pieces of code scattered all over the place and
gathered together by bitbake, that does not help readability.

My experience is opposite, developers tend to copy whole recipes without
explaining why it was copied and what's the important diff they needed
compared to upstream recipes (or .inc file).

It's difficult to untangle when someone few years ago copied newer version
of foo from warrior to thud based build in product repo, then someone else
from different team cherry-picks "Add that feature" commit to dunfell based
build (and now foo is downgraded from version in dunfell to the old version
from warrior and the cherry-picked commit doesn't say why this version was
duplicated). Then when I'm upgrading from dunfell to kirkstone it's really
late to try finding the original warrior commit from which it was copied to
see if there were some intentional changes in packaging during the
"duplication".

e.g. duplication in
https://github.com/shr-project/meta-webosose/commit/1d61ee9739d61c061256077b231eb3fceed68f04
replaced with shorter bbappend in
https://github.com/shr-project/meta-webosose/commit/d333fec00fb0bc57d38375ddcda269780837ce79

Yes developers should write better commit messages and provide useful
comments in imported files, but at least in my experience very few do that
and duplicating more hurts readability, for everything else there is
bitbake -e to see where and how bitbake gathered the pieces.

Having short .bbappend helps them to do only necessary changes. Changing
the version in .bbappend is usually bad (especially when the original _
1.0.bb recipe is versioned and the _1.0.bbappend sets PV = "2.0"), so
reusing as much as possible from .inc file if it exists is the next best
thing to keep the "diff" short.

.bbappends also stop working from time to time when the upstream recipe
changes, reusing .inc files isn't very different from that.

Cheers,

On Tue, Nov 1, 2022 at 10:11 AM Alexander Kanavin <alex.kanavin@gmail.com>
wrote:

> On Tue, 1 Nov 2022 at 09:12, Peter Bergin <peter@berginkonsult.se> wrote:
> > This opened my eyes for the way systemd recipes are built up in oe-core.
> > It seems a bit the other way around. systemd.inc contains almost only
> > version information. systemd_251.4.bb is huge and contains a lot of
> > thing that I think can be shared among versions. If it had been the
> > other way around it had been easy to have a systemd_git.bb, including
> > systemd.inc, in my local layer (or upstream if desired) that build
> > systemd main branch. What do you think of that? Is is worth working on
> > such a patch? Or are there reasons for that setup?
>
> I prefer the opposite actually. Where possible, I fold the .inc files
> into the main recipe because that makes maintenance easier, and if
> someone needs to change that in non-upstreamable manner, they have to
> perform a full fork in a private layer. I do not like bits and pieces
> of code scattered all over the place and gathered together by bitbake,
> that does not help readability.
>
> For systemd, I think the reason for systemd.inc is that something else
> in the same folder is using it.
>
> Alex
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#172353):
> https://lists.openembedded.org/g/openembedded-core/message/172353
> Mute This Topic: https://lists.openembedded.org/mt/94696056/3617156
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> Martin.Jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

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

Re: [OE-core] [PATCH] systemd: change syntax for SRC_URI append

[Mikko Rapeli]

Hi,

On Tue, Nov 01, 2022 at 10:10:45AM +0100, Alexander Kanavin wrote:
> On Tue, 1 Nov 2022 at 09:12, Peter Bergin <peter@berginkonsult.se> wrote:
> > This opened my eyes for the way systemd recipes are built up in oe-core.
> > It seems a bit the other way around. systemd.inc contains almost only
> > version information. systemd_251.4.bb is huge and contains a lot of
> > thing that I think can be shared among versions. If it had been the
> > other way around it had been easy to have a systemd_git.bb, including
> > systemd.inc, in my local layer (or upstream if desired) that build
> > systemd main branch. What do you think of that? Is is worth working on
> > such a patch? Or are there reasons for that setup?
> 
> I prefer the opposite actually. Where possible, I fold the .inc files
> into the main recipe because that makes maintenance easier, and if
> someone needs to change that in non-upstreamable manner, they have to
> perform a full fork in a private layer. I do not like bits and pieces
> of code scattered all over the place and gathered together by bitbake,
> that does not help readability.

systemd contains both compiled code and large amounts of config files
which frequently need to be changed in various ways in real products. Think of
all the defaults for systemd-networkd, systemd-journald, service files etc.

Thus, it is common to have a bbappend for it which either patches
or otherwise changes these config files (e.g. sed in a
do_install_append()).

In these cases full fork of the poky side recipe is not needed or even
wanted.

What breaks these use cases is use of :append to change various
variables which users also need to change. A SRC_URI:append with 10's of
patches needs SRC_URI:remove for all of them, possibly increasing with
every security update gets really annoying in a .bbappend and users will
need to fully fork the recipes.

The .bb or .inc way of handling the main recipe doesn't really matter as
long it easy in a .bbappend to amend things to the variables like
SRC_URI and that these amendments work when there are small updates to
the recipe from e.g. LTS branch.

Cheers,

-Mikko