* [pve-devel] [PATCH installer 0/7] refactor; improve installation progress
@ 2023-07-26 14:03 Christoph Heiss
2023-07-26 14:03 ` [pve-devel] [PATCH installer 1/7] tui: move install progress dialog into own view module Christoph Heiss
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: Christoph Heiss @ 2023-07-26 14:03 UTC (permalink / raw)
To: pve-devel
First 5 patches splits out the gigantic install progress dialog into its
own module; then applying some divide-and-conquer to make it more
readable and get the inner >150 lines closure down to a more
maintainable state.
Patch 6 does some code de-duplication for the low-lever installer.
Patch 7 improves the auto-reboot message/UX by counting down the
timeout, to match its GUI counterpart.
Christoph Heiss (7):
tui: move install progress dialog into own view module
tui: install progress: split out low-level installer spawing into own
function
tui: install progress: split out reboot handling into own function
tui: install progress: split out prompt logic into own function
tui: install progress: handle errors in ui message loop more
gracefully
low-level: avoid open-coding config reading, parsing and merging
low-level, tui: count down auto-reboot timeout
proxmox-low-level-installer | 70 +++--
proxmox-tui-installer/src/main.rs | 235 +--------------
.../src/views/install_progress.rs | 275 ++++++++++++++++++
proxmox-tui-installer/src/views/mod.rs | 3 +
4 files changed, 318 insertions(+), 265 deletions(-)
create mode 100644 proxmox-tui-installer/src/views/install_progress.rs
--
2.41.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [pve-devel] [PATCH installer 1/7] tui: move install progress dialog into own view module 2023-07-26 14:03 [pve-devel] [PATCH installer 0/7] refactor; improve installation progress Christoph Heiss @ 2023-07-26 14:03 ` Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 2/7] tui: install progress: split out low-level installer spawing into own function Christoph Heiss ` (5 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Christoph Heiss @ 2023-07-26 14:03 UTC (permalink / raw) To: pve-devel While doing so, also pull the main progress thread task into a separate function, as well as the spawning of the low-level installer. No functional changes. Signed-off-by: Christoph Heiss <c.heiss@proxmox.com> --- proxmox-tui-installer/src/main.rs | 235 +--------------- .../src/views/install_progress.rs | 251 ++++++++++++++++++ proxmox-tui-installer/src/views/mod.rs | 3 + 3 files changed, 261 insertions(+), 228 deletions(-) create mode 100644 proxmox-tui-installer/src/views/install_progress.rs diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs index 7bfaf9b..a356f6d 100644 --- a/proxmox-tui-installer/src/main.rs +++ b/proxmox-tui-installer/src/main.rs @@ -1,24 +1,12 @@ -use std::{ - collections::HashMap, - env, - io::{BufRead, BufReader, Write}, - net::IpAddr, - path::PathBuf, - str::FromStr, - sync::{Arc, Mutex}, - thread, - time::Duration, -}; +use std::{collections::HashMap, env, net::IpAddr, path::PathBuf}; use cursive::{ event::Event, theme::{ColorStyle, Effect, PaletteColor, Style}, - utils::Counter, view::{Nameable, Offset, Resizable, ViewWrapper}, views::{ Button, Checkbox, Dialog, DummyView, EditView, Layer, LinearLayout, PaddedView, Panel, - ProgressBar, ResizedView, ScrollView, SelectView, StackView, TextContent, TextView, - ViewRef, + ResizedView, ScrollView, SelectView, StackView, TextView, ViewRef, }, Cursive, CursiveRunnable, ScreenId, View, XY, }; @@ -29,7 +17,7 @@ mod options; use options::*; mod setup; -use setup::{InstallConfig, LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo}; +use setup::{LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo}; mod system; @@ -38,8 +26,8 @@ use utils::Fqdn; mod views; use views::{ - BootdiskOptionsView, CidrAddressEditView, FormView, TableView, TableViewItem, - TimezoneOptionsView, + BootdiskOptionsView, CidrAddressEditView, FormView, InstallProgressView, TableView, + TableViewItem, TimezoneOptionsView, }; // TextView::center() seems to garble the first two lines, so fix it manually here. @@ -720,215 +708,6 @@ fn install_progress_dialog(siv: &mut Cursive) -> InstallerView { // Ensure the screen is updated independently of keyboard events and such siv.set_autorefresh(true); - let cb_sink = siv.cb_sink().clone(); - let state = siv.user_data::<InstallerState>().unwrap(); - let progress_text = TextContent::new("starting the installation .."); - - let progress_task = { - let progress_text = progress_text.clone(); - let options = state.options.clone(); - move |counter: Counter| { - let child = { - use std::process::{Command, Stdio}; - - #[cfg(not(debug_assertions))] - let (path, args, envs): (&str, [&str; 1], [(&str, &str); 0]) = - ("proxmox-low-level-installer", ["start-session"], []); - - #[cfg(debug_assertions)] - let (path, args, envs) = ( - PathBuf::from("./proxmox-low-level-installer"), - ["-t", "start-session-test"], - [("PERL5LIB", ".")], - ); - - Command::new(path) - .args(args) - .envs(envs) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .spawn() - }; - - let mut child = match child { - Ok(child) => child, - Err(err) => { - let _ = cb_sink.send(Box::new(move |siv| { - siv.add_layer( - Dialog::text(err.to_string()) - .title("Error") - .button("Ok", Cursive::quit), - ); - })); - return; - } - }; - - let inner = || { - let reader = child.stdout.take().map(BufReader::new)?; - let mut writer = child.stdin.take()?; - - serde_json::to_writer(&mut writer, &InstallConfig::from(options)).unwrap(); - writeln!(writer).unwrap(); - - let writer = Arc::new(Mutex::new(writer)); - - for line in reader.lines() { - let line = match line { - Ok(line) => line, - Err(_) => break, - }; - - let msg = match line.parse::<UiMessage>() { - Ok(msg) => msg, - Err(stray) => { - eprintln!("low-level installer: {stray}"); - continue; - } - }; - - match msg { - UiMessage::Info(s) => cb_sink.send(Box::new(|siv| { - siv.add_layer(Dialog::info(s).title("Information")); - })), - UiMessage::Error(s) => cb_sink.send(Box::new(|siv| { - siv.add_layer(Dialog::info(s).title("Error")); - })), - UiMessage::Prompt(s) => cb_sink.send({ - let writer = writer.clone(); - Box::new(move |siv| { - yes_no_dialog( - siv, - "Prompt", - &s, - Box::new({ - let writer = writer.clone(); - move |_| { - if let Ok(mut writer) = writer.lock() { - let _ = writeln!(writer, "ok"); - } - } - }), - Box::new(move |_| { - if let Ok(mut writer) = writer.lock() { - let _ = writeln!(writer); - } - }), - ); - }) - }), - UiMessage::Progress(ratio, s) => { - counter.set(ratio); - progress_text.set_content(s); - Ok(()) - } - UiMessage::Finished(success, msg) => { - counter.set(100); - progress_text.set_content(msg.to_owned()); - cb_sink.send(Box::new(move |siv| { - let title = if success { "Success" } else { "Failure" }; - - // For rebooting, we just need to quit the installer, - // our caller does the actual reboot. - siv.add_layer( - Dialog::text(msg) - .title(title) - .button("Reboot now", Cursive::quit), - ); - - let autoreboot = siv - .user_data::<InstallerState>() - .map(|state| state.options.autoreboot) - .unwrap_or_default(); - - if autoreboot && success { - let cb_sink = siv.cb_sink(); - thread::spawn({ - let cb_sink = cb_sink.clone(); - move || { - thread::sleep(Duration::from_secs(5)); - let _ = cb_sink.send(Box::new(Cursive::quit)); - } - }); - } - })) - } - } - .unwrap(); - } - - Some(()) - }; - - if inner().is_none() { - cb_sink - .send(Box::new(|siv| { - siv.add_layer( - Dialog::text("low-level installer exited early") - .title("Error") - .button("Exit", Cursive::quit), - ); - })) - .unwrap(); - } - } - }; - - let progress_bar = ProgressBar::new().with_task(progress_task).full_width(); - let inner = PaddedView::lrtb( - 1, - 1, - 1, - 1, - LinearLayout::vertical() - .child(PaddedView::lrtb(1, 1, 0, 0, progress_bar)) - .child(DummyView) - .child(TextView::new_with_content(progress_text).center()) - .child(PaddedView::lrtb( - 1, - 1, - 1, - 0, - LinearLayout::horizontal().child(abort_install_button()), - )), - ); - - InstallerView::with_raw(state, inner) -} - -enum UiMessage { - Info(String), - Error(String), - Prompt(String), - Finished(bool, String), - Progress(usize, String), -} - -impl FromStr for UiMessage { - type Err = String; - - fn from_str(s: &str) -> Result<Self, Self::Err> { - let (ty, rest) = s.split_once(": ").ok_or("invalid message: no type")?; - - match ty { - "message" => Ok(UiMessage::Info(rest.to_owned())), - "error" => Ok(UiMessage::Error(rest.to_owned())), - "prompt" => Ok(UiMessage::Prompt(rest.to_owned())), - "finished" => { - let (state, rest) = rest.split_once(", ").ok_or("invalid message: no state")?; - Ok(UiMessage::Finished(state == "ok", rest.to_owned())) - } - "progress" => { - let (percent, rest) = rest.split_once(' ').ok_or("invalid progress message")?; - Ok(UiMessage::Progress( - percent - .parse::<f64>() - .map(|v| (v * 100.).floor() as usize) - .map_err(|err| err.to_string())?, - rest.to_owned(), - )) - } - unknown => Err(format!("invalid message type {unknown}, rest: {rest}")), - } - } + let state = siv.user_data::<InstallerState>().cloned().unwrap(); + InstallerView::with_raw(&state, InstallProgressView::new(siv)) } diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs new file mode 100644 index 0000000..e9d764a --- /dev/null +++ b/proxmox-tui-installer/src/views/install_progress.rs @@ -0,0 +1,251 @@ +use std::{ + io::{self, BufRead, BufReader, Write}, + process, + str::FromStr, + sync::{Arc, Mutex}, + thread, + time::Duration, +}; + +use cursive::{ + utils::Counter, + view::{Resizable, ViewWrapper}, + views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextContent, TextView}, + CbSink, Cursive, +}; + +use crate::{ + abort_install_button, options::InstallerOptions, setup::InstallConfig, yes_no_dialog, + InstallerState, +}; + +pub struct InstallProgressView { + view: PaddedView<LinearLayout>, +} + +impl InstallProgressView { + pub fn new(siv: &mut Cursive) -> Self { + let cb_sink = siv.cb_sink().clone(); + let state = siv.user_data::<InstallerState>().unwrap(); + let progress_text = TextContent::new("starting the installation .."); + + let progress_task = { + let progress_text = progress_text.clone(); + let options = state.options.clone(); + move |counter: Counter| progress_task(counter, cb_sink, options, progress_text) + }; + + let progress_bar = ProgressBar::new().with_task(progress_task).full_width(); + let view = PaddedView::lrtb( + 1, + 1, + 1, + 1, + LinearLayout::vertical() + .child(PaddedView::lrtb(1, 1, 0, 0, progress_bar)) + .child(DummyView) + .child(TextView::new_with_content(progress_text).center()) + .child(PaddedView::lrtb( + 1, + 1, + 1, + 0, + LinearLayout::horizontal().child(abort_install_button()), + )), + ); + + Self { view } + } +} + +fn progress_task( + counter: Counter, + cb_sink: CbSink, + options: InstallerOptions, + progress_text: TextContent, +) { + let child = { + use std::process::{Command, Stdio}; + + #[cfg(not(debug_assertions))] + let (path, args, envs): (&str, [&str; 1], [(&str, &str); 0]) = + ("proxmox-low-level-installer", ["start-session"], []); + + #[cfg(debug_assertions)] + let (path, args, envs) = ( + std::path::PathBuf::from("./proxmox-low-level-installer"), + ["-t", "start-session-test"], + [("PERL5LIB", ".")], + ); + + Command::new(path) + .args(args) + .envs(envs) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + }; + + let mut child = match child { + Ok(child) => child, + Err(err) => { + let _ = cb_sink.send(Box::new(move |siv| { + siv.add_layer( + Dialog::text(err.to_string()) + .title("Error") + .button("Ok", Cursive::quit), + ); + })); + return; + } + }; + + let inner = || { + let reader = child.stdout.take().map(BufReader::new)?; + let mut writer = child.stdin.take()?; + + serde_json::to_writer(&mut writer, &InstallConfig::from(options)).unwrap(); + writeln!(writer).unwrap(); + + let writer = Arc::new(Mutex::new(writer)); + + for line in reader.lines() { + let line = match line { + Ok(line) => line, + Err(_) => break, + }; + + let msg = match line.parse::<UiMessage>() { + Ok(msg) => msg, + Err(stray) => { + eprintln!("low-level installer: {stray}"); + continue; + } + }; + + match msg { + UiMessage::Info(s) => cb_sink.send(Box::new(|siv| { + siv.add_layer(Dialog::info(s).title("Information")); + })), + UiMessage::Error(s) => cb_sink.send(Box::new(|siv| { + siv.add_layer(Dialog::info(s).title("Error")); + })), + UiMessage::Prompt(s) => cb_sink.send({ + let writer = writer.clone(); + Box::new(move |siv| { + yes_no_dialog( + siv, + "Prompt", + &s, + Box::new({ + let writer = writer.clone(); + move |_| { + if let Ok(mut writer) = writer.lock() { + let _ = writeln!(writer, "ok"); + } + } + }), + Box::new(move |_| { + if let Ok(mut writer) = writer.lock() { + let _ = writeln!(writer); + } + }), + ); + }) + }), + UiMessage::Progress(ratio, s) => { + counter.set(ratio); + progress_text.set_content(s); + Ok(()) + } + UiMessage::Finished(success, msg) => { + counter.set(100); + progress_text.set_content(msg.to_owned()); + cb_sink.send(Box::new(move |siv| { + let title = if success { "Success" } else { "Failure" }; + + // For rebooting, we just need to quit the installer, + // our caller does the actual reboot. + siv.add_layer( + Dialog::text(msg) + .title(title) + .button("Reboot now", Cursive::quit), + ); + + let autoreboot = siv + .user_data::<InstallerState>() + .map(|state| state.options.autoreboot) + .unwrap_or_default(); + + if autoreboot && success { + let cb_sink = siv.cb_sink(); + thread::spawn({ + let cb_sink = cb_sink.clone(); + move || { + thread::sleep(Duration::from_secs(5)); + let _ = cb_sink.send(Box::new(Cursive::quit)); + } + }); + } + })) + } + } + .unwrap(); + } + + Some(()) + }; + + if inner().is_none() { + cb_sink + .send(Box::new(|siv| { + siv.add_layer( + Dialog::text("low-level installer exited early") + .title("Error") + .button("Exit", Cursive::quit), + ); + })) + .unwrap(); + } +} + +impl ViewWrapper for InstallProgressView { + cursive::wrap_impl!(self.view: PaddedView<LinearLayout>); +} + +enum UiMessage { + Info(String), + Error(String), + Prompt(String), + Finished(bool, String), + Progress(usize, String), +} + +impl FromStr for UiMessage { + type Err = String; + + fn from_str(s: &str) -> Result<Self, Self::Err> { + let (ty, rest) = s.split_once(": ").ok_or("invalid message: no type")?; + + match ty { + "message" => Ok(UiMessage::Info(rest.to_owned())), + "error" => Ok(UiMessage::Error(rest.to_owned())), + "prompt" => Ok(UiMessage::Prompt(rest.to_owned())), + "finished" => { + let (state, rest) = rest.split_once(", ").ok_or("invalid message: no state")?; + Ok(UiMessage::Finished(state == "ok", rest.to_owned())) + } + "progress" => { + let (percent, rest) = rest.split_once(' ').ok_or("invalid progress message")?; + Ok(UiMessage::Progress( + percent + .parse::<f64>() + .map(|v| (v * 100.).floor() as usize) + .map_err(|err| err.to_string())?, + rest.to_owned(), + )) + } + unknown => Err(format!("invalid message type {unknown}, rest: {rest}")), + } + } +} diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs index aa24fa4..18dbdcf 100644 --- a/proxmox-tui-installer/src/views/mod.rs +++ b/proxmox-tui-installer/src/views/mod.rs @@ -12,6 +12,9 @@ use crate::utils::CidrAddress; mod bootdisk; pub use bootdisk::*; +mod install_progress; +pub use install_progress::*; + mod table_view; pub use table_view::*; -- 2.41.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer 2/7] tui: install progress: split out low-level installer spawing into own function 2023-07-26 14:03 [pve-devel] [PATCH installer 0/7] refactor; improve installation progress Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 1/7] tui: move install progress dialog into own view module Christoph Heiss @ 2023-07-26 14:03 ` Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 3/7] tui: install progress: split out reboot handling " Christoph Heiss ` (4 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Christoph Heiss @ 2023-07-26 14:03 UTC (permalink / raw) To: pve-devel No functional changes. Signed-off-by: Christoph Heiss <c.heiss@proxmox.com> --- .../src/views/install_progress.rs | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs index e9d764a..870c6a3 100644 --- a/proxmox-tui-installer/src/views/install_progress.rs +++ b/proxmox-tui-installer/src/views/install_progress.rs @@ -58,35 +58,35 @@ impl InstallProgressView { } } +fn spawn_low_level_installer() -> io::Result<process::Child> { + use std::process::{Command, Stdio}; + + #[cfg(not(debug_assertions))] + let (path, args, envs): (&str, [&str; 1], [(&str, &str); 0]) = + ("proxmox-low-level-installer", ["start-session"], []); + + #[cfg(debug_assertions)] + let (path, args, envs) = ( + std::path::PathBuf::from("./proxmox-low-level-installer"), + ["-t", "start-session-test"], + [("PERL5LIB", ".")], + ); + + Command::new(path) + .args(args) + .envs(envs) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() +} + fn progress_task( counter: Counter, cb_sink: CbSink, options: InstallerOptions, progress_text: TextContent, ) { - let child = { - use std::process::{Command, Stdio}; - - #[cfg(not(debug_assertions))] - let (path, args, envs): (&str, [&str; 1], [(&str, &str); 0]) = - ("proxmox-low-level-installer", ["start-session"], []); - - #[cfg(debug_assertions)] - let (path, args, envs) = ( - std::path::PathBuf::from("./proxmox-low-level-installer"), - ["-t", "start-session-test"], - [("PERL5LIB", ".")], - ); - - Command::new(path) - .args(args) - .envs(envs) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .spawn() - }; - - let mut child = match child { + let mut child = match spawn_low_level_installer() { Ok(child) => child, Err(err) => { let _ = cb_sink.send(Box::new(move |siv| { -- 2.41.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer 3/7] tui: install progress: split out reboot handling into own function 2023-07-26 14:03 [pve-devel] [PATCH installer 0/7] refactor; improve installation progress Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 1/7] tui: move install progress dialog into own view module Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 2/7] tui: install progress: split out low-level installer spawing into own function Christoph Heiss @ 2023-07-26 14:03 ` Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 4/7] tui: install progress: split out prompt logic " Christoph Heiss ` (3 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Christoph Heiss @ 2023-07-26 14:03 UTC (permalink / raw) To: pve-devel No functional changes. Signed-off-by: Christoph Heiss <c.heiss@proxmox.com> --- .../src/views/install_progress.rs | 55 ++++++++++--------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs index 870c6a3..bd27628 100644 --- a/proxmox-tui-installer/src/views/install_progress.rs +++ b/proxmox-tui-installer/src/views/install_progress.rs @@ -80,6 +80,33 @@ fn spawn_low_level_installer() -> io::Result<process::Child> { .spawn() } +fn prepare_for_reboot(siv: &mut Cursive, success: bool, msg: &str) { + let title = if success { "Success" } else { "Failure" }; + + // For rebooting, we just need to quit the installer, our caller does the actual reboot. + siv.add_layer( + Dialog::text(msg) + .title(title) + .button("Reboot now", Cursive::quit), + ); + + let autoreboot = siv + .user_data::<InstallerState>() + .map(|state| state.options.autoreboot) + .unwrap_or_default(); + + if autoreboot && success { + let cb_sink = siv.cb_sink(); + thread::spawn({ + let cb_sink = cb_sink.clone(); + move || { + thread::sleep(Duration::from_secs(5)); + let _ = cb_sink.send(Box::new(Cursive::quit)); + } + }); + } +} + fn progress_task( counter: Counter, cb_sink: CbSink, @@ -161,33 +188,7 @@ fn progress_task( UiMessage::Finished(success, msg) => { counter.set(100); progress_text.set_content(msg.to_owned()); - cb_sink.send(Box::new(move |siv| { - let title = if success { "Success" } else { "Failure" }; - - // For rebooting, we just need to quit the installer, - // our caller does the actual reboot. - siv.add_layer( - Dialog::text(msg) - .title(title) - .button("Reboot now", Cursive::quit), - ); - - let autoreboot = siv - .user_data::<InstallerState>() - .map(|state| state.options.autoreboot) - .unwrap_or_default(); - - if autoreboot && success { - let cb_sink = siv.cb_sink(); - thread::spawn({ - let cb_sink = cb_sink.clone(); - move || { - thread::sleep(Duration::from_secs(5)); - let _ = cb_sink.send(Box::new(Cursive::quit)); - } - }); - } - })) + cb_sink.send(Box::new(move |siv| prepare_for_reboot(siv, success, &msg))) } } .unwrap(); -- 2.41.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer 4/7] tui: install progress: split out prompt logic into own function 2023-07-26 14:03 [pve-devel] [PATCH installer 0/7] refactor; improve installation progress Christoph Heiss ` (2 preceding siblings ...) 2023-07-26 14:03 ` [pve-devel] [PATCH installer 3/7] tui: install progress: split out reboot handling " Christoph Heiss @ 2023-07-26 14:03 ` Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 5/7] tui: install progress: handle errors in ui message loop more gracefully Christoph Heiss ` (2 subsequent siblings) 6 siblings, 0 replies; 8+ messages in thread From: Christoph Heiss @ 2023-07-26 14:03 UTC (permalink / raw) To: pve-devel No functional changes. Signed-off-by: Christoph Heiss <c.heiss@proxmox.com> --- .../src/views/install_progress.rs | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs index bd27628..3046a26 100644 --- a/proxmox-tui-installer/src/views/install_progress.rs +++ b/proxmox-tui-installer/src/views/install_progress.rs @@ -80,6 +80,27 @@ fn spawn_low_level_installer() -> io::Result<process::Child> { .spawn() } +fn show_prompt<W: Write + 'static>(siv: &mut Cursive, text: &str, writer: Arc<Mutex<W>>) { + yes_no_dialog( + siv, + "Prompt", + text, + Box::new({ + let writer = writer.clone(); + move |_| { + if let Ok(mut writer) = writer.lock() { + let _ = writeln!(writer, "ok"); + } + } + }), + Box::new(move |_| { + if let Ok(mut writer) = writer.lock() { + let _ = writeln!(writer); + } + }), + ); +} + fn prepare_for_reboot(siv: &mut Cursive, success: bool, msg: &str) { let title = if success { "Success" } else { "Failure" }; @@ -159,26 +180,7 @@ fn progress_task( })), UiMessage::Prompt(s) => cb_sink.send({ let writer = writer.clone(); - Box::new(move |siv| { - yes_no_dialog( - siv, - "Prompt", - &s, - Box::new({ - let writer = writer.clone(); - move |_| { - if let Ok(mut writer) = writer.lock() { - let _ = writeln!(writer, "ok"); - } - } - }), - Box::new(move |_| { - if let Ok(mut writer) = writer.lock() { - let _ = writeln!(writer); - } - }), - ); - }) + Box::new(move |siv| show_prompt(siv, &s, writer)) }), UiMessage::Progress(ratio, s) => { counter.set(ratio); -- 2.41.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer 5/7] tui: install progress: handle errors in ui message loop more gracefully 2023-07-26 14:03 [pve-devel] [PATCH installer 0/7] refactor; improve installation progress Christoph Heiss ` (3 preceding siblings ...) 2023-07-26 14:03 ` [pve-devel] [PATCH installer 4/7] tui: install progress: split out prompt logic " Christoph Heiss @ 2023-07-26 14:03 ` Christoph Heiss 2023-07-26 14:04 ` [pve-devel] [PATCH installer 6/7] low-level: avoid open-coding config reading, parsing and merging Christoph Heiss 2023-07-26 14:04 ` [pve-devel] [PATCH installer 7/7] low-level, tui: count down auto-reboot timeout Christoph Heiss 6 siblings, 0 replies; 8+ messages in thread From: Christoph Heiss @ 2023-07-26 14:03 UTC (permalink / raw) To: pve-devel This at least gives /some/ feedback to the user he can potentially report, instead of a single, hardcoded message. Signed-off-by: Christoph Heiss <c.heiss@proxmox.com> --- .../src/views/install_progress.rs | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs index 3046a26..2025a4c 100644 --- a/proxmox-tui-installer/src/views/install_progress.rs +++ b/proxmox-tui-installer/src/views/install_progress.rs @@ -149,29 +149,36 @@ fn progress_task( }; let inner = || { - let reader = child.stdout.take().map(BufReader::new)?; - let mut writer = child.stdin.take()?; + let reader = child + .stdout + .take() + .map(BufReader::new) + .ok_or("failed to get stdin reader")?; - serde_json::to_writer(&mut writer, &InstallConfig::from(options)).unwrap(); - writeln!(writer).unwrap(); + let mut writer = child.stdin.take().ok_or("failed to get stdin writer")?; + + serde_json::to_writer(&mut writer, &InstallConfig::from(options)) + .map_err(|err| format!("failed to serialize install config: {err}"))?; + writeln!(writer).map_err(|err| format!("failed to write install config: {err}"))?; let writer = Arc::new(Mutex::new(writer)); for line in reader.lines() { let line = match line { Ok(line) => line, - Err(_) => break, + Err(err) => return Err(format!("low-level installer exited early: {err}")), }; let msg = match line.parse::<UiMessage>() { Ok(msg) => msg, Err(stray) => { + // Not a fatal error, so don't abort the installation by returning eprintln!("low-level installer: {stray}"); continue; } }; - match msg { + let result = match msg.clone() { UiMessage::Info(s) => cb_sink.send(Box::new(|siv| { siv.add_layer(Dialog::info(s).title("Information")); })), @@ -192,18 +199,23 @@ fn progress_task( progress_text.set_content(msg.to_owned()); cb_sink.send(Box::new(move |siv| prepare_for_reboot(siv, success, &msg))) } + }; + + if let Err(err) = result { + eprintln!("error during message handling: {err}"); + eprintln!(" message was: '{msg:?}'"); } - .unwrap(); } - Some(()) + Ok(()) }; - if inner().is_none() { + if let Err(err) = inner() { + let message = format!("install failed: {err}"); cb_sink .send(Box::new(|siv| { siv.add_layer( - Dialog::text("low-level installer exited early") + Dialog::text(message) .title("Error") .button("Exit", Cursive::quit), ); @@ -216,6 +228,7 @@ impl ViewWrapper for InstallProgressView { cursive::wrap_impl!(self.view: PaddedView<LinearLayout>); } +#[derive(Debug, Clone)] enum UiMessage { Info(String), Error(String), -- 2.41.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer 6/7] low-level: avoid open-coding config reading, parsing and merging 2023-07-26 14:03 [pve-devel] [PATCH installer 0/7] refactor; improve installation progress Christoph Heiss ` (4 preceding siblings ...) 2023-07-26 14:03 ` [pve-devel] [PATCH installer 5/7] tui: install progress: handle errors in ui message loop more gracefully Christoph Heiss @ 2023-07-26 14:04 ` Christoph Heiss 2023-07-26 14:04 ` [pve-devel] [PATCH installer 7/7] low-level, tui: count down auto-reboot timeout Christoph Heiss 6 siblings, 0 replies; 8+ messages in thread From: Christoph Heiss @ 2023-07-26 14:04 UTC (permalink / raw) To: pve-devel Signed-off-by: Christoph Heiss <c.heiss@proxmox.com> --- proxmox-low-level-installer | 45 +++++++++++++++---------------------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/proxmox-low-level-installer b/proxmox-low-level-installer index 814961e..745402a 100755 --- a/proxmox-low-level-installer +++ b/proxmox-low-level-installer @@ -53,6 +53,22 @@ sub usage { exit($cmd ne 'help' ? 1 : 0); } +sub read_and_merge_config { + my $config_raw; + while (my $line = <>) { + if ($line =~ /^\s*\{/) { + $config_raw = $line; + last; + } + } + + my $config = eval { from_json($config_raw, { utf8 => 1 }) }; + die "failed to parse config from stdin - $@\n" if $@; + + Proxmox::Install::Config::merge($config); + log_info("got installation config: ". to_json(Proxmox::Install::Config::get(), { utf8 => 1, canonical => 1 }) ."\n"); +} + my $cmd = shift; if (!$cmd || $cmd eq 'help' || !exists($commands->{$cmd})) { usage($cmd // ''); @@ -87,19 +103,7 @@ if ($cmd eq 'dump-env') { file_write_all($run_env_file, $run_env_serialized); } elsif ($cmd eq 'start-session') { Proxmox::UI::init_stdio({}, $env); - - my $config_raw; - while (my $line = <>) { - if ($line =~ /^\s*\{/) { - $config_raw = $line; - last; - } - } - my $config = eval { from_json($config_raw, { utf8 => 1 }) }; - die "failed to parse config from stdin - $@\n" if $@; - - Proxmox::Install::Config::merge($config); - log_info("got installation config: ". to_json(Proxmox::Install::Config::get(), { utf8 => 1, canonical => 1 }) ."\n"); + read_and_merge_config(); eval { Proxmox::Install::extract_data() }; if (my $err = $@) { @@ -119,20 +123,7 @@ if ($cmd eq 'dump-env') { } } elsif ($cmd eq 'start-session-test') { Proxmox::UI::init_stdio({}, $env); - - my $config_raw; - while (my $line = <>) { - if ($line =~ /^\s*\{/) { - $config_raw = $line; - last; - } - } - my $config = eval { from_json($config_raw, { utf8 => 1 }) }; - die "failed to parse config from stdin - $@\n" if $@; - - Proxmox::Install::Config::merge($config); - - print STDERR "got config: ". to_json(Proxmox::Install::Config::get(), { utf8 => 1, canonical => 1 }) ."\n"; + read_and_merge_config(); my $res = Proxmox::UI::prompt("Reply anything?") ? 'ok' : 'not ok'; Proxmox::UI::message("Test Message - got $res"); -- 2.41.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH installer 7/7] low-level, tui: count down auto-reboot timeout 2023-07-26 14:03 [pve-devel] [PATCH installer 0/7] refactor; improve installation progress Christoph Heiss ` (5 preceding siblings ...) 2023-07-26 14:04 ` [pve-devel] [PATCH installer 6/7] low-level: avoid open-coding config reading, parsing and merging Christoph Heiss @ 2023-07-26 14:04 ` Christoph Heiss 6 siblings, 0 replies; 8+ messages in thread From: Christoph Heiss @ 2023-07-26 14:04 UTC (permalink / raw) To: pve-devel The GUI installer already has the same functionality, with this the TUI installer gains the same. It is a nice touch anyway, primarily to indicate to the user that the installer is not frozen or similar. Signed-off-by: Christoph Heiss <c.heiss@proxmox.com> --- proxmox-low-level-installer | 25 +++++++++++-------- .../src/views/install_progress.rs | 12 +++++++-- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/proxmox-low-level-installer b/proxmox-low-level-installer index 745402a..0141170 100755 --- a/proxmox-low-level-installer +++ b/proxmox-low-level-installer @@ -69,6 +69,19 @@ sub read_and_merge_config { log_info("got installation config: ". to_json(Proxmox::Install::Config::get(), { utf8 => 1, canonical => 1 }) ."\n"); } +sub send_reboot_ui_message { + if (Proxmox::Install::Config::get_autoreboot()) { + my $secs = 5; + while ($secs > 0) { + Proxmox::UI::finished(1, "Installation finished - auto-rebooting in $secs seconds .."); + sleep 1; + $secs -= 1; + } + } else { + Proxmox::UI::finished(1, "Installation complete - reboot now?"); + } +} + my $cmd = shift; if (!$cmd || $cmd eq 'help' || !exists($commands->{$cmd})) { usage($cmd // ''); @@ -115,11 +128,7 @@ if ($cmd eq 'dump-env') { Proxmox::UI::finished(0, $err); } } else { - if (Proxmox::Install::Config::get_autoreboot()) { - Proxmox::UI::finished(1, "Installation finished - auto-rebooting in ~ 5 seconds"); - } else { - Proxmox::UI::finished(1, "Installation complete - reboot now?"); - } + send_reboot_ui_message(); } } elsif ($cmd eq 'start-session-test') { Proxmox::UI::init_stdio({}, $env); @@ -137,11 +146,7 @@ if ($cmd eq 'dump-env') { } } - if (Proxmox::Install::Config::get_autoreboot()) { - Proxmox::UI::finished(1, "Installation finished - auto-rebooting in ~ 5 seconds"); - } else { - Proxmox::UI::finished(1, "Installation complete - reboot now?"); - } + send_reboot_ui_message(); } exit(0); diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs index 2025a4c..c8d9aab 100644 --- a/proxmox-tui-installer/src/views/install_progress.rs +++ b/proxmox-tui-installer/src/views/install_progress.rs @@ -9,7 +9,7 @@ use std::{ use cursive::{ utils::Counter, - view::{Resizable, ViewWrapper}, + view::{Nameable, Resizable, ViewWrapper}, views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextContent, TextView}, CbSink, Cursive, }; @@ -102,13 +102,21 @@ fn show_prompt<W: Write + 'static>(siv: &mut Cursive, text: &str, writer: Arc<Mu } fn prepare_for_reboot(siv: &mut Cursive, success: bool, msg: &str) { + const DIALOG_ID: &str = "autoreboot-dialog"; let title = if success { "Success" } else { "Failure" }; + // If the dialog was previously created, just update its content and we're done. + if let Some(mut dialog) = siv.find_name::<Dialog>(DIALOG_ID) { + dialog.set_content(TextView::new(msg)); + return; + } + // For rebooting, we just need to quit the installer, our caller does the actual reboot. siv.add_layer( Dialog::text(msg) .title(title) - .button("Reboot now", Cursive::quit), + .button("Reboot now", Cursive::quit) + .with_name(DIALOG_ID), ); let autoreboot = siv -- 2.41.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-07-26 14:04 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-26 14:03 [pve-devel] [PATCH installer 0/7] refactor; improve installation progress Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 1/7] tui: move install progress dialog into own view module Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 2/7] tui: install progress: split out low-level installer spawing into own function Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 3/7] tui: install progress: split out reboot handling " Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 4/7] tui: install progress: split out prompt logic " Christoph Heiss 2023-07-26 14:03 ` [pve-devel] [PATCH installer 5/7] tui: install progress: handle errors in ui message loop more gracefully Christoph Heiss 2023-07-26 14:04 ` [pve-devel] [PATCH installer 6/7] low-level: avoid open-coding config reading, parsing and merging Christoph Heiss 2023-07-26 14:04 ` [pve-devel] [PATCH installer 7/7] low-level, tui: count down auto-reboot timeout Christoph Heiss
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.Service provided by Proxmox Server Solutions GmbH | Privacy | Legal