Merge pull request #164034 from helsinki-systems/clean/stc

nixos/switch-to-configuration: Major script cleanup

2 files changed, 378 insertions, 284 deletions

diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl
index 459d09faa53b5..9e5b760434a05 100755
--- a/nixos/modules/system/activation/switch-to-configuration.pl
+++ b/nixos/modules/system/activation/switch-to-configuration.pl
@@ -8,18 +8,29 @@ use File::Basename;
 use File::Slurp qw(read_file write_file edit_file);
 use Net::DBus;
 use Sys::Syslog qw(:standard :macros);
-use Cwd 'abs_path';
+use Cwd qw(abs_path);
 
+## no critic(ControlStructures::ProhibitDeepNests)
+## no critic(ErrorHandling::RequireCarping)
 ## no critic(CodeLayout::ProhibitParensWithBuiltins)
+## no critic(Variables::ProhibitPunctuationVars, Variables::RequireLocalizedPunctuationVars)
+## no critic(InputOutput::RequireCheckedSyscalls, InputOutput::RequireBracedFileHandleWithPrint, InputOutput::RequireBriefOpen)
+## no critic(ValuesAndExpressions::ProhibitNoisyQuotes, ValuesAndExpressions::ProhibitMagicNumbers, ValuesAndExpressions::ProhibitEmptyQuotes, ValuesAndExpressions::ProhibitInterpolationOfLiterals)
+## no critic(RegularExpressions::ProhibitEscapedMetacharacters)
 
+# System closure path to switch to
 my $out = "@out@";
-
-my $curSystemd = abs_path("/run/current-system/sw/bin");
+# Path to the directory containing systemd tools of the old system
+my $cur_systemd = abs_path("/run/current-system/sw/bin");
+# Path to the systemd store path of the new system
+my $new_systemd = "@systemd@";
 
 # To be robust against interruption, record what units need to be started etc.
-my $startListFile = "/run/nixos/start-list";
-my $restartListFile = "/run/nixos/restart-list";
-my $reloadListFile = "/run/nixos/reload-list";
+# We read these files again every time this script starts to make sure we continue
+# where the old (interrupted) script left off.
+my $start_list_file = "/run/nixos/start-list";
+my $restart_list_file = "/run/nixos/restart-list";
+my $reload_list_file = "/run/nixos/reload-list";
 
 # Parse restart/reload requests by the activation script.
 # Activation scripts may write newline-separated units to the restart
@@ -31,21 +42,23 @@ my $reloadListFile = "/run/nixos/reload-list";
 # The reload file asks the script to reload a unit. This is the same as
 # specifying a reload trigger in the NixOS module and can be ignored if
 # the unit is restarted in this activation.
-my $restartByActivationFile = "/run/nixos/activation-restart-list";
-my $reloadByActivationFile = "/run/nixos/activation-reload-list";
-my $dryRestartByActivationFile = "/run/nixos/dry-activation-restart-list";
-my $dryReloadByActivationFile = "/run/nixos/dry-activation-reload-list";
-
-make_path("/run/nixos", { mode => oct(755) });
+my $restart_by_activation_file = "/run/nixos/activation-restart-list";
+my $reload_by_activation_file = "/run/nixos/activation-reload-list";
+my $dry_restart_by_activation_file = "/run/nixos/dry-activation-restart-list";
+my $dry_reload_by_activation_file = "/run/nixos/dry-activation-reload-list";
 
+# The action that is to be performed (like switch, boot, test, dry-activate)
+# Also exposed via environment variable from now on
 my $action = shift(@ARGV);
+$ENV{NIXOS_ACTION} = $action;
 
+# Expose the locale archive as an environment variable for systemctl and the activation script
 if ("@localeArchive@" ne "") {
     $ENV{LOCALE_ARCHIVE} = "@localeArchive@";
 }
 
 if (!defined($action) || ($action ne "switch" && $action ne "boot" && $action ne "test" && $action ne "dry-activate")) {
-    print STDERR <<EOF;
+    print STDERR <<"EOF";
 Usage: $0 [switch|boot|test]
 
 switch:       make the configuration the boot default and activate now
@@ -56,37 +69,41 @@ EOF
     exit(1);
 }
 
-$ENV{NIXOS_ACTION} = $action;
-
 # This is a NixOS installation if it has /etc/NIXOS or a proper
 # /etc/os-release.
-die("This is not a NixOS installation!\n") unless
-    -f "/etc/NIXOS" || (read_file("/etc/os-release", err_mode => 'quiet') // "") =~ /ID="?nixos"?/s;
+if (!-f "/etc/NIXOS" && (read_file("/etc/os-release", err_mode => "quiet") // "") !~ /^ID="?nixos"?/msx) {
+    die("This is not a NixOS installation!\n");
+}
 
+make_path("/run/nixos", { mode => oct(755) });
 openlog("nixos", "", LOG_USER);
 
 # Install or update the bootloader.
 if ($action eq "switch" || $action eq "boot") {
-    chomp(my $installBootLoader = <<'EOFBOOTLOADER');
+    chomp(my $install_boot_loader = <<'EOFBOOTLOADER');
 @installBootLoader@
 EOFBOOTLOADER
-    system("$installBootLoader $out") == 0 or exit 1;
+    system("$install_boot_loader $out") == 0 or exit 1;
 }
 
 # Just in case the new configuration hangs the system, do a sync now.
-system("@coreutils@/bin/sync", "-f", "/nix/store") unless ($ENV{"NIXOS_NO_SYNC"} // "") eq "1";
+if (($ENV{"NIXOS_NO_SYNC"} // "") ne "1") {
+    system("@coreutils@/bin/sync", "-f", "/nix/store");
+}
 
-exit(0) if $action eq "boot";
+if ($action eq "boot") {
+    exit(0);
+}
 
 # Check if we can activate the new configuration.
-my $oldVersion = read_file("/run/current-system/init-interface-version", err_mode => 'quiet') // "";
-my $newVersion = read_file("$out/init-interface-version");
+my $cur_init_interface_version = read_file("/run/current-system/init-interface-version", err_mode => "quiet") // "";
+my $new_init_interface_version = read_file("$out/init-interface-version");
 
-if ($newVersion ne $oldVersion) {
-    print STDERR <<EOF;
+if ($new_init_interface_version ne $cur_init_interface_version) {
+    print STDERR <<'EOF';
 Warning: the new NixOS configuration has an ‘init’ that is
 incompatible with the current configuration.  The new configuration
-won\'t take effect until you reboot the system.
+won't take effect until you reboot the system.
 EOF
     exit(100);
 }
@@ -95,41 +112,56 @@ EOF
 # virtual console 1 and we restart the "tty1" unit.
 $SIG{PIPE} = "IGNORE";
 
-sub getActiveUnits {
+# Asks the currently running systemd instance via dbus which units are active.
+# Returns a hash where the key is the name of each unit and the value a hash
+# of load, state, substate.
+sub get_active_units {
     my $mgr = Net::DBus->system->get_service("org.freedesktop.systemd1")->get_object("/org/freedesktop/systemd1");
     my $units = $mgr->ListUnitsByPatterns([], []);
     my $res = {};
-    for my $item (@$units) {
+    for my $item (@{$units}) {
         my ($id, $description, $load_state, $active_state, $sub_state,
-            $following, $unit_path, $job_id, $job_type, $job_path) = @$item;
-        next unless $following eq '';
-        next if $job_id == 0 and $active_state eq 'inactive';
+            $following, $unit_path, $job_id, $job_type, $job_path) = @{$item};
+        if ($following ne "") {
+            next;
+        }
+        if ($job_id == 0 and $active_state eq "inactive") {
+            next;
+        }
         $res->{$id} = { load => $load_state, state => $active_state, substate => $sub_state };
     }
     return $res;
 }
 
-# Returns whether a systemd unit is active
+# Asks the currently running systemd instance whether a unit is currently active.
+# Takes the name of the unit as an argument and returns a bool whether the unit is active or not.
 sub unit_is_active {
     my ($unit_name) = @_;
 
-    my $mgr = Net::DBus->system->get_service('org.freedesktop.systemd1')->get_object('/org/freedesktop/systemd1');
+    my $mgr = Net::DBus->system->get_service("org.freedesktop.systemd1")->get_object("/org/freedesktop/systemd1");
     my $units = $mgr->ListUnitsByNames([$unit_name]);
     if (scalar(@{$units}) == 0) {
         return 0;
     }
-    my $active_state = $units->[0]->[3]; ## no critic (ValuesAndExpressions::ProhibitMagicNumbers)
-    return $active_state eq 'active' || $active_state eq 'activating';
+    my $active_state = $units->[0]->[3];
+    return $active_state eq "active" || $active_state eq "activating";
 }
 
-sub parseFstab {
+# Parse a fstab file, given its path.
+# Returns a tuple of filesystems and swaps.
+#
+# Filesystems is a hash of mountpoint and { device, fsType, options }
+# Swaps is a hash of device and { options }
+sub parse_fstab {
     my ($filename) = @_;
     my ($fss, $swaps);
-    foreach my $line (read_file($filename, err_mode => 'quiet')) {
+    foreach my $line (read_file($filename, err_mode => "quiet")) {
         chomp($line);
-        $line =~ s/^\s*#.*//;
-        next if $line =~ /^\s*$/;
-        my @xs = split(/ /, $line);
+        $line =~ s/^\s*\#.*//msx;
+        if ($line =~ /^\s*$/msx) {
+            next;
+        }
+        my @xs = split(/\s+/msx, $line);
         if ($xs[2] eq "swap") {
             $swaps->{$xs[0]} = { options => $xs[3] // "" };
         } else {
@@ -148,35 +180,35 @@ sub parseFstab {
 #
 # Instead of returning the hash, this subroutine takes a hashref to return the data in. This
 # allows calling the subroutine multiple times with the same hash to parse override files.
-sub parseSystemdIni {
-    my ($unitContents, $path) = @_;
+sub parse_systemd_ini {
+    my ($unit_contents, $path) = @_;
     # Tie the ini file to a hash for easier access
-    tie(my %fileContents, 'Config::IniFiles', (-file => $path, -allowempty => 1, -allowcontinue => 1)); ## no critic(Miscellanea::ProhibitTies)
+    tie(my %file_contents, "Config::IniFiles", (-file => $path, -allowempty => 1, -allowcontinue => 1)); ## no critic(Miscellanea::ProhibitTies)
 
     # Copy over all sections
-    foreach my $sectionName (keys(%fileContents)) {
-        if ($sectionName eq "Install") {
+    foreach my $section_name (keys(%file_contents)) {
+        if ($section_name eq "Install") {
             # Skip the [Install] section because it has no relevant keys for us
             next;
         }
         # Copy over all keys
-        foreach my $iniKey (keys(%{$fileContents{$sectionName}})) {
+        foreach my $ini_key (keys(%{$file_contents{$section_name}})) {
             # Ensure the value is an array so it's easier to work with
-            my $iniValue = $fileContents{$sectionName}{$iniKey};
-            my @iniValues;
-            if (ref($iniValue) eq "ARRAY") {
-                @iniValues = @{$iniValue};
+            my $ini_value = $file_contents{$section_name}{$ini_key};
+            my @ini_values;
+            if (ref($ini_value) eq "ARRAY") {
+                @ini_values = @{$ini_value};
             } else {
-                @iniValues = $iniValue;
+                @ini_values = $ini_value;
             }
             # Go over all values
-            for my $iniValue (@iniValues) {
+            for my $ini_value (@ini_values) {
                 # If a value is empty, it's an override that tells us to clean the value
-                if ($iniValue eq "") {
-                    delete $unitContents->{$sectionName}->{$iniKey};
+                if ($ini_value eq "") {
+                    delete $unit_contents->{$section_name}->{$ini_key};
                     next;
                 }
-                push(@{$unitContents->{$sectionName}->{$iniKey}}, $iniValue);
+                push(@{$unit_contents->{$section_name}->{$ini_key}}, $ini_value);
             }
         }
     }
@@ -185,7 +217,7 @@ sub parseSystemdIni {
 
 # This subroutine takes the path to a systemd configuration file (like a unit configuration),
 # parses it, and returns a hash that contains the contents. The contents of this hash are
-# explained in the `parseSystemdIni` subroutine. Neither the sections nor the keys inside
+# explained in the `parse_systemd_ini` subroutine. Neither the sections nor the keys inside
 # the sections are consistently sorted.
 #
 # If a directory with the same basename ending in .d exists next to the unit file, it will be
@@ -199,36 +231,44 @@ sub parse_unit {
     # Valid characters in unit names are ASCII letters, digits, ":", "-", "_", ".", and "\"
     $unit_path =~ s/\\/\\\\/gmsx;
     foreach (glob("${unit_path}{,.d/*.conf}")) {
-        parseSystemdIni(\%unit_data, "$_")
+        parse_systemd_ini(\%unit_data, "$_")
     }
     return %unit_data;
 }
 
 # Checks whether a specified boolean in a systemd unit is true
 # or false, with a default that is applied when the value is not set.
-sub parseSystemdBool {
-    my ($unitConfig, $sectionName, $boolName, $default) = @_;
+sub parse_systemd_bool {
+    my ($unit_config, $section_name, $bool_name, $default) = @_;
 
-    my @values = @{$unitConfig->{$sectionName}{$boolName} // []};
+    my @values = @{$unit_config->{$section_name}{$bool_name} // []};
     # Return default if value is not set
-    if (scalar(@values) lt 1 || not defined($values[-1])) {
+    if ((scalar(@values) < 1) || (not defined($values[-1]))) {
         return $default;
     }
     # If value is defined multiple times, use the last definition
-    my $last = $values[-1];
+    my $last_value = $values[-1];
     # These are valid values as of systemd.syntax(7)
-    return $last eq "1" || $last eq "yes" || $last eq "true" || $last eq "on";
+    return $last_value eq "1" || $last_value eq "yes" || $last_value eq "true" || $last_value eq "on";
 }
 
-sub recordUnit {
+# Writes a unit name into a given file to be more resilient against
+# crashes of the script. Does nothing when the action is dry-activate.
+sub record_unit {
     my ($fn, $unit) = @_;
-    write_file($fn, { append => 1 }, "$unit\n") if $action ne "dry-activate";
+    if ($action ne "dry-activate") {
+        write_file($fn, { append => 1 }, "$unit\n");
+    }
+    return;
 }
 
-# The opposite of recordUnit, removes a unit name from a file
+# The opposite of record_unit, removes a unit name from a file
 sub unrecord_unit {
     my ($fn, $unit) = @_;
-    edit_file(sub { s/^$unit\n//msx }, $fn) if $action ne "dry-activate";
+    if ($action ne "dry-activate") {
+        edit_file(sub { s/^$unit\n//msx }, $fn);
+    }
+    return;
 }
 
 # Compare the contents of two unit files and return whether the unit
@@ -240,8 +280,8 @@ sub unrecord_unit {
 # - 0 if the units are equal
 # - 1 if the units are different and a restart action is required
 # - 2 if the units are different and a reload action is required
-sub compare_units {
-    my ($old_unit, $new_unit) = @_;
+sub compare_units { ## no critic(Subroutines::ProhibitExcessComplexity)
+    my ($cur_unit, $new_unit) = @_;
     my $ret = 0;
     # Keys to ignore in the [Unit] section
     my %unit_section_ignores = map { $_ => 1 } qw(
@@ -262,13 +302,13 @@ sub compare_units {
     # Comparison hash for the sections
     my %section_cmp = map { $_ => 1 } keys(%{$new_unit});
     # Iterate over the sections
-    foreach my $section_name (keys(%{$old_unit})) {
+    foreach my $section_name (keys(%{$cur_unit})) {
         # Missing section in the new unit?
         if (not exists($section_cmp{$section_name})) {
             # If the [Unit] section was removed, make sure that only keys
             # were in it that are ignored
-            if ($section_name eq 'Unit') {
-                foreach my $ini_key (keys(%{$old_unit->{'Unit'}})) {
+            if ($section_name eq "Unit") {
+                foreach my $ini_key (keys(%{$cur_unit->{"Unit"}})) {
                     if (not defined($unit_section_ignores{$ini_key})) {
                         return 1;
                     }
@@ -277,7 +317,7 @@ sub compare_units {
             } else {
                 return 1;
             }
-            if ($section_name eq 'Unit' and %{$old_unit->{'Unit'}} == 1 and defined(%{$old_unit->{'Unit'}}{'X-Reload-Triggers'})) {
+            if ($section_name eq "Unit" and %{$cur_unit->{"Unit"}} == 1 and defined(%{$cur_unit->{"Unit"}}{"X-Reload-Triggers"})) {
                 # If a new [Unit] section was removed that only contained X-Reload-Triggers,
                 # do nothing.
                 next;
@@ -289,23 +329,23 @@ sub compare_units {
         # Comparison hash for the section contents
         my %ini_cmp = map { $_ => 1 } keys(%{$new_unit->{$section_name}});
         # Iterate over the keys of the section
-        foreach my $ini_key (keys(%{$old_unit->{$section_name}})) {
+        foreach my $ini_key (keys(%{$cur_unit->{$section_name}})) {
             delete $ini_cmp{$ini_key};
-            my @old_value = @{$old_unit->{$section_name}{$ini_key}};
+            my @cur_value = @{$cur_unit->{$section_name}{$ini_key}};
             # If the key is missing in the new unit, they are different...
             if (not $new_unit->{$section_name}{$ini_key}) {
                 # ... unless the key that is now missing is one of the ignored keys
-                if ($section_name eq 'Unit' and defined($unit_section_ignores{$ini_key})) {
+                if ($section_name eq "Unit" and defined($unit_section_ignores{$ini_key})) {
                     next;
                 }
                 return 1;
             }
             my @new_value = @{$new_unit->{$section_name}{$ini_key}};
             # If the contents are different, the units are different
-            if (not $comp_array->(\@old_value, \@new_value)) {
+            if (not $comp_array->(\@cur_value, \@new_value)) {
                 # Check if only the reload triggers changed or one of the ignored keys
-                if ($section_name eq 'Unit') {
-                    if ($ini_key eq 'X-Reload-Triggers') {
+                if ($section_name eq "Unit") {
+                    if ($ini_key eq "X-Reload-Triggers") {
                         $ret = 2;
                         next;
                     } elsif (defined($unit_section_ignores{$ini_key})) {
@@ -315,11 +355,11 @@ sub compare_units {
                 return 1;
             }
         }
-        # A key was introduced that was missing in the old unit
+        # A key was introduced that was missing in the previous unit
         if (%ini_cmp) {
-            if ($section_name eq 'Unit') {
+            if ($section_name eq "Unit") {
                 foreach my $ini_key (keys(%ini_cmp)) {
-                    if ($ini_key eq 'X-Reload-Triggers') {
+                    if ($ini_key eq "X-Reload-Triggers") {
                         $ret = 2;
                     } elsif (defined($unit_section_ignores{$ini_key})) {
                         next;
@@ -332,13 +372,13 @@ sub compare_units {
             }
         };
     }
-    # A section was introduced that was missing in the old unit
+    # A section was introduced that was missing in the previous unit
     if (%section_cmp) {
-        if (%section_cmp == 1 and defined($section_cmp{'Unit'})) {
-            foreach my $ini_key (keys(%{$new_unit->{'Unit'}})) {
+        if (%section_cmp == 1 and defined($section_cmp{"Unit"})) {
+            foreach my $ini_key (keys(%{$new_unit->{"Unit"}})) {
                 if (not defined($unit_section_ignores{$ini_key})) {
                     return 1;
-                } elsif ($ini_key eq 'X-Reload-Triggers') {
+                } elsif ($ini_key eq "X-Reload-Triggers") {
                     $ret = 2;
                 }
             }
@@ -350,76 +390,78 @@ sub compare_units {
     return $ret;
 }
 
-sub handleModifiedUnit {
-    my ($unit, $baseName, $newUnitFile, $newUnitInfo, $activePrev, $unitsToStop, $unitsToStart, $unitsToReload, $unitsToRestart, $unitsToSkip) = @_;
+# Called when a unit exists in both the old systemd and the new system and the units
+# differ. This figures out of what units are to be stopped, restarted, reloaded, started, and skipped.
+sub handle_modified_unit { ## no critic(Subroutines::ProhibitManyArgs, Subroutines::ProhibitExcessComplexity)
+    my ($unit, $base_name, $new_unit_file, $new_unit_info, $active_cur, $units_to_stop, $units_to_start, $units_to_reload, $units_to_restart, $units_to_skip) = @_;
 
-    if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target" || $unit =~ /\.path$/ || $unit =~ /\.slice$/) {
+    if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target" || $unit =~ /\.path$/msx || $unit =~ /\.slice$/msx) {
         # Do nothing.  These cannot be restarted directly.
 
         # Slices and Paths don't have to be restarted since
         # properties (resource limits and inotify watches)
         # seem to get applied on daemon-reload.
-    } elsif ($unit =~ /\.mount$/) {
+    } elsif ($unit =~ /\.mount$/msx) {
         # Reload the changed mount unit to force a remount.
         # FIXME: only reload when Options= changed, restart otherwise
-        $unitsToReload->{$unit} = 1;
-        recordUnit($reloadListFile, $unit);
-    } elsif ($unit =~ /\.socket$/) {
+        $units_to_reload->{$unit} = 1;
+        record_unit($reload_list_file, $unit);
+    } elsif ($unit =~ /\.socket$/msx) {
         # FIXME: do something?
         # Attempt to fix this: https://github.com/NixOS/nixpkgs/pull/141192
         # Revert of the attempt: https://github.com/NixOS/nixpkgs/pull/147609
         # More details: https://github.com/NixOS/nixpkgs/issues/74899#issuecomment-981142430
     } else {
-        my %unitInfo = $newUnitInfo ? %{$newUnitInfo} : parse_unit($newUnitFile);
-        if (parseSystemdBool(\%unitInfo, "Service", "X-ReloadIfChanged", 0) and not $unitsToRestart->{$unit} and not $unitsToStop->{$unit}) {
-            $unitsToReload->{$unit} = 1;
-            recordUnit($reloadListFile, $unit);
+        my %new_unit_info = $new_unit_info ? %{$new_unit_info} : parse_unit($new_unit_file);
+        if (parse_systemd_bool(\%new_unit_info, "Service", "X-ReloadIfChanged", 0) and not $units_to_restart->{$unit} and not $units_to_stop->{$unit}) {
+            $units_to_reload->{$unit} = 1;
+            record_unit($reload_list_file, $unit);
         }
-        elsif (!parseSystemdBool(\%unitInfo, "Service", "X-RestartIfChanged", 1) || parseSystemdBool(\%unitInfo, "Unit", "RefuseManualStop", 0) || parseSystemdBool(\%unitInfo, "Unit", "X-OnlyManualStart", 0)) {
-            $unitsToSkip->{$unit} = 1;
+        elsif (!parse_systemd_bool(\%new_unit_info, "Service", "X-RestartIfChanged", 1) || parse_systemd_bool(\%new_unit_info, "Unit", "RefuseManualStop", 0) || parse_systemd_bool(\%new_unit_info, "Unit", "X-OnlyManualStart", 0)) {
+            $units_to_skip->{$unit} = 1;
         } else {
             # It doesn't make sense to stop and start non-services because
             # they can't have ExecStop=
-            if (!parseSystemdBool(\%unitInfo, "Service", "X-StopIfChanged", 1) || $unit !~ /\.service$/) {
+            if (!parse_systemd_bool(\%new_unit_info, "Service", "X-StopIfChanged", 1) || $unit !~ /\.service$/msx) {
                 # This unit should be restarted instead of
                 # stopped and started.
-                $unitsToRestart->{$unit} = 1;
-                recordUnit($restartListFile, $unit);
+                $units_to_restart->{$unit} = 1;
+                record_unit($restart_list_file, $unit);
                 # Remove from units to reload so we don't restart and reload
-                if ($unitsToReload->{$unit}) {
-                    delete $unitsToReload->{$unit};
-                    unrecord_unit($reloadListFile, $unit);
+                if ($units_to_reload->{$unit}) {
+                    delete $units_to_reload->{$unit};
+                    unrecord_unit($reload_list_file, $unit);
                 }
             } else {
                 # If this unit is socket-activated, then stop the
                 # socket unit(s) as well, and restart the
                 # socket(s) instead of the service.
                 my $socket_activated = 0;
-                if ($unit =~ /\.service$/) {
-                    my @sockets = split(/ /, join(" ", @{$unitInfo{Service}{Sockets} // []}));
+                if ($unit =~ /\.service$/msx) {
+                    my @sockets = split(/\s+/msx, join(" ", @{$new_unit_info{Service}{Sockets} // []}));
                     if (scalar(@sockets) == 0) {
-                        @sockets = ("$baseName.socket");
+                        @sockets = ("$base_name.socket");
                     }
                     foreach my $socket (@sockets) {
-                        if (defined($activePrev->{$socket})) {
+                        if (defined($active_cur->{$socket})) {
                             # We can now be sure this is a socket-activate unit
 
-                            $unitsToStop->{$socket} = 1;
+                            $units_to_stop->{$socket} = 1;
                             # Only restart sockets that actually
                             # exist in new configuration:
                             if (-e "$out/etc/systemd/system/$socket") {
-                                $unitsToStart->{$socket} = 1;
-                                if ($unitsToStart eq $unitsToRestart) {
-                                    recordUnit($restartListFile, $socket);
+                                $units_to_start->{$socket} = 1;
+                                if ($units_to_start eq $units_to_restart) {
+                                    record_unit($restart_list_file, $socket);
                                 } else {
-                                    recordUnit($startListFile, $socket);
+                                    record_unit($start_list_file, $socket);
                                 }
                                 $socket_activated = 1;
                             }
                             # Remove from units to reload so we don't restart and reload
-                            if ($unitsToReload->{$unit}) {
-                                delete $unitsToReload->{$unit};
-                                unrecord_unit($reloadListFile, $unit);
+                            if ($units_to_reload->{$unit}) {
+                                delete $units_to_reload->{$unit};
+                                unrecord_unit($reload_list_file, $unit);
                             }
                         }
                     }
@@ -430,64 +472,67 @@ sub handleModifiedUnit {
                 # We write this to a file to ensure that the
                 # service gets restarted if we're interrupted.
                 if (!$socket_activated) {
-                    $unitsToStart->{$unit} = 1;
-                    if ($unitsToStart eq $unitsToRestart) {
-                        recordUnit($restartListFile, $unit);
+                    $units_to_start->{$unit} = 1;
+                    if ($units_to_start eq $units_to_restart) {
+                        record_unit($restart_list_file, $unit);
                     } else {
-                        recordUnit($startListFile, $unit);
+                        record_unit($start_list_file, $unit);
                     }
                 }
 
-                $unitsToStop->{$unit} = 1;
+                $units_to_stop->{$unit} = 1;
                 # Remove from units to reload so we don't restart and reload
-                if ($unitsToReload->{$unit}) {
-                    delete $unitsToReload->{$unit};
-                    unrecord_unit($reloadListFile, $unit);
+                if ($units_to_reload->{$unit}) {
+                    delete $units_to_reload->{$unit};
+                    unrecord_unit($reload_list_file, $unit);
                 }
             }
         }
     }
+    return;
 }
 
 # Figure out what units need to be stopped, started, restarted or reloaded.
-my (%unitsToStop, %unitsToSkip, %unitsToStart, %unitsToRestart, %unitsToReload);
+my (%units_to_stop, %units_to_skip, %units_to_start, %units_to_restart, %units_to_reload);
 
-my %unitsToFilter; # units not shown
+my %units_to_filter; # units not shown
 
-$unitsToStart{$_} = 1 foreach
-    split('\n', read_file($startListFile, err_mode => 'quiet') // "");
+%units_to_start = map { $_ => 1 }
+    split(/\n/msx, read_file($start_list_file, err_mode => "quiet") // "");
 
-$unitsToRestart{$_} = 1 foreach
-    split('\n', read_file($restartListFile, err_mode => 'quiet') // "");
+%units_to_restart = map { $_ => 1 }
+    split(/\n/msx, read_file($restart_list_file, err_mode => "quiet") // "");
 
-$unitsToReload{$_} = 1 foreach
-    split('\n', read_file($reloadListFile, err_mode => 'quiet') // "");
+%units_to_reload = map { $_ => 1 }
+    split(/\n/msx, read_file($reload_list_file, err_mode => "quiet") // "");
 
-my $activePrev = getActiveUnits();
-while (my ($unit, $state) = each(%{$activePrev})) {
-    my $baseUnit = $unit;
+my $active_cur = get_active_units();
+while (my ($unit, $state) = each(%{$active_cur})) {
+    my $base_unit = $unit;
 
-    my $prevUnitFile = "/etc/systemd/system/$baseUnit";
-    my $newUnitFile = "$out/etc/systemd/system/$baseUnit";
+    my $cur_unit_file = "/etc/systemd/system/$base_unit";
+    my $new_unit_file = "$out/etc/systemd/system/$base_unit";
 
     # Detect template instances.
-    if (!-e $prevUnitFile && !-e $newUnitFile && $unit =~ /^(.*)@[^\.]*\.(.*)$/) {
-      $baseUnit = "$1\@.$2";
-      $prevUnitFile = "/etc/systemd/system/$baseUnit";
-      $newUnitFile = "$out/etc/systemd/system/$baseUnit";
+    if (!-e $cur_unit_file && !-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/msx) {
+      $base_unit = "$1\@.$2";
+      $cur_unit_file = "/etc/systemd/system/$base_unit";
+      $new_unit_file = "$out/etc/systemd/system/$base_unit";
     }
 
-    my $baseName = $baseUnit;
-    $baseName =~ s/\.[a-z]*$//;
+    my $base_name = $base_unit;
+    $base_name =~ s/\.[[:lower:]]*$//msx;
 
-    if (-e $prevUnitFile && ($state->{state} eq "active" || $state->{state} eq "activating")) {
-        if (! -e $newUnitFile || abs_path($newUnitFile) eq "/dev/null") {
-            my %unitInfo = parse_unit($prevUnitFile);
-            $unitsToStop{$unit} = 1 if parseSystemdBool(\%unitInfo, "Unit", "X-StopOnRemoval", 1);
+    if (-e $cur_unit_file && ($state->{state} eq "active" || $state->{state} eq "activating")) {
+        if (! -e $new_unit_file || abs_path($new_unit_file) eq "/dev/null") {
+            my %cur_unit_info = parse_unit($cur_unit_file);
+            if (parse_systemd_bool(\%cur_unit_info, "Unit", "X-StopOnRemoval", 1)) {
+                $units_to_stop{$unit} = 1;
+            }
         }
 
-        elsif ($unit =~ /\.target$/) {
-            my %unitInfo = parse_unit($newUnitFile);
+        elsif ($unit =~ /\.target$/msx) {
+            my %new_unit_info = parse_unit($new_unit_file);
 
             # Cause all active target units to be restarted below.
             # This should start most changed units we stop here as
@@ -496,11 +541,11 @@ while (my ($unit, $state) = each(%{$activePrev})) {
             # active after the system has resumed, which probably
             # should not be the case.  Just ignore it.
             if ($unit ne "suspend.target" && $unit ne "hibernate.target" && $unit ne "hybrid-sleep.target") {
-                unless (parseSystemdBool(\%unitInfo, "Unit", "RefuseManualStart", 0) || parseSystemdBool(\%unitInfo, "Unit", "X-OnlyManualStart", 0)) {
-                    $unitsToStart{$unit} = 1;
-                    recordUnit($startListFile, $unit);
+                if (!(parse_systemd_bool(\%new_unit_info, "Unit", "RefuseManualStart", 0) || parse_systemd_bool(\%new_unit_info, "Unit", "X-OnlyManualStart", 0))) {
+                    $units_to_start{$unit} = 1;
+                    record_unit($start_list_file, $unit);
                     # Don't spam the user with target units that always get started.
-                    $unitsToFilter{$unit} = 1;
+                    $units_to_filter{$unit} = 1;
                 }
             }
 
@@ -515,33 +560,35 @@ while (my ($unit, $state) = each(%{$activePrev})) {
             # Stopping a target generally has no effect on other units
             # (unless there is a PartOf dependency), so this is just a
             # bookkeeping thing to get systemd to do the right thing.
-            if (parseSystemdBool(\%unitInfo, "Unit", "X-StopOnReconfiguration", 0)) {
-                $unitsToStop{$unit} = 1;
+            if (parse_systemd_bool(\%new_unit_info, "Unit", "X-StopOnReconfiguration", 0)) {
+                $units_to_stop{$unit} = 1;
             }
         }
 
         else {
-            my %old_unit_info = parse_unit($prevUnitFile);
-            my %new_unit_info = parse_unit($newUnitFile);
-            my $diff = compare_units(\%old_unit_info, \%new_unit_info);
+            my %cur_unit_info = parse_unit($cur_unit_file);
+            my %new_unit_info = parse_unit($new_unit_file);
+            my $diff = compare_units(\%cur_unit_info, \%new_unit_info);
             if ($diff == 1) {
-                handleModifiedUnit($unit, $baseName, $newUnitFile, \%new_unit_info, $activePrev, \%unitsToStop, \%unitsToStart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip);
-            } elsif ($diff == 2 and not $unitsToRestart{$unit}) {
-                $unitsToReload{$unit} = 1;
-                recordUnit($reloadListFile, $unit);
+                handle_modified_unit($unit, $base_name, $new_unit_file, \%new_unit_info, $active_cur, \%units_to_stop, \%units_to_start, \%units_to_reload, \%units_to_restart, \%units_to_skip);
+            } elsif ($diff == 2 and not $units_to_restart{$unit}) {
+                $units_to_reload{$unit} = 1;
+                record_unit($reload_list_file, $unit);
             }
         }
     }
 }
 
-sub pathToUnitName {
+# Converts a path to the name of a systemd mount unit that would be responsible
+# for mounting this path.
+sub path_to_unit_name {
     my ($path) = @_;
     # Use current version of systemctl binary before daemon is reexeced.
-    open(my $cmd, "-|", "$curSystemd/systemd-escape", "--suffix=mount", "-p", $path)
+    open(my $cmd, "-|", "$cur_systemd/systemd-escape", "--suffix=mount", "-p", $path)
         or die "Unable to escape $path!\n";
-    my $escaped = join("", <$cmd>);
+    my $escaped = do { local $/ = undef; <$cmd> };
     chomp($escaped);
-    close($cmd) or die('Unable to close systemd-escape pipe');
+    close($cmd) or die("Unable to close systemd-escape pipe");
     return $escaped;
 }
 
@@ -550,31 +597,31 @@ sub pathToUnitName {
 # automatically by starting local-fs.target.  FIXME: might be nicer if
 # we generated units for all mounts; then we could unify this with the
 # unit checking code above.
-my ($prevFss, $prevSwaps) = parseFstab("/etc/fstab");
-my ($newFss, $newSwaps) = parseFstab("$out/etc/fstab");
-foreach my $mountPoint (keys(%$prevFss)) {
-    my $prev = $prevFss->{$mountPoint};
-    my $new = $newFss->{$mountPoint};
-    my $unit = pathToUnitName($mountPoint);
+my ($cur_fss, $cur_swaps) = parse_fstab("/etc/fstab");
+my ($new_fss, $new_swaps) = parse_fstab("$out/etc/fstab");
+foreach my $mount_point (keys(%{$cur_fss})) {
+    my $cur = $cur_fss->{$mount_point};
+    my $new = $new_fss->{$mount_point};
+    my $unit = path_to_unit_name($mount_point);
     if (!defined($new)) {
         # Filesystem entry disappeared, so unmount it.
-        $unitsToStop{$unit} = 1;
-    } elsif ($prev->{fsType} ne $new->{fsType} || $prev->{device} ne $new->{device}) {
+        $units_to_stop{$unit} = 1;
+    } elsif ($cur->{fsType} ne $new->{fsType} || $cur->{device} ne $new->{device}) {
         # Filesystem type or device changed, so unmount and mount it.
-        $unitsToStop{$unit} = 1;
-        $unitsToStart{$unit} = 1;
-        recordUnit($startListFile, $unit);
-    } elsif ($prev->{options} ne $new->{options}) {
+        $units_to_stop{$unit} = 1;
+        $units_to_start{$unit} = 1;
+        record_unit($start_list_file, $unit);
+    } elsif ($cur->{options} ne $new->{options}) {
         # Mount options changes, so remount it.
-        $unitsToReload{$unit} = 1;
-        recordUnit($reloadListFile, $unit);
+        $units_to_reload{$unit} = 1;
+        record_unit($reload_list_file, $unit);
     }
 }
 
 # Also handles swap devices.
-foreach my $device (keys(%$prevSwaps)) {
-    my $prev = $prevSwaps->{$device};
-    my $new = $newSwaps->{$device};
+foreach my $device (keys(%{$cur_swaps})) {
+    my $cur = $cur_swaps->{$device};
+    my $new = $new_swaps->{$device};
     if (!defined($new)) {
         # Swap entry disappeared, so turn it off.  Can't use
         # "systemctl stop" here because systemd has lots of alias
@@ -592,97 +639,109 @@ foreach my $device (keys(%$prevSwaps)) {
 
 
 # Should we have systemd re-exec itself?
-my $prevSystemd = abs_path("/proc/1/exe") // "/unknown";
-my $prevSystemdSystemConfig = abs_path("/etc/systemd/system.conf") // "/unknown";
-my $newSystemd = abs_path("@systemd@/lib/systemd/systemd") or die;
-my $newSystemdSystemConfig = abs_path("$out/etc/systemd/system.conf") // "/unknown";
+my $cur_pid1_path = abs_path("/proc/1/exe") // "/unknown";
+my $cur_systemd_system_config = abs_path("/etc/systemd/system.conf") // "/unknown";
+my $new_pid1_path = abs_path("$new_systemd/lib/systemd/systemd") or die;
+my $new_systemd_system_config = abs_path("$out/etc/systemd/system.conf") // "/unknown";
 
-my $restartSystemd = $prevSystemd ne $newSystemd;
-if ($prevSystemdSystemConfig ne $newSystemdSystemConfig) {
-    $restartSystemd = 1;
+my $restart_systemd = $cur_pid1_path ne $new_pid1_path;
+if ($cur_systemd_system_config ne $new_systemd_system_config) {
+    $restart_systemd = 1;
 }
 
-
-sub filterUnits {
+# Takes an array of unit names and returns an array with the same elements,
+# except all units that are also in the global variable `unitsToFilter`.
+sub filter_units {
     my ($units) = @_;
     my @res;
     foreach my $unit (sort(keys(%{$units}))) {
-        push(@res, $unit) if !defined($unitsToFilter{$unit});
+        if (!defined($units_to_filter{$unit})) {
+            push(@res, $unit);
+        }
     }
     return @res;
 }
 
-my @unitsToStopFiltered = filterUnits(\%unitsToStop);
+my @units_to_stop_filtered = filter_units(\%units_to_stop);
 
 
 # Show dry-run actions.
 if ($action eq "dry-activate") {
-    print STDERR "would stop the following units: ", join(", ", @unitsToStopFiltered), "\n"
-        if scalar(@unitsToStopFiltered) > 0;
-    print STDERR "would NOT stop the following changed units: ", join(", ", sort(keys(%unitsToSkip))), "\n"
-        if scalar(keys(%unitsToSkip)) > 0;
+    if (scalar(@units_to_stop_filtered) > 0) {
+        print STDERR "would stop the following units: ", join(", ", @units_to_stop_filtered), "\n";
+    }
+    if (scalar(keys(%units_to_skip)) > 0) {
+        print STDERR "would NOT stop the following changed units: ", join(", ", sort(keys(%units_to_skip))), "\n";
+    }
 
     print STDERR "would activate the configuration...\n";
     system("$out/dry-activate", "$out");
 
     # Handle the activation script requesting the restart or reload of a unit.
-    foreach (split('\n', read_file($dryRestartByActivationFile, err_mode => 'quiet') // "")) {
+    foreach (split(/\n/msx, read_file($dry_restart_by_activation_file, err_mode => "quiet") // "")) {
         my $unit = $_;
-        my $baseUnit = $unit;
-        my $newUnitFile = "$out/etc/systemd/system/$baseUnit";
+        my $base_unit = $unit;
+        my $new_unit_file = "$out/etc/systemd/system/$base_unit";
 
         # Detect template instances.
-        if (!-e $newUnitFile && $unit =~ /^(.*)@[^\.]*\.(.*)$/) {
-          $baseUnit = "$1\@.$2";
-          $newUnitFile = "$out/etc/systemd/system/$baseUnit";
+        if (!-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/msx) {
+          $base_unit = "$1\@.$2";
+          $new_unit_file = "$out/etc/systemd/system/$base_unit";
         }
 
-        my $baseName = $baseUnit;
-        $baseName =~ s/\.[a-z]*$//;
+        my $base_name = $base_unit;
+        $base_name =~ s/\.[[:lower:]]*$//msx;
 
         # Start units if they were not active previously
-        if (not defined($activePrev->{$unit})) {
-            $unitsToStart{$unit} = 1;
+        if (not defined($active_cur->{$unit})) {
+            $units_to_start{$unit} = 1;
             next;
         }
 
-        handleModifiedUnit($unit, $baseName, $newUnitFile, undef, $activePrev, \%unitsToRestart, \%unitsToRestart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip);
+        handle_modified_unit($unit, $base_name, $new_unit_file, undef, $active_cur, \%units_to_restart, \%units_to_restart, \%units_to_reload, \%units_to_restart, \%units_to_skip);
     }
-    unlink($dryRestartByActivationFile);
+    unlink($dry_restart_by_activation_file);
 
-    foreach (split('\n', read_file($dryReloadByActivationFile, err_mode => 'quiet') // "")) {
+    foreach (split(/\n/msx, read_file($dry_reload_by_activation_file, err_mode => "quiet") // "")) {
         my $unit = $_;
 
-        if (defined($activePrev->{$unit}) and not $unitsToRestart{$unit} and not $unitsToStop{$unit}) {
-            $unitsToReload{$unit} = 1;
-            recordUnit($reloadListFile, $unit);
+        if (defined($active_cur->{$unit}) and not $units_to_restart{$unit} and not $units_to_stop{$unit}) {
+            $units_to_reload{$unit} = 1;
+            record_unit($reload_list_file, $unit);
         }
     }
-    unlink($dryReloadByActivationFile);
+    unlink($dry_reload_by_activation_file);
 
-    print STDERR "would restart systemd\n" if $restartSystemd;
-    print STDERR "would reload the following units: ", join(", ", sort(keys(%unitsToReload))), "\n"
-        if scalar(keys(%unitsToReload)) > 0;
-    print STDERR "would restart the following units: ", join(", ", sort(keys(%unitsToRestart))), "\n"
-        if scalar(keys(%unitsToRestart)) > 0;
-    my @unitsToStartFiltered = filterUnits(\%unitsToStart);
-    print STDERR "would start the following units: ", join(", ", @unitsToStartFiltered), "\n"
-        if scalar(@unitsToStartFiltered);
+    if ($restart_systemd) {
+        print STDERR "would restart systemd\n";
+    }
+    if (scalar(keys(%units_to_reload)) > 0) {
+        print STDERR "would reload the following units: ", join(", ", sort(keys(%units_to_reload))), "\n";
+    }
+    if (scalar(keys(%units_to_restart)) > 0) {
+        print STDERR "would restart the following units: ", join(", ", sort(keys(%units_to_restart))), "\n";
+    }
+    my @units_to_start_filtered = filter_units(\%units_to_start);
+    if (scalar(@units_to_start_filtered)) {
+        print STDERR "would start the following units: ", join(", ", @units_to_start_filtered), "\n";
+    }
     exit 0;
 }
 
 
 syslog(LOG_NOTICE, "switching to system configuration $out");
 
-if (scalar(keys(%unitsToStop)) > 0) {
-    print STDERR "stopping the following units: ", join(", ", @unitsToStopFiltered), "\n"
-        if scalar(@unitsToStopFiltered);
+if (scalar(keys(%units_to_stop)) > 0) {
+    if (scalar(@units_to_stop_filtered)) {
+        print STDERR "stopping the following units: ", join(", ", @units_to_stop_filtered), "\n";
+    }
     # Use current version of systemctl binary before daemon is reexeced.
-    system("$curSystemd/systemctl", "stop", "--", sort(keys(%unitsToStop)));
+    system("$cur_systemd/systemctl", "stop", "--", sort(keys(%units_to_stop)));
 }
 
-print STDERR "NOT restarting the following changed units: ", join(", ", sort(keys(%unitsToSkip))), "\n"
-    if scalar(keys(%unitsToSkip)) > 0;
+if (scalar(keys(%units_to_skip)) > 0) {
+    print STDERR "NOT restarting the following changed units: ", join(", ", sort(keys(%units_to_skip))), "\n";
+}
 
 # Activate the new configuration (i.e., update /etc, make accounts,
 # and so on).
@@ -691,108 +750,110 @@ print STDERR "activating the configuration...\n";
 system("$out/activate", "$out") == 0 or $res = 2;
 
 # Handle the activation script requesting the restart or reload of a unit.
-foreach (split('\n', read_file($restartByActivationFile, err_mode => 'quiet') // "")) {
+foreach (split(/\n/msx, read_file($restart_by_activation_file, err_mode => "quiet") // "")) {
     my $unit = $_;
-    my $baseUnit = $unit;
-    my $newUnitFile = "$out/etc/systemd/system/$baseUnit";
+    my $base_unit = $unit;
+    my $new_unit_file = "$out/etc/systemd/system/$base_unit";
 
     # Detect template instances.
-    if (!-e $newUnitFile && $unit =~ /^(.*)@[^\.]*\.(.*)$/) {
-      $baseUnit = "$1\@.$2";
-      $newUnitFile = "$out/etc/systemd/system/$baseUnit";
+    if (!-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/msx) {
+      $base_unit = "$1\@.$2";
+      $new_unit_file = "$out/etc/systemd/system/$base_unit";
     }
 
-    my $baseName = $baseUnit;
-    $baseName =~ s/\.[a-z]*$//;
+    my $base_name = $base_unit;
+    $base_name =~ s/\.[[:lower:]]*$//msx;
 
     # Start units if they were not active previously
-    if (not defined($activePrev->{$unit})) {
-        $unitsToStart{$unit} = 1;
-        recordUnit($startListFile, $unit);
+    if (not defined($active_cur->{$unit})) {
+        $units_to_start{$unit} = 1;
+        record_unit($start_list_file, $unit);
         next;
     }
 
-    handleModifiedUnit($unit, $baseName, $newUnitFile, undef, $activePrev, \%unitsToRestart, \%unitsToRestart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip);
+    handle_modified_unit($unit, $base_name, $new_unit_file, undef, $active_cur, \%units_to_restart, \%units_to_restart, \%units_to_reload, \%units_to_restart, \%units_to_skip);
 }
 # We can remove the file now because it has been propagated to the other restart/reload files
-unlink($restartByActivationFile);
+unlink($restart_by_activation_file);
 
-foreach (split('\n', read_file($reloadByActivationFile, err_mode => 'quiet') // "")) {
+foreach (split(/\n/msx, read_file($reload_by_activation_file, err_mode => "quiet") // "")) {
     my $unit = $_;
 
-    if (defined($activePrev->{$unit}) and not $unitsToRestart{$unit} and not $unitsToStop{$unit}) {
-        $unitsToReload{$unit} = 1;
-        recordUnit($reloadListFile, $unit);
+    if (defined($active_cur->{$unit}) and not $units_to_restart{$unit} and not $units_to_stop{$unit}) {
+        $units_to_reload{$unit} = 1;
+        record_unit($reload_list_file, $unit);
     }
 }
 # We can remove the file now because it has been propagated to the other reload file
-unlink($reloadByActivationFile);
+unlink($reload_by_activation_file);
 
 # Restart systemd if necessary. Note that this is done using the
 # current version of systemd, just in case the new one has trouble
 # communicating with the running pid 1.
-if ($restartSystemd) {
+if ($restart_systemd) {
     print STDERR "restarting systemd...\n";
-    system("$curSystemd/systemctl", "daemon-reexec") == 0 or $res = 2;
+    system("$cur_systemd/systemctl", "daemon-reexec") == 0 or $res = 2;
 }
 
 # Forget about previously failed services.
-system("@systemd@/bin/systemctl", "reset-failed");
+system("$new_systemd/bin/systemctl", "reset-failed");
 
 # Make systemd reload its units.
-system("@systemd@/bin/systemctl", "daemon-reload") == 0 or $res = 3;
+system("$new_systemd/bin/systemctl", "daemon-reload") == 0 or $res = 3;
 
 # Reload user units
-open(my $listActiveUsers, '-|', '@systemd@/bin/loginctl', 'list-users', '--no-legend');
-while (my $f = <$listActiveUsers>) {
-    next unless $f =~ /^\s*(?<uid>\d+)\s+(?<user>\S+)/;
+open(my $list_active_users, "-|", "$new_systemd/bin/loginctl", "list-users", "--no-legend") || die("Unable to call loginctl");
+while (my $f = <$list_active_users>) {
+    if ($f !~ /^\s*(?<uid>\d+)\s+(?<user>\S+)/msx) {
+        next;
+    }
     my ($uid, $name) = ($+{uid}, $+{user});
     print STDERR "reloading user units for $name...\n";
 
     system("@su@", "-s", "@shell@", "-l", $name, "-c",
            "export XDG_RUNTIME_DIR=/run/user/$uid; " .
-           "$curSystemd/systemctl --user daemon-reexec; " .
-           "@systemd@/bin/systemctl --user start nixos-activation.service");
+           "$cur_systemd/systemctl --user daemon-reexec; " .
+           "$new_systemd/bin/systemctl --user start nixos-activation.service");
 }
 
-close($listActiveUsers);
+close($list_active_users) || die("Unable to close the file handle to loginctl");
 
 # Set the new tmpfiles
 print STDERR "setting up tmpfiles\n";
-system("@systemd@/bin/systemd-tmpfiles", "--create", "--remove", "--exclude-prefix=/dev") == 0 or $res = 3;
+system("$new_systemd/bin/systemd-tmpfiles", "--create", "--remove", "--exclude-prefix=/dev") == 0 or $res = 3;
 
 # Before reloading we need to ensure that the units are still active. They may have been
 # deactivated because one of their requirements got stopped. If they are inactive
 # but should have been reloaded, the user probably expects them to be started.
-if (scalar(keys(%unitsToReload)) > 0) {
-    for my $unit (keys(%unitsToReload)) {
+if (scalar(keys(%units_to_reload)) > 0) {
+    for my $unit (keys(%units_to_reload)) {
         if (!unit_is_active($unit)) {
             # Figure out if we need to start the unit
             my %unit_info = parse_unit("$out/etc/systemd/system/$unit");
-            if (!(parseSystemdBool(\%unit_info, 'Unit', 'RefuseManualStart', 0) || parseSystemdBool(\%unit_info, 'Unit', 'X-OnlyManualStart', 0))) {
-                $unitsToStart{$unit} = 1;
-                recordUnit($startListFile, $unit);
+            if (!(parse_systemd_bool(\%unit_info, "Unit", "RefuseManualStart", 0) || parse_systemd_bool(\%unit_info, "Unit", "X-OnlyManualStart", 0))) {
+                $units_to_start{$unit} = 1;
+                record_unit($start_list_file, $unit);
             }
             # Don't reload the unit, reloading would fail
-            delete %unitsToReload{$unit};
-            unrecord_unit($reloadListFile, $unit);
+            delete %units_to_reload{$unit};
+            unrecord_unit($reload_list_file, $unit);
         }
     }
 }
 # Reload units that need it. This includes remounting changed mount
 # units.
-if (scalar(keys(%unitsToReload)) > 0) {
-    print STDERR "reloading the following units: ", join(", ", sort(keys(%unitsToReload))), "\n";
-    system("@systemd@/bin/systemctl", "reload", "--", sort(keys(%unitsToReload))) == 0 or $res = 4;
-    unlink($reloadListFile);
+if (scalar(keys(%units_to_reload)) > 0) {
+    print STDERR "reloading the following units: ", join(", ", sort(keys(%units_to_reload))), "\n";
+    system("$new_systemd/bin/systemctl", "reload", "--", sort(keys(%units_to_reload))) == 0 or $res = 4;
+    unlink($reload_list_file);
 }
 
 # Restart changed services (those that have to be restarted rather
 # than stopped and started).
-if (scalar(keys(%unitsToRestart)) > 0) {
-    print STDERR "restarting the following units: ", join(", ", sort(keys(%unitsToRestart))), "\n";
-    system("@systemd@/bin/systemctl", "restart", "--", sort(keys(%unitsToRestart))) == 0 or $res = 4;
-    unlink($restartListFile);
+if (scalar(keys(%units_to_restart)) > 0) {
+    print STDERR "restarting the following units: ", join(", ", sort(keys(%units_to_restart))), "\n";
+    system("$new_systemd/bin/systemctl", "restart", "--", sort(keys(%units_to_restart))) == 0 or $res = 4;
+    unlink($restart_list_file);
 }
 
 # Start all active targets, as well as changed units we stopped above.
@@ -801,17 +862,18 @@ if (scalar(keys(%unitsToRestart)) > 0) {
 # that are symlinks to other units.  We shouldn't start both at the
 # same time because we'll get a "Failed to add path to set" error from
 # systemd.
-my @unitsToStartFiltered = filterUnits(\%unitsToStart);
-print STDERR "starting the following units: ", join(", ", @unitsToStartFiltered), "\n"
-    if scalar(@unitsToStartFiltered);
-system("@systemd@/bin/systemctl", "start", "--", sort(keys(%unitsToStart))) == 0 or $res = 4;
-unlink($startListFile);
+my @units_to_start_filtered = filter_units(\%units_to_start);
+if (scalar(@units_to_start_filtered)) {
+    print STDERR "starting the following units: ", join(", ", @units_to_start_filtered), "\n"
+}
+system("$new_systemd/bin/systemctl", "start", "--", sort(keys(%units_to_start))) == 0 or $res = 4;
+unlink($start_list_file);
 
 
 # Print failed and new units.
 my (@failed, @new);
-my $activeNew = getActiveUnits();
-while (my ($unit, $state) = each(%{$activeNew})) {
+my $active_new = get_active_units();
+while (my ($unit, $state) = each(%{$active_new})) {
     if ($state->{state} eq "failed") {
         push(@failed, $unit);
         next;
@@ -819,7 +881,9 @@ while (my ($unit, $state) = each(%{$activeNew})) {
 
     if ($state->{substate} eq "auto-restart") {
         # A unit in auto-restart substate is a failure *if* it previously failed to start
-        my $main_status = `@systemd@/bin/systemctl show --value --property=ExecMainStatus '$unit'`;
+        open(my $main_status_fd, "-|", "$new_systemd/bin/systemctl", "show", "--value", "--property=ExecMainStatus", $unit) || die("Unable to call 'systemctl show'");
+        my $main_status = do { local $/ = undef; <$main_status_fd> };
+        close($main_status_fd) || die("Unable to close 'systemctl show' fd");
         chomp($main_status);
 
         if ($main_status ne "0") {
@@ -831,7 +895,7 @@ while (my ($unit, $state) = each(%{$activeNew})) {
     # Ignore scopes since they are not managed by this script but rather
     # created and managed by third-party services via the systemd dbus API.
     # This only lists units that are not failed (including ones that are in auto-restart but have not failed previously)
-    if ($state->{state} ne "failed" && !defined($activePrev->{$unit}) && $unit !~ /\.scope$/msx) {
+    if ($state->{state} ne "failed" && !defined($active_cur->{$unit}) && $unit !~ /\.scope$/msx) {
         push(@new, $unit);
     }
 }
@@ -843,7 +907,7 @@ if (scalar(@new) > 0) {
 if (scalar(@failed) > 0) {
     my @failed_sorted = sort(@failed);
     print STDERR "warning: the following units failed: ", join(", ", @failed_sorted), "\n\n";
-    system("@systemd@/bin/systemctl status --no-pager --full '" . join("' '", @failed_sorted) . "' >&2");
+    system("$new_systemd/bin/systemctl status --no-pager --full '" . join("' '", @failed_sorted) . "' >&2");
     $res = 4;
 }
 
diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix
index 93eee4babc2de..0198866b6ff83 100644
--- a/nixos/tests/switch-test.nix
+++ b/nixos/tests/switch-test.nix
@@ -51,6 +51,12 @@ in {
       environment.systemPackages = [ pkgs.socat ]; # for the socket activation stuff
       users.mutableUsers = false;
 
+      # For boot/switch testing
+      system.build.installBootLoader = lib.mkForce (pkgs.writeShellScript "install-dummy-loader" ''
+        echo "installing dummy bootloader"
+        touch /tmp/bootloader-installed
+      '');
+
       specialisation = rec {
         simpleService.configuration = {
           systemd.services.test = {
@@ -502,10 +508,33 @@ in {
     machine.succeed(
         "${stderrRunner} ${originalSystem}/bin/switch-to-configuration test"
     )
+    # This tests whether the /etc/os-release parser works which is a fallback
+    # when /etc/NIXOS is missing. If the parser does not work, switch-to-configuration
+    # would fail.
+    machine.succeed("rm /etc/NIXOS")
     machine.succeed(
         "${stderrRunner} ${otherSystem}/bin/switch-to-configuration test"
     )
 
+
+    with subtest("actions"):
+        # boot action
+        machine.fail("test -f /tmp/bootloader-installed")
+        out = switch_to_specialisation("${machine}", "simpleService", action="boot")
+        assert_contains(out, "installing dummy bootloader")
+        assert_lacks(out, "activating the configuration...")  # good indicator of a system activation
+        machine.succeed("test -f /tmp/bootloader-installed")
+        machine.succeed("rm /tmp/bootloader-installed")
+
+        # switch action
+        machine.fail("test -f /tmp/bootloader-installed")
+        out = switch_to_specialisation("${machine}", "", action="switch")
+        assert_contains(out, "installing dummy bootloader")
+        assert_contains(out, "activating the configuration...")  # good indicator of a system activation
+        machine.succeed("test -f /tmp/bootloader-installed")
+
+        # test and dry-activate actions are tested further down below
+
     with subtest("services"):
         switch_to_specialisation("${machine}", "")
         # Nothing happens when nothing is changed
@@ -519,6 +548,7 @@ in {
 
         # Start a simple service
         out = switch_to_specialisation("${machine}", "simpleService")
+        assert_lacks(out, "installing dummy bootloader")  # test does not install a bootloader
         assert_lacks(out, "stopping the following units:")
         assert_lacks(out, "NOT restarting the following changed units:")
         assert_contains(out, "reloading the following units: dbus.service\n")  # huh