public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH installer v2 0/8] refactor and improve installation progress
@ 2023-11-10 14:17 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
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Christoph Heiss @ 2023-11-10 14:17 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. Purely code movement without any functional changes.

Patch #6 improves the error handling during the interaction with the
low-level installer, by providing the user with better error messages in
case something goes wrong.

Patch #7 does some code de-duplication for the low-lever installer.

Patch #8 improves the auto-reboot message/UX by counting down the
timeout, to match its GUI counterpart.

v1: https://lists.proxmox.com/pipermail/pve-devel/2023-July/058510.html

Notable changes v1 -> v2:
  * rebased on latest master
  * added patch #2, split out from #1

Christoph Heiss (8):
  tui: move install progress dialog into own view module
  tui: install_progress: move progress task into own function
  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-installer-common/src/setup.rs         |  22 +-
 proxmox-low-level-installer                   |  70 +++--
 proxmox-tui-installer/src/main.rs             | 233 +---------------
 .../src/views/install_progress.rs             | 253 ++++++++++++++++++
 proxmox-tui-installer/src/views/mod.rs        |   3 +
 5 files changed, 316 insertions(+), 265 deletions(-)
 create mode 100644 proxmox-tui-installer/src/views/install_progress.rs

--
2.41.0





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [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

end of thread, other threads:[~2023-11-10 14:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [pve-devel] [PATCH installer v2 3/8] tui: install_progress: split out low-level installer spawing " Christoph Heiss
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 ` [pve-devel] [PATCH installer v2 5/8] tui: install_progress: split out prompt logic " 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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal