[pve-devel] [PATCH installer] tui: fix interface sort order

[pve-devel] [PATCH installer] tui: fix interface sort order

[Stoiko Ivanov]

currently when multiple nics are present in a system the TUI
sometimes selects the wrong interface (not the one that has the
default gateway/dhcp lease)

I assume this is due to HashMap's values yielding an iterator in
arbitrary order

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
sadly a bit difficult to test due to the randomnes - but at least the 3
tests on a VM were consistent.

 proxmox-tui-installer/src/main.rs | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 4c14482..85b6811 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -488,16 +488,23 @@ fn network_dialog(siv: &mut Cursive) -> InstallerView {
     let state = siv.user_data::<InstallerState>().unwrap();
     let options = &state.options.network;
     let ifaces = state.runtime_info.network.interfaces.values();
-    let ifnames = ifaces
+    let ifname_entries = ifaces
         .clone()
         .map(|iface| (iface.render(), iface.name.clone()));
-    let mut ifaces_selection = SelectView::new().popup().with_all(ifnames.clone());
+    let mut ifaces_selection = SelectView::new().popup().with_all(ifname_entries.clone());
+
+    let mut ifnames = ifaces
+        .clone()
+        .map(|iface| iface.name.clone())
+        .collect::<Vec<String>>();
+    ifnames.sort();
 
     ifaces_selection.sort();
     ifaces_selection.set_selection(
         ifnames
+            .iter()
             .clone()
-            .position(|iface| iface.1 == options.ifname)
+            .position(|iface| *iface == options.ifname)
             .unwrap_or(ifaces.len() - 1),
     );
 
-- 
2.39.2

[pve-devel] applied: [PATCH installer] tui: fix interface sort order

[Thomas Lamprecht]

Am 17/11/2023 um 18:30 schrieb Stoiko Ivanov:
> currently when multiple nics are present in a system the TUI
> sometimes selects the wrong interface (not the one that has the
> default gateway/dhcp lease)
> 
> I assume this is due to HashMap's values yielding an iterator in
> arbitrary order
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> sadly a bit difficult to test due to the randomnes - but at least the 3
> tests on a VM were consistent.
> 
>  proxmox-tui-installer/src/main.rs | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
>

applied, thanks!

as talked off-list, I remolded this to avoid a separate vector by re-using the
SelectView's data, which is already sorted:

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 4c14482..4b6b5b2 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -493,13 +493,14 @@ fn network_dialog(siv: &mut Cursive) -> InstallerView {
         .map(|iface| (iface.render(), iface.name.clone()));
     let mut ifaces_selection = SelectView::new().popup().with_all(ifnames.clone());
 
+    // sort first to always have stable view
     ifaces_selection.sort();
-    ifaces_selection.set_selection(
-        ifnames
-            .clone()
-            .position(|iface| iface.1 == options.ifname)
-            .unwrap_or(ifaces.len() - 1),
-    );
+    let selected = ifaces_selection
+        .iter()
+        .position(|(_label, iface)| *iface == options.ifname)
+        .unwrap_or(ifaces.len() - 1);
+
+    ifaces_selection.set_selection(selected);
 
     let inner = FormView::new()
         .child("Management interface", ifaces_selection)