Skip to content

Conversation

@keszybz
Copy link
Member

@keszybz keszybz commented May 16, 2022

rpms can be installed in two different modes: into a chroot, where the system
is not running, and onto a live system. In the first mode, where should create
all changes that are "permanent", and in the second mode, all changes which are
"permanent" but also those which only affect the running system. Thus, changes
like new modprobe rules, tmpfiles rules, binfmt rules, udev rules, etc., are
guarded by 'test -d "/run/systemd/system"' which is the official way to check
if systemd is running, so that they are not executed when installed into a
chroot. But the same logic does not apply to sysusers, hwdb, and the journal
catalog: all those files can and should result in changes being performed
immediately to the system. This makes the creation of immutable images possible
(because there are no permanent changes to executed after a reboot), and allows
other packages to depend on the the effect of those changes.

Thus, the guard to check if we're not in a chroot is dropped from triggers for
sysusers, hwdb, and the journal catalog. This means that those triggers will
execute, and no subsequent work is needed. systemd-sysusers.service,
systemd-journal-catalog-update.service, and systemd-hwdb-update.service.in all
have ConditionNeedsUpdate= so they they generally won't be invoked after a
reboot. (systemd.rpm does not touch /usr to trigger the condition, because the
%transfiletriggers make that unnecessary.)

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2085481

rpms can be installed in two different modes: into a chroot, where the system
is not running, and onto a live system. In the first mode, where should create
all changes that are "permanent", and in the second mode, all changes which are
"permanent" but also those which only affect the running system. Thus, changes
like new modprobe rules, tmpfiles rules, binfmt rules, udev rules, etc., are
guarded by 'test -d "/run/systemd/system"' which is the official way to check
if systemd is running, so that they are *not* executed when installed into a
chroot. But the same logic does not apply to sysusers, hwdb, and the journal
catalog: all those files can and should result in changes being performed
immediately to the system. This makes the creation of immutable images possible
(because there are no permanent changes to executed after a reboot), and allows
other packages to depend on the the effect of those changes.

Thus, the guard to check if we're not in a chroot is dropped from triggers for
sysusers, hwdb, and the journal catalog. This means that those triggers will
execute, and no subsequent work is needed. systemd-sysusers.service,
systemd-journal-catalog-update.service, and systemd-hwdb-update.service.in all
have ConditionNeedsUpdate= so they they generally won't be invoked after a
reboot. (systemd.rpm does not touch /usr to trigger the condition, because the
%transfiletriggers make that unnecessary.)

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2085481
@bluca bluca added the rpm label May 16, 2022
@bluca
Copy link
Member

bluca commented May 16, 2022

Is this safe enough for v251?

@keszybz
Copy link
Member Author

keszybz commented May 16, 2022

Yes. It's a clear bug fix.

I tested the build in rawhide and it now works as expected.

@bluca bluca added the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label May 16, 2022
@bluca bluca merged commit 2fd7ecd into systemd:main May 16, 2022
@keszybz keszybz deleted the rpm-update-conditionalization branch May 18, 2022 06:27
@keszybz
Copy link
Member Author

keszybz commented Jun 9, 2022

v249-stable now.

@keszybz keszybz removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed needs-stable-backport labels Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants