From 5c00fe6b1bfba588f4e9c7fdcc9de8c3b7b191f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 13 Mar 2022 19:09:06 +0100 Subject: [PATCH 1/7] nixos/switchTest: Also test the os-release parser --- nixos/tests/switch-test.nix | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix index 93eee4babc2..4136aa336ac 100644 --- a/nixos/tests/switch-test.nix +++ b/nixos/tests/switch-test.nix @@ -502,6 +502,10 @@ 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" ) From bdcd55881231b43e3e72393aace962ce32ae9909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 13 Mar 2022 19:15:08 +0100 Subject: [PATCH 2/7] nixos/switch-to-configuration: Get rid of all postfixes and unlesses --- .../activation/switch-to-configuration.pl | 97 ++++++++++++------- 1 file changed, 64 insertions(+), 33 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 459d09faa53..9bf24694cae 100755 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -60,8 +60,9 @@ $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"?/s) { + die("This is not a NixOS installation!\n"); +} openlog("nixos", "", LOG_USER); @@ -74,9 +75,13 @@ EOFBOOTLOADER } # 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') // ""; @@ -102,8 +107,12 @@ sub getActiveUnits { 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'; + 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; @@ -128,7 +137,9 @@ sub parseFstab { foreach my $line (read_file($filename, err_mode => 'quiet')) { chomp($line); $line =~ s/^\s*#.*//; - next if $line =~ /^\s*$/; + if ($line =~ /^\s*$/) { + next; + } my @xs = split(/ /, $line); if ($xs[2] eq "swap") { $swaps->{$xs[0]} = { options => $xs[3] // "" }; @@ -222,13 +233,17 @@ sub parseSystemdBool { sub recordUnit { 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"); + } } # The opposite of recordUnit, 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); + } } # Compare the contents of two unit files and return whether the unit @@ -454,13 +469,13 @@ my (%unitsToStop, %unitsToSkip, %unitsToStart, %unitsToRestart, %unitsToReload); my %unitsToFilter; # units not shown -$unitsToStart{$_} = 1 foreach +%unitsToStart = map { $_ => 1 } split('\n', read_file($startListFile, err_mode => 'quiet') // ""); -$unitsToRestart{$_} = 1 foreach +%unitsToRestart = map { $_ => 1 } split('\n', read_file($restartListFile, err_mode => 'quiet') // ""); -$unitsToReload{$_} = 1 foreach +%unitsToReload = map { $_ => 1 } split('\n', read_file($reloadListFile, err_mode => 'quiet') // ""); my $activePrev = getActiveUnits(); @@ -483,7 +498,9 @@ while (my ($unit, $state) = each(%{$activePrev})) { 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 (parseSystemdBool(\%unitInfo, "Unit", "X-StopOnRemoval", 1)) { + $unitsToStop{$unit} = 1; + } } elsif ($unit =~ /\.target$/) { @@ -496,7 +513,7 @@ 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)) { + if (!(parseSystemdBool(\%unitInfo, 'Unit', 'RefuseManualStart', 0) || parseSystemdBool(\%unitInfo, 'Unit', 'X-OnlyManualStart', 0))) { $unitsToStart{$unit} = 1; recordUnit($startListFile, $unit); # Don't spam the user with target units that always get started. @@ -607,7 +624,9 @@ sub filterUnits { my ($units) = @_; my @res; foreach my $unit (sort(keys(%{$units}))) { - push(@res, $unit) if !defined($unitsToFilter{$unit}); + if (!defined($unitsToFilter{$unit})) { + push(@res, $unit); + } } return @res; } @@ -617,10 +636,12 @@ my @unitsToStopFiltered = filterUnits(\%unitsToStop); # 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(@unitsToStopFiltered) > 0) { + print STDERR "would stop the following units: ", join(", ", @unitsToStopFiltered), "\n"; + } + if (scalar(keys(%unitsToSkip)) > 0) { + print STDERR "would NOT stop the following changed units: ", join(", ", sort(keys(%unitsToSkip))), "\n"; + } print STDERR "would activate the configuration...\n"; system("$out/dry-activate", "$out"); @@ -660,14 +681,19 @@ if ($action eq "dry-activate") { } unlink($dryReloadByActivationFile); - 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; + if ($restartSystemd) { + print STDERR "would restart systemd\n"; + } + if (scalar(keys(%unitsToReload)) > 0) { + print STDERR "would reload the following units: ", join(", ", sort(keys(%unitsToReload))), "\n"; + } + if (scalar(keys(%unitsToRestart)) > 0) { + print STDERR "would restart the following units: ", join(", ", sort(keys(%unitsToRestart))), "\n"; + } my @unitsToStartFiltered = filterUnits(\%unitsToStart); - print STDERR "would start the following units: ", join(", ", @unitsToStartFiltered), "\n" - if scalar(@unitsToStartFiltered); + if (scalar(@unitsToStartFiltered)) { + print STDERR "would start the following units: ", join(", ", @unitsToStartFiltered), "\n"; + } exit 0; } @@ -675,14 +701,16 @@ if ($action eq "dry-activate") { 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(@unitsToStopFiltered)) { + print STDERR "stopping the following units: ", join(", ", @unitsToStopFiltered), "\n"; + } # Use current version of systemctl binary before daemon is reexeced. system("$curSystemd/systemctl", "stop", "--", sort(keys(%unitsToStop))); } -print STDERR "NOT restarting the following changed units: ", join(", ", sort(keys(%unitsToSkip))), "\n" - if scalar(keys(%unitsToSkip)) > 0; +if (scalar(keys(%unitsToSkip)) > 0) { + print STDERR "NOT restarting the following changed units: ", join(", ", sort(keys(%unitsToSkip))), "\n"; +} # Activate the new configuration (i.e., update /etc, make accounts, # and so on). @@ -745,7 +773,9 @@ system("@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*(?\d+)\s+(?\S+)/; + if ($f !~ /^\s*(?\d+)\s+(?\S+)/) { + next; + } my ($uid, $name) = ($+{uid}, $+{user}); print STDERR "reloading user units for $name...\n"; @@ -802,8 +832,9 @@ if (scalar(keys(%unitsToRestart)) > 0) { # 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); +if (scalar(@unitsToStartFiltered)) { + print STDERR "starting the following units: ", join(", ", @unitsToStartFiltered), "\n" +} system("@systemd@/bin/systemctl", "start", "--", sort(keys(%unitsToStart))) == 0 or $res = 4; unlink($startListFile); From 23ea9965bbe51ec506518fd5084a8648b78aa32c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 13 Mar 2022 19:21:15 +0100 Subject: [PATCH 3/7] nixos/switch-to-configuration: Enforce consistent quoting --- .../activation/switch-to-configuration.pl | 73 ++++++++++--------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 9bf24694cae..bffa3e2d711 100755 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -1,5 +1,8 @@ #! @perl@/bin/perl +## no critic(CodeLayout::ProhibitParensWithBuiltins) +## no critic(ValuesAndExpressions::ProhibitInterpolationOfLiterals) + use strict; use warnings; use Config::IniFiles; @@ -8,9 +11,7 @@ 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'; - -## no critic(CodeLayout::ProhibitParensWithBuiltins) +use Cwd "abs_path"; my $out = "@out@"; @@ -60,7 +61,7 @@ $ENV{NIXOS_ACTION} = $action; # This is a NixOS installation if it has /etc/NIXOS or a proper # /etc/os-release. -if (!-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"?/s) { die("This is not a NixOS installation!\n"); } @@ -79,12 +80,12 @@ if (($ENV{"NIXOS_NO_SYNC"} // "") ne "1") { system("@coreutils@/bin/sync", "-f", "/nix/store"); } -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 $oldVersion = read_file("/run/current-system/init-interface-version", err_mode => "quiet") // ""; my $newVersion = read_file("$out/init-interface-version"); if ($newVersion ne $oldVersion) { @@ -107,10 +108,10 @@ sub getActiveUnits { for my $item (@$units) { my ($id, $description, $load_state, $active_state, $sub_state, $following, $unit_path, $job_id, $job_type, $job_path) = @$item; - if ($following ne '') { + if ($following ne "") { next; } - if ($job_id == 0 and $active_state eq 'inactive') { + if ($job_id == 0 and $active_state eq "inactive") { next; } $res->{$id} = { load => $load_state, state => $active_state, substate => $sub_state }; @@ -122,19 +123,19 @@ sub getActiveUnits { 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'; + return $active_state eq "active" || $active_state eq "activating"; } sub parseFstab { 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*#.*//; if ($line =~ /^\s*$/) { @@ -162,7 +163,7 @@ sub parseFstab { sub parseSystemdIni { my ($unitContents, $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 %fileContents, "Config::IniFiles", (-file => $path, -allowempty => 1, -allowcontinue => 1)); ## no critic(Miscellanea::ProhibitTies) # Copy over all sections foreach my $sectionName (keys(%fileContents)) { @@ -233,7 +234,7 @@ sub parseSystemdBool { sub recordUnit { my ($fn, $unit) = @_; - if ($action ne 'dry-activate') { + if ($action ne "dry-activate") { write_file($fn, { append => 1 }, "$unit\n"); } } @@ -241,7 +242,7 @@ sub recordUnit { # The opposite of recordUnit, removes a unit name from a file sub unrecord_unit { my ($fn, $unit) = @_; - if ($action ne 'dry-activate') { + if ($action ne "dry-activate") { edit_file(sub { s/^$unit\n//msx }, $fn); } } @@ -282,8 +283,8 @@ sub compare_units { 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(%{$old_unit->{"Unit"}})) { if (not defined($unit_section_ignores{$ini_key})) { return 1; } @@ -292,7 +293,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 %{$old_unit->{"Unit"}} == 1 and defined(%{$old_unit->{"Unit"}}{"X-Reload-Triggers"})) { # If a new [Unit] section was removed that only contained X-Reload-Triggers, # do nothing. next; @@ -310,7 +311,7 @@ sub compare_units { # 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; @@ -319,8 +320,8 @@ sub compare_units { # If the contents are different, the units are different if (not $comp_array->(\@old_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})) { @@ -332,9 +333,9 @@ sub compare_units { } # A key was introduced that was missing in the old 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; @@ -349,11 +350,11 @@ sub compare_units { } # A section was introduced that was missing in the old 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; } } @@ -470,13 +471,13 @@ my (%unitsToStop, %unitsToSkip, %unitsToStart, %unitsToRestart, %unitsToReload); my %unitsToFilter; # units not shown %unitsToStart = map { $_ => 1 } - split('\n', read_file($startListFile, err_mode => 'quiet') // ""); + split("\n", read_file($startListFile, err_mode => "quiet") // ""); %unitsToRestart = map { $_ => 1 } - split('\n', read_file($restartListFile, err_mode => 'quiet') // ""); + split("\n", read_file($restartListFile, err_mode => "quiet") // ""); %unitsToReload = map { $_ => 1 } - split('\n', read_file($reloadListFile, err_mode => 'quiet') // ""); + split("\n", read_file($reloadListFile, err_mode => "quiet") // ""); my $activePrev = getActiveUnits(); while (my ($unit, $state) = each(%{$activePrev})) { @@ -513,7 +514,7 @@ 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") { - if (!(parseSystemdBool(\%unitInfo, 'Unit', 'RefuseManualStart', 0) || parseSystemdBool(\%unitInfo, 'Unit', 'X-OnlyManualStart', 0))) { + if (!(parseSystemdBool(\%unitInfo, "Unit", "RefuseManualStart", 0) || parseSystemdBool(\%unitInfo, "Unit", "X-OnlyManualStart", 0))) { $unitsToStart{$unit} = 1; recordUnit($startListFile, $unit); # Don't spam the user with target units that always get started. @@ -558,7 +559,7 @@ sub pathToUnitName { or die "Unable to escape $path!\n"; my $escaped = join("", <$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; } @@ -647,7 +648,7 @@ if ($action eq "dry-activate") { 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", read_file($dryRestartByActivationFile, err_mode => "quiet") // "")) { my $unit = $_; my $baseUnit = $unit; my $newUnitFile = "$out/etc/systemd/system/$baseUnit"; @@ -671,7 +672,7 @@ if ($action eq "dry-activate") { } unlink($dryRestartByActivationFile); - foreach (split('\n', read_file($dryReloadByActivationFile, err_mode => 'quiet') // "")) { + foreach (split("\n", read_file($dryReloadByActivationFile, err_mode => "quiet") // "")) { my $unit = $_; if (defined($activePrev->{$unit}) and not $unitsToRestart{$unit} and not $unitsToStop{$unit}) { @@ -719,7 +720,7 @@ 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", read_file($restartByActivationFile, err_mode => "quiet") // "")) { my $unit = $_; my $baseUnit = $unit; my $newUnitFile = "$out/etc/systemd/system/$baseUnit"; @@ -745,7 +746,7 @@ foreach (split('\n', read_file($restartByActivationFile, err_mode => 'quiet') // # We can remove the file now because it has been propagated to the other restart/reload files unlink($restartByActivationFile); -foreach (split('\n', read_file($reloadByActivationFile, err_mode => 'quiet') // "")) { +foreach (split("\n", read_file($reloadByActivationFile, err_mode => "quiet") // "")) { my $unit = $_; if (defined($activePrev->{$unit}) and not $unitsToRestart{$unit} and not $unitsToStop{$unit}) { @@ -771,7 +772,7 @@ system("@systemd@/bin/systemctl", "reset-failed"); system("@systemd@/bin/systemctl", "daemon-reload") == 0 or $res = 3; # Reload user units -open(my $listActiveUsers, '-|', '@systemd@/bin/loginctl', 'list-users', '--no-legend'); +open(my $listActiveUsers, "-|", "@systemd@/bin/loginctl", "list-users", "--no-legend"); while (my $f = <$listActiveUsers>) { if ($f !~ /^\s*(?\d+)\s+(?\S+)/) { next; @@ -799,7 +800,7 @@ if (scalar(keys(%unitsToReload)) > 0) { 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))) { + if (!(parseSystemdBool(\%unit_info, "Unit", "RefuseManualStart", 0) || parseSystemdBool(\%unit_info, "Unit", "X-OnlyManualStart", 0))) { $unitsToStart{$unit} = 1; recordUnit($startListFile, $unit); } From 67f84b4b87d3a1df5fc0f23e7fe5db52b5ac030b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 13 Mar 2022 19:37:06 +0100 Subject: [PATCH 4/7] nixos/switch-to-configuration: Snake-case all subroutines and add comments --- .../activation/switch-to-configuration.pl | 108 ++++++++++-------- 1 file changed, 62 insertions(+), 46 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index bffa3e2d711..aeeb60dcd68 100755 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -101,7 +101,10 @@ 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 = {}; @@ -119,7 +122,8 @@ sub getActiveUnits { 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) = @_; @@ -132,7 +136,12 @@ sub unit_is_active { 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")) { @@ -160,7 +169,7 @@ 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 { +sub parse_systemd_ini { my ($unitContents, $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) @@ -197,7 +206,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 @@ -211,14 +220,14 @@ 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 { +sub parse_systemd_bool { my ($unitConfig, $sectionName, $boolName, $default) = @_; my @values = @{$unitConfig->{$sectionName}{$boolName} // []}; @@ -232,14 +241,16 @@ sub parseSystemdBool { return $last eq "1" || $last eq "yes" || $last eq "true" || $last 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) = @_; if ($action ne "dry-activate") { write_file($fn, { append => 1 }, "$unit\n"); } } -# 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) = @_; if ($action ne "dry-activate") { @@ -366,7 +377,9 @@ sub compare_units { return $ret; } -sub handleModifiedUnit { +# 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 { my ($unit, $baseName, $newUnitFile, $newUnitInfo, $activePrev, $unitsToStop, $unitsToStart, $unitsToReload, $unitsToRestart, $unitsToSkip) = @_; if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target" || $unit =~ /\.path$/ || $unit =~ /\.slice$/) { @@ -379,7 +392,7 @@ sub handleModifiedUnit { # Reload the changed mount unit to force a remount. # FIXME: only reload when Options= changed, restart otherwise $unitsToReload->{$unit} = 1; - recordUnit($reloadListFile, $unit); + record_unit($reloadListFile, $unit); } elsif ($unit =~ /\.socket$/) { # FIXME: do something? # Attempt to fix this: https://github.com/NixOS/nixpkgs/pull/141192 @@ -387,20 +400,20 @@ sub handleModifiedUnit { # 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}) { + if (parse_systemd_bool(\%unitInfo, "Service", "X-ReloadIfChanged", 0) and not $unitsToRestart->{$unit} and not $unitsToStop->{$unit}) { $unitsToReload->{$unit} = 1; - recordUnit($reloadListFile, $unit); + record_unit($reloadListFile, $unit); } - elsif (!parseSystemdBool(\%unitInfo, "Service", "X-RestartIfChanged", 1) || parseSystemdBool(\%unitInfo, "Unit", "RefuseManualStop", 0) || parseSystemdBool(\%unitInfo, "Unit", "X-OnlyManualStart", 0)) { + elsif (!parse_systemd_bool(\%unitInfo, "Service", "X-RestartIfChanged", 1) || parse_systemd_bool(\%unitInfo, "Unit", "RefuseManualStop", 0) || parse_systemd_bool(\%unitInfo, "Unit", "X-OnlyManualStart", 0)) { $unitsToSkip->{$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(\%unitInfo, "Service", "X-StopIfChanged", 1) || $unit !~ /\.service$/) { # This unit should be restarted instead of # stopped and started. $unitsToRestart->{$unit} = 1; - recordUnit($restartListFile, $unit); + record_unit($restartListFile, $unit); # Remove from units to reload so we don't restart and reload if ($unitsToReload->{$unit}) { delete $unitsToReload->{$unit}; @@ -426,9 +439,9 @@ sub handleModifiedUnit { if (-e "$out/etc/systemd/system/$socket") { $unitsToStart->{$socket} = 1; if ($unitsToStart eq $unitsToRestart) { - recordUnit($restartListFile, $socket); + record_unit($restartListFile, $socket); } else { - recordUnit($startListFile, $socket); + record_unit($startListFile, $socket); } $socket_activated = 1; } @@ -448,9 +461,9 @@ sub handleModifiedUnit { if (!$socket_activated) { $unitsToStart->{$unit} = 1; if ($unitsToStart eq $unitsToRestart) { - recordUnit($restartListFile, $unit); + record_unit($restartListFile, $unit); } else { - recordUnit($startListFile, $unit); + record_unit($startListFile, $unit); } } @@ -479,7 +492,7 @@ my %unitsToFilter; # units not shown %unitsToReload = map { $_ => 1 } split("\n", read_file($reloadListFile, err_mode => "quiet") // ""); -my $activePrev = getActiveUnits(); +my $activePrev = get_active_units(); while (my ($unit, $state) = each(%{$activePrev})) { my $baseUnit = $unit; @@ -499,7 +512,7 @@ while (my ($unit, $state) = each(%{$activePrev})) { 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); - if (parseSystemdBool(\%unitInfo, "Unit", "X-StopOnRemoval", 1)) { + if (parse_systemd_bool(\%unitInfo, "Unit", "X-StopOnRemoval", 1)) { $unitsToStop{$unit} = 1; } } @@ -514,9 +527,9 @@ 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") { - if (!(parseSystemdBool(\%unitInfo, "Unit", "RefuseManualStart", 0) || parseSystemdBool(\%unitInfo, "Unit", "X-OnlyManualStart", 0))) { + if (!(parse_systemd_bool(\%unitInfo, "Unit", "RefuseManualStart", 0) || parse_systemd_bool(\%unitInfo, "Unit", "X-OnlyManualStart", 0))) { $unitsToStart{$unit} = 1; - recordUnit($startListFile, $unit); + record_unit($startListFile, $unit); # Don't spam the user with target units that always get started. $unitsToFilter{$unit} = 1; } @@ -533,7 +546,7 @@ 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)) { + if (parse_systemd_bool(\%unitInfo, "Unit", "X-StopOnReconfiguration", 0)) { $unitsToStop{$unit} = 1; } } @@ -543,16 +556,18 @@ while (my ($unit, $state) = each(%{$activePrev})) { my %new_unit_info = parse_unit($newUnitFile); my $diff = compare_units(\%old_unit_info, \%new_unit_info); if ($diff == 1) { - handleModifiedUnit($unit, $baseName, $newUnitFile, \%new_unit_info, $activePrev, \%unitsToStop, \%unitsToStart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip); + handle_modified_unit($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); + record_unit($reloadListFile, $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) @@ -568,12 +583,12 @@ 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"); +my ($prevFss, $prevSwaps) = parse_fstab("/etc/fstab"); +my ($newFss, $newSwaps) = parse_fstab("$out/etc/fstab"); foreach my $mountPoint (keys(%$prevFss)) { my $prev = $prevFss->{$mountPoint}; my $new = $newFss->{$mountPoint}; - my $unit = pathToUnitName($mountPoint); + my $unit = path_to_unit_name($mountPoint); if (!defined($new)) { # Filesystem entry disappeared, so unmount it. $unitsToStop{$unit} = 1; @@ -581,11 +596,11 @@ foreach my $mountPoint (keys(%$prevFss)) { # Filesystem type or device changed, so unmount and mount it. $unitsToStop{$unit} = 1; $unitsToStart{$unit} = 1; - recordUnit($startListFile, $unit); + record_unit($startListFile, $unit); } elsif ($prev->{options} ne $new->{options}) { # Mount options changes, so remount it. $unitsToReload{$unit} = 1; - recordUnit($reloadListFile, $unit); + record_unit($reloadListFile, $unit); } } @@ -620,8 +635,9 @@ if ($prevSystemdSystemConfig ne $newSystemdSystemConfig) { $restartSystemd = 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}))) { @@ -632,7 +648,7 @@ sub filterUnits { return @res; } -my @unitsToStopFiltered = filterUnits(\%unitsToStop); +my @unitsToStopFiltered = filter_units(\%unitsToStop); # Show dry-run actions. @@ -668,7 +684,7 @@ if ($action eq "dry-activate") { next; } - handleModifiedUnit($unit, $baseName, $newUnitFile, undef, $activePrev, \%unitsToRestart, \%unitsToRestart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip); + handle_modified_unit($unit, $baseName, $newUnitFile, undef, $activePrev, \%unitsToRestart, \%unitsToRestart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip); } unlink($dryRestartByActivationFile); @@ -677,7 +693,7 @@ if ($action eq "dry-activate") { if (defined($activePrev->{$unit}) and not $unitsToRestart{$unit} and not $unitsToStop{$unit}) { $unitsToReload{$unit} = 1; - recordUnit($reloadListFile, $unit); + record_unit($reloadListFile, $unit); } } unlink($dryReloadByActivationFile); @@ -691,7 +707,7 @@ if ($action eq "dry-activate") { if (scalar(keys(%unitsToRestart)) > 0) { print STDERR "would restart the following units: ", join(", ", sort(keys(%unitsToRestart))), "\n"; } - my @unitsToStartFiltered = filterUnits(\%unitsToStart); + my @unitsToStartFiltered = filter_units(\%unitsToStart); if (scalar(@unitsToStartFiltered)) { print STDERR "would start the following units: ", join(", ", @unitsToStartFiltered), "\n"; } @@ -737,11 +753,11 @@ foreach (split("\n", read_file($restartByActivationFile, err_mode => "quiet") // # Start units if they were not active previously if (not defined($activePrev->{$unit})) { $unitsToStart{$unit} = 1; - recordUnit($startListFile, $unit); + record_unit($startListFile, $unit); next; } - handleModifiedUnit($unit, $baseName, $newUnitFile, undef, $activePrev, \%unitsToRestart, \%unitsToRestart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip); + handle_modified_unit($unit, $baseName, $newUnitFile, undef, $activePrev, \%unitsToRestart, \%unitsToRestart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip); } # We can remove the file now because it has been propagated to the other restart/reload files unlink($restartByActivationFile); @@ -751,7 +767,7 @@ foreach (split("\n", read_file($reloadByActivationFile, err_mode => "quiet") // if (defined($activePrev->{$unit}) and not $unitsToRestart{$unit} and not $unitsToStop{$unit}) { $unitsToReload{$unit} = 1; - recordUnit($reloadListFile, $unit); + record_unit($reloadListFile, $unit); } } # We can remove the file now because it has been propagated to the other reload file @@ -800,9 +816,9 @@ if (scalar(keys(%unitsToReload)) > 0) { 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))) { + if (!(parse_systemd_bool(\%unit_info, "Unit", "RefuseManualStart", 0) || parse_systemd_bool(\%unit_info, "Unit", "X-OnlyManualStart", 0))) { $unitsToStart{$unit} = 1; - recordUnit($startListFile, $unit); + record_unit($startListFile, $unit); } # Don't reload the unit, reloading would fail delete %unitsToReload{$unit}; @@ -832,7 +848,7 @@ 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); +my @unitsToStartFiltered = filter_units(\%unitsToStart); if (scalar(@unitsToStartFiltered)) { print STDERR "starting the following units: ", join(", ", @unitsToStartFiltered), "\n" } @@ -842,7 +858,7 @@ unlink($startListFile); # Print failed and new units. my (@failed, @new); -my $activeNew = getActiveUnits(); +my $activeNew = get_active_units(); while (my ($unit, $state) = each(%{$activeNew})) { if ($state->{state} eq "failed") { push(@failed, $unit); From 9c494b57732076fd01ba7a1c8ac0f228285c43c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 13 Mar 2022 19:59:55 +0100 Subject: [PATCH 5/7] nixos/switch-to-configuration: Snake-case all variables Also add a lot of comments and reorder some assignments --- .../activation/switch-to-configuration.pl | 470 +++++++++--------- 1 file changed, 238 insertions(+), 232 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index aeeb60dcd68..5532cfe6afe 100755 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -1,7 +1,7 @@ #! @perl@/bin/perl ## no critic(CodeLayout::ProhibitParensWithBuiltins) -## no critic(ValuesAndExpressions::ProhibitInterpolationOfLiterals) +## no critic(ValuesAndExpressions::ProhibitInterpolationOfLiterals, Variables::ProhibitPunctuationVars) use strict; use warnings; @@ -13,14 +13,19 @@ use Net::DBus; use Sys::Syslog qw(:standard :macros); use Cwd "abs_path"; +# 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 @@ -32,15 +37,17 @@ 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@"; } @@ -57,22 +64,21 @@ EOF exit(1); } -$ENV{NIXOS_ACTION} = $action; - # This is a NixOS installation if it has /etc/NIXOS or a proper # /etc/os-release. if (!-f "/etc/NIXOS" && (read_file("/etc/os-release", err_mode => "quiet") // "") !~ /ID="?nixos"?/s) { 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. @@ -85,10 +91,10 @@ if ($action eq "boot") { } # 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) { +if ($new_init_interface_version ne $cur_init_interface_version) { print STDERR < $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); } } } @@ -228,17 +234,17 @@ sub parse_unit { # 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 parse_systemd_bool { - my ($unitConfig, $sectionName, $boolName, $default) = @_; + 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])) { 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"; } # Writes a unit name into a given file to be more resilient against @@ -268,7 +274,7 @@ sub unrecord_unit { # - 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) = @_; + my ($cur_unit, $new_unit) = @_; my $ret = 0; # Keys to ignore in the [Unit] section my %unit_section_ignores = map { $_ => 1 } qw( @@ -289,13 +295,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"}})) { + foreach my $ini_key (keys(%{$cur_unit->{"Unit"}})) { if (not defined($unit_section_ignores{$ini_key})) { return 1; } @@ -304,7 +310,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; @@ -316,9 +322,9 @@ 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 @@ -329,7 +335,7 @@ sub compare_units { } 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") { @@ -342,7 +348,7 @@ 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 previos unit if (%ini_cmp) { if ($section_name eq "Unit") { foreach my $ini_key (keys(%ini_cmp)) { @@ -359,7 +365,7 @@ sub compare_units { } }; } - # A section was introduced that was missing in the old unit + # A section was introduced that was missing in the previos unit if (%section_cmp) { if (%section_cmp == 1 and defined($section_cmp{"Unit"})) { foreach my $ini_key (keys(%{$new_unit->{"Unit"}})) { @@ -380,7 +386,7 @@ sub compare_units { # 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 { - my ($unit, $baseName, $newUnitFile, $newUnitInfo, $activePrev, $unitsToStop, $unitsToStart, $unitsToReload, $unitsToRestart, $unitsToSkip) = @_; + 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$/) { # Do nothing. These cannot be restarted directly. @@ -391,33 +397,33 @@ sub handle_modified_unit { } elsif ($unit =~ /\.mount$/) { # Reload the changed mount unit to force a remount. # FIXME: only reload when Options= changed, restart otherwise - $unitsToReload->{$unit} = 1; - record_unit($reloadListFile, $unit); + $units_to_reload->{$unit} = 1; + record_unit($reload_list_file, $unit); } elsif ($unit =~ /\.socket$/) { # 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 (parse_systemd_bool(\%unitInfo, "Service", "X-ReloadIfChanged", 0) and not $unitsToRestart->{$unit} and not $unitsToStop->{$unit}) { - $unitsToReload->{$unit} = 1; - record_unit($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 (!parse_systemd_bool(\%unitInfo, "Service", "X-RestartIfChanged", 1) || parse_systemd_bool(\%unitInfo, "Unit", "RefuseManualStop", 0) || parse_systemd_bool(\%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 (!parse_systemd_bool(\%unitInfo, "Service", "X-StopIfChanged", 1) || $unit !~ /\.service$/) { + if (!parse_systemd_bool(\%new_unit_info, "Service", "X-StopIfChanged", 1) || $unit !~ /\.service$/) { # This unit should be restarted instead of # stopped and started. - $unitsToRestart->{$unit} = 1; - record_unit($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 @@ -425,30 +431,30 @@ sub handle_modified_unit { # socket(s) instead of the service. my $socket_activated = 0; if ($unit =~ /\.service$/) { - my @sockets = split(/ /, join(" ", @{$unitInfo{Service}{Sockets} // []})); + my @sockets = split(/ /, 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) { - record_unit($restartListFile, $socket); + $units_to_start->{$socket} = 1; + if ($units_to_start eq $units_to_restart) { + record_unit($restart_list_file, $socket); } else { - record_unit($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); } } } @@ -459,19 +465,19 @@ sub handle_modified_unit { # 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) { - record_unit($restartListFile, $unit); + $units_to_start->{$unit} = 1; + if ($units_to_start eq $units_to_restart) { + record_unit($restart_list_file, $unit); } else { - record_unit($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); } } } @@ -479,46 +485,46 @@ sub handle_modified_unit { } # 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 = map { $_ => 1 } - split("\n", read_file($startListFile, err_mode => "quiet") // ""); +%units_to_start = map { $_ => 1 } + split("\n", read_file($start_list_file, err_mode => "quiet") // ""); -%unitsToRestart = map { $_ => 1 } - split("\n", read_file($restartListFile, err_mode => "quiet") // ""); +%units_to_restart = map { $_ => 1 } + split("\n", read_file($restart_list_file, err_mode => "quiet") // ""); -%unitsToReload = map { $_ => 1 } - split("\n", read_file($reloadListFile, err_mode => "quiet") // ""); +%units_to_reload = map { $_ => 1 } + split("\n", read_file($reload_list_file, err_mode => "quiet") // ""); -my $activePrev = get_active_units(); -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 =~ /^(.*)@[^\.]*\.(.*)$/) { + $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/\.[a-z]*$//; - 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); - if (parse_systemd_bool(\%unitInfo, "Unit", "X-StopOnRemoval", 1)) { - $unitsToStop{$unit} = 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); + 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 @@ -527,11 +533,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") { - if (!(parse_systemd_bool(\%unitInfo, "Unit", "RefuseManualStart", 0) || parse_systemd_bool(\%unitInfo, "Unit", "X-OnlyManualStart", 0))) { - $unitsToStart{$unit} = 1; - record_unit($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; } } @@ -546,20 +552,20 @@ 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 (parse_systemd_bool(\%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) { - handle_modified_unit($unit, $baseName, $newUnitFile, \%new_unit_info, $activePrev, \%unitsToStop, \%unitsToStart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip); - } elsif ($diff == 2 and not $unitsToRestart{$unit}) { - $unitsToReload{$unit} = 1; - record_unit($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); } } } @@ -570,7 +576,7 @@ while (my ($unit, $state) = each(%{$activePrev})) { 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>); chomp($escaped); @@ -583,31 +589,31 @@ sub path_to_unit_name { # 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) = parse_fstab("/etc/fstab"); -my ($newFss, $newSwaps) = parse_fstab("$out/etc/fstab"); -foreach my $mountPoint (keys(%$prevFss)) { - my $prev = $prevFss->{$mountPoint}; - my $new = $newFss->{$mountPoint}; - my $unit = path_to_unit_name($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; - record_unit($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; - record_unit($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 @@ -625,14 +631,14 @@ 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; } # Takes an array of unit names and returns an array with the same elements, @@ -641,75 +647,75 @@ sub filter_units { my ($units) = @_; my @res; foreach my $unit (sort(keys(%{$units}))) { - if (!defined($unitsToFilter{$unit})) { + if (!defined($units_to_filter{$unit})) { push(@res, $unit); } } return @res; } -my @unitsToStopFiltered = filter_units(\%unitsToStop); +my @units_to_stop_filtered = filter_units(\%units_to_stop); # Show dry-run actions. if ($action eq "dry-activate") { - if (scalar(@unitsToStopFiltered) > 0) { - print STDERR "would stop the following units: ", join(", ", @unitsToStopFiltered), "\n"; + if (scalar(@units_to_stop_filtered) > 0) { + print STDERR "would stop the following units: ", join(", ", @units_to_stop_filtered), "\n"; } - if (scalar(keys(%unitsToSkip)) > 0) { - print STDERR "would NOT stop the following changed units: ", join(", ", sort(keys(%unitsToSkip))), "\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", 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 =~ /^(.*)@[^\.]*\.(.*)$/) { + $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/\.[a-z]*$//; # 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; } - handle_modified_unit($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", 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; - record_unit($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); - if ($restartSystemd) { + if ($restart_systemd) { print STDERR "would restart systemd\n"; } - if (scalar(keys(%unitsToReload)) > 0) { - print STDERR "would reload the following units: ", join(", ", sort(keys(%unitsToReload))), "\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(%unitsToRestart)) > 0) { - print STDERR "would restart the following units: ", join(", ", sort(keys(%unitsToRestart))), "\n"; + if (scalar(keys(%units_to_restart)) > 0) { + print STDERR "would restart the following units: ", join(", ", sort(keys(%units_to_restart))), "\n"; } - my @unitsToStartFiltered = filter_units(\%unitsToStart); - if (scalar(@unitsToStartFiltered)) { - print STDERR "would start the following units: ", join(", ", @unitsToStartFiltered), "\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; } @@ -717,16 +723,16 @@ if ($action eq "dry-activate") { syslog(LOG_NOTICE, "switching to system configuration $out"); -if (scalar(keys(%unitsToStop)) > 0) { - if (scalar(@unitsToStopFiltered)) { - print STDERR "stopping the following units: ", join(", ", @unitsToStopFiltered), "\n"; +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))); } -if (scalar(keys(%unitsToSkip)) > 0) { - print STDERR "NOT restarting the following changed units: ", join(", ", sort(keys(%unitsToSkip))), "\n"; +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, @@ -736,60 +742,60 @@ 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", 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 =~ /^(.*)@[^\.]*\.(.*)$/) { + $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/\.[a-z]*$//; # Start units if they were not active previously - if (not defined($activePrev->{$unit})) { - $unitsToStart{$unit} = 1; - record_unit($startListFile, $unit); + if (not defined($active_cur->{$unit})) { + $units_to_start{$unit} = 1; + record_unit($start_list_file, $unit); next; } - handle_modified_unit($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", 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; - record_unit($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>) { +open(my $list_active_users, "-|", "$new_systemd/bin/loginctl", "list-users", "--no-legend"); +while (my $f = <$list_active_users>) { if ($f !~ /^\s*(?\d+)\s+(?\S+)/) { next; } @@ -798,48 +804,48 @@ while (my $f = <$listActiveUsers>) { 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); # 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 (!(parse_systemd_bool(\%unit_info, "Unit", "RefuseManualStart", 0) || parse_systemd_bool(\%unit_info, "Unit", "X-OnlyManualStart", 0))) { - $unitsToStart{$unit} = 1; - record_unit($startListFile, $unit); + $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. @@ -848,18 +854,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 = filter_units(\%unitsToStart); -if (scalar(@unitsToStartFiltered)) { - print STDERR "starting the following units: ", join(", ", @unitsToStartFiltered), "\n" +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("@systemd@/bin/systemctl", "start", "--", sort(keys(%unitsToStart))) == 0 or $res = 4; -unlink($startListFile); +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 = get_active_units(); -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; @@ -867,7 +873,7 @@ 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'`; + my $main_status = `$new_systemd/bin/systemctl show --value --property=ExecMainStatus '$unit'`; chomp($main_status); if ($main_status ne "0") { @@ -879,7 +885,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); } } @@ -891,7 +897,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; } From 85874efcb01fe6266c78761900b2c526cf929cd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 13 Mar 2022 20:33:56 +0100 Subject: [PATCH 6/7] nixos/switch-to-configuration: Make perlcritic happy --- .../activation/switch-to-configuration.pl | 102 ++++++++++-------- 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 5532cfe6afe..9e5b760434a 100755 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -1,8 +1,5 @@ #! @perl@/bin/perl -## no critic(CodeLayout::ProhibitParensWithBuiltins) -## no critic(ValuesAndExpressions::ProhibitInterpolationOfLiterals, Variables::ProhibitPunctuationVars) - use strict; use warnings; use Config::IniFiles; @@ -11,7 +8,15 @@ 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@"; @@ -53,7 +58,7 @@ if ("@localeArchive@" ne "") { } if (!defined($action) || ($action ne "switch" && $action ne "boot" && $action ne "test" && $action ne "dry-activate")) { - print STDERR < "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"); } @@ -95,10 +100,10 @@ my $cur_init_interface_version = read_file("/run/current-system/init-interface-v my $new_init_interface_version = read_file("$out/init-interface-version"); if ($new_init_interface_version ne $cur_init_interface_version) { - print STDERR <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; + $following, $unit_path, $job_id, $job_type, $job_path) = @{$item}; if ($following ne "") { next; } @@ -138,7 +143,7 @@ sub unit_is_active { if (scalar(@{$units}) == 0) { return 0; } - my $active_state = $units->[0]->[3]; ## no critic (ValuesAndExpressions::ProhibitMagicNumbers) + my $active_state = $units->[0]->[3]; return $active_state eq "active" || $active_state eq "activating"; } @@ -152,11 +157,11 @@ sub parse_fstab { my ($fss, $swaps); foreach my $line (read_file($filename, err_mode => "quiet")) { chomp($line); - $line =~ s/^\s*#.*//; - if ($line =~ /^\s*$/) { + $line =~ s/^\s*\#.*//msx; + if ($line =~ /^\s*$/msx) { next; } - my @xs = split(/ /, $line); + my @xs = split(/\s+/msx, $line); if ($xs[2] eq "swap") { $swaps->{$xs[0]} = { options => $xs[3] // "" }; } else { @@ -238,7 +243,7 @@ sub parse_systemd_bool { 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 @@ -254,6 +259,7 @@ sub record_unit { if ($action ne "dry-activate") { write_file($fn, { append => 1 }, "$unit\n"); } + return; } # The opposite of record_unit, removes a unit name from a file @@ -262,6 +268,7 @@ sub unrecord_unit { 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 @@ -273,7 +280,7 @@ 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 { +sub compare_units { ## no critic(Subroutines::ProhibitExcessComplexity) my ($cur_unit, $new_unit) = @_; my $ret = 0; # Keys to ignore in the [Unit] section @@ -348,7 +355,7 @@ sub compare_units { return 1; } } - # A key was introduced that was missing in the previos unit + # A key was introduced that was missing in the previous unit if (%ini_cmp) { if ($section_name eq "Unit") { foreach my $ini_key (keys(%ini_cmp)) { @@ -365,7 +372,7 @@ sub compare_units { } }; } - # A section was introduced that was missing in the previos 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"}})) { @@ -385,21 +392,21 @@ sub compare_units { # 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 { +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 $units_to_reload->{$unit} = 1; record_unit($reload_list_file, $unit); - } elsif ($unit =~ /\.socket$/) { + } 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 @@ -415,7 +422,7 @@ sub handle_modified_unit { } else { # It doesn't make sense to stop and start non-services because # they can't have ExecStop= - if (!parse_systemd_bool(\%new_unit_info, "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. $units_to_restart->{$unit} = 1; @@ -430,8 +437,8 @@ sub handle_modified_unit { # 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(" ", @{$new_unit_info{Service}{Sockets} // []})); + if ($unit =~ /\.service$/msx) { + my @sockets = split(/\s+/msx, join(" ", @{$new_unit_info{Service}{Sockets} // []})); if (scalar(@sockets) == 0) { @sockets = ("$base_name.socket"); } @@ -482,6 +489,7 @@ sub handle_modified_unit { } } } + return; } # Figure out what units need to be stopped, started, restarted or reloaded. @@ -490,13 +498,13 @@ my (%units_to_stop, %units_to_skip, %units_to_start, %units_to_restart, %units_t my %units_to_filter; # units not shown %units_to_start = map { $_ => 1 } - split("\n", read_file($start_list_file, err_mode => "quiet") // ""); + split(/\n/msx, read_file($start_list_file, err_mode => "quiet") // ""); %units_to_restart = map { $_ => 1 } - split("\n", read_file($restart_list_file, err_mode => "quiet") // ""); + split(/\n/msx, read_file($restart_list_file, err_mode => "quiet") // ""); %units_to_reload = map { $_ => 1 } - split("\n", read_file($reload_list_file, err_mode => "quiet") // ""); + split(/\n/msx, read_file($reload_list_file, err_mode => "quiet") // ""); my $active_cur = get_active_units(); while (my ($unit, $state) = each(%{$active_cur})) { @@ -506,14 +514,14 @@ while (my ($unit, $state) = each(%{$active_cur})) { my $new_unit_file = "$out/etc/systemd/system/$base_unit"; # Detect template instances. - if (!-e $cur_unit_file && !-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/) { + 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 $base_name = $base_unit; - $base_name =~ s/\.[a-z]*$//; + $base_name =~ s/\.[[:lower:]]*$//msx; 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") { @@ -523,7 +531,7 @@ while (my ($unit, $state) = each(%{$active_cur})) { } } - elsif ($unit =~ /\.target$/) { + elsif ($unit =~ /\.target$/msx) { my %new_unit_info = parse_unit($new_unit_file); # Cause all active target units to be restarted below. @@ -578,7 +586,7 @@ sub path_to_unit_name { # Use current version of systemctl binary before daemon is reexeced. 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"); return $escaped; @@ -591,7 +599,7 @@ sub path_to_unit_name { # unit checking code above. 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)) { +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); @@ -611,7 +619,7 @@ foreach my $mount_point (keys(%$cur_fss)) { } # Also handles swap devices. -foreach my $device (keys(%$cur_swaps)) { +foreach my $device (keys(%{$cur_swaps})) { my $cur = $cur_swaps->{$device}; my $new = $new_swaps->{$device}; if (!defined($new)) { @@ -670,19 +678,19 @@ if ($action eq "dry-activate") { system("$out/dry-activate", "$out"); # Handle the activation script requesting the restart or reload of a unit. - foreach (split("\n", read_file($dry_restart_by_activation_file, err_mode => "quiet") // "")) { + foreach (split(/\n/msx, read_file($dry_restart_by_activation_file, err_mode => "quiet") // "")) { my $unit = $_; my $base_unit = $unit; my $new_unit_file = "$out/etc/systemd/system/$base_unit"; # Detect template instances. - if (!-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/) { + if (!-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/msx) { $base_unit = "$1\@.$2"; $new_unit_file = "$out/etc/systemd/system/$base_unit"; } my $base_name = $base_unit; - $base_name =~ s/\.[a-z]*$//; + $base_name =~ s/\.[[:lower:]]*$//msx; # Start units if they were not active previously if (not defined($active_cur->{$unit})) { @@ -694,7 +702,7 @@ if ($action eq "dry-activate") { } unlink($dry_restart_by_activation_file); - foreach (split("\n", read_file($dry_reload_by_activation_file, err_mode => "quiet") // "")) { + foreach (split(/\n/msx, read_file($dry_reload_by_activation_file, err_mode => "quiet") // "")) { my $unit = $_; if (defined($active_cur->{$unit}) and not $units_to_restart{$unit} and not $units_to_stop{$unit}) { @@ -742,19 +750,19 @@ 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($restart_by_activation_file, err_mode => "quiet") // "")) { +foreach (split(/\n/msx, read_file($restart_by_activation_file, err_mode => "quiet") // "")) { my $unit = $_; my $base_unit = $unit; my $new_unit_file = "$out/etc/systemd/system/$base_unit"; # Detect template instances. - if (!-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/) { + if (!-e $new_unit_file && $unit =~ /^(.*)@[^\.]*\.(.*)$/msx) { $base_unit = "$1\@.$2"; $new_unit_file = "$out/etc/systemd/system/$base_unit"; } my $base_name = $base_unit; - $base_name =~ s/\.[a-z]*$//; + $base_name =~ s/\.[[:lower:]]*$//msx; # Start units if they were not active previously if (not defined($active_cur->{$unit})) { @@ -768,7 +776,7 @@ foreach (split("\n", read_file($restart_by_activation_file, err_mode => "quiet") # We can remove the file now because it has been propagated to the other restart/reload files unlink($restart_by_activation_file); -foreach (split("\n", read_file($reload_by_activation_file, err_mode => "quiet") // "")) { +foreach (split(/\n/msx, read_file($reload_by_activation_file, err_mode => "quiet") // "")) { my $unit = $_; if (defined($active_cur->{$unit}) and not $units_to_restart{$unit} and not $units_to_stop{$unit}) { @@ -794,9 +802,9 @@ system("$new_systemd/bin/systemctl", "reset-failed"); system("$new_systemd/bin/systemctl", "daemon-reload") == 0 or $res = 3; # Reload user units -open(my $list_active_users, "-|", "$new_systemd/bin/loginctl", "list-users", "--no-legend"); +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*(?\d+)\s+(?\S+)/) { + if ($f !~ /^\s*(?\d+)\s+(?\S+)/msx) { next; } my ($uid, $name) = ($+{uid}, $+{user}); @@ -808,7 +816,7 @@ while (my $f = <$list_active_users>) { "$new_systemd/bin/systemctl --user start nixos-activation.service"); } -close($list_active_users); +close($list_active_users) || die("Unable to close the file handle to loginctl"); # Set the new tmpfiles print STDERR "setting up tmpfiles\n"; @@ -873,7 +881,9 @@ while (my ($unit, $state) = each(%{$active_new})) { if ($state->{substate} eq "auto-restart") { # A unit in auto-restart substate is a failure *if* it previously failed to start - my $main_status = `$new_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") { From 2473cce829b30a35655baf1501e392337549a179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Mon, 14 Mar 2022 10:08:38 +0100 Subject: [PATCH 7/7] nixos/switchTest: Also test boot/switch actions --- nixos/tests/switch-test.nix | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix index 4136aa336ac..0198866b6ff 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 = { @@ -510,6 +516,25 @@ in { "${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 @@ -523,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