* [pve-devel] [PATCH installer v2 1/8] tui: move install progress dialog into own view module
2023-11-10 14:17 [pve-devel] [PATCH installer v2 0/8] refactor and improve installation progress Christoph Heiss
@ 2023-11-10 14:17 ` Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 2/8] tui: install_progress: move progress task into own function Christoph Heiss
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2023-11-10 14:17 UTC (permalink / raw)
To: pve-devel
While at it, convert it to a proper `View`-impl, instead of a functional
component.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* moved separation of progress task function to separate patch
proxmox-tui-installer/src/main.rs | 233 +----------------
.../src/views/install_progress.rs | 241 ++++++++++++++++++
proxmox-tui-installer/src/views/mod.rs | 3 +
3 files changed, 250 insertions(+), 227 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 a7b466f..e1411c6 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -1,25 +1,14 @@
#![forbid(unsafe_code)]
-use std::{
- collections::HashMap,
- env,
- io::{BufRead, BufReader, Write},
- net::IpAddr,
- str::FromStr,
- sync::{Arc, Mutex},
- thread,
- time::Duration,
-};
+use std::{collections::HashMap, env, net::IpAddr};
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,
};
@@ -36,14 +25,13 @@ use proxmox_installer_common::{
};
mod setup;
-use setup::InstallConfig;
mod system;
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.
@@ -673,215 +661,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 in_test_mode = state.in_test_mode;
- 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};
-
- let (path, args, envs): (&str, &[&str], Vec<(&str, &str)>) = if in_test_mode {
- (
- "./proxmox-low-level-installer",
- &["-t", "start-session-test"],
- vec![("PERL5LIB", ".")],
- )
- } else {
- ("proxmox-low-level-installer", &["start-session"], vec![])
- };
-
- 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..4dca81b
--- /dev/null
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -0,0 +1,241 @@
+use std::{
+ io::{BufRead, BufReader, Write},
+ str::FromStr,
+ sync::{Arc, Mutex},
+ thread,
+ time::Duration,
+};
+
+use cursive::{
+ utils::Counter,
+ view::{Resizable, ViewWrapper},
+ views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextContent, TextView},
+ Cursive,
+};
+
+use crate::{abort_install_button, 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 state = state.clone();
+ move |counter: Counter| {
+ let child = {
+ use std::process::{Command, Stdio};
+
+ let (path, args, envs): (&str, &[&str], Vec<(&str, &str)>) =
+ if state.in_test_mode {
+ (
+ "./proxmox-low-level-installer",
+ &["-t", "start-session-test"],
+ vec![("PERL5LIB", ".")],
+ )
+ } else {
+ ("proxmox-low-level-installer", &["start-session"], vec![])
+ };
+
+ 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(state.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 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 }
+ }
+}
+
+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 4d27532..3244e76 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -12,6 +12,9 @@ use proxmox_installer_common::utils::CidrAddress;
mod bootdisk;
pub use bootdisk::*;
+mod install_progress;
+pub use install_progress::*;
+
mod table_view;
pub use table_view::*;
--
2.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH installer v2 2/8] tui: install_progress: move progress task into own function
2023-11-10 14:17 [pve-devel] [PATCH installer v2 0/8] refactor and improve installation progress Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 1/8] tui: move install progress dialog into own view module Christoph Heiss
@ 2023-11-10 14:17 ` Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 3/8] tui: install_progress: split out low-level installer spawing " Christoph Heiss
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2023-11-10 14:17 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* new patch, separated out from patch #1
* use static member function instead of top-level function
.../src/views/install_progress.rs | 299 +++++++++---------
1 file changed, 152 insertions(+), 147 deletions(-)
diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs
index 4dca81b..ccf53ad 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -10,7 +10,7 @@ use cursive::{
utils::Counter,
view::{Resizable, ViewWrapper},
views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextContent, TextView},
- Cursive,
+ CbSink, Cursive,
};
use crate::{abort_install_button, setup::InstallConfig, yes_no_dialog, InstallerState};
@@ -28,152 +28,7 @@ impl InstallProgressView {
let progress_task = {
let progress_text = progress_text.clone();
let state = state.clone();
- move |counter: Counter| {
- let child = {
- use std::process::{Command, Stdio};
-
- let (path, args, envs): (&str, &[&str], Vec<(&str, &str)>) =
- if state.in_test_mode {
- (
- "./proxmox-low-level-installer",
- &["-t", "start-session-test"],
- vec![("PERL5LIB", ".")],
- )
- } else {
- ("proxmox-low-level-installer", &["start-session"], vec![])
- };
-
- 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(state.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();
- }
- }
+ move |counter: Counter| Self::progress_task(counter, cb_sink, state, progress_text)
};
let progress_bar = ProgressBar::new().with_task(progress_task).full_width();
@@ -197,6 +52,156 @@ impl InstallProgressView {
Self { view }
}
+
+ fn progress_task(
+ counter: Counter,
+ cb_sink: CbSink,
+ state: InstallerState,
+ progress_text: TextContent,
+ ) {
+ let child = {
+ use std::process::{Command, Stdio};
+
+ let (path, args, envs): (&str, &[&str], Vec<(&str, &str)>) = if state.in_test_mode {
+ (
+ "./proxmox-low-level-installer",
+ &["-t", "start-session-test"],
+ vec![("PERL5LIB", ".")],
+ )
+ } else {
+ ("proxmox-low-level-installer", &["start-session"], vec![])
+ };
+
+ 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(state.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 {
--
2.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH installer v2 3/8] tui: install_progress: split out low-level installer spawing into own function
2023-11-10 14:17 [pve-devel] [PATCH installer v2 0/8] refactor and improve installation progress Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 1/8] tui: move install progress dialog into own view module Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 2/8] tui: install_progress: move progress task into own function Christoph Heiss
@ 2023-11-10 14:17 ` Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 4/8] tui: install_progress: split out reboot handling " Christoph Heiss
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2023-11-10 14:17 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* moved spawn_low_level_installer() to common crate
proxmox-installer-common/src/setup.rs | 22 ++++++++++++++++-
.../src/views/install_progress.rs | 24 ++-----------------
2 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 28a58f3..70bdc3c 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -3,9 +3,10 @@ use std::{
collections::HashMap,
fmt,
fs::File,
- io::BufReader,
+ io::{self, BufReader},
net::IpAddr,
path::{Path, PathBuf},
+ process::{self, Command, Stdio},
};
use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
@@ -367,3 +368,22 @@ impl Interface {
format!("{} {}", self.state.render(), self.name)
}
}
+
+pub fn spawn_low_level_installer(test_mode: bool) -> io::Result<process::Child> {
+ let (path, args, envs): (&str, &[&str], Vec<(&str, &str)>) = if test_mode {
+ (
+ "./proxmox-low-level-installer",
+ &["-t", "start-session-test"],
+ vec![("PERL5LIB", ".")],
+ )
+ } else {
+ ("proxmox-low-level-installer", &["start-session"], vec![])
+ };
+
+ Command::new(path)
+ .args(args)
+ .envs(envs)
+ .stdin(Stdio::piped())
+ .stdout(Stdio::piped())
+ .spawn()
+}
diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs
index ccf53ad..a70b6cb 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -14,6 +14,7 @@ use cursive::{
};
use crate::{abort_install_button, setup::InstallConfig, yes_no_dialog, InstallerState};
+use proxmox_installer_common::setup::spawn_low_level_installer;
pub struct InstallProgressView {
view: PaddedView<LinearLayout>,
@@ -59,28 +60,7 @@ impl InstallProgressView {
state: InstallerState,
progress_text: TextContent,
) {
- let child = {
- use std::process::{Command, Stdio};
-
- let (path, args, envs): (&str, &[&str], Vec<(&str, &str)>) = if state.in_test_mode {
- (
- "./proxmox-low-level-installer",
- &["-t", "start-session-test"],
- vec![("PERL5LIB", ".")],
- )
- } else {
- ("proxmox-low-level-installer", &["start-session"], vec![])
- };
-
- 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(state.in_test_mode) {
Ok(child) => child,
Err(err) => {
let _ = cb_sink.send(Box::new(move |siv| {
--
2.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH installer v2 4/8] tui: install_progress: split out reboot handling into own function
2023-11-10 14:17 [pve-devel] [PATCH installer v2 0/8] refactor and improve installation progress Christoph Heiss
` (2 preceding siblings ...)
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 3/8] tui: install_progress: split out low-level installer spawing " Christoph Heiss
@ 2023-11-10 14:17 ` Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 5/8] tui: install_progress: split out prompt logic " Christoph Heiss
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2023-11-10 14:17 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* use static member function instead of top-level function
.../src/views/install_progress.rs | 54 ++++++++++---------
1 file changed, 29 insertions(+), 25 deletions(-)
diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs
index a70b6cb..9f4b8c4 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -136,31 +136,7 @@ impl InstallProgressView {
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));
- }
- });
- }
+ Self::prepare_for_reboot(siv, success, &msg)
}))
}
}
@@ -182,6 +158,34 @@ impl InstallProgressView {
.unwrap();
}
}
+
+ 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));
+ }
+ });
+ }
+ }
}
impl ViewWrapper for InstallProgressView {
--
2.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH installer v2 5/8] tui: install_progress: split out prompt logic into own function
2023-11-10 14:17 [pve-devel] [PATCH installer v2 0/8] refactor and improve installation progress Christoph Heiss
` (3 preceding siblings ...)
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 4/8] tui: install_progress: split out reboot handling " Christoph Heiss
@ 2023-11-10 14:17 ` Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 6/8] tui: install_progress: handle errors in ui message loop more gracefully Christoph Heiss
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2023-11-10 14:17 UTC (permalink / raw)
To: pve-devel
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* use static member function instead of top-level function
.../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 9f4b8c4..fceb785 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -106,26 +106,7 @@ impl InstallProgressView {
})),
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| Self::show_prompt(siv, &s, writer))
}),
UiMessage::Progress(ratio, s) => {
counter.set(ratio);
@@ -186,6 +167,27 @@ impl InstallProgressView {
});
}
}
+
+ 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);
+ }
+ }),
+ );
+ }
}
impl ViewWrapper for InstallProgressView {
--
2.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH installer v2 6/8] tui: install_progress: handle errors in ui message loop more gracefully
2023-11-10 14:17 [pve-devel] [PATCH installer v2 0/8] refactor and improve installation progress Christoph Heiss
` (4 preceding siblings ...)
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 5/8] tui: install_progress: split out prompt logic " Christoph Heiss
@ 2023-11-10 14:17 ` Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 7/8] low-level: avoid open-coding config reading, parsing and merging Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 8/8] low-level, tui: count down auto-reboot timeout Christoph Heiss
7 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2023-11-10 14:17 UTC (permalink / raw)
To: pve-devel
This at least gives _some_ feedback to the user he can potentially
report or try to address, instead of a single, hardcoded message.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
.../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 fceb785..96e62f8 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -75,29 +75,36 @@ impl InstallProgressView {
};
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(state.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(state.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"));
})),
@@ -120,18 +127,23 @@ impl InstallProgressView {
Self::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!("installation 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),
);
@@ -194,6 +206,7 @@ impl ViewWrapper for InstallProgressView {
cursive::wrap_impl!(self.view: PaddedView<LinearLayout>);
}
+#[derive(Clone, Debug)]
enum UiMessage {
Info(String),
Error(String),
--
2.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH installer v2 7/8] low-level: avoid open-coding config reading, parsing and merging
2023-11-10 14:17 [pve-devel] [PATCH installer v2 0/8] refactor and improve installation progress Christoph Heiss
` (5 preceding siblings ...)
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 6/8] tui: install_progress: handle errors in ui message loop more gracefully Christoph Heiss
@ 2023-11-10 14:17 ` Christoph Heiss
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 8/8] low-level, tui: count down auto-reboot timeout Christoph Heiss
7 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2023-11-10 14:17 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
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 0f2bf4f..b8269d7 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.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH installer v2 8/8] low-level, tui: count down auto-reboot timeout
2023-11-10 14:17 [pve-devel] [PATCH installer v2 0/8] refactor and improve installation progress Christoph Heiss
` (6 preceding siblings ...)
2023-11-10 14:17 ` [pve-devel] [PATCH installer v2 7/8] low-level: avoid open-coding config reading, parsing and merging Christoph Heiss
@ 2023-11-10 14:17 ` Christoph Heiss
7 siblings, 0 replies; 9+ messages in thread
From: Christoph Heiss @ 2023-11-10 14:17 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>
---
Changes v1 -> v2:
* no changes
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 b8269d7..9b4b773 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 96e62f8..76dd518 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -8,7 +8,7 @@ use std::{
use cursive::{
utils::Counter,
- view::{Resizable, ViewWrapper},
+ view::{Nameable, Resizable, ViewWrapper},
views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextContent, TextView},
CbSink, Cursive,
};
@@ -153,14 +153,22 @@ impl InstallProgressView {
}
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.42.0
^ permalink raw reply [flat|nested] 9+ messages in thread