nixos: refactor OsRebuildArgs impl

Author: faukah

src/nixos.rs

@@ -98,21 +98,9 @@ impl OsRebuildArgs {
     final_attr: Option<String>,
     elevation: ElevationStrategy,
   ) -> Result<()> {
-    use OsRebuildVariant::{Boot, Build, BuildVm, Switch, Test};
+    use OsRebuildVariant::{Build, BuildVm};
 
-    if self.build_host.is_some() || self.target_host.is_some() {
-      // This can fail, we only care about prompting the user
-      // for ssh key login beforehand.
-      let _ = ensure_ssh_key_login();
-    }
-
-    let elevate = check_and_get_elevation_status(self.bypass_root_check)?;
-
-    if self.update_args.update_all || self.update_args.update_input.is_some() {
-      update(&self.common.installable, self.update_args.update_input)?;
-    }
-
-    let target_hostname = get_resolved_hostname(self.hostname.clone())?;
+    let (elevate, target_hostname) = self.setup_build_context()?;
 
     // Only show the warning if we're explicitly building a VM
     // and no hostname was explicitly provided (--hostname was None)
@@ -129,49 +117,138 @@ impl OsRebuildArgs {
       );
     }
 
-    let (out_path, _tempdir_guard) = match self.common.out_link {
-      Some(p) => (p, None),
+    let (out_path, _tempdir_guard) = self.determine_output_path(variant)?;
+
+    let toplevel =
+      self.resolve_installable_and_toplevel(&target_hostname, final_attr)?;
+
+    let message = match variant {
+      BuildVm => "Building NixOS VM image",
+      _ => "Building NixOS configuration",
+    };
+
+    self.execute_build_command(toplevel, &out_path, message)?;
+
+    let target_profile = self.resolve_specialisation_and_profile(&out_path)?;
+
+    self.handle_dix_diff(&target_profile)?;
+
+    if self.common.dry || matches!(variant, Build | BuildVm) {
+      if self.common.ask {
+        warn!("--ask has no effect as dry run was requested");
+      }
+
+      // For VM builds, print instructions on how to run the VM
+      if matches!(variant, BuildVm) && !self.common.dry {
+        print_vm_instructions(&out_path)?;
+      }
+
+      return Ok(());
+    }
+
+    self.activate_rebuilt_config(
+      variant,
+      &out_path,
+      &target_profile,
+      elevate,
+      elevation,
+    )?;
+
+    Ok(())
+  }
+
+  /// Performs initial setup and gathers context for an OS rebuild operation.
+  ///
+  /// This includes:
+  /// - Ensuring SSH key login if a remote build/target host is involved.
+  /// - Checking and determining elevation status.
+  /// - Performing updates to Nix inputs if specified.
+  /// - Resolving the target hostname for the build.
+  ///
+  /// # Returns
+  /// A `Result` containing a tuple:
+  /// - `bool`: `true` if elevation is required, `false` otherwise.
+  /// - `String`: The resolved target hostname.
+  fn setup_build_context(&self) -> Result<(bool, String)> {
+    if self.build_host.is_some() || self.target_host.is_some() {
+      // This can fail, we only care about prompting the user
+      // for ssh key login beforehand.
+      let _ = ensure_ssh_key_login();
+    }
+
+    let elevate = check_and_get_elevation_status(self.bypass_root_check)?;
+
+    if self.update_args.update_all || self.update_args.update_input.is_some() {
+      update(
+        &self.common.installable,
+        self.update_args.update_input.clone(),
+      )?;
+    }
+
+    let target_hostname = get_resolved_hostname(self.hostname.clone())?;
+    Ok((elevate, target_hostname))
+  }
+
+  fn determine_output_path(
+    &self,
+    variant: &OsRebuildVariant,
+  ) -> Result<(PathBuf, Option<tempfile::TempDir>)> {
+    use OsRebuildVariant::{Build, BuildVm};
+    match self.common.out_link.clone() {
+      Some(p) => Ok((p, None)),
       None => {
         let (path, guard) = if matches!(variant, BuildVm | Build) {
           (PathBuf::from("result"), None)
         } else {
           let dir = tempfile::Builder::new().prefix("nh-os").tempdir()?;
           (dir.as_ref().join("result"), Some(dir))
         };
-        (path, guard)
+        Ok((path, guard))
       },
-    };
+    }
+  }
 
-    // Use NH_OS_FLAKE if available, otherwise use the provided installable
+  fn resolve_installable_and_toplevel(
+    &self,
+    target_hostname: &str,
+    final_attr: Option<String>,
+  ) -> Result<Installable> {
     let installable = if let Some(flake_installable) = parse_nh_os_flake_env()?
     {
       flake_installable
     } else {
       self.common.installable.clone()
     };
 
-    let toplevel = toplevel_for(
-      &target_hostname,
+    Ok(toplevel_for(
+      target_hostname,
       installable,
       final_attr.as_deref().unwrap_or("toplevel"),
-    );
-
-    let message = match variant {
-      BuildVm => "Building NixOS VM image",
-      _ => "Building NixOS configuration",
-    };
+    ))
+  }
 
+  fn execute_build_command(
+    &self,
+    toplevel: Installable,
+    out_path: &Path,
+    message: &str,
+  ) -> Result<()> {
     commands::Build::new(toplevel)
       .extra_arg("--out-link")
-      .extra_arg(&out_path)
+      .extra_arg(out_path)
       .extra_args(&self.extra_args)
       .passthrough(&self.common.passthrough)
       .builder(self.build_host.clone())
       .message(message)
       .nom(!self.common.no_nom)
       .run()
-      .wrap_err("Failed to build configuration")?;
+      .wrap_err("Failed to build configuration")
+  }
 
