Re: [OE-core] [PATCHv2 5/5] systemd: split networkd into its own package

["Freihofer, Adrian" <adrian.freihofer@siemens.com>]

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

On Sun, 2024-12-15 at 23:41 -0500, Bruce Ashfield wrote:


On Fri, Dec 13, 2024 at 4:48 PM Adrian Freihofer <adrian.freihofer@gmail.com<mailto:adrian.freihofer@gmail.com>> wrote:
Am Fr., 13. Dez. 2024 um 14:54 Uhr schrieb Bruce Ashfield <bruce.ashfield@gmail.com<mailto:bruce.ashfield@gmail.com>>:


On Thu, Dec 12, 2024 at 11:44 AM Adrian Freihofer via lists.openembedded.org<http://lists.openembedded.org/> <adrian.freihofer=gmail.com@lists.openembedded.org<mailto:gmail.com@lists.openembedded.org>> wrote:
From: Adrian Freihofer <adrian.freihofer@siemens.com<mailto:adrian.freihofer@siemens.com>>

Support of images with different network management implementations
without having to recompile systemd and other components.

Fedora does this as well since systemd version 246.6-2:
https://src.fedoraproject.org/rpms/systemd/blob/rawhide/f/systemd.spec#_578
This proves that it is technically possible and officially supported by
systemd.

Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com<mailto:adrian.freihofer@siemens.com>>
---
 meta/recipes-core/systemd/systemd_256.8.bb<http://systemd_256.8.bb/> | 28 +++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-core/systemd/systemd_256.8.bb<http://systemd_256.8.bb/> b/meta/recipes-core/systemd/systemd_256.8.bb<http://systemd_256.8.bb/>
