| Message ID | 20211119113429.502652-1-luca.boccassi@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/7] systemd: skip chown when building for nativesdk | expand |
On Fri, 2021-11-19 at 11:34 +0000, Luca Bocassi wrote: > From: Luca Boccassi <luca.boccassi@microsoft.com> > > The useradd class is a no-op in the nativesdk case, so chown will fail. > Skip them. > > Signed-off-by: Luca Boccassi <luca.boccassi@microsoft.com> > --- > v2: use "${PN}" = "${BPN}" as suggested by reviewers I think that was bad advice since this would break multilib variants of the systemd recipe and I'd much prefer this was conditional on nativesdk. > meta/recipes-core/systemd/systemd_249.5.bb | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/meta/recipes-core/systemd/systemd_249.5.bb b/meta/recipes-core/systemd/systemd_249.5.bb > index 8bdc0ca028..2df2de0cf3 100644 > --- a/meta/recipes-core/systemd/systemd_249.5.bb > +++ b/meta/recipes-core/systemd/systemd_249.5.bb > @@ -275,7 +275,10 @@ do_install() { > # which is expected to be empty. > rm -rf ${D}${localstatedir}/log > else > - chown root:systemd-journal ${D}${localstatedir}/log/journal > + # The useradd class is a no-op in the nativesdk case, so chown will fail > + if [ "${PN}" = "${BPN}" ]; then > + chown root:systemd-journal ${D}${localstatedir}/log/journal > + fi > > # journal-remote creates this at start > rm -rf ${D}${localstatedir}/log/journal/remote I'm guessing this is only failing on systems that don't have a systemd-jounral group as it built ok for me? The better way to fix this is probably to replicate what we have for native, i.e. the entry in the class: native.bbclass:PATH:prepend = "${COREBASE}/scripts/native-intercept:" which puts a chown and chgrp into PATH which doesn't do anything. We could do something similar for nativesdk and it would avoid the need for these if statements and solve the problem generically. I am also a bit concerned about some of the other "creeping" dependencies so I experimented a little with master to see how much it could be cut down. I could get working builds with the lines below: """ PACKAGECONFIG:remove:class-native = "vconsole xkbcommon sysvinit" PACKAGECONFIG:append:class-native = " serial-getty-generator" RDEPENDS:${PN}:remove:class-native = "volatile-binds" RRECOMMENDS:${PN}:remove:class-native = "os-release systemd-conf" RRECOMMENDS:${PN}-vconsole-setup:class-native = "" PACKAGECONFIG:remove:class-nativesdk = "vconsole xkbcommon sysvinit" PACKAGECONFIG:append:class-nativesdk = " serial-getty-generator" RDEPENDS:${PN}:remove:class-nativesdk = "volatile-binds" RRECOMMENDS:${PN}:remove:class-nativesdk = "os-release systemd-conf" RRECOMMENDS:${PN}-vconsole-setup:class-nativesdk = "" # Nothing picks up /var in the nativesdk case do_install:append:class-nativesdk () { rm -rf ${D}/var } BBCLASSEXTEND = "native nativesdk" """ which removes the need to change os-release, kbd, systemd-conf and systemd- getty. To merge, we'd want to restructure this to alter the variable construction so we can avoid the use of the remove operator but it is an easy way to test and evaluate the extent of changes needed. The above also nearly has native builds working as well. To get that to build I had to patch meson.build: Index: git/meson.build =================================================================== --- git.orig/meson.buildIndex: git/meson.build =================================================================== --- git.orig/meson.build +++ git/meson.build @@ -745,7 +745,7 @@ conf.set('CONTAINER_UID_BASE_MAX', conta nobody_user = get_option('nobody-user') nobody_group = get_option('nobody-group') -if not meson.is_cross_build() +if false and not meson.is_cross_build() getent_result = run_command('getent', 'passwd', '65534') if getent_result.returncode() == 0 name = getent_result.stdout().split(':')[0] since we want to use the "cross" codepath there regardless. That lets everything build but I did then see errors due to absolute path symlinks which would likely be fixable. I did this mainly as I wanted to understand how much of systemd is being build and packaged since many of these packages will not make sense in a SDK or a native build. I think the final piece of patch which we'd need to be able to merge something like this is to trim down what is being packaged up to the pieces which are actually useful in the native or nativesdk cases. Cheers, Richard
I have sort of a general question regarding this patch series. Last time I checked (and yeah it's been a while back) systemd-analyze wasn't self-containing, meaning it would have to have a running systemd process and at least a running dbus iirc. Is that still the case? If yes, how should that work here? Do we want to spawn a systemd per workspace/SDK? What about the weird setting that systemd somehow requires us to assign PID 1 to it? What about systems that have already a systemd instance running - and what about the systems that don't? And if not (and all of sudden systemd project finally decided to recognize the cross-compile use case), does this only apply to systemd-analyze? I would be happy if you could shed some light on these questions. Thx On 19.11.21 12:34, Luca Bocassi wrote: > From: Luca Boccassi <luca.boccassi@microsoft.com> > > The useradd class is a no-op in the nativesdk case, so chown will fail. > Skip them. > > Signed-off-by: Luca Boccassi <luca.boccassi@microsoft.com> > --- > v2: use "${PN}" = "${BPN}" as suggested by reviewers > > meta/recipes-core/systemd/systemd_249.5.bb | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/meta/recipes-core/systemd/systemd_249.5.bb b/meta/recipes-core/systemd/systemd_249.5.bb > index 8bdc0ca028..2df2de0cf3 100644 > --- a/meta/recipes-core/systemd/systemd_249.5.bb > +++ b/meta/recipes-core/systemd/systemd_249.5.bb > @@ -275,7 +275,10 @@ do_install() { > # which is expected to be empty. > rm -rf ${D}${localstatedir}/log > else > - chown root:systemd-journal ${D}${localstatedir}/log/journal > + # The useradd class is a no-op in the nativesdk case, so chown will fail > + if [ "${PN}" = "${BPN}" ]; then > + chown root:systemd-journal ${D}${localstatedir}/log/journal > + fi > > # journal-remote creates this at start > rm -rf ${D}${localstatedir}/log/journal/remote > @@ -319,7 +322,10 @@ do_install() { > if ${@bb.utils.contains('PACKAGECONFIG', 'polkit', 'true', 'false', d)}; then > if [ -d ${D}${datadir}/polkit-1/rules.d ]; then > chmod 700 ${D}${datadir}/polkit-1/rules.d > - chown polkitd:root ${D}${datadir}/polkit-1/rules.d > + # The useradd class is a no-op in the nativesdk case, so chown will fail > + if [ "${PN}" = "${BPN}" ]; then > + chown polkitd:root ${D}${datadir}/polkit-1/rules.d > + fi > fi > fi > > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#158507): https://lists.openembedded.org/g/openembedded-core/message/158507 > Mute This Topic: https://lists.openembedded.org/mt/87165491/3647476 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [kweihmann@outlook.com] > -=-=-=-=-=-=-=-=-=-=-=- >
Indeed; in the absence of tests that exercise this functionality - either SDK tests, or direct bitbake tests - it's hard to say if this is an experiment that may not be sustainable long term, or something systemd upstream is actually committed to. Alex On Mon, 22 Nov 2021 at 14:57, Konrad Weihmann <kweihmann@outlook.com> wrote: > I have sort of a general question regarding this patch series. > > Last time I checked (and yeah it's been a while back) systemd-analyze > wasn't self-containing, meaning it would have to have a running systemd > process and at least a running dbus iirc. > > Is that still the case? > If yes, how should that work here? > Do we want to spawn a systemd per workspace/SDK? > What about the weird setting that systemd somehow requires us to assign > PID 1 to it? > What about systems that have already a systemd instance running - and > what about the systems that don't? > > And if not (and all of sudden systemd project finally decided to > recognize the cross-compile use case), does this only apply to > systemd-analyze? > > I would be happy if you could shed some light on these questions. Thx > > On 19.11.21 12:34, Luca Bocassi wrote: > > From: Luca Boccassi <luca.boccassi@microsoft.com> > > > > The useradd class is a no-op in the nativesdk case, so chown will fail. > > Skip them. > > > > Signed-off-by: Luca Boccassi <luca.boccassi@microsoft.com> > > --- > > v2: use "${PN}" = "${BPN}" as suggested by reviewers > > > > meta/recipes-core/systemd/systemd_249.5.bb | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/meta/recipes-core/systemd/systemd_249.5.bb > b/meta/recipes-core/systemd/systemd_249.5.bb > > index 8bdc0ca028..2df2de0cf3 100644 > > --- a/meta/recipes-core/systemd/systemd_249.5.bb > > +++ b/meta/recipes-core/systemd/systemd_249.5.bb > > @@ -275,7 +275,10 @@ do_install() { > > # which is expected to be empty. > > rm -rf ${D}${localstatedir}/log > > else > > - chown root:systemd-journal ${D}${localstatedir}/log/journal > > + # The useradd class is a no-op in the nativesdk case, so > chown will fail > > + if [ "${PN}" = "${BPN}" ]; then > > + chown root:systemd-journal > ${D}${localstatedir}/log/journal > > + fi > > > > # journal-remote creates this at start > > rm -rf ${D}${localstatedir}/log/journal/remote > > @@ -319,7 +322,10 @@ do_install() { > > if ${@bb.utils.contains('PACKAGECONFIG', 'polkit', 'true', > 'false', d)}; then > > if [ -d ${D}${datadir}/polkit-1/rules.d ]; then > > chmod 700 ${D}${datadir}/polkit-1/rules.d > > - chown polkitd:root ${D}${datadir}/polkit-1/rules.d > > + # The useradd class is a no-op in the nativesdk > case, so chown will fail > > + if [ "${PN}" = "${BPN}" ]; then > > + chown polkitd:root > ${D}${datadir}/polkit-1/rules.d > > + fi > > fi > > fi > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#158569): > https://lists.openembedded.org/g/openembedded-core/message/158569 > Mute This Topic: https://lists.openembedded.org/mt/87165491/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
On Mon, 2021-11-22 at 14:57 +0100, Konrad Weihmann wrote: > I have sort of a general question regarding this patch series. > > Last time I checked (and yeah it's been a while back) systemd-analyze > wasn't self-containing, meaning it would have to have a running > systemd > process and at least a running dbus iirc. > > Is that still the case? That was the case only for some verbs - 'verify' does not need a running instance. From v250 (next release) it will also be able to operate on images/root directories. The 'security' verb did need a running instance, but there's a new -- offline switch in v250.
On 22.11.21 23:47, Luca Boccassi wrote: > On Mon, 2021-11-22 at 14:57 +0100, Konrad Weihmann wrote: >> I have sort of a general question regarding this patch series. >> >> Last time I checked (and yeah it's been a while back) systemd-analyze >> wasn't self-containing, meaning it would have to have a running >> systemd >> process and at least a running dbus iirc. >> >> Is that still the case? > > That was the case only for some verbs - 'verify' does not need a > running instance. From v250 (next release) it will also be able to > operate on images/root directories. > The 'security' verb did need a running instance, but there's a new -- > offline switch in v250. > Thanks for the background information. I kind of agree with the voices raised in the discussion so far. As the feature will be available only in a yet to be released version, I would propose to - wait for the release of systemd 2.50 - build a native variant of the tools first - package only what is really suitable for "offline" use - add a nativesdk variant later on And I think there should be some demo patch to use maybe the analyze part in insane bbclass or so, so we could first of all see the actual benefit of it and even more important can easily track down future regressions. BTW you said you tested this primary on dunfell, what kind of makes me think how these pieces fit together, as dunfell (likely) will never get the support for the tools mentioned
On Wed, 2021-11-24 at 09:09 +0100, Konrad Weihmann wrote: > > On 22.11.21 23:47, Luca Boccassi wrote: > > On Mon, 2021-11-22 at 14:57 +0100, Konrad Weihmann wrote: > > > I have sort of a general question regarding this patch series. > > > > > > Last time I checked (and yeah it's been a while back) systemd-analyze > > > wasn't self-containing, meaning it would have to have a running > > > systemd > > > process and at least a running dbus iirc. > > > > > > Is that still the case? > > > > That was the case only for some verbs - 'verify' does not need a > > running instance. From v250 (next release) it will also be able to > > operate on images/root directories. > > The 'security' verb did need a running instance, but there's a new -- > > offline switch in v250. > > > > Thanks for the background information. > I kind of agree with the voices raised in the discussion so far. > > As the feature will be available only in a yet to be released version, I > would propose to > > - wait for the release of systemd 2.50 > - build a native variant of the tools first > - package only what is really suitable for "offline" use > - add a nativesdk variant later on > > And I think there should be some demo patch to use maybe the analyze > part in insane bbclass or so, so we could first of all see the actual > benefit of it and even more important can easily track down future > regressions. As mentioned earlier, I have no use for 'native' variants and whatever an 'insane bbclass' is, so I will not be spending several weeks to make these things work, sorry. I've got no issue at all if you don't want to take this series, it's absolutely fine, all of these changes can be done via bbappend anyway. My experience of working with yocto software is so horribly painful and time-consuming that I'm not going to spend one minute more on it than I have to. > BTW you said you tested this primary on dunfell, what kind of makes me > think how these pieces fit together, as dunfell (likely) will never get > the support for the tools mentioned It's tested _only_ on dunfell. We forwarded the systemd recipe to v247 on top of it, and then additional patches including the ones for this functionality are backported. It's very easy to backport them, if you are curious you can see the whole tree here: https://github.com/bluca/systemd/commits/dunfell-msft-247
diff --git a/meta/recipes-core/systemd/systemd_249.5.bb b/meta/recipes-core/systemd/systemd_249.5.bb index 8bdc0ca028..2df2de0cf3 100644 --- a/meta/recipes-core/systemd/systemd_249.5.bb +++ b/meta/recipes-core/systemd/systemd_249.5.bb @@ -275,7 +275,10 @@ do_install() { # which is expected to be empty. rm -rf ${D}${localstatedir}/log else - chown root:systemd-journal ${D}${localstatedir}/log/journal + # The useradd class is a no-op in the nativesdk case, so chown will fail + if [ "${PN}" = "${BPN}" ]; then + chown root:systemd-journal ${D}${localstatedir}/log/journal + fi # journal-remote creates this at start rm -rf ${D}${localstatedir}/log/journal/remote @@ -319,7 +322,10 @@ do_install() { if ${@bb.utils.contains('PACKAGECONFIG', 'polkit', 'true', 'false', d)}; then if [ -d ${D}${datadir}/polkit-1/rules.d ]; then chmod 700 ${D}${datadir}/polkit-1/rules.d - chown polkitd:root ${D}${datadir}/polkit-1/rules.d + # The useradd class is a no-op in the nativesdk case, so chown will fail + if [ "${PN}" = "${BPN}" ]; then + chown polkitd:root ${D}${datadir}/polkit-1/rules.d + fi fi fi