From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leo-Andres Hofmann To: development@lists.ipfire.org Subject: [PATCH 1/2] pakfire: Implement feedback from mailing list discussion Date: Mon, 27 Dec 2021 14:21:36 +0100 Message-ID: <20211227132137.1355-1-hofmann@leo-andres.de> In-Reply-To: <20211202153955.1126-1-hofmann@leo-andres.de> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1942648513114218601==" List-Id: --===============1942648513114218601== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable - Improve lockfile test: Return immediately if lockfile is present, to prevent unnecessary and expensive "pidof" calls - Add better explanation to the log file reading command and JS - Change user interface: If no errors occurred, the page returns to the main screen (after a short delay). If an error occurred, the log output remains and a message is shown. Signed-off-by: Leo-Andres Hofmann --- html/cgi-bin/pakfire.cgi | 42 ++++++++++++---- html/html/include/pakfire.js | 96 ++++++++++++++++++++++++++++++++++-- langs/de/cgi-bin/de.pl | 3 +- langs/en/cgi-bin/en.pl | 3 +- 4 files changed, 127 insertions(+), 17 deletions(-) diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi index e14658ffb..8516b07b1 100644 --- a/html/cgi-bin/pakfire.cgi +++ b/html/cgi-bin/pakfire.cgi @@ -20,6 +20,7 @@ ############################################################################= ### =20 use strict; +use List::Util qw(any); =20 # enable only the following on debugging purpose #use warnings; @@ -54,13 +55,20 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') { # Send HTTP headers _start_json_output(); =20 - # Collect Pakfire status and log messages + # Read /var/log/messages backwards until a "Pakfire started" header is foun= d, + # to capture all messages of the last (i.e. current) Pakfire run + my @messages =3D `tac /var/log/messages | sed -n '/pakfire:/{p;/Pakfire.*st= arted/q}'`; + + # Test if the log contains an error message (fastest implementation, stops = at first match) + my $failure =3D any{ index($_, 'ERROR') !=3D -1 } @messages; + + # Collect Pakfire status my %status =3D ( 'running' =3D> &_is_pakfire_busy() || "0", 'running_since' =3D> &General::age("$Pakfire::lockfile") || "0s", - 'reboot' =3D> (-e "/var/run/need_reboot") || "0" + 'reboot' =3D> (-e "/var/run/need_reboot") || "0", + 'failure' =3D> $failure || "0" ); - my @messages =3D `tac /var/log/messages | sed -n '/pakfire:/{p;/Pakfire.*st= arted/q}'`; =20 # Start JSON file print "{\n"; @@ -128,14 +136,18 @@ my $extraHead =3D <$Lang::tr{'pakfire return'= }', 'link_reboot': '$Lang::tr{'needreboot'}<= /a>' }); -=09 - // AJAX auto refresh interval - pakfire.refreshInterval =3D 1000; + + // AJAX auto refresh interval (in ms, default: 1000) + //pakfire.refreshInterval =3D 1000; + + // Enable returning to main screen (delay in ms) + pakfire.setupPageReload(true, 3000); END ; @@ -276,6 +288,7 @@ if(&_is_pakfire_busy()) { 

 
=20
@@ -401,13 +414,22 @@ END
=20
 # Check if pakfire is already running (extend test here if necessary)
 sub _is_pakfire_busy {
-	# Get PID of a running pakfire instance
+	# Return immediately if lockfile is present
+	if(-e "$Pakfire::lockfile") {
+		return 1;
+	}
+
+	# Check if a PID of a running pakfire instance is found
 	# (The system backpipe command is safe, because no user input is computed.)
 	my $pakfire_pid =3D `pidof -s /usr/local/bin/pakfire`;
 	chomp($pakfire_pid);
=20
-	# Test presence of PID or lockfile
-	return (($pakfire_pid) || (-e "$Pakfire::lockfile"));
+	if($pakfire_pid) {
+		return 1;
+	}
+
+	# Pakfire isn't running
+	return 0;
 }
=20
 # Send HTTP headers
diff --git a/html/html/include/pakfire.js b/html/html/include/pakfire.js
index 0950870e0..44a40c75f 100644
--- a/html/html/include/pakfire.js
+++ b/html/html/include/pakfire.js
@@ -32,12 +32,13 @@ class PakfireJS {
 		this._states =3D Object.create(null);
 		this._states.running =3D false;
 		this._states.reboot =3D false;
+		this._states.failure =3D false;
=20
 		// Status refresh helper
 		this._autoRefresh =3D {
-			delay: 1000, //Delay between requests (default: 1s)
+			delay: 1000, //Delay between requests (minimum: 500, default: 1s)
 			jsonAction: 'getstatus', //CGI POST action parameter
-			timeout: 5000, //XHR timeout (5s)
+			timeout: 5000, //XHR timeout (0 to disable, default: 5s)
=20
 			delayTimer: null, //setTimeout reference
 			jqXHR: undefined, //jQuery.ajax promise reference
@@ -51,10 +52,32 @@ class PakfireJS {
 				return (this.runningDelay || this.runningXHR);
 			}
 		};
+
+		// Return to main screen helper
+		this._pageReload =3D {
+			delay: 1000, //Delay before page reload (default: 1s)
+			enabled: false, //Reload disabled by default
+
+			delayTimer: null, //setTimeout reference
+			get isTriggered() { //Reload timer started
+				return (this.delayTimer !=3D=3D null);
+			}
+		};
 	}
=20
 	//### Public properties ###
=20
+	// Note on using the status flags
+	// running: Pakfire is performing a task.
+	//    Writing "true" activates the periodic AJAX/JSON status polling, writi=
ng "false" stops polling.
+	//    When the task has been completed, status polling stops and this retur=
ns to "false".
+	//    The page can then be reloaded to go back to the main screen. Writing =
"false" does not trigger a reload.
+	//    "refreshInterval" and "setupPageReload" can be used to adjust the res=
pective behaviour.
+	// reboot: An update requires a reboot.
+	//    If set to "true", a link to the reboot menu is shown after the task i=
s completed.
+	// failure: An error has occured.
+	//    To display the error log, the page does not return to the main screen.
+
 	// Pakfire is running (true/false)
 	set running(state) {
 		if(this._states.running !=3D=3D state) {
@@ -77,6 +100,17 @@ class PakfireJS {
 		return this._states.reboot;
 	}
=20
+	// Error encountered (true/false)
+	set failure(state) {
+		if(this._states.failure !=3D=3D state) {
+			this._states.failure =3D state;
+			this._states_onChange('failure');
+		}
+	}
+	get failure() {
+		return this._states.failure;
+	}
+
 	// Status refresh interval in ms
 	set refreshInterval(delay) {
 		if(delay < 500) {
@@ -88,6 +122,16 @@ class PakfireJS {
 		return this._autoRefresh.delay;
 	}
=20
+	// Configure page reload after successful task (returns to main screen)
+	// delay: In ms
+	setupPageReload(enabled, delay) {
+		if(delay < 0) {
+			delay =3D 0;
+		}
+		this._pageReload.delay =3D delay;
+		this._pageReload.enabled =3D enabled;
+	}
+
 	// Document loaded (call once from jQuery.ready)
 	documentReady() {
 		// Status refresh late start
@@ -96,6 +140,12 @@ class PakfireJS {
 		}
 	}
=20
+	// Reload entire CGI page (clears POST/GET data from history)
+	documentReload() {
+		let url =3D window.location.origin + window.location.pathname;
+		window.location.replace(url);
+	}
+
 	//### Private properties ###
=20
 	// Pakfire status change handler
@@ -106,9 +156,13 @@ class PakfireJS {
 			$('#pflog-status').text(this.i18n.get('working'));
 			$('#pflog-action').empty();
 		} else {
-			$('#pflog-status').text(this.i18n.get('finished'));
+			if(this.failure) {
+				$('#pflog-status').text(this.i18n.get('finished error'));
+			} else {
+				$('#pflog-status').text(this.i18n.get('finished'));
+			}
 			if(this.reboot) { //Enable return or reboot links in UI
-				$('#pflog-action').html(this.i18n.get('link_reboot'));
+				$('#pflog-action').html(this.i18n.get('link_return') + " • " + this=
.i18n.get('link_reboot'));
 			} else {
 				$('#pflog-action').html(this.i18n.get('link_return'));
 			}
@@ -122,6 +176,13 @@ class PakfireJS {
 				this._autoRefresh_clearSchedule();
 			}
 		}
+
+		// Always stay in the log viewer if Pakfire failed
+		if(property =3D=3D=3D 'failure') {
+			if(this.failure) {
+				this._pageReload_cancel();
+			}
+		}
 	}
=20
 	//--- Status refresh scheduling functions ---
@@ -164,6 +225,25 @@ class PakfireJS {
 		}
 	}
=20
+	// Start delayed page reload to return to main screen
+	_pageReload_trigger() {
+		if((! this._pageReload.enabled) || this._pageReload.isTriggered) {
+			return; // Disabled or already started
+		}
+		this._pageReload.delayTimer =3D window.setTimeout(function() {
+			this._pageReload.delayTimer =3D null;
+			this.documentReload();
+		}.bind(this), this._pageReload.delay);
+	}
+
+	// Stop scheduled reload
+	_pageReload_cancel() {
+		if(this._pageReload.isTriggered) {
+			window.clearTimeout(this._pageReload.delayTimer);
+			this._pageReload.delayTimer =3D null;
+		}
+	}
+
 	//--- JSON request & data handling ---
=20
 	// Load JSON data from Pakfire CGI, using a POST request
@@ -192,10 +272,11 @@ class PakfireJS {
 			// Update status flags
 			this.running =3D (data['running'] !=3D '0');
 			this.reboot =3D (data['reboot'] !=3D '0');
+			this.failure =3D (data['failure'] !=3D '0');
=20
 			// Update timer display
 			if(this.running && data['running_since']) {
-				$('#pflog-time').text(this.i18n.get('since') + data['running_since']);
+				$('#pflog-time').text(this.i18n.get('since') + " " + data['running_since=
']);
 			} else {
 				$('#pflog-time').empty();
 			}
@@ -206,6 +287,11 @@ class PakfireJS {
 				messages +=3D `${line}\n`;
 			});
 			$('#pflog-messages').text(messages);
+
+			// Pakfire finished without errors, return to main screen
+			if((! this.running) && (! this.failure)) {
+				this._pageReload_trigger();
+			}
 		}
 	}
 }
diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
index 490879c90..dd946081d 100644
--- a/langs/de/cgi-bin/de.pl
+++ b/langs/de/cgi-bin/de.pl
@@ -1974,7 +1974,8 @@
 'pakfire configuration' =3D> 'Pakfire Konfiguration',
 'pakfire core update auto' =3D> 'Core- und Addon-Updates automatisch install=
ieren:',
 'pakfire core update level' =3D> 'Core-Update-Level',
-'pakfire finished' =3D> 'Pakfire ist fertig!. Bitte =C3=BCberpr=C3=BCfen Sie=
 die Log Ausgabe.',
+'pakfire finished' =3D> 'Pakfire ist fertig! Kehre zur=C3=BCck...',
+'pakfire finished error' =3D> 'Pakfire ist fertig! Fehler sind aufgetreten, =
bitte =C3=BCberpr=C3=BCfen Sie die Log-Ausgabe, bevor Sie fortfahren.',
 'pakfire health check' =3D> 'Mirrors auf Erreichbarkeit pr=C3=BCfen (Ping):',
 'pakfire install description' =3D> 'W=C3=A4hlen Sie ein oder mehrere Pakete =
zur Installation aus und dr=C3=BCcken Sie auf das plus-Symbol.',
 'pakfire install package' =3D> 'Sie m=C3=B6chten folgende Pakete installiere=
n: ',
diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
index 4442ea772..fc5a57cb0 100644
--- a/langs/en/cgi-bin/en.pl
+++ b/langs/en/cgi-bin/en.pl
@@ -2009,7 +2009,8 @@
 'pakfire configuration' =3D> 'Pakfire Configuration',
 'pakfire core update auto' =3D> 'Install core and addon updates automaticall=
y:',
 'pakfire core update level' =3D> 'Core-Update-Level',
-'pakfire finished' =3D> 'Pakfire is finished! Please check the log output.',
+'pakfire finished' =3D> 'Pakfire has finished! Returning...',
+'pakfire finished error' =3D> 'Pakfire has finished! Errors occurred, please=
 check the log output before proceeding.',
 'pakfire health check' =3D> 'Check if mirror is reachable (ping):',
 'pakfire install description' =3D> 'Please choose one or more items from the=
 list below and click the plus to install.',
 'pakfire install package' =3D> 'You want to install the following packages: =
',
--=20
2.27.0.windows.1


--===============1942648513114218601==--