index c1915d7e941..0710377d32d 100644
--- a/meta/recipes-core/systemd/systemd_256.8.bb<http://systemd_256.8.bb/>
+++ b/meta/recipes-core/systemd/systemd_256.8.bb<http://systemd_256.8.bb/>
@@ -412,6 +412,7 @@ PACKAGE_BEFORE_PN = "\
     ${PN}-journal-remote \
     ${PN}-kernel-install \
     ${PN}-mime \
+    ${PN}-networkd \
     ${PN}-rpm-macros \
     ${PN}-udev-rules \
     ${PN}-vconsole-setup \
@@ -440,6 +441,7 @@ SYSTEMD_PACKAGES = "${@bb.utils.contains('PACKAGECONFIG', 'binfmt', '${PN}-binfm
                     ${@bb.utils.contains('PACKAGECONFIG', 'microhttpd', '${PN}-journal-gatewayd', '', d)} \
                     ${@bb.utils.contains('PACKAGECONFIG', 'microhttpd', '${PN}-journal-remote', '', d)} \
                     ${@bb.utils.contains('PACKAGECONFIG', 'journal-upload', '${PN}-journal-upload', '', d)} \
+                    ${@bb.utils.contains('PACKAGECONFIG', 'networkd', '${PN}-networkd', '', d)} \
 "
 SYSTEMD_SERVICE:${PN}-binfmt = "systemd-binfmt.service"

@@ -448,12 +450,13 @@ USERADD_PACKAGES = "${PN} \
                     ${@bb.utils.contains('PACKAGECONFIG', 'microhttpd', '${PN}-journal-gatewayd', '', d)} \
                     ${@bb.utils.contains('PACKAGECONFIG', 'microhttpd', '${PN}-journal-remote', '', d)} \
                     ${@bb.utils.contains('PACKAGECONFIG', 'journal-upload', '${PN}-journal-upload', '', d)} \
+                    ${@bb.utils.contains('PACKAGECONFIG', 'networkd', '${PN}-networkd', '', d)} \
 "


Possibly an already asked and answered question (I scanned quickly, but I didn't see it).

Is there any reason why networkd shouldn't be in the default PACKAGECONFIG ? This is a change of behaviour, that should be opt-in, not opt-out.

I'm not sure what you mean. networkd was and remains part of the default PACKAGECONFIG.
My intention is not to change the default behavior. Could you give me a hint where this is mistakenly happening anyway in this case?


I almost always read patches looking at the patch itself, and don't
do any research or looking outside of the patch itself. If something
isn't explained in the patch or causes me to wonder, it usually means
that the patch commit needs some tweaking.

In this case from the context of this patch, I see you packaging
based on a PACKAGECONFIG, but since the PACKAGECONFIG
isn't in the patch itself, I have no idea what the default of the recipe
in question is.

I also only see RRECOMMENDS for the new package on the main
PN, which means that someone without rrecommends enabled
will see different behaviour.

So my point would be that the comment that the default isn't
changed (and then describe what that is so), as well as commenting
on rrecommends vs rdepends, should be captured in the commit
log.

Thank you for the explanation. Now it's understandable. Your suggesting that I should add something like the following sentence to the commit message:

The systemd package RRECOMMENDS the new package systemd-networkd. As long as the RRECOMMENDED packages are not excluded, the change is expected to be fully backward compatible.
However, for builds where the RRECOMMENDED packages are not installed, systemd-networkd was installed before this commit, but no longer after this commit.

Adrian


Bruce





Adrian

Bruce


 GROUPADD_PARAM:${PN} = "-r systemd-journal;"
 GROUPADD_PARAM:udev = "-r render"
 GROUPADD_PARAM:${PN} += "${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', '-r systemd-hostname;', '', d)}"
 USERADD_PARAM:${PN} += "${@bb.utils.contains('PACKAGECONFIG', 'coredump', '--system -d / -M --shell /sbin/nologin systemd-coredump;', '', d)}"
-USERADD_PARAM:${PN} += "${@bb.utils.contains('PACKAGECONFIG', 'networkd', '--system -d / -M --shell /sbin/nologin systemd-network;', '', d)}"
+USERADD_PARAM:${PN}-networkd = "--system -d / -M --shell /sbin/nologin systemd-network"
 USERADD_PARAM:${PN} += "${@bb.utils.contains('PACKAGECONFIG', 'polkit', '--system --no-create-home --user-group --home-dir ${datadir}/polkit-1 polkitd;', '', d)}"
 USERADD_PARAM:${PN} += "${@bb.utils.contains('PACKAGECONFIG', 'resolved', '--system -d / -M --shell /sbin/nologin systemd-resolve;', '', d)}"
 USERADD_PARAM:${PN} += "${@bb.utils.contains('PACKAGECONFIG', 'timesyncd', '--system -d / -M --shell /sbin/nologin systemd-timesync;', '', d)}"
@@ -620,6 +623,27 @@ FILES:${PN}-extra-utils = "\
 FILES:${PN}-mime = "${MIMEDIR}"
 RRECOMMENDS:${PN} += "${PN}-mime"

+FILES:${PN}-networkd = "\
+    ${bindir}/networkctl \
+    ${datadir}/dbus-1/system-services/org.freedesktop.network1.service \
+    ${datadir}/dbus-1/system.d/org.freedesktop.network1.conf \
+    ${datadir}/polkit-1/actions/org.freedesktop.network1.policy \
+    ${nonarch_libdir}/sysusers.d/systemd-network.conf \
+    ${nonarch_libdir}/tmpfiles.d/systemd-network.conf \
+    ${sysconfdir}/systemd/networkd.conf \
+    ${systemd_system_unitdir}/systemd-networkd* \
+    ${systemd_unitdir}/network/*.network \
+    ${systemd_unitdir}/network/*.network.example \
+    ${systemd_unitdir}/networkd.conf \
+    ${systemd_unitdir}/systemd-networkd* \
+"
+# systemd-networkd-persistent-storage.service BindsTo=systemd-networkd.service
+# systemd-networkd.service has Also=systemd-networkd-wait-online.service
+SYSTEMD_SERVICE:${PN}-networkd = "systemd-networkd.service"
+CONFFILES:${PN}-networkd = "${sysconfdir}/systemd/networkd.conf"
+RDEPENDS:${PN}-networkd += "${PN}"
+RRECOMMENDS:${PN} += "${@bb.utils.contains('PACKAGECONFIG', 'networkd', '${PN}-networkd', '', d)}"
+
 FILES:${PN}-udev-rules = "\
                         ${nonarch_libdir}/udev/rules.d/70-uaccess.rules \
                         ${nonarch_libdir}/udev/rules.d/71-seat.rules \
@@ -630,7 +654,6 @@ FILES:${PN}-udev-rules = "\
 CONFFILES:${PN} = "${sysconfdir}/systemd/coredump.conf \
        ${sysconfdir}/systemd/journald.conf \
        ${sysconfdir}/systemd/logind.conf \
-       ${sysconfdir}/systemd/networkd.conf \
        ${sysconfdir}/systemd/pstore.conf \
        ${sysconfdir}/systemd/resolved.conf \
        ${sysconfdir}/systemd/sleep.conf \
@@ -698,7 +721,6 @@ FILES:${PN} = " ${base_bindir}/* \
                 ${nonarch_libdir}/modprobe.d/README \
                 ${datadir}/dbus-1/system.d/org.freedesktop.timedate1.conf \
                 ${datadir}/dbus-1/system.d/org.freedesktop.locale1.conf \
-                ${datadir}/dbus-1/system.d/org.freedesktop.network1.conf \
                 ${datadir}/dbus-1/system.d/org.freedesktop.resolve1.conf \
                 ${datadir}/dbus-1/system.d/org.freedesktop.systemd1.conf \
                 ${@bb.utils.contains('PACKAGECONFIG', 'polkit_hostnamed_fallback', '${datadir}/dbus-1/system.d/org.freedesktop.hostname1_no_polkit.conf', '', d)} \






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