From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter =?utf-8?q?M=C3=BCller?= To: development@lists.ipfire.org Subject: Re: [PATCH 1/2] pakfire: Implement feedback from mailing list discussion Date: Tue, 28 Dec 2021 23:11:43 +0100 Message-ID: <6741292c-17ff-4aa5-8b4b-1ce2628e8cbd@ipfire.org> In-Reply-To: <20211227132137.1355-1-hofmann@leo-andres.de> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7789480164409022864==" List-Id: --===============7789480164409022864== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Acked-by: Peter M=C3=BCller > - Improve lockfile test: Return immediately if lockfile is present, > to prevent unnecessary and expensive "pidof" calls >=20 > - Add better explanation to the log file reading command and JS >=20 > - 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. >=20 > 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(-) >=20 > 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 fo= und, > + # to capture all messages of the last (i.e. current) Pakfire run > + my @messages =3D `tac /var/log/messages | sed -n '/pakfire:/{p;/Pakfire.*= started/q}'`; > + > + # Test if the log contains an error message (fastest implementation, stop= s 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.*= started/q}'`; > =20 > # Start JSON file > print "{\n"; > @@ -128,14 +136,18 @@ my $extraHead =3D < pakfire.i18n.load({ > 'working': '$Lang::tr{'pakfire working'}', > 'finished': '$Lang::tr{'pakfire finished'}', > - 'since': '$Lang::tr{'since'} ', //(space is intentional) > + 'finished error': '$Lang::tr{'pakfire finished error'}', > + 'since': '$Lang::tr{'since'}', > =20 > 'link_return': '$Lang::tr{'pakfire retur= n'}', > 'link_reboot': '$Lang::tr{'needreboot'= }' > }); > -=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, wri=
ting "false" stops polling.
> +	//    When the task has been completed, status polling stops and this ret=
urns to "false".
> +	//    The page can then be reloaded to go back to the main screen. Writin=
g "false" does not trigger a reload.
> +	//    "refreshInterval" and "setupPageReload" can be used to adjust the r=
espective behaviour.
> +	// reboot: An update requires a reboot.
> +	//    If set to "true", a link to the reboot menu is shown after the task=
 is completed.
> +	// failure: An error has occured.
> +	//    To display the error log, the page does not return to the main scre=
en.
> +
>  	// 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') + " • " + th=
is.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_sin=
ce']);
>  			} 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 insta=
llieren:',
>  'pakfire core update level' =3D> 'Core-Update-Level',
> -'pakfire finished' =3D> 'Pakfire ist fertig!. Bitte =C3=BCberpr=C3=BCfen S=
ie 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 Paket=
e zur Installation aus und dr=C3=BCcken Sie auf das plus-Symbol.',
>  'pakfire install package' =3D> 'Sie m=C3=B6chten folgende Pakete installie=
ren: ',
> 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 automatica=
lly:',
>  '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, plea=
se 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 t=
he list below and click the plus to install.',
>  'pakfire install package' =3D> 'You want to install the following packages=
: ',
>=20

--===============7789480164409022864==--