Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
965 changes: 819 additions & 146 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ hostname = "0.4.1"
humantime = "2.3.0"
inquire = { default-features = false, version = "0.9.1", features = [ "crossterm" ] }
nix = { default-features = false, features = [ "fs", "user" ], version = "0.30.1" }
notify-rust = "4.11.7"
regex = "1.11.3"
reqwest = { default-features = false, features = [
"rustls-tls-native-roots",
Expand Down
38 changes: 27 additions & 11 deletions src/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ use nix::{
unistd::{AccessFlags, faccessat},
};
use regex::Regex;
use tracing::{Level, debug, info, instrument, span, warn};
use tracing::{Level, debug, instrument, span, warn};
use yansi::{Color, Paint};

use crate::{
Result,
commands::{Command, ElevationStrategy},
interface,
interface::{self, NotifyAskMode},
nh_info,
notify::NotificationSender,
};

// Nix impl:
Expand Down Expand Up @@ -249,7 +251,7 @@ impl interface::CleanMode {
Err(err) => {
warn!(?err, ?now, "Failed to compare time!");
},
Ok(val) if val <= args.keep_since.into() => {
Ok(val) if val <= std::time::Duration::from(args.keep_since) => {
gcroots_tagged.insert(dst, false);
},
Ok(_) => {
Expand Down Expand Up @@ -333,12 +335,26 @@ impl interface::CleanMode {
}

// Clean the paths
if args.ask
&& !Confirm::new("Confirm the cleanup plan?")
.with_default(false)
.prompt()?
{
bail!("User rejected the cleanup plan");
if let Some(ask) = &args.ask {
let confirmation = match ask {
NotifyAskMode::Prompt => {
Confirm::new("Confirm the cleanup plan?")
.with_default(false)
.prompt()?
},
#[cfg(all(unix, not(target_os = "macos")))]
NotifyAskMode::Notify => {
NotificationSender::new(
"nh clean",
"Do you want to confirm the cleanup plan?",
)
.ask()
},
};

if !confirmation {
bail!("User rejected the cleanup plan");
}
}

if !args.dry {
Expand Down Expand Up @@ -488,7 +504,7 @@ fn cleanable_generations(
Err(err) => {
warn!(?err, ?now, ?generation, "Failed to compare time!");
},
Ok(val) if val <= keep_since.into() => {
Ok(val) if val <= std::time::Duration::from(keep_since) => {
*tbr = false;
},
Ok(_) => {},
Expand All @@ -504,7 +520,7 @@ fn cleanable_generations(
}

fn remove_path_nofail(path: &Path) {
info!("Removing {}", path.to_string_lossy());
nh_info!("Removing {}", path.to_string_lossy());
if let Err(err) = std::fs::remove_file(path) {
warn!(?path, ?err, "Failed to remove path");
}
Expand Down
14 changes: 9 additions & 5 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@ use color_eyre::{
use secrecy::{ExposeSecret, SecretString};
use subprocess::{Exec, ExitStatus, Redirection};
use thiserror::Error;
use tracing::{debug, info, warn};
use tracing::{debug, warn};
use which::which;

use crate::{installable::Installable, interface::NixBuildPassthroughArgs};
use crate::{
installable::Installable,
interface::NixBuildPassthroughArgs,
nh_info,
};

static PASSWORD_CACHE: OnceLock<Mutex<HashMap<String, SecretString>>> =
OnceLock::new();
Expand Down Expand Up @@ -539,7 +543,7 @@ impl Command {
);

if let Some(m) = &self.message {
info!("{m}");
nh_info!("{m}");
}

debug!(?cmd);
Expand Down Expand Up @@ -589,7 +593,7 @@ impl Command {
);

if let Some(m) = &self.message {
info!("{m}");
nh_info!("{m}");
}

debug!(?cmd);
Expand Down Expand Up @@ -670,7 +674,7 @@ impl Build {
/// Returns an error if the build command fails to execute.
pub fn run(&self) -> Result<()> {
if let Some(m) = &self.message {
info!("{m}");
nh_info!("{m}");
}

let installable_args = self.installable.to_args();
Expand Down
42 changes: 34 additions & 8 deletions src/darwin.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{env, path::PathBuf};
use std::{env, fmt, path::PathBuf};

use color_eyre::eyre::{Context, bail, eyre};
use tracing::{debug, warn};
Expand All @@ -14,8 +14,10 @@ use crate::{
DarwinReplArgs,
DarwinSubcommand,
DiffType,
NotifyAskMode,
},
nixos::toplevel_for,
notify::NotificationSender,
update::update,
util::{get_hostname, print_dix_diff},
};
Expand All @@ -34,7 +36,7 @@ impl DarwinArgs {
match self.subcommand {
DarwinSubcommand::Switch(args) => args.rebuild(&Switch, elevation),
DarwinSubcommand::Build(args) => {
if args.common.ask || args.common.dry {
if args.common.ask.is_some() || args.common.dry {
warn!("`--ask` and `--dry` have no effect for `nh darwin build`");
}
args.rebuild(&Build, elevation)
Expand All @@ -49,6 +51,16 @@ enum DarwinRebuildVariant {
Build,
}

impl fmt::Display for DarwinRebuildVariant {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let s = match self {
DarwinRebuildVariant::Build => "build",
DarwinRebuildVariant::Switch => "switch",
};
write!(f, "{s}")
}
}

impl DarwinRebuildArgs {
fn rebuild(
self,
Expand Down Expand Up @@ -144,13 +156,27 @@ impl DarwinRebuildArgs {
let _ = print_dix_diff(&PathBuf::from(CURRENT_PROFILE), &target_profile);
}

if self.common.ask && !self.common.dry && !matches!(variant, Build) {
let confirmation = inquire::Confirm::new("Apply the config?")
.with_default(false)
.prompt()?;
if !self.common.dry && !matches!(variant, Build) {
if let Some(ask) = self.common.ask {
let confirmation = match ask {
NotifyAskMode::Prompt => {
inquire::Confirm::new("Apply the config?")
.with_default(false)
.prompt()?
},
#[cfg(all(unix, not(target_os = "macos")))]
NotifyAskMode::Notify => {
NotificationSender::new(
&format!("nh darwin {variant}"),
"Do you want to apply the Darwin configuration?",
)
.ask()
},
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +168 to +175
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Handle notification backend failures gracefully.

NotificationSender::ask() still unwraps notify-rust’s show() internally, so if the notification daemon is missing or D-Bus errors, this branch will panic before the user ever sees a terminal prompt. Please make ask() return Result<bool> and fall back to the existing Confirm prompt on error so nh darwin does not crash.

         #[cfg(all(unix, not(target_os = "macos")))]
         NotifyAskMode::Notify => {
-          NotificationSender::new(
-            &format!("nh darwin {variant}"),
-            "Do you want to apply the Darwin configuration?",
-          )
-          .ask()
+          match NotificationSender::new(
+            &format!("nh darwin {variant}"),
+            "Do you want to apply the Darwin configuration?",
+          )
+          .ask()
+          {
+            Ok(confirmed) => confirmed,
+            Err(e) => {
+              debug!(?e, "Notification prompt failed; falling back to terminal prompt");
+              inquire::Confirm::new("Apply the config?")
+                .with_default(false)
+                .prompt()?
+            }
+          }
         },

Do the corresponding NotificationSender::ask() change (returning Result<bool>) so this compiles cleanly. The same adjustment will be needed at the other call sites in this PR.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/darwin.rs around lines 168 to 175, the Notify branch calls
NotificationSender::ask() which currently panics on backend failures; change the
call to handle a Result return from ask() — update NotificationSender::ask() to
return Result<bool, E> (or anyhow::Error) and here call
.ask().unwrap_or_else(|_| Confirm::new("Do you want to apply the Darwin
configuration?").interact().map_err(|e| e.into())?). Specifically: modify
NotificationSender::ask() to return Result<bool>, then in this match arm call
ask(), match on Ok(true)/Ok(false) and on Err(_) fall back to the existing
Confirm prompt (prompt synchronously and return its boolean), propagating errors
as a Result from this function so compilation remains correct; apply analogous
changes at the other call sites in this PR.


if !confirmation {
bail!("User rejected the new config");
if !confirmation {
bail!("User rejected the new config");
}
}
}

Expand Down
39 changes: 30 additions & 9 deletions src/home.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,22 @@ use color_eyre::{
Result,
eyre::{Context, bail, eyre},
};
use tracing::{debug, info, warn};
use tracing::{debug, warn};

use crate::{
commands,
commands::Command,
installable::Installable,
interface::{self, DiffType, HomeRebuildArgs, HomeReplArgs, HomeSubcommand},
interface::{
self,
DiffType,
HomeRebuildArgs,
HomeReplArgs,
HomeSubcommand,
NotifyAskMode,
},
nh_info,
notify::NotificationSender,
update::update,
util::{get_hostname, print_dix_diff},
};
Expand All @@ -26,7 +35,7 @@ impl interface::HomeArgs {
match self.subcommand {
HomeSubcommand::Switch(args) => args.rebuild(&Switch),
HomeSubcommand::Build(args) => {
if args.common.ask || args.common.dry {
if args.common.ask.is_some() || args.common.dry {
warn!("`--ask` and `--dry` have no effect for `nh home build`");
}
args.rebuild(&Build)
Expand Down Expand Up @@ -150,24 +159,36 @@ impl HomeRebuildArgs {
}

if self.common.dry || matches!(variant, Build) {
if self.common.ask {
if self.common.ask.is_some() {
warn!("--ask has no effect as dry run was requested");
}
return Ok(());
}

if self.common.ask {
let confirmation = inquire::Confirm::new("Apply the config?")
.with_default(false)
.prompt()?;
if let Some(ask) = &self.common.ask {
let confirmation = match ask {
NotifyAskMode::Prompt => {
inquire::Confirm::new("Apply the config?")
.with_default(false)
.prompt()?
},
#[cfg(all(unix, not(target_os = "macos")))]
NotifyAskMode::Notify => {
NotificationSender::new(
"nh home switch",
"Do you want to apply the Home Manager configuration?",
)
.ask()
},
};

if !confirmation {
bail!("User rejected the new config");
}
}

if let Some(ext) = &self.backup_extension {
info!("Using {} as the backup extension", ext);
nh_info!("Using {} as the backup extension", ext);
unsafe {
env::set_var("HOME_MANAGER_BACKUP_EXT", ext);
}
Expand Down
21 changes: 15 additions & 6 deletions src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,15 +264,24 @@ pub enum DiffType {
Never,
}

#[derive(Debug, Clone, ValueEnum, PartialEq)]
pub enum NotifyAskMode {
/// Ask in the terminal (stdin prompt)
Prompt,
/// Ask via a desktop notification action
#[cfg(all(unix, not(target_os = "macos")))]
Notify,
}

#[derive(Debug, Args)]
pub struct OsRollbackArgs {
/// Only print actions, without performing them
#[arg(long, short = 'n')]
pub dry: bool,

/// Ask for confirmation
#[arg(long, short)]
pub ask: bool,
#[arg(long, short, value_enum, default_missing_value = "prompt", num_args = 0..=1)]
pub ask: Option<NotifyAskMode>,
Comment on lines +283 to +284
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: Missing value_enum attribute causes CLI parsing failures.

OsRollbackArgs.ask (line 284) includes value_enum in its clap attribute, but CommonRebuildArgs.ask (line 316) and CleanArgs.ask (line 448) do not. Without value_enum, clap cannot parse the enum variants from the command line, so --ask=notify and --ask=both will fail with a parsing error.

Apply this fix:

   /// Ask for confirmation
-  #[arg(long, short, default_missing_value = "prompt", num_args = 0..=1)]
+  #[arg(long, short, value_enum, default_missing_value = "prompt", num_args = 0..=1)]
   pub ask: Option<NotifyAskMode>,

for both CommonRebuildArgs (line 316) and CleanArgs (line 448).

Run this script to verify all ask fields consistently use value_enum:

Also applies to: 316-317, 448-449


🏁 Script executed:

#!/bin/bash
# Find all ask field declarations and their attributes
rg -n -A1 -B1 'pub ask: Option<NotifyAskMode>' src/interface.rs

# Check which ones have value_enum in their clap attribute
rg -n -B2 'pub ask: Option<NotifyAskMode>' src/interface.rs | grep -E '(#\[arg|pub ask)'

Length of output: 740




🏁 Script executed:

#!/bin/bash
# Find NotifyAskMode definition and its traits/derives
rg -n -A10 'enum NotifyAskMode' src/interface.rs

# Check if NotifyAskMode is derived with any trait that enables FromStr
rg -n -B5 'enum NotifyAskMode' src/interface.rs | head -20

Length of output: 440


🏁 Script executed:

#!/bin/bash
# Search for any FromStr implementation for NotifyAskMode
rg -n 'impl.*FromStr.*NotifyAskMode|NotifyAskMode.*FromStr' src/

# Also search for clap derive or value_enum usage patterns
rg -n '#\[derive.*Clap|#\[derive.*ValueEnum' src/interface.rs | head -10

Length of output: 136




🏁 Script executed:

#!/bin/bash
# Search for tests exercising the --ask flag
rg -n '\-\-ask|ask.*Option.*NotifyAskMode' tests/ --type rust 2>/dev/null | head -20

# Also check if there's any test or integration test
fd -e rs tests/ 2>/dev/null | head -10

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check clap's ValueEnum behavior - verify that derived ValueEnum works without explicit value_enum attribute
# Search for any documentation or comments about this pattern
rg -n 'ValueEnum|value_enum' src/interface.rs | head -20

Length of output: 438


Add missing value_enum attribute to fix CLI parsing failures.

The verification confirms the issue: NotifyAskMode derives ValueEnum, but the value_enum attribute in the #[arg(...)] macro is still required to enable proper enum parsing. Without it, clap cannot parse enum variants from command-line arguments.

  • Line 284 (OsRollbackArgs): ✓ Correct, has value_enum
  • Line 316 (CommonRebuildArgs): ✗ Missing value_enum
  • Line 448 (CleanArgs): ✗ Missing value_enum

Apply this fix to lines 316 and 448:

-  #[arg(long, short, default_missing_value = "prompt", num_args = 0..=1)]
+  #[arg(long, short, value_enum, default_missing_value = "prompt", num_args = 0..=1)]
   pub ask: Option<NotifyAskMode>,

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/interface.rs around lines 316 and 448, the #[arg(...)] macros for the
fields using NotifyAskMode are missing the value_enum attribute, which prevents
clap from parsing enum variants even though NotifyAskMode derives ValueEnum;
update those two arg attributes to include value_enum (matching the one at line
284) so the enum is parsed correctly from CLI arguments.


/// Explicitly select some specialisation
#[arg(long, short)]
Expand Down Expand Up @@ -303,8 +312,8 @@ pub struct CommonRebuildArgs {
pub dry: bool,

/// Ask for confirmation
#[arg(long, short)]
pub ask: bool,
#[arg(long, short, default_missing_value = "prompt", num_args = 0..=1)]
pub ask: Option<NotifyAskMode>,

#[command(flatten)]
pub installable: Installable,
Expand Down Expand Up @@ -435,8 +444,8 @@ pub struct CleanArgs {
pub dry: bool,

/// Ask for confirmation
#[arg(long, short)]
pub ask: bool,
#[arg(long, short, default_missing_value = "prompt", num_args = 0..=1)]
pub ask: Option<NotifyAskMode>,

/// Don't run nix store --gc
#[arg(long = "no-gc", alias = "nogc")]
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub mod interface;
pub mod json;
pub mod logging;
pub mod nixos;
pub mod notify;
pub mod search;
pub mod update;
pub mod util;
Expand Down
32 changes: 29 additions & 3 deletions src/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ where

match *level {
Level::ERROR => {
write!(writer, "{} ", Paint::new("ERROR").fg(Color::Red))?
write!(writer, "{} ", Paint::new("ERROR").fg(Color::Red))?;
},
Level::WARN => write!(writer, "{} ", Paint::new("!").fg(Color::Yellow))?,
Level::INFO => write!(writer, "{} ", Paint::new(">").fg(Color::Green))?,
Level::DEBUG => {
write!(writer, "{} ", Paint::new("DEBUG").fg(Color::Blue))?
write!(writer, "{} ", Paint::new("DEBUG").fg(Color::Blue))?;
},
Level::TRACE => {
write!(writer, "{} ", Paint::new("TRACE").fg(Color::Cyan))?
write!(writer, "{} ", Paint::new("TRACE").fg(Color::Cyan))?;
},
}

Expand Down Expand Up @@ -94,3 +94,29 @@ pub fn setup_logging(

Ok(())
}

#[macro_export]
macro_rules! nh_info {
($($arg:tt)*) => {{
use notify_rust::Urgency;
use $crate::notify::NotificationSender;
let message = format!($($arg)*);
tracing::info!($($arg)*);
if let Err(e) = NotificationSender::new("nh info", &message).urgency(Urgency::Normal).send() {
tracing::error!("{e}");
};
}};
}

#[macro_export]
macro_rules! nh_warn {
($($arg:tt)*) => {{
use notify_rust::Urgency;
use $crate::notify::NotificationSender;
let message = format!($($arg)*);
tracing::warn!($($arg)*);
if let Err(e) = NotificationSender::new("nh warn", &message).urgency(Urgency::Normal).send() {
tracing::error!("{e}");
};
}};
}
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod interface;
mod json;
mod logging;
mod nixos;
mod notify;
mod search;
mod update;
mod util;
Expand Down
Loading