Control: tags -1 + patch

Hello again.

On Tue, Mar 01, 2016 at 05:42:19PM +0100, Andreas Henriksson wrote:
> Hello!
> 
> I'm attaching a completely untested attempt at prefering "systemctl
> preset" over just enabling unconditionally.
[...]

Ok, I actually tested it now and except for a minor mistake (added '== 0'
to 'system(...) == 0 or error "...";') it actually seems to work.
Updated patch attached.

Tested under both sysvinit-core (with systemctl available = systemd
package installed) and under systemd-sysv (pid1 = systemd).
(Under sysvinit-core without systemctl/systemd package installed is
just assumed to still work since it's all old code as usual then.)

Testcases used was hdparm (ships init script but no native service file)
and fancontrol (ships both init script and native service file).

Please note that perl warns about the smartmatch being experimental.
Someone should probably replace that part with a better check....

Please also note that if you actually add a preset file which disables
the service (eg. echo "disable *" >> \
/lib/systemd/system-preset/test.preset ) you'll get confusing message
(from systemctl preset) saying the service file has no Install section.

Anyway, it works! Ship it! ;)

Regards,
Andreas Henriksson
From: Andreas Henriksson <[email protected]>
Subject: hacky attempt at prefering systemctl presets over enable.

https://bugs.debian.org/772555

diff --git a/script/deb-systemd-helper b/script/deb-systemd-helper
index 2a87cb4..a8aa834 100755
--- a/script/deb-systemd-helper
+++ b/script/deb-systemd-helper
@@ -218,11 +218,56 @@ sub get_link_closure {
     return @links;
 }
 
+sub is_enabled {
+    my ($scriptname, $service_path) = @_;
+
+    my @links = get_link_closure($scriptname, $service_path);
+    my @missing_links = grep { ! -l $_->{src} } @links;
+
+    return (@missing_links == 0);
+}
+
 sub make_systemd_links {
     my ($scriptname, $service_path) = @_;
 
     my $dsh_state = dsh_state_path($scriptname);
+    my $has_preset = 0;
+
+    # prefer using systemctl preset, but if that's not available
+    # we use our own built-in hardcoded-to-enable fallback.
+    if (-x "/bin/systemctl") {
+        debug "Using systemctl preset to enable $scriptname";
+        $has_preset = 1;
+        # Note: two consecutive is_enabled() will not properly
+        # tell us if something changed. We should compare the
+        # result from two get_link_closure() instead.
+        #my $prev_enabled = is_enabled($scriptname, $service_path);
+        my @prev_links = get_link_closure($scriptname, $service_path);
+        system("/bin/systemctl", "--preset-mode=enable-only",
+            "preset", $scriptname) == 0 or
+            error("systemctl preset failed on $scriptname: $!");
+        #my $now_enabled = is_enabled($scriptname, $service_path);
+        my @links = get_link_closure($scriptname, $service_path);
+        #$changed_sth = ($prev_enabled != $now_enabled);
+        # N.B. Trying a "Smart Matching operator" posing like I actually
+        # know perl or if this even does the right thing.
+        $changed_sth = !(@prev_links ~~ @links);
+        # FIXME: We should really update statefile here when we still
+        # know something about the changed links instead of badly
+        # piggy-backing on the fallback which has no idea.
+        return unless $changed_sth;
+    }
 
+    # This fallback has dual purpose, if we used preset above
+    # it is still used to record links to dsh statefile.
+    #
+    # Note: this does not properly track state when using preset.
+    # (Consider the case where administrator has manually enabled
+    # bar.service. You're now installing foo(.service) which contains
+    # an Also=bar.service, which never got touched since it was already
+    # enabled before running the preset. It'll now be tracked as if
+    # we just enabled bar and will be disabled when uninstalling foo.
+    # OTOH, was this case even properly handled before?!)
     my @links = get_link_closure($scriptname, $service_path);
     for my $link (@links) {
         my $service_path = $link->{dest};
@@ -234,7 +279,7 @@ sub make_systemd_links {
         $statefile =~ s,^/etc/systemd/system/,$enabled_state_dir/,;
         next if -e $statefile;
 
-        if (! -l $service_link) {
+        if (!$has_preset && ! -l $service_link) {
             make_path(dirname($service_link));
             symlink($service_path, $service_link) or
                 error("unable to link $service_link to $service_path: $!");
@@ -472,9 +517,7 @@ for my $scriptname (@ARGV) {
     debug "action = $action, scriptname = $scriptname, service_path = $service_path";
 
     if ($action eq 'is-enabled') {
-        my @links = get_link_closure($scriptname, $service_path);
-        my @missing_links = grep { ! -l $_->{src} } @links;
-        my $enabled = (@missing_links == 0);
+        my $enabled = is_enabled($scriptname, $service_path);
         print STDERR ($enabled ? "enabled\n" : "disabled\n") unless $quiet;
         $rc = 0 if $enabled;
     }
_______________________________________________
Pkg-systemd-maintainers mailing list
[email protected]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-systemd-maintainers

Reply via email to