all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements
@ 2024-11-25 11:27 Christoph Heiss
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate Christoph Heiss
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
  To: pve-devel

This series tries to improve upon some small things around the
installation progress reporting, esp. in the auto-installer, as well as
some small fixes & code-deduplication.

History
=======

v1: https://lore.proxmox.com/pve-devel/20240516133945.1087246-1-c.heiss@proxmox.com/

Changes v1 -> v2:
  * dropped already-applied patches
  * rebased on latest master
  * added new patch #6

Diffstat
========

Christoph Heiss (6):
  auto, tui: move low-level installer message struct to common crate
  auto: log non-json low-level messages into separate file
  low-level: stdio: fix: make progress text properly optional
  low-level: add proper message to 100% progress ratio update
  auto: avoid printing unnecessary progress update lines
  tui: tests: catch EOF from proxmox-low-level-installer early

 Proxmox/Install.pm                            |  2 +-
 Proxmox/UI/StdIO.pm                           |  8 +-
 .../src/bin/proxmox-auto-installer.rs         | 41 ++++++++--
 proxmox-auto-installer/src/utils.rs           | 23 ------
 proxmox-installer-common/src/setup.rs         | 23 ++++++
 .../src/views/install_progress.rs             | 76 ++++++++-----------
 test/ui2-stdio.pl                             |  2 +-
 7 files changed, 98 insertions(+), 77 deletions(-)

-- 
2.44.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


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

* [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate
  2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 2/6] auto: log non-json low-level messages into separate file Christoph Heiss
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
  To: pve-devel

.. effectively de-duplicating the struct, which currently is defined in
both the auto-installer and the tui-installer separately.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * no changes

 .../src/bin/proxmox-auto-installer.rs         |  5 +-
 proxmox-auto-installer/src/utils.rs           | 23 ---------
 proxmox-installer-common/src/setup.rs         | 23 +++++++++
 .../src/views/install_progress.rs             | 50 +++++--------------
 4 files changed, 39 insertions(+), 62 deletions(-)

diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 4b9d73d..151694f 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -10,7 +10,8 @@ use std::{
 use proxmox_installer_common::{
     http,
     setup::{
-        installer_setup, read_json, spawn_low_level_installer, LocaleInfo, RuntimeInfo, SetupInfo,
+        installer_setup, read_json, spawn_low_level_installer, LocaleInfo, LowLevelMessage,
+        RuntimeInfo, SetupInfo,
     },
     FIRST_BOOT_EXEC_MAX_SIZE, FIRST_BOOT_EXEC_NAME, RUNTIME_DIR,
 };
@@ -19,7 +20,7 @@ use proxmox_auto_installer::{
     answer::{Answer, FirstBootHookInfo, FirstBootHookSourceMode},
     log::AutoInstLogger,
     udevinfo::UdevInfo,
-    utils::{parse_answer, LowLevelMessage},
+    utils::parse_answer,
 };
 
 static LOGGER: AutoInstLogger = AutoInstLogger;
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index ea7176a..2be81ea 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -441,26 +441,3 @@ pub fn parse_answer(
 
     Ok(config)
 }
-
-#[derive(Clone, Debug, Deserialize, PartialEq)]
-#[serde(tag = "type", rename_all = "lowercase")]
-pub enum LowLevelMessage {
-    #[serde(rename = "message")]
-    Info {
-        message: String,
-    },
-    Error {
-        message: String,
-    },
-    Prompt {
-        query: String,
-    },
-    Finished {
-        state: String,
-        message: String,
-    },
-    Progress {
-        ratio: f32,
-        text: String,
-    },
-}
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index 1a9a71c..e22ce4e 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -551,3 +551,26 @@ pub struct InstallConfig {
 
     pub first_boot: InstallFirstBootSetup,
 }
+
+#[derive(Clone, Debug, Deserialize, PartialEq)]
+#[serde(tag = "type", rename_all = "lowercase")]
+pub enum LowLevelMessage {
+    #[serde(rename = "message")]
+    Info {
+        message: String,
+    },
+    Error {
+        message: String,
+    },
+    Prompt {
+        query: String,
+    },
+    Finished {
+        state: String,
+        message: String,
+    },
+    Progress {
+        ratio: f32,
+        text: String,
+    },
+}
diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs
index 4b4a418..585877c 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -4,7 +4,6 @@ use cursive::{
     views::{Dialog, DummyView, LinearLayout, PaddedView, ProgressBar, TextView},
     CbSink, Cursive,
 };
-use serde::Deserialize;
 use std::{
     fs::File,
     io::{BufRead, BufReader, Write},
@@ -14,7 +13,7 @@ use std::{
 };
 
 use crate::{abort_install_button, prompt_dialog, InstallerState};
-use proxmox_installer_common::setup::{spawn_low_level_installer, InstallConfig};
+use proxmox_installer_common::setup::{spawn_low_level_installer, InstallConfig, LowLevelMessage};
 
 pub struct InstallProgressView {
     view: PaddedView<LinearLayout>,
@@ -105,7 +104,7 @@ impl InstallProgressView {
                     continue;
                 }
 
-                let msg = match serde_json::from_str::<UiMessage>(&line) {
+                let msg = match serde_json::from_str::<LowLevelMessage>(&line) {
                     Ok(msg) => msg,
                     Err(err) => {
                         // Not a fatal error, so don't abort the installation by returning
@@ -116,17 +115,17 @@ impl InstallProgressView {
                 };
 
                 let result = match msg.clone() {
-                    UiMessage::Info { message } => cb_sink.send(Box::new(|siv| {
+                    LowLevelMessage::Info { message } => cb_sink.send(Box::new(|siv| {
                         siv.add_layer(Dialog::info(message).title("Information"));
                     })),
-                    UiMessage::Error { message } => cb_sink.send(Box::new(|siv| {
+                    LowLevelMessage::Error { message } => cb_sink.send(Box::new(|siv| {
                         siv.add_layer(Dialog::info(message).title("Error"));
                     })),
-                    UiMessage::Prompt { query } => cb_sink.send({
+                    LowLevelMessage::Prompt { query } => cb_sink.send({
                         let writer = writer.clone();
                         Box::new(move |siv| Self::show_prompt(siv, &query, writer))
                     }),
-                    UiMessage::Progress { ratio, text } => {
+                    LowLevelMessage::Progress { ratio, text } => {
                         counter.set((ratio * 100.).floor() as usize);
                         cb_sink.send(Box::new(move |siv| {
                             siv.call_on_name(Self::PROGRESS_TEXT_VIEW_ID, |v: &mut TextView| {
@@ -134,7 +133,7 @@ impl InstallProgressView {
                             });
                         }))
                     }
-                    UiMessage::Finished { state, message } => {
+                    LowLevelMessage::Finished { state, message } => {
                         counter.set(100);
                         cb_sink.send(Box::new(move |siv| {
                             siv.call_on_name(Self::PROGRESS_TEXT_VIEW_ID, |v: &mut TextView| {
@@ -245,39 +244,16 @@ impl ViewWrapper for InstallProgressView {
     cursive::wrap_impl!(self.view: PaddedView<LinearLayout>);
 }
 
-#[derive(Clone, Debug, Deserialize, PartialEq)]
-#[serde(tag = "type", rename_all = "lowercase")]
-enum UiMessage {
-    #[serde(rename = "message")]
-    Info {
-        message: String,
-    },
-    Error {
-        message: String,
-    },
-    Prompt {
-        query: String,
-    },
-    Finished {
-        state: String,
-        message: String,
-    },
-    Progress {
-        ratio: f32,
-        text: String,
-    },
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
     use std::env;
 
-    fn next_msg<R: BufRead>(reader: &mut R) -> Option<UiMessage> {
+    fn next_msg<R: BufRead>(reader: &mut R) -> Option<LowLevelMessage> {
         let mut line = String::new();
         reader.read_line(&mut line).expect("a line");
 
-        match serde_json::from_str::<UiMessage>(&line) {
+        match serde_json::from_str::<LowLevelMessage>(&line) {
             Ok(msg) => Some(msg),
             Err(err) => {
                 eprintln!("invalid json: '{err}'");
@@ -310,7 +286,7 @@ mod tests {
 
         assert_eq!(
             next_msg(&mut reader),
-            Some(UiMessage::Prompt {
+            Some(LowLevelMessage::Prompt {
                 query: "Reply anything?".to_owned()
             }),
         );
@@ -324,7 +300,7 @@ mod tests {
 
         assert_eq!(
             next_msg(&mut reader),
-            Some(UiMessage::Info {
+            Some(LowLevelMessage::Info {
                 message: "Test Message - got ok".to_owned()
             }),
         );
@@ -332,7 +308,7 @@ mod tests {
         for i in (1..=1000).step_by(3) {
             assert_eq!(
                 next_msg(&mut reader),
-                Some(UiMessage::Progress {
+                Some(LowLevelMessage::Progress {
                     ratio: (i as f32) / 1000.,
                     text: format!("foo {i}"),
                 }),
@@ -341,7 +317,7 @@ mod tests {
 
         assert_eq!(
             next_msg(&mut reader),
-            Some(UiMessage::Finished {
+            Some(LowLevelMessage::Finished {
                 state: "ok".to_owned(),
                 message: "Installation finished - reboot now?".to_owned(),
             }),
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


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

* [pve-devel] [PATCH installer v2 2/6] auto: log non-json low-level messages into separate file
  2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 3/6] low-level: stdio: fix: make progress text properly optional Christoph Heiss
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
  To: pve-devel

Same as the TUI does, as introduced in [0].

[0] 22de6e5 ("tui: install_progress: write low-level non-JSON messages to separate file")

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * no changes

 .../src/bin/proxmox-auto-installer.rs         | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 151694f..2143838 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -1,7 +1,8 @@
 use anyhow::{bail, format_err, Result};
 use log::{error, info, LevelFilter};
 use std::{
-    env, fs,
+    env,
+    fs::{self, File},
     io::{BufRead, BufReader, Write},
     path::PathBuf,
     process::ExitCode,
@@ -169,15 +170,29 @@ fn run_installation(
             .map_err(|err| format_err!("failed to serialize install config: {err}"))?;
         writeln!(writer).map_err(|err| format_err!("failed to write install config: {err}"))?;
 
+        let mut lowlevel_log = File::create("/tmp/install-low-level.log")
+            .map_err(|err| format_err!("failed to open low-level installer logfile: {err}"))?;
+
         for line in reader.lines() {
             let line = match line {
                 Ok(line) => line,
                 Err(_) => break,
             };
+
+            // The low-level installer also spews the output of any command it runs on its
+            // stdout. Use a very simple heuricstic to determine whether it is actually JSON
+            // or not.
+            if !line.starts_with('{') || !line.ends_with('}') {
+                let _ = writeln!(lowlevel_log, "{}", line);
+                continue;
+            }
+
             let msg = match serde_json::from_str::<LowLevelMessage>(&line) {
                 Ok(msg) => msg,
-                Err(_) => {
+                Err(err) => {
                     // Not a fatal error, so don't abort the installation by returning
+                    eprintln!("low-level installer: error while parsing message: '{err}'");
+                    eprintln!("    original message was: '{line}'");
                     continue;
                 }
             };
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


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

* [pve-devel] [PATCH installer v2 3/6] low-level: stdio: fix: make progress text properly optional
  2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate Christoph Heiss
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 2/6] auto: log non-json low-level messages into separate file Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 4/6] low-level: add proper message to 100% progress ratio update Christoph Heiss
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
  To: pve-devel

.. such that receivers can differentiate between these two cases more
clearly.

Sometimes, the `progress` sub does not get passed a text (on purpose!),
just updating the progress ratio. This would cause log messages to be
written out which could indicate missing text and look rather weird.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * no changes

 Proxmox/UI/StdIO.pm                           |  8 +++++---
 .../src/bin/proxmox-auto-installer.rs         |  6 +++++-
 proxmox-installer-common/src/setup.rs         |  2 +-
 .../src/views/install_progress.rs             | 19 +++++++++++++------
 test/ui2-stdio.pl                             |  2 +-
 5 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/Proxmox/UI/StdIO.pm b/Proxmox/UI/StdIO.pm
index 2838fcb..ec9eac9 100644
--- a/Proxmox/UI/StdIO.pm
+++ b/Proxmox/UI/StdIO.pm
@@ -70,9 +70,11 @@ sub display_html {
 sub progress {
     my ($self, $ratio, $text) = @_;
 
-    $text = '' if !defined($text);
-
-    send_msg('progress', ratio => $ratio, text => $text);
+    if (defined($text)) {
+	send_msg('progress', ratio => $ratio, text => $text);
+    } else {
+	send_msg('progress', ratio => $ratio);
+    }
 }
 
 sub process_events {
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 2143838..9d1dff4 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -205,7 +205,11 @@ fn run_installation(
                 }
                 LowLevelMessage::Progress { ratio, text } => {
                     let percentage = ratio * 100.;
-                    info!("progress {percentage:>5.1} % - {text}");
+                    if let Some(text) = text {
+                        info!("progress {percentage:>5.1} % - {text}");
+                    } else {
+                        info!("progress {percentage:>5.1} %");
+                    }
                 }
                 LowLevelMessage::Finished { state, message } => {
                     if state == "err" {
diff --git a/proxmox-installer-common/src/setup.rs b/proxmox-installer-common/src/setup.rs
index e22ce4e..4b4920e 100644
--- a/proxmox-installer-common/src/setup.rs
+++ b/proxmox-installer-common/src/setup.rs
@@ -571,6 +571,6 @@ pub enum LowLevelMessage {
     },
     Progress {
         ratio: f32,
-        text: String,
+        text: Option<String>,
     },
 }
diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs
index 585877c..303da0e 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -127,11 +127,18 @@ impl InstallProgressView {
                     }),
                     LowLevelMessage::Progress { ratio, text } => {
                         counter.set((ratio * 100.).floor() as usize);
-                        cb_sink.send(Box::new(move |siv| {
-                            siv.call_on_name(Self::PROGRESS_TEXT_VIEW_ID, |v: &mut TextView| {
-                                v.set_content(text);
-                            });
-                        }))
+                        if let Some(text) = text {
+                            cb_sink.send(Box::new(move |siv| {
+                                siv.call_on_name(
+                                    Self::PROGRESS_TEXT_VIEW_ID,
+                                    |v: &mut TextView| {
+                                        v.set_content(text);
+                                    },
+                                );
+                            }))
+                        } else {
+                            Ok(())
+                        }
                     }
                     LowLevelMessage::Finished { state, message } => {
                         counter.set(100);
@@ -310,7 +317,7 @@ mod tests {
                 next_msg(&mut reader),
                 Some(LowLevelMessage::Progress {
                     ratio: (i as f32) / 1000.,
-                    text: format!("foo {i}"),
+                    text: Some(format!("foo {i}")),
                 }),
             );
         }
diff --git a/test/ui2-stdio.pl b/test/ui2-stdio.pl
index 01f323d..eebb5c9 100755
--- a/test/ui2-stdio.pl
+++ b/test/ui2-stdio.pl
@@ -81,7 +81,7 @@ if ($child_pid) {
 	'should get 20% done progress message');
 
     is_deeply(
-	$next_msg->(), { type => 'progress', ratio => 0.2, text => '' },
+	$next_msg->(), { type => 'progress', ratio => 0.2, },
 	'should get progress continuation message');
 
     is_deeply(
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


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

* [pve-devel] [PATCH installer v2 4/6] low-level: add proper message to 100% progress ratio update
  2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
                   ` (2 preceding siblings ...)
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 3/6] low-level: stdio: fix: make progress text properly optional Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 5/6] auto: avoid printing unnecessary progress update lines Christoph Heiss
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * no changes

 Proxmox/Install.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index b891383..9074709 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -1460,7 +1460,7 @@ _EOD
 
     my $err = $@;
 
-    update_progress(1, 0, 1, "");
+    update_progress(1, 0, 1, "installation finished");
 
     print STDERR $err if $err && $err ne "\n";
 
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


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

* [pve-devel] [PATCH installer v2 5/6] auto: avoid printing unnecessary progress update lines
  2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
                   ` (3 preceding siblings ...)
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 4/6] low-level: add proper message to 100% progress ratio update Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 6/6] tui: tests: catch EOF from proxmox-low-level-installer early Christoph Heiss
  2024-11-25 22:25 ` [pve-devel] applied-series: [PATCH installer v2 0/6] small, overall install progress improvements Thomas Lamprecht
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
  To: pve-devel

In case the progress message did not contain any text and the percentage
did not change, don't print another (useless) line.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * if the percentage was updated w/o any text, reuse the last message

 .../src/bin/proxmox-auto-installer.rs             | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 9d1dff4..ece9a94 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -173,6 +173,9 @@ fn run_installation(
         let mut lowlevel_log = File::create("/tmp/install-low-level.log")
             .map_err(|err| format_err!("failed to open low-level installer logfile: {err}"))?;
 
+        let mut last_progress_percentage = 0.;
+        let mut last_progress_text = None;
+
         for line in reader.lines() {
             let line = match line {
                 Ok(line) => line,
@@ -207,8 +210,16 @@ fn run_installation(
                     let percentage = ratio * 100.;
                     if let Some(text) = text {
                         info!("progress {percentage:>5.1} % - {text}");
-                    } else {
-                        info!("progress {percentage:>5.1} %");
+                        last_progress_percentage = percentage;
+                        last_progress_text = Some(text);
+                    } else if (percentage - last_progress_percentage) > 0.1 {
+                        if let Some(text) = &last_progress_text {
+                            info!("progress {percentage:>5.1} % - {text}");
+                        } else {
+                            info!("progress {percentage:>5.1} %");
+                        }
+
+                        last_progress_percentage = percentage;
                     }
                 }
                 LowLevelMessage::Finished { state, message } => {
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


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

* [pve-devel] [PATCH installer v2 6/6] tui: tests: catch EOF from proxmox-low-level-installer early
  2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
                   ` (4 preceding siblings ...)
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 5/6] auto: avoid printing unnecessary progress update lines Christoph Heiss
@ 2024-11-25 11:27 ` Christoph Heiss
  2024-11-25 22:25 ` [pve-devel] applied-series: [PATCH installer v2 0/6] small, overall install progress improvements Thomas Lamprecht
  6 siblings, 0 replies; 8+ messages in thread
From: Christoph Heiss @ 2024-11-25 11:27 UTC (permalink / raw)
  To: pve-devel

This can happen if e.g. not all required testdata is available, such as
.cd-info - in which case proxmox-low-level-installer will fail early
with

  could not open CD info file '/.cd-info' - No such file or directory at Proxmox/Install/ISOEnv.pm line 95.

Currently, next_msg() would just be recursively called until the stack
overflowed in such a case.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * new patch

 proxmox-tui-installer/src/views/install_progress.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/proxmox-tui-installer/src/views/install_progress.rs b/proxmox-tui-installer/src/views/install_progress.rs
index 303da0e..d001451 100644
--- a/proxmox-tui-installer/src/views/install_progress.rs
+++ b/proxmox-tui-installer/src/views/install_progress.rs
@@ -258,7 +258,12 @@ mod tests {
 
     fn next_msg<R: BufRead>(reader: &mut R) -> Option<LowLevelMessage> {
         let mut line = String::new();
-        reader.read_line(&mut line).expect("a line");
+
+        match reader.read_line(&mut line) {
+            Ok(0) => return None, /* reached EOF */
+            Err(err) => panic!("failed to read message: {err}"),
+            _ => {}
+        }
 
         match serde_json::from_str::<LowLevelMessage>(&line) {
             Ok(msg) => Some(msg),
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


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

* [pve-devel] applied-series:  [PATCH installer v2 0/6] small, overall install progress improvements
  2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
                   ` (5 preceding siblings ...)
  2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 6/6] tui: tests: catch EOF from proxmox-low-level-installer early Christoph Heiss
@ 2024-11-25 22:25 ` Thomas Lamprecht
  6 siblings, 0 replies; 8+ messages in thread
From: Thomas Lamprecht @ 2024-11-25 22:25 UTC (permalink / raw)
  To: Proxmox VE development discussion, Christoph Heiss

Am 25.11.24 um 12:27 schrieb Christoph Heiss:
> This series tries to improve upon some small things around the
> installation progress reporting, esp. in the auto-installer, as well as
> some small fixes & code-deduplication.
> 
> History
> =======
> 
> v1: https://lore.proxmox.com/pve-devel/20240516133945.1087246-1-c.heiss@proxmox.com/
> 
> Changes v1 -> v2:
>   * dropped already-applied patches
>   * rebased on latest master
>   * added new patch #6
> 
> Diffstat
> ========
> 
> Christoph Heiss (6):
>   auto, tui: move low-level installer message struct to common crate
>   auto: log non-json low-level messages into separate file
>   low-level: stdio: fix: make progress text properly optional
>   low-level: add proper message to 100% progress ratio update
>   auto: avoid printing unnecessary progress update lines
>   tui: tests: catch EOF from proxmox-low-level-installer early
> 
>  Proxmox/Install.pm                            |  2 +-
>  Proxmox/UI/StdIO.pm                           |  8 +-
>  .../src/bin/proxmox-auto-installer.rs         | 41 ++++++++--
>  proxmox-auto-installer/src/utils.rs           | 23 ------
>  proxmox-installer-common/src/setup.rs         | 23 ++++++
>  .../src/views/install_progress.rs             | 76 ++++++++-----------
>  test/ui2-stdio.pl                             |  2 +-
>  7 files changed, 98 insertions(+), 77 deletions(-)
> 


applied series, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


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

end of thread, other threads:[~2024-11-25 22:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-25 11:27 [pve-devel] [PATCH installer v2 0/6] small, overall install progress improvements Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 1/6] auto, tui: move low-level installer message struct to common crate Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 2/6] auto: log non-json low-level messages into separate file Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 3/6] low-level: stdio: fix: make progress text properly optional Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 4/6] low-level: add proper message to 100% progress ratio update Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 5/6] auto: avoid printing unnecessary progress update lines Christoph Heiss
2024-11-25 11:27 ` [pve-devel] [PATCH installer v2 6/6] tui: tests: catch EOF from proxmox-low-level-installer early Christoph Heiss
2024-11-25 22:25 ` [pve-devel] applied-series: [PATCH installer v2 0/6] small, overall install progress improvements Thomas Lamprecht

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal