Update and unify limits across systemd and macOS. (#1126)

* Raise the NOFILE limit to 512 * 1024 * 1024 after hitting limits talking to caches

See: NixOS/nix#11387

* Set the default stack limit to 64M

Amazon Linux 2023 and likely CentOS / RedHat sets the default stack limit to 10M, and is generally effective at building nixpkgs. One notable exception is Nix, which typically sets it stack size to 64M. When it could not do that, a test around the stack overflow behavior failed. This was fixed in NixOS/nix#10903 to use less stack. However, since Nix typically can use a 64M stack we set that as the default max.

We should not treat this value as magically correct. If this value causes problems, we should freely raise it. It is possible infinity is the right answer. 64M is attempting to be a "good guess" at a universally applicable good start.

For future readers, this bug report to systemd regarding LimitSTACK and DefaultLimitSTACK might be useful: systemd/systemd#34193.

* Echo TasksMax from the systemd unit to the macOS plist

---

Also:

* Fixup the unused init parameter to ProvisionDeterminateNixd

* Fixup clippy warnings around borrows

* Delete dead code in configure_init_service

src/action/base/add_user_to_group.rs

@@ -184,13 +184,6 @@ impl Action for AddUserToGroup {
 
     #[tracing::instrument(level = "debug", skip_all)]
     async fn execute(&mut self) -> Result<(), ActionError> {
-        let Self {
-            name,
-            uid: _,
-            groupname,
-            gid: _,
-        } = self;
-
         use target_lexicon::OperatingSystem;
         match OperatingSystem::host() {
             OperatingSystem::MacOSX {
@@ -205,10 +198,10 @@ impl Action for AddUserToGroup {
                         .args([
                             ".",
                             "-append",
-                            &format!("/Groups/{groupname}"),
+                            &format!("/Groups/{}", self.groupname),
                             "GroupMembership",
                         ])
-                        .arg(&name)
+                        .arg(&self.name)
                         .stdin(std::process::Stdio::null()),
                 )
                 .await
@@ -218,10 +211,10 @@ impl Action for AddUserToGroup {
                         .process_group(0)
                         .args(["-o", "edit"])
                         .arg("-a")
-                        .arg(&name)
+                        .arg(&self.name)
                         .arg("-t")
-                        .arg(&name)
-                        .arg(groupname)
+                        .arg(&self.name)
+                        .arg(&self.groupname)
                         .stdin(std::process::Stdio::null()),
                 )
                 .await
@@ -233,7 +226,7 @@ impl Action for AddUserToGroup {
                         Command::new("gpasswd")
                             .process_group(0)
                             .args(["-a"])
-                            .args([name, groupname])
+                            .args([&self.name, &self.groupname])
                             .stdin(std::process::Stdio::null()),
                     )
                     .await
@@ -242,7 +235,7 @@ impl Action for AddUserToGroup {
                     execute_command(
                         Command::new("addgroup")
                             .process_group(0)
-                            .args([name, groupname])
+                            .args([&self.name, &self.groupname])
                             .stdin(std::process::Stdio::null()),
                     )
                     .await
@@ -291,7 +284,7 @@ impl Action for AddUserToGroup {
                     Command::new("/usr/bin/dscl")
                         .process_group(0)
                         .args([".", "-delete", &format!("/Groups/{groupname}"), "users"])
-                        .arg(&name)
+                        .arg(name)
                         .stdin(std::process::Stdio::null()),
                 )
                 .await

src/action/base/create_file.rs

@@ -178,39 +178,30 @@ impl Action for CreateFile {
 
     #[tracing::instrument(level = "debug", skip_all)]
     async fn execute(&mut self) -> Result<(), ActionError> {
-        let Self {
-            path,
-            user,
-            group,
-            mode,
-            buf,
-            force: _,
-        } = self;
-
         if tracing::enabled!(tracing::Level::TRACE) {
             let span = tracing::Span::current();
-            span.record("buf", &buf);
+            span.record("buf", &self.buf);
         }
 
         let mut options = OpenOptions::new();
         options.create_new(true).write(true).read(true);
 
-        if let Some(mode) = mode {
-            options.mode(*mode);
+        if let Some(mode) = self.mode {
+            options.mode(mode);
         }
 
         let mut file = options
-            .open(&path)
+            .open(&self.path)
             .await
-            .map_err(|e| ActionErrorKind::Open(path.to_owned(), e))
+            .map_err(|e| ActionErrorKind::Open(self.path.to_owned(), e))
             .map_err(Self::error)?;
 
-        file.write_all(buf.as_bytes())
+        file.write_all(self.buf.as_bytes())
             .await
-            .map_err(|e| ActionErrorKind::Write(path.to_owned(), e))
+            .map_err(|e| ActionErrorKind::Write(self.path.to_owned(), e))
             .map_err(Self::error)?;
 
-        let gid = if let Some(group) = group {
+        let gid = if let Some(ref group) = self.group {
             Some(
                 Group::from_name(group.as_str())
                     .map_err(|e| ActionErrorKind::GettingGroupId(group.clone(), e))
@@ -222,7 +213,7 @@ impl Action for CreateFile {
         } else {
             None
         };
-        let uid = if let Some(user) = user {
+        let uid = if let Some(ref user) = self.user {
             Some(
                 User::from_name(user.as_str())
                     .map_err(|e| ActionErrorKind::GettingUserId(user.clone(), e))
@@ -234,8 +225,8 @@ impl Action for CreateFile {
         } else {
             None
         };
-        chown(path, uid, gid)
-            .map_err(|e| ActionErrorKind::Chown(path.clone(), e))
+        chown(&self.path, uid, gid)
+            .map_err(|e| ActionErrorKind::Chown(self.path.clone(), e))
             .map_err(Self::error)?;
 
         Ok(())

src/action/base/create_or_merge_nix_config.rs

@@ -200,8 +200,7 @@ impl Action for CreateOrMergeNixConfig {
         if tracing::enabled!(tracing::Level::TRACE) {
             span.record(
                 "pending_nix_config",
-                &self
-                    .pending_nix_config
+                self.pending_nix_config
                     .settings()
                     .iter()
                     .map(|(k, v)| format!("{k}=\"{v}\""))

src/action/common/configure_determinate_nixd_init_service/mod.rs

@@ -201,6 +201,8 @@ pub struct DeterminateNixDaemonPlist {
 #[serde(rename_all = "PascalCase")]
 pub struct ResourceLimits {
     number_of_files: usize,
+    number_of_processes: usize,
+    stack: usize,
 }
 
 #[derive(Deserialize, Clone, Debug, Serialize, PartialEq)]
@@ -225,10 +227,14 @@ fn generate_plist() -> DeterminateNixDaemonPlist {
         standard_error_path: "/var/log/determinate-nix-daemon.log".into(),
         standard_out_path: "/var/log/determinate-nix-daemon.log".into(),
         soft_resource_limits: ResourceLimits {
-            number_of_files: 1048576,
+            number_of_files: 512 * 1024 * 1024,
+            number_of_processes: 1024 * 1024,
+            stack: 64 * 1024 * 1024,
         },
         hard_resource_limits: ResourceLimits {
-            number_of_files: 1048576 * 2,
+            number_of_files: 512 * 1024 * 1024,
+            number_of_processes: 1024 * 1024,
+            stack: 64 * 1024 * 1024,
         },
         sockets: HashMap::from([
             (

src/action/common/configure_determinate_nixd_init_service/nix-daemon.determinate-nixd.service

@@ -9,7 +9,8 @@ ConditionPathIsReadWrite=/nix/var/nix/daemon-socket
 [Service]
 ExecStart=@/usr/local/bin/determinate-nixd determinate-nixd
 KillMode=process
-LimitNOFILE=1048576
+LimitNOFILE=536870912
+LimitSTACK=64M
 TasksMax=1048576
 
 [Install]

src/action/common/configure_init_service.rs

@@ -1,7 +1,6 @@
 use std::path::Path;
 use std::path::PathBuf;
 
-use serde::{Deserialize, Serialize};
 use tokio::process::Command;
 use tracing::{span, Span};
 
@@ -713,24 +712,6 @@ pub enum ConfigureNixDaemonServiceError {
     InitNotSupported,
 }
 
-#[derive(Deserialize, Clone, Debug, Serialize, PartialEq)]
-#[serde(rename_all = "PascalCase")]
-pub struct DeterminateNixDaemonPlist {
-    label: String,
-    program: String,
-    keep_alive: bool,
-    run_at_load: bool,
-    standard_error_path: String,
-    standard_out_path: String,
-    soft_resource_limits: ResourceLimits,
-}
-
-#[derive(Deserialize, Clone, Debug, Serialize, PartialEq)]
-#[serde(rename_all = "PascalCase")]
-pub struct ResourceLimits {
-    number_of_files: usize,
-}
-
 async fn stop(unit: &str) -> Result<(), ActionErrorKind> {
     let mut command = Command::new("systemctl");
     command.arg("stop");

src/action/common/provision_determinate_nixd.rs

@@ -7,7 +7,6 @@ use tracing::{span, Span};
 use crate::action::{
     Action, ActionDescription, ActionError, ActionErrorKind, ActionTag, StatefulAction,
 };
-use crate::settings::InitSystem;
 
 const DETERMINATE_NIXD_BINARY_PATH: &str = "/usr/local/bin/determinate-nixd";
 /**
@@ -21,7 +20,7 @@ pub struct ProvisionDeterminateNixd {
 
 impl ProvisionDeterminateNixd {
     #[tracing::instrument(level = "debug", skip_all)]
-    pub async fn plan(init: InitSystem) -> Result<StatefulAction<Self>, ActionError> {
+    pub async fn plan() -> Result<StatefulAction<Self>, ActionError> {
         crate::settings::DETERMINATE_NIXD_BINARY
             .ok_or_else(|| Self::error(ActionErrorKind::DeterminateNixUnavailable))?;
 

src/action/linux/start_systemd_unit.rs

@@ -82,7 +82,7 @@ impl Action for StartSystemdUnit {
                         .process_group(0)
                         .arg("enable")
                         .arg("--now")
-                        .arg(&unit)
+                        .arg(unit)
                         .stdin(std::process::Stdio::null()),
                 )
                 .await
@@ -94,7 +94,7 @@ impl Action for StartSystemdUnit {
                     Command::new("systemctl")
                         .process_group(0)
                         .arg("start")
-                        .arg(&unit)
+                        .arg(unit)
                         .stdin(std::process::Stdio::null()),
                 )
                 .await

src/action/macos/bootstrap_launchctl_service.rs

@@ -132,8 +132,8 @@ impl Action for BootstrapLaunchctlService {
                 Command::new("launchctl")
                     .process_group(0)
                     .arg("bootstrap")
-                    .arg(&domain)
-                    .arg(&path)
+                    .arg(domain)
+                    .arg(path)
                     .stdin(std::process::Stdio::null()),
             )
             .await

src/action/macos/enable_ownership.rs

@@ -53,14 +53,12 @@ impl Action for EnableOwnership {
 
     #[tracing::instrument(level = "debug", skip_all)]
     async fn execute(&mut self) -> Result<(), ActionError> {
-        let Self { path } = self;
-
         let should_enable_ownership = {
             let buf = execute_command(
                 Command::new("/usr/sbin/diskutil")
                     .process_group(0)
                     .args(["info", "-plist"])
-                    .arg(&path)
+                    .arg(&self.path)
                     .stdin(std::process::Stdio::null()),
             )
             .await
@@ -77,7 +75,7 @@ impl Action for EnableOwnership {
                 Command::new("/usr/sbin/diskutil")
                     .process_group(0)
                     .arg("enableOwnership")
-                    .arg(path)
+                    .arg(&self.path)
                     .stdin(std::process::Stdio::null()),
             )
             .await

src/action/macos/encrypt_apfs_volume.rs

@@ -142,12 +142,6 @@ impl Action for EncryptApfsVolume {
         disk = %self.disk.display(),
     ))]
     async fn execute(&mut self) -> Result<(), ActionError> {
-        let Self {
-            determinate_nix,
-            disk,
-            name,
-        } = self;
-
         // Generate a random password.
         let password: String = {
             const CHARSET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZ\
@@ -164,18 +158,22 @@ impl Action for EncryptApfsVolume {
                 .collect()
         };
 
-        let disk_str = disk.to_str().expect("Could not turn disk into string"); /* Should not reasonably ever fail */
+        let disk_str = &self.disk.to_str().expect("Could not turn disk into string"); /* Should not reasonably ever fail */
 
-        execute_command(Command::new("/usr/sbin/diskutil").arg("mount").arg(&name))
-            .await
-            .map_err(Self::error)?;
+        execute_command(
+            Command::new("/usr/sbin/diskutil")
+                .arg("mount")
+                .arg(&self.name),
+        )
+        .await
+        .map_err(Self::error)?;
 
         // Add the password to the user keychain so they can unlock it later.
         let mut cmd = Command::new("/usr/bin/security");
         cmd.process_group(0).args([
             "add-generic-password",
             "-a",
-            name.as_str(),
+            self.name.as_str(),
             "-s",
             "Nix Store",
             "-l",
@@ -195,7 +193,7 @@ impl Action for EncryptApfsVolume {
             "/usr/bin/security",
         ]);
 
-        if *determinate_nix {
+        if self.determinate_nix {
             cmd.args(["-T", "/usr/local/bin/determinate-nixd"]);
         }
 
@@ -208,7 +206,7 @@ impl Action for EncryptApfsVolume {
         execute_command(Command::new("/usr/sbin/diskutil").process_group(0).args([
             "apfs",
             "encryptVolume",
-            name.as_str(),
+            self.name.as_str(),
             "-user",
             "disk",
             "-passphrase",
@@ -222,7 +220,7 @@ impl Action for EncryptApfsVolume {
                 .process_group(0)
                 .arg("unmount")
                 .arg("force")
-                .arg(&name),
+                .arg(&self.name),
         )
         .await
         .map_err(Self::error)?;