[pve-devel] [PATCH installer 0/3] memorize keyboard layout and run setupcon in background

[pve-devel] [PATCH installer 0/3] memorize keyboard layout and run setupcon in background

[Stoiko Ivanov]

This patchset addresses the issue found by Oguz with setupcon slowing down the
GUI.

After some discussion off-list with Thomas and Oguz 2 thinks were considered
worth improving:
* memorize the chosen keyboard layout and don't jump back to the one defined
by the country - when using the Previous/Next buttons
* still set the keymap of the installer system, but do it in the background

I tested the patches locally and think they work as they should (but then again
the possible combinations of changed country/changed keymap/freshly drawn
window confused me a bit - so would be grateful for some testing)


Stoiko Ivanov (3):
  memorize keyboard layout selection
  add run_in_background
  set the keymap on the installer console

 proxinstall | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

-- 
2.20.1

[pve-devel] [PATCH installer 1/3] memorize keyboard layout selection

[Stoiko Ivanov]

currently when using the previous/next buttons the keyboard layout gets
defined based on the detected/selected country, even if it was set to a
different value explicitly.

This patch changes the behaviour to only update the layout and set it in
the installer if it got actively changed, or if a different country was
selected

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 proxinstall | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/proxinstall b/proxinstall
index 9879df4..9977f44 100755
--- a/proxinstall
+++ b/proxinstall
@@ -2549,6 +2549,8 @@ sub get_device_desc {
     }
 }
 
+my $last_layout;
+my $country_layout;
 sub update_layout {
     my ($cb, $kmap) = @_;
 
@@ -2562,7 +2564,14 @@ sub update_layout {
 	$i++;
     }
 
-    $cb->set_active($ind || $def || 0);
+    my $val = $ind || $def || 0;
+
+    if (!defined($kmap)) {
+	$last_layout //= $val;
+    } elsif (!defined($country_layout) || $country_layout != $val) {
+	$last_layout = $country_layout = $val;
+    }
+    $cb->set_active($last_layout);
 }
 
 my $lastzonecb;
@@ -2699,6 +2708,7 @@ sub create_password_view {
 
 }
 
+my $installer_kmap;
 sub create_country_view {
 
     cleanup_view();
@@ -2744,11 +2754,16 @@ sub create_country_view {
 
     $kmapcb->signal_connect ('changed' => sub {
 	my $sel = $kmapcb->get_active_text();
+	$last_layout = $kmapcb->get_active();
 	if (my $kmap = $cmap->{kmaphash}->{$sel}) {
 	    my $xkmap = $cmap->{kmap}->{$kmap}->{x11};
 	    my $xvar = $cmap->{kmap}->{$kmap}->{x11var};
 	    $keymap = $kmap;
 
+	    return if (defined($installer_kmap) && $installer_kmap eq $kmap);
+
+	    $installer_kmap = $keymap;
+
 	    if (! $opt_testmode) {
 		syscmd ("setxkbmap $xkmap $xvar");
 	    }
-- 
2.20.1

[pve-devel] [PATCH installer 2/3] add run_in_background

[Stoiko Ivanov]

certain tasks done during the installer need not block the GUI (e.g. setting
the keymap of the console, to have the correct one set in the shell on vt3)
and take a longer time.

This patch adds a simple run_in_background method, which forks and runs the
provided code in the child. Before exiting the children get reaped.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 proxinstall | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/proxinstall b/proxinstall
index 9977f44..1551e18 100755
--- a/proxinstall
+++ b/proxinstall
@@ -21,6 +21,7 @@ use Data::Dumper;
 use File::Basename;
 use File::Path;
 use Time::HiRes;
+use POSIX ":sys_wait_h";
 
 use ProxmoxInstallerSetup;
 
@@ -484,6 +485,23 @@ sub run_command {
     return $ostream;
 }
 
+# forks and runs the provided coderef in the child
+# do not use syscmd or run_command as both confuse the GTK mainloop if
+# run from a child process
+sub run_in_background {
+    my ($cmd) = @_;
+
+    my $pid = fork() // die "fork failed: $!\n";
+    if (!$pid) {
+	eval { $cmd->(); };
+	if (my $err = $@) {
+	    warn "run_in_background error: $err\n";
+	    POSIX::_exit(1);
+	}
+	POSIX::_exit(0);
+    }
+}
+
 sub detect_country {
 
     print "trying to detect country...\n";
@@ -3583,4 +3601,9 @@ create_intro_view () if !$initial_error;
 
 Gtk3->main;
 
+# reap left over zombie processes
+while ((my $child = waitpid(-1, POSIX::WNOHANG)) > 0) {
+    print "reaped child $child\n";
+}
+
 exit 0;
-- 
2.20.1

[pve-devel] [PATCH installer 3/3] set the keymap on the installer console

[Stoiko Ivanov]

this patch partially reverts 8bc528041ba85e1b9bd4c17638a2302088bc19ce
by writing /etc/default/keyboard and running `setupcon` in the background
the delay should not harm the UX in the installer

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 proxinstall | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/proxinstall b/proxinstall
index 1551e18..2bc30e8 100755
--- a/proxinstall
+++ b/proxinstall
@@ -2784,6 +2784,18 @@ sub create_country_view {
 
 	    if (! $opt_testmode) {
 		syscmd ("setxkbmap $xkmap $xvar");
+
+		my $kbd_config = qq{
+		    XKBLAYOUT="$xkmap"
+		    XKBVARIANT="$xvar"
+		    BACKSPACE="guess"
+		};
+		$kbd_config =~ s/^\s+//gm;
+
+		run_in_background( sub {
+		    write_config($kbd_config, '/etc/default/keyboard');
+		    system("setupcon");
+		});
 	    }
 	}
     });
-- 
2.20.1

Re: [pve-devel] [PATCH installer 0/3] memorize keyboard layout and run setupcon in background

[Oguz Bektas]

ACK from me, fixes the described issue and keeps the chosen layout.

Tested-by: Oguz Bektas <o.bektas@proxmox.com>

On Tue, Nov 10, 2020 at 03:15:27PM +0100, Stoiko Ivanov wrote:
> This patchset addresses the issue found by Oguz with setupcon slowing down the
> GUI.
> 
> After some discussion off-list with Thomas and Oguz 2 thinks were considered
> worth improving:
> * memorize the chosen keyboard layout and don't jump back to the one defined
> by the country - when using the Previous/Next buttons
> * still set the keymap of the installer system, but do it in the background
> 
> I tested the patches locally and think they work as they should (but then again
> the possible combinations of changed country/changed keymap/freshly drawn
> window confused me a bit - so would be grateful for some testing)
> 
> 
> Stoiko Ivanov (3):
>   memorize keyboard layout selection
>   add run_in_background
>   set the keymap on the installer console
> 
>  proxinstall | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

[pve-devel] applied-series: [PATCH installer 0/3] memorize keyboard layout and run setupcon in background

[Thomas Lamprecht]

On 10.11.20 15:15, Stoiko Ivanov wrote:
> This patchset addresses the issue found by Oguz with setupcon slowing down the
> GUI.
> 
> After some discussion off-list with Thomas and Oguz 2 thinks were considered
> worth improving:
> * memorize the chosen keyboard layout and don't jump back to the one defined
> by the country - when using the Previous/Next buttons
> * still set the keymap of the installer system, but do it in the background
> 
> I tested the patches locally and think they work as they should (but then again
> the possible combinations of changed country/changed keymap/freshly drawn
> window confused me a bit - so would be grateful for some testing)
> 
> 
> Stoiko Ivanov (3):
>   memorize keyboard layout selection
>   add run_in_background
>   set the keymap on the installer console
> 
>  proxinstall | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 



applied series, thanks!