diff --git a/src/commands.rs b/src/commands.rs index 94976b8f..064faea1 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -29,6 +29,7 @@ pub struct Command { args: Vec, elevate: bool, ssh: Option, + show_output: bool, } impl Command { @@ -40,6 +41,7 @@ impl Command { args: vec![], elevate: false, ssh: None, + show_output: false, } } @@ -53,6 +55,11 @@ impl Command { self } + pub const fn show_output(mut self, show_output: bool) -> Self { + self.show_output = show_output; + self + } + pub fn ssh(mut self, ssh: Option) -> Self { self.ssh = ssh; self @@ -113,8 +120,15 @@ impl Command { } else { Exec::cmd(&self.command).args(&self.args) }; - let cmd = - ssh_wrap(cmd.stderr(Redirection::None), self.ssh.as_deref()).stdout(Redirection::None); + // Configure output redirection based on show_output setting + let cmd = ssh_wrap( + if self.show_output { + cmd.stderr(Redirection::Merge) + } else { + cmd.stderr(Redirection::None).stdout(Redirection::None) + }, + self.ssh.as_deref(), + ); if let Some(m) = &self.message { info!("{}", m); diff --git a/src/darwin.rs b/src/darwin.rs index fb1b4427..c5753145 100644 --- a/src/darwin.rs +++ b/src/darwin.rs @@ -103,6 +103,16 @@ impl DarwinRebuildArgs { let target_profile = out_path.get_path().to_owned(); + // Take a strong reference to out_path to prevent premature dropping + // We need to keep this alive through the entire function scope to prevent + // the tempdir from being dropped early, which would cause nvd diff to fail + #[allow(unused_variables)] + let keep_alive = out_path.get_path().to_owned(); + debug!( + "Registered keep_alive reference to: {}", + keep_alive.display() + ); + target_profile.try_exists().context("Doesn't exist")?; Command::new("nvd") @@ -110,6 +120,7 @@ impl DarwinRebuildArgs { .arg(CURRENT_PROFILE) .arg(&target_profile) .message("Comparing changes") + .show_output(true) .run()?; if self.common.ask && !self.common.dry && !matches!(variant, Build) { @@ -151,6 +162,10 @@ impl DarwinRebuildArgs { // Make sure out_path is not accidentally dropped // https://docs.rs/tempfile/3.12.0/tempfile/index.html#early-drop-pitfall + debug!( + "Completed operation with output path: {:?}", + out_path.get_path() + ); drop(out_path); Ok(()) diff --git a/src/generations.rs b/src/generations.rs index 1e00f200..b1ea8b74 100644 --- a/src/generations.rs +++ b/src/generations.rs @@ -43,7 +43,7 @@ pub fn from_dir(generation_dir: &Path) -> Option { }) } -pub fn describe(generation_dir: &Path, current_profile: &Path) -> Option { +pub fn describe(generation_dir: &Path) -> Option { let generation_number = from_dir(generation_dir)?; // Get metadata once and reuse for both date and existence checks diff --git a/src/home.rs b/src/home.rs index 1817ea9b..4d7bd62e 100644 --- a/src/home.rs +++ b/src/home.rs @@ -112,10 +112,17 @@ impl HomeRebuildArgs { debug!("target_specialisation: {target_specialisation:?}"); - let target_profile: Box = match &target_specialisation { - None => out_path, - Some(spec) => Box::new(out_path.get_path().join("specialisation").join(spec)), - }; + let target_profile: Box = + if let Some(spec) = &target_specialisation { + Box::new(out_path.get_path().join("specialisation").join(spec)) + } else { + out_path + }; + + // Take a strong reference to ensure the TempDir isn't dropped + // This prevents early dropping of the tempdir, which causes nvd diff to fail + #[allow(unused_variables)] + let keep_alive = target_profile.get_path().to_owned(); // just do nothing for None case (fresh installs) if let Some(generation) = prev_generation { @@ -124,6 +131,7 @@ impl HomeRebuildArgs { .arg(generation) .arg(target_profile.get_path()) .message("Comparing changes") + .show_output(true) .run()?; } @@ -154,6 +162,10 @@ impl HomeRebuildArgs { // Make sure out_path is not accidentally dropped // https://docs.rs/tempfile/3.12.0/tempfile/index.html#early-drop-pitfall + debug!( + "Completed operation with output path: {:?}", + target_profile.get_path() + ); drop(target_profile); Ok(()) diff --git a/src/nixos.rs b/src/nixos.rs index 775828d7..6400d262 100644 --- a/src/nixos.rs +++ b/src/nixos.rs @@ -56,6 +56,7 @@ enum OsRebuildVariant { impl OsBuildVmArgs { fn build_vm(self) -> Result<()> { let final_attr = get_final_attr(true, self.with_bootloader); + debug!("Building VM with attribute: {}", final_attr); self.common .rebuild(OsRebuildVariant::BuildVm, Some(final_attr)) } @@ -97,7 +98,16 @@ impl OsRebuildArgs { Some(h) => h.to_owned(), None => match &system_hostname { Some(hostname) => { - tracing::warn!("Guessing system is {hostname} for a VM image. If this isn't intended, use --hostname to change."); + // Only show the warning if we're explicitly building a VM + // by directly calling build_vm(), not when the BuildVm variant + // is used internally via other code paths + if matches!(variant, OsRebuildVariant::BuildVm) + && final_attr + .as_deref() + .map_or(false, |attr| attr == "vm" || attr == "vmWithBootLoader") + { + tracing::warn!("Guessing system is {hostname} for a VM image. If this isn't intended, use --hostname to change."); + } hostname.clone() } None => return Err(eyre!("Unable to fetch hostname, and no hostname supplied.")), @@ -143,7 +153,6 @@ impl OsRebuildArgs { BuildVm => "Building NixOS VM image", _ => "Building NixOS configuration", }; - commands::Build::new(toplevel) .extra_arg("--out-link") .extra_arg(out_path.get_path()) @@ -168,19 +177,38 @@ impl OsRebuildArgs { Some(spec) => out_path.get_path().join("specialisation").join(spec), }; - debug!("exists: {}", target_profile.exists()); + debug!("Output path: {}", out_path.get_path().display()); + debug!("Target profile path: {}", target_profile.display()); + debug!("Target profile exists: {}", target_profile.exists()); + + // Note: out_path itself is kept alive until the end of this function, + // which prevents the tempdir (if any) from being dropped early - target_profile.try_exists().context("Doesn't exist")?; + if !target_profile + .try_exists() + .context("Failed to check if target profile exists")? + { + return Err(eyre!( + "Target profile path does not exist: {}", + target_profile.display() + )); + } if self.build_host.is_none() && self.target_host.is_none() && system_hostname.map_or(true, |h| h == target_hostname) { + debug!( + "Comparing with target profile: {}", + target_profile.display() + ); + Command::new("nvd") .arg("diff") .arg(CURRENT_PROFILE) .arg(&target_profile) .message("Comparing changes") + .show_output(true) .run()?; } else { debug!("Not running nvd as the target hostname is different from the system hostname."); @@ -253,8 +281,12 @@ impl OsRebuildArgs { .run()?; } - // Make sure out_path is not accidentally dropped + // Make sure out_path is not acidentally dropped // https://docs.rs/tempfile/3.12.0/tempfile/index.html#early-drop-pitfall + debug!( + "Completed operation with output path: {:?}", + out_path.get_path() + ); drop(out_path); Ok(()) @@ -305,6 +337,7 @@ impl OsRollbackArgs { .arg(CURRENT_PROFILE) .arg(&generation_link) .message("Comparing changes") + .show_output(true) .run()?; if self.dry { @@ -345,9 +378,6 @@ impl OsRollbackArgs { .message("Setting system profile") .run()?; - // Set up rollback protection flag - let mut rollback_profile = false; - // Determine the correct profile to use with specialisations let final_profile = match &target_specialisation { None => generation_link, @@ -383,10 +413,8 @@ impl OsRollbackArgs { ); } Err(e) => { - rollback_profile = true; - // If activation fails, rollback the profile - if rollback_profile && current_gen_number > 0 { + if current_gen_number > 0 { let current_gen_link = profile_dir.join(format!("system-{current_gen_number}-link")); @@ -421,7 +449,7 @@ fn find_previous_generation() -> Result { if let Some(filename) = path.file_name() { if let Some(name) = filename.to_str() { if name.starts_with("system-") && name.ends_with("-link") { - return generations::describe(&path, &profile_path); + return generations::describe(&path); } } } @@ -467,7 +495,7 @@ fn find_generation_by_number(number: u64) -> Result if let Some(filename) = path.file_name() { if let Some(name) = filename.to_str() { if name.starts_with("system-") && name.ends_with("-link") { - return generations::describe(&path, &profile_path); + return generations::describe(&path); } } } @@ -492,11 +520,7 @@ fn get_current_generation_number() -> Result { .parent() .unwrap_or(Path::new("/nix/var/nix/profiles")), )? - .filter_map(|entry| { - entry - .ok() - .and_then(|e| generations::describe(&e.path(), &profile_path)) - }) + .filter_map(|entry| entry.ok().and_then(|e| generations::describe(&e.path()))) .collect(); let current_gen = generations @@ -641,7 +665,7 @@ impl OsGenerationsArgs { let descriptions: Vec = generations .iter() - .filter_map(|gen_dir| generations::describe(gen_dir, &profile)) + .filter_map(|gen_dir| generations::describe(gen_dir)) .collect(); generations::print_info(descriptions);