+  fn resolve_specialisation_and_profile(
+    &self,
+    out_path: &Path,
+  ) -> Result<PathBuf> {
     let current_specialisation = std::fs::read_to_string(SPEC_LOCATION).ok();
 
     let target_specialisation = if self.no_specialisation {
@@ -183,7 +260,7 @@ impl OsRebuildArgs {
     debug!("Target specialisation: {target_specialisation:?}");
 
     let target_profile = target_specialisation.as_ref().map_or_else(
-      || out_path.clone(),
+      || out_path.to_path_buf(),
       |spec| out_path.join("specialisation").join(spec),
     );
 
@@ -201,10 +278,13 @@ impl OsRebuildArgs {
       ));
     }
 
+    Ok(target_profile)
+  }
+
+  fn handle_dix_diff(&self, target_profile: &Path) -> Result<()> {
     match self.common.diff {
       DiffType::Always => {
-        let _ =
-          print_dix_diff(&PathBuf::from(CURRENT_PROFILE), &target_profile);
+        let _ = print_dix_diff(&PathBuf::from(CURRENT_PROFILE), target_profile);
       },
       DiffType::Never => {
         debug!("Not running dix as the --diff flag is set to never.");
@@ -221,7 +301,7 @@ impl OsRebuildArgs {
             target_profile.display()
           );
           let _ =
-            print_dix_diff(&PathBuf::from(CURRENT_PROFILE), &target_profile);
+            print_dix_diff(&PathBuf::from(CURRENT_PROFILE), target_profile);
         } else {
           debug!(
             "Not running dix as a remote host is involved or an explicit \
@@ -230,19 +310,19 @@ impl OsRebuildArgs {
         }
       },
     }
+    Ok(())
+  }
 
-    if self.common.dry || matches!(variant, Build | BuildVm) {
-      if self.common.ask {
-        warn!("--ask has no effect as dry run was requested");
-      }
-
-      // For VM builds, print instructions on how to run the VM
-      if matches!(variant, BuildVm) && !self.common.dry {
-        print_vm_instructions(&out_path)?;
-      }
-
-      return Ok(());
-    }
+  #[expect(clippy::too_many_arguments)]
+  fn activate_rebuilt_config(
+    &self,
+    variant: &OsRebuildVariant,
+    out_path: &Path,
+    target_profile: &Path,
+    elevate: bool,
+    elevation: ElevationStrategy,
+  ) -> Result<()> {
+    use OsRebuildVariant::{Boot, Switch, Test};
 
     if self.common.ask {
       let confirmation = inquire::Confirm::new("Apply the config?")
@@ -315,8 +395,8 @@ impl OsRebuildArgs {
 
       let mut cmd = Command::new(switch_to_configuration)
         .arg("boot")
-        .ssh(self.target_host)
-        .elevate(elevate.then_some(elevation))
+        .ssh(self.target_host.clone())
+        .elevate(elevate.then_some(elevation.clone()))
         .message("Adding configuration to bootloader")
         .preserve_envs(["NIXOS_INSTALL_BOOTLOADER"]);
 
@@ -331,7 +411,6 @@ impl OsRebuildArgs {
     }
 
     debug!("Completed {variant:?} operation with output path: {out_path:?}");
-
     Ok(())
   }
 }
@@ -517,42 +596,40 @@ impl OsRollbackArgs {
 fn find_vm_script(out_path: &Path) -> Result<PathBuf> {
   let bin_dir = out_path.join("bin");
 
-  if !bin_dir.exists() {
+  if !bin_dir.is_dir() {
     bail!(
       "VM build output missing bin directory at {}",
       bin_dir.display()
     );
   }
 
-  let entries = fs::read_dir(&bin_dir).wrap_err_with(|| {
-    format!("Failed to read bin directory at {}", bin_dir.display())
-  })?;
-
-  let mut vm_script: Option<PathBuf> = None;
-  for entry_result in entries {
-    match entry_result {
-      Ok(entry) => {
-        let fname = entry.file_name();
-        if fname
-          .to_str()
-          .is_some_and(|name| name.starts_with("run-") && name.ends_with("-vm"))
-        {
-          vm_script = Some(entry.path());
-          break;
-        }
-      },
-      Err(e) => {
-        warn!(
-          "Error reading entry in VM bin directory ({}): {}",
-          bin_dir.display(),
-          e
-        );
-      },
-    }
-  }
-  let vm_script = vm_script.ok_or_else(|| {
-    eyre!("Could not find VM runner script in {}", bin_dir.display())
-  })?;
+  let vm_script = fs::read_dir(&bin_dir)
+    .wrap_err_with(|| {
+      format!("Failed to read directory {}", bin_dir.display())
+    })?
+    .filter_map(|entry_result| {
+      match entry_result {
+        Ok(entry) => Some(entry),
+        Err(e) => {
+          warn!("Error reading entry in {}: {}", bin_dir.display(), e);
+          None
+        },
+      }
+    })
+    .find_map(|entry| {
+      let fname = entry.file_name();
+      if fname
+        .to_str()
+        .is_some_and(|name| name.starts_with("run-") && name.ends_with("-vm"))
+      {
+        Some(entry.path())
+      } else {
+        None
+      }
+    })
+    .ok_or_else(|| {
+      eyre!("Could not find VM runner script in {}", bin_dir.display())
+    })?;
 
   Ok(vm_script)
 }