On Tue, Oct 19, 2021 at 07:48:03PM -0000, Chad Smith wrote: > For the comment about "the other checks are either superfluous, or > actively harmful."
> I'm not certain how this section will be harmful here, if we are in > chroots, we should be skipping this section in general because our > initial `if [ -d /run/systemd/system ]` check. If this code happens to > run in a chroot w/ run/systemd/system present we also avoid non-zero > exit in the failure case if systemctl or deb-systremd-helper error. The point is that, if the system is *currently* mounted as a chroot, and this postinst runs, it will not to disable the new service in accordance with the admin's choice regarding the old service. If furthermore in the course of normal operation this is a booted system and not a chroot, this means that on next boot (and subsequently), the new service will be started when the intent was that it not be. This is an unlikely scenario in the grand scheme of things, but is not impossible. If someone has disabled this service; and the instance ends up in a broken state, so the user mounts the root drive from another instance, chroots into it, and runs package updates as part of fixing it; then you could have exactly this situation. So not applying the "disable" policy when running the postinst in a chroot is a bug. We should avoid introducing bugs in SRUs when possible, however corner case. > I presumed this was also why debhelper snippets is automatically adding > the similar check prior to invoking `systemctl ...`. debhelper doesn't wrap calls to 'deb-systemd-helper enable' with checks for whether the system currently has a running systemd. The 'deb-systemd-helper enable' and 'deb-systemd-helper disable' calls are explicitly intended to update the state of the unit even if systemd is not currently running. > Are you suggesting we don't use systemctl or deb-systemd-helper tools > directly and instead just attempt to unlink systemd timer/service link > files to ensure we continue to disable ua-timer.timer if ua- > messaging.timer is not represented as enabled? That would be an option, but seems unnecessary vs just calling 'deb-systemd-helper disable $UA_TIMER_NAME > /dev/null 2>&1 || true' here. On Fri, Oct 15, 2021 at 05:59:56PM -0000, Grant Orndorff wrote: > 4. Running deb-systemd-helper enable is confusing. > The use of enable was intentional. We attempted to explain in the comment > above that line. The reason is that we want to take advantage of d-s-h's > autosnippet for enabling/updating config for ua-timer.timer which appends > the following to the bottom of postinst: > # Automatically added by dh_systemd_enable > # This will only remove masks created by d-s-h on package removal. > deb-systemd-helper unmask ua-timer.timer >/dev/null || true > # was-enabled defaults to true, so new installations run enable. > if deb-systemd-helper --quiet was-enabled ua-timer.timer; then > # Enables the unit on first installation, creates new > # symlinks on upgrades if the unit file has changed. > deb-systemd-helper enable ua-timer.timer >/dev/null || true > else > # Update the statefile to add new symlinks (if any), which need to be > # cleaned up on purge. Also remove old symlinks. > deb-systemd-helper update-state ua-timer.timer >/dev/null || true > fi > # End automatically added section Ok, now I see - but that means deb-systemd-helper's auxiliary state will be permanently out of sync with the systemd state, I think? Also, this dh_systemd_enable snippet calls deb-systemd-helper enable if 'was-enabled' was true, but I don't see that you want to call enable again, as this is either a no-op, or it will enable the service which we explicitly are aiming to disable. In what case do you see incorrect behavior by calling (the more obviously named) 'deb-systemd-helper disable'? -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developer https://www.debian.org/ slanga...@ubuntu.com vor...@debian.org -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1942929 Title: [SRU] ubuntu-advantage-tools (27.2.2 -> 27.3) Xenial, Bionic, Focal, Hirsute To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/ubuntu-advantage-tools/+bug/1942929/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
