diff --git a/ares-install/src/install.rs b/ares-install/src/install.rs index 93e04ad..42f837a 100644 --- a/ares-install/src/install.rs +++ b/ares-install/src/install.rs @@ -19,6 +19,11 @@ pub(crate) trait InstallApp { #[derive(Debug)] pub enum InstallError { Response { error_code: i32, reason: String }, + /// The install stream ended without ever saying how it went. + NoVerdict, + /// The connection went down with the install already under way, so what + /// the device did with the package is not known. + Interrupted { device: String }, ChecksumMismatch { expected: String, actual: String }, Luna(LunaError), Transfer(TransferError), @@ -31,6 +36,17 @@ impl Display for InstallError { InstallError::Response { error_code, reason } => { write!(f, "{reason} (error {error_code})") } + InstallError::NoVerdict => write!( + f, + "the device stopped reporting before it said whether the package installed" + ), + InstallError::Interrupted { device } => write!( + f, + "lost the connection to {device} while the install was running. The device may \ + have installed the package anyway - `ares-install -d {device} --listfull` \ + prints the version that is on it. `--list` alone only names the app, which \ + says nothing about which build landed" + ), InstallError::ChecksumMismatch { expected, actual } => write!( f, "uploaded package is corrupted: expected sha256 {expected}, device has {actual}" @@ -74,9 +90,8 @@ impl InstallApp for DeviceSession { IoError::new( ErrorKind::Other, format!( - "Failed to generate checksum for {}: {:?}", - package.as_ref().to_string_lossy(), - e + "Failed to generate checksum for {}: {e}", + package.as_ref().to_string_lossy() ), ) })?; @@ -114,7 +129,7 @@ impl InstallApp for DeviceSession { pb.set_message("Checking uploaded package"); let verified = verify_upload(self, &ipk_path, &checksum); if let Err(e) = &verified { - pb.suspend(|| eprintln!("Upload of {package_display_name} is broken: {e:?}")); + pb.suspend(|| eprintln!("Upload of {package_display_name} is broken: {e}")); } let result = verified.and_then(|_| { @@ -152,22 +167,51 @@ impl InstallApp for DeviceSession { ) }) .next() - .unwrap_or_else(|| Ok(String::new())), + // Reaching the end of the stream having seen neither + // "installed" nor a failure is not a success. Reporting one + // claims a package is on the device that may well not be. + .unwrap_or(Err(InstallError::NoVerdict)), Err(e) => Err(e.into()), } }); + // The install request is sent before the device answers it, so a + // connection that dies here says nothing about what the device did - + // and on some sets the install goes through regardless. Calling that a + // failed install is a guess, and the wrong one often enough to matter. + let result = result.map_err(|e| match e { + InstallError::Luna(LunaError::Session(_)) | InstallError::Io(_) + if !self.is_connected() => + { + InstallError::Interrupted { + device: self.device.name.clone(), + } + } + other => other, + }); + if let Ok(package_id) = &result { pb.suspend(|| println!("Installed package {}!", package_id)); } - pb.suspend(|| println!("Deleting uploaded package...")); pb.set_prefix("Cleanup"); pb.set_message("Deleting uploaded package"); - if let Err(e) = self.rm(&ipk_path) { + // Cleaning up over a connection that is already gone only produces a + // second, confusing error - and it lands before the one that explains + // the run, because this happens on the way out. Say what was left + // behind instead, and let the real error through. + if self.is_connected() { + pb.suspend(|| println!("Deleting uploaded package...")); + if let Err(e) = self.rm(&ipk_path) { + pb.suspend(|| eprintln!("Failed to delete {ipk_path}: {e}")); + } + } else { pb.suspend(|| { - eprintln!("Failed to delete {}: {:?}", ipk_path, e); + eprintln!( + "Lost the connection to {}, so {ipk_path} is still on the device.", + self.device.name + ); }); } pb.finish_and_clear(); diff --git a/ares-install/src/main.rs b/ares-install/src/main.rs index 28364e3..839b65f 100644 --- a/ares-install/src/main.rs +++ b/ares-install/src/main.rs @@ -5,7 +5,7 @@ use ares_connection_lib::session::NewSession; use ares_device_lib::DeviceManager; use ares_device_lib::cli::unwrap_or_exit; use clap::Parser; -use install::InstallApp; +use install::{InstallApp, InstallError}; use list::ListApps; use crate::remove::RemoveApp; @@ -80,6 +80,13 @@ fn main() { } else if let Some(package) = cli.package { match session.install_app(package) { Ok(_) => {} + // An interrupted install reports itself: "Failed to install" would + // contradict the one thing the message has to say, which is that + // nobody knows yet. + Err(e @ InstallError::Interrupted { .. }) => { + eprintln!("{e}"); + exit(1); + } Err(e) => { eprintln!("Failed to install: {e}"); exit(1);