Various fixups (#1329)

* CreateUser: retry dscl command if it gets SIGKILLed

* AddUserToGroup: disambiguate between "group not found" and "user not found"

* AddUserToGroup: handle case where user and group both already exist

* fixup: bogus arg to dseditgroup

This has always been wrong:

     -t recordtype
              The type of the record to be added to or deleted from the group specified by groupname.
              Valid values are user, computer, group, or computergroup.

but I guess macOS just ignores / autodetects the correct type when
provided with a bogus value.

Validated by doing various operations with `-t jfkdlsjfkldasjl` and
seeing no errors, but passing the "wrong type", i.e. `-t group` in this
invocation, did error with "Record was not found".

* SetupDefaultProfile: prevent post-build-hook from messing nix-env up

...if it's configured, we'll skip it for the nix-env invocations.

src/action/base/add_user_to_group.rs

@@ -93,6 +93,7 @@ impl AddUserToGroup {
                         .await
                         .map_err(|e| ActionErrorKind::command(&command, e))
                         .map_err(Self::error)?;
+                    let stderr = String::from_utf8_lossy(&output.stderr);
                     match output.status.code() {
                         Some(0) => {
                             // yes {user} is a member of {groupname}
@@ -104,15 +105,27 @@ impl AddUserToGroup {
                             );
                             return Ok(StatefulAction::completed(this));
                         },
-                        Some(64) => {
-                            // 64 is the exit code for "Group not found"
+                        // 64 is the exit code for "Group not found" or "Unable to find the user
+                        // record", so we have to disambiguate by checking stderr for this string
+                        Some(64) if stderr.contains("Group not found") => {
                             tracing::trace!(
                                 "Will add user `{}` to newly created group `{}`",
                                 this.name,
                                 this.groupname
                             );
                             // The group will be created by the installer
                         },
+                        Some(67) => {
+                            // 67 is the exit code for "user is not a member of the group", i.e.:
+                            //
+                            //     no _nixbld1 is NOT a member of nixbld
+                            tracing::trace!(
+                                "Will add existing user `{}` to existing group `{}`",
+                                this.name,
+                                this.groupname
+                            );
+                            // The user will be added to this group
+                        },
                         _ => {
                             // Some other issue
                             return Err(Self::error(ActionErrorKind::command_output(
@@ -212,8 +225,6 @@ impl Action for AddUserToGroup {
                         .args(["-o", "edit"])
                         .arg("-a")
                         .arg(&self.name)
-                        .arg("-t")
-                        .arg(&self.name)
                         .arg(&self.groupname)
                         .stdin(std::process::Stdio::null()),
                 )

src/action/base/create_user.rs

@@ -1,3 +1,5 @@
+use std::os::unix::process::ExitStatusExt;
+
 use nix::unistd::User;
 use target_lexicon::OperatingSystem;
 use tokio::process::Command;
@@ -240,9 +242,7 @@ impl Action for CreateUser {
 }
 
 #[tracing::instrument]
-pub(crate) async fn execute_command_retry_on_enotyetimplemented(
-    dscl_args: &[&str],
-) -> Result<(), ActionErrorKind> {
+async fn execute_dscl_retry_on_specific_errors(dscl_args: &[&str]) -> Result<(), ActionErrorKind> {
     let mut retry_tokens: usize = 10;
     loop {
         let mut command = Command::new("/usr/bin/dscl");
@@ -267,13 +267,16 @@ pub(crate) async fn execute_command_retry_on_enotyetimplemented(
         } else if retry_tokens == 0 {
             return Err(ActionErrorKind::command_output(&command, output))?;
         } else {
-            match output.status.code() {
-                Some(140) if stderr.contains("-14988 (eNotYetImplemented)") => {
-                    // Retry due to buggy macOS user behavior?
-                    // https://github.com/DeterminateSystems/nix-installer/issues/1300
-                    // https://github.com/ansible/ansible/issues/73505
-                },
-                _ => return Err(ActionErrorKind::command_output(&command, output)),
+            if output.status.code() == Some(140) && stderr.contains("-14988 (eNotYetImplemented)") {
+                // Retry due to buggy macOS user behavior?
+                // https://github.com/DeterminateSystems/nix-installer/issues/1300
+                // https://github.com/ansible/ansible/issues/73505
+            } else if output.status.signal() == Some(9) {
+                // If the command was SIGKILLed, let's retry and hope it doesn't happen again.
+            } else {
+                // If the command failed for a reason that we weren't "expecting", return that as an
+                // error.
+                return Err(ActionErrorKind::command_output(&command, output));
             }
 
             retry_tokens = retry_tokens.saturating_sub(1);
@@ -287,50 +290,49 @@ pub(crate) async fn execute_command_retry_on_enotyetimplemented(
 
 #[tracing::instrument(level = "debug", skip_all)]
 async fn create_user_macos(name: &str, uid: u32, gid: u32) -> Result<(), ActionErrorKind> {
-    execute_command_retry_on_enotyetimplemented(&[".", "-create", &format!("/Users/{name}")])
-        .await?;
+    execute_dscl_retry_on_specific_errors(&[".", "-create", &format!("/Users/{name}")]).await?;
 
-    execute_command_retry_on_enotyetimplemented(&[
+    execute_dscl_retry_on_specific_errors(&[
         ".",
         "-create",
         &format!("/Users/{name}"),
         "UniqueID",
         &format!("{uid}"),
     ])
     .await?;
-    execute_command_retry_on_enotyetimplemented(&[
+    execute_dscl_retry_on_specific_errors(&[
         ".",
         "-create",
         &format!("/Users/{name}"),
         "PrimaryGroupID",
         &format!("{gid}"),
     ])
     .await?;
-    execute_command_retry_on_enotyetimplemented(&[
+    execute_dscl_retry_on_specific_errors(&[
         ".",
         "-create",
         &format!("/Users/{name}"),
         "NFSHomeDirectory",
         "/var/empty",
     ])
     .await?;
-    execute_command_retry_on_enotyetimplemented(&[
+    execute_dscl_retry_on_specific_errors(&[
         ".",
         "-create",
         &format!("/Users/{name}"),
         "UserShell",
         "/sbin/nologin",
     ])
     .await?;
-    execute_command_retry_on_enotyetimplemented(&[
+    execute_dscl_retry_on_specific_errors(&[
         ".",
         "-create",
         &format!("/Users/{name}"),
         "IsHidden",
         "1",
     ])
     .await?;
-    execute_command_retry_on_enotyetimplemented(&[
+    execute_dscl_retry_on_specific_errors(&[
         ".",
         "-create",
         &format!("/Users/{name}"),

src/action/base/setup_default_profile.rs

@@ -119,6 +119,7 @@ impl Action for SetupDefaultProfile {
             Command::new(nix_pkg.join("bin/nix-env"))
                 .process_group(0)
                 .args(["--option", "substitute", "false"])
+                .args(["--option", "post-build-hook", ""])
                 .arg("-i")
                 .arg(&nix_pkg)
                 .stdin(std::process::Stdio::null())
@@ -140,6 +141,7 @@ impl Action for SetupDefaultProfile {
             Command::new(nix_pkg.join("bin/nix-env"))
                 .process_group(0)
                 .args(["--option", "substitute", "false"])
+                .args(["--option", "post-build-hook", ""])
                 .arg("-i")
                 .arg(&nss_ca_cert_pkg)
                 .stdin(std::process::Stdio::null())