[1/2] pakfire: Implement feedback from mailing list discussion
Commit Message
- 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 <hofmann@leo-andres.de>
---
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(-)
Comments
Acked-by: Peter Müller <peter.mueller@ipfire.org>
> - 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 <hofmann@leo-andres.de>
> ---
> 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 @@
> ###############################################################################
>
> use strict;
> +use List::Util qw(any);
>
> # 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();
>
> - # Collect Pakfire status and log messages
> + # Read /var/log/messages backwards until a "Pakfire started" header is found,
> + # to capture all messages of the last (i.e. current) Pakfire run
> + my @messages = `tac /var/log/messages | sed -n '/pakfire:/{p;/Pakfire.*started/q}'`;
> +
> + # Test if the log contains an error message (fastest implementation, stops at first match)
> + my $failure = any{ index($_, 'ERROR') != -1 } @messages;
> +
> + # Collect Pakfire status
> my %status = (
> 'running' => &_is_pakfire_busy() || "0",
> 'running_since' => &General::age("$Pakfire::lockfile") || "0s",
> - 'reboot' => (-e "/var/run/need_reboot") || "0"
> + 'reboot' => (-e "/var/run/need_reboot") || "0",
> + 'failure' => $failure || "0"
> );
> - my @messages = `tac /var/log/messages | sed -n '/pakfire:/{p;/Pakfire.*started/q}'`;
>
> # Start JSON file
> print "{\n";
> @@ -128,14 +136,18 @@ my $extraHead = <<END
> 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'}',
>
> 'link_return': '<a href="$ENV{'SCRIPT_NAME'}">$Lang::tr{'pakfire return'}</a>',
> 'link_reboot': '<a href="/cgi-bin/shutdown.cgi">$Lang::tr{'needreboot'}</a>'
> });
> -
> - // AJAX auto refresh interval
> - pakfire.refreshInterval = 1000;
> +
> + // AJAX auto refresh interval (in ms, default: 1000)
> + //pakfire.refreshInterval = 1000;
> +
> + // Enable returning to main screen (delay in ms)
> + pakfire.setupPageReload(true, 3000);
> </script>
> END
> ;
> @@ -276,6 +288,7 @@ if(&_is_pakfire_busy()) {
> <!-- Pakfire log messages -->
> <pre id="pflog-messages"></pre>
> <script>
> + // Start automatic log refresh
> pakfire.running = true;
> </script>
>
> @@ -401,13 +414,22 @@ END
>
> # 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 = `pidof -s /usr/local/bin/pakfire`;
> chomp($pakfire_pid);
>
> - # Test presence of PID or lockfile
> - return (($pakfire_pid) || (-e "$Pakfire::lockfile"));
> + if($pakfire_pid) {
> + return 1;
> + }
> +
> + # Pakfire isn't running
> + return 0;
> }
>
> # 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 = Object.create(null);
> this._states.running = false;
> this._states.reboot = false;
> + this._states.failure = false;
>
> // Status refresh helper
> this._autoRefresh = {
> - 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)
>
> 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 = {
> + 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 !== null);
> + }
> + };
> }
>
> //### Public properties ###
>
> + // Note on using the status flags
> + // running: Pakfire is performing a task.
> + // Writing "true" activates the periodic AJAX/JSON status polling, writing "false" stops polling.
> + // When the task has been completed, status polling stops and this returns 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 respective 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 screen.
> +
> // Pakfire is running (true/false)
> set running(state) {
> if(this._states.running !== state) {
> @@ -77,6 +100,17 @@ class PakfireJS {
> return this._states.reboot;
> }
>
> + // Error encountered (true/false)
> + set failure(state) {
> + if(this._states.failure !== state) {
> + this._states.failure = 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;
> }
>
> + // Configure page reload after successful task (returns to main screen)
> + // delay: In ms
> + setupPageReload(enabled, delay) {
> + if(delay < 0) {
> + delay = 0;
> + }
> + this._pageReload.delay = delay;
> + this._pageReload.enabled = enabled;
> + }
> +
> // Document loaded (call once from jQuery.ready)
> documentReady() {
> // Status refresh late start
> @@ -96,6 +140,12 @@ class PakfireJS {
> }
> }
>
> + // Reload entire CGI page (clears POST/GET data from history)
> + documentReload() {
> + let url = window.location.origin + window.location.pathname;
> + window.location.replace(url);
> + }
> +
> //### Private properties ###
>
> // 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 === 'failure') {
> + if(this.failure) {
> + this._pageReload_cancel();
> + }
> + }
> }
>
> //--- Status refresh scheduling functions ---
> @@ -164,6 +225,25 @@ class PakfireJS {
> }
> }
>
> + // 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 = window.setTimeout(function() {
> + this._pageReload.delayTimer = 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 = null;
> + }
> + }
> +
> //--- JSON request & data handling ---
>
> // Load JSON data from Pakfire CGI, using a POST request
> @@ -192,10 +272,11 @@ class PakfireJS {
> // Update status flags
> this.running = (data['running'] != '0');
> this.reboot = (data['reboot'] != '0');
> + this.failure = (data['failure'] != '0');
>
> // 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 += `${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' => 'Pakfire Konfiguration',
> 'pakfire core update auto' => 'Core- und Addon-Updates automatisch installieren:',
> 'pakfire core update level' => 'Core-Update-Level',
> -'pakfire finished' => 'Pakfire ist fertig!. Bitte überprüfen Sie die Log Ausgabe.',
> +'pakfire finished' => 'Pakfire ist fertig! Kehre zurück...',
> +'pakfire finished error' => 'Pakfire ist fertig! Fehler sind aufgetreten, bitte überprüfen Sie die Log-Ausgabe, bevor Sie fortfahren.',
> 'pakfire health check' => 'Mirrors auf Erreichbarkeit prüfen (Ping):',
> 'pakfire install description' => 'Wählen Sie ein oder mehrere Pakete zur Installation aus und drücken Sie auf das plus-Symbol.',
> 'pakfire install package' => 'Sie möchten folgende Pakete installieren: ',
> 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' => 'Pakfire Configuration',
> 'pakfire core update auto' => 'Install core and addon updates automatically:',
> 'pakfire core update level' => 'Core-Update-Level',
> -'pakfire finished' => 'Pakfire is finished! Please check the log output.',
> +'pakfire finished' => 'Pakfire has finished! Returning...',
> +'pakfire finished error' => 'Pakfire has finished! Errors occurred, please check the log output before proceeding.',
> 'pakfire health check' => 'Check if mirror is reachable (ping):',
> 'pakfire install description' => 'Please choose one or more items from the list below and click the plus to install.',
> 'pakfire install package' => 'You want to install the following packages: ',
>
@@ -20,6 +20,7 @@
###############################################################################
use strict;
+use List::Util qw(any);
# 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();
- # Collect Pakfire status and log messages
+ # Read /var/log/messages backwards until a "Pakfire started" header is found,
+ # to capture all messages of the last (i.e. current) Pakfire run
+ my @messages = `tac /var/log/messages | sed -n '/pakfire:/{p;/Pakfire.*started/q}'`;
+
+ # Test if the log contains an error message (fastest implementation, stops at first match)
+ my $failure = any{ index($_, 'ERROR') != -1 } @messages;
+
+ # Collect Pakfire status
my %status = (
'running' => &_is_pakfire_busy() || "0",
'running_since' => &General::age("$Pakfire::lockfile") || "0s",
- 'reboot' => (-e "/var/run/need_reboot") || "0"
+ 'reboot' => (-e "/var/run/need_reboot") || "0",
+ 'failure' => $failure || "0"
);
- my @messages = `tac /var/log/messages | sed -n '/pakfire:/{p;/Pakfire.*started/q}'`;
# Start JSON file
print "{\n";
@@ -128,14 +136,18 @@ my $extraHead = <<END
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'}',
'link_return': '<a href="$ENV{'SCRIPT_NAME'}">$Lang::tr{'pakfire return'}</a>',
'link_reboot': '<a href="/cgi-bin/shutdown.cgi">$Lang::tr{'needreboot'}</a>'
});
-
- // AJAX auto refresh interval
- pakfire.refreshInterval = 1000;
+
+ // AJAX auto refresh interval (in ms, default: 1000)
+ //pakfire.refreshInterval = 1000;
+
+ // Enable returning to main screen (delay in ms)
+ pakfire.setupPageReload(true, 3000);
</script>
END
;
@@ -276,6 +288,7 @@ if(&_is_pakfire_busy()) {
<!-- Pakfire log messages -->
<pre id="pflog-messages"></pre>
<script>
+ // Start automatic log refresh
pakfire.running = true;
</script>
@@ -401,13 +414,22 @@ END
# 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 = `pidof -s /usr/local/bin/pakfire`;
chomp($pakfire_pid);
- # Test presence of PID or lockfile
- return (($pakfire_pid) || (-e "$Pakfire::lockfile"));
+ if($pakfire_pid) {
+ return 1;
+ }
+
+ # Pakfire isn't running
+ return 0;
}
# Send HTTP headers
@@ -32,12 +32,13 @@ class PakfireJS {
this._states = Object.create(null);
this._states.running = false;
this._states.reboot = false;
+ this._states.failure = false;
// Status refresh helper
this._autoRefresh = {
- 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)
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 = {
+ 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 !== null);
+ }
+ };
}
//### Public properties ###
+ // Note on using the status flags
+ // running: Pakfire is performing a task.
+ // Writing "true" activates the periodic AJAX/JSON status polling, writing "false" stops polling.
+ // When the task has been completed, status polling stops and this returns 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 respective 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 screen.
+
// Pakfire is running (true/false)
set running(state) {
if(this._states.running !== state) {
@@ -77,6 +100,17 @@ class PakfireJS {
return this._states.reboot;
}
+ // Error encountered (true/false)
+ set failure(state) {
+ if(this._states.failure !== state) {
+ this._states.failure = 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;
}
+ // Configure page reload after successful task (returns to main screen)
+ // delay: In ms
+ setupPageReload(enabled, delay) {
+ if(delay < 0) {
+ delay = 0;
+ }
+ this._pageReload.delay = delay;
+ this._pageReload.enabled = enabled;
+ }
+
// Document loaded (call once from jQuery.ready)
documentReady() {
// Status refresh late start
@@ -96,6 +140,12 @@ class PakfireJS {
}
}
+ // Reload entire CGI page (clears POST/GET data from history)
+ documentReload() {
+ let url = window.location.origin + window.location.pathname;
+ window.location.replace(url);
+ }
+
//### Private properties ###
// 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 === 'failure') {
+ if(this.failure) {
+ this._pageReload_cancel();
+ }
+ }
}
//--- Status refresh scheduling functions ---
@@ -164,6 +225,25 @@ class PakfireJS {
}
}
+ // 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 = window.setTimeout(function() {
+ this._pageReload.delayTimer = 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 = null;
+ }
+ }
+
//--- JSON request & data handling ---
// Load JSON data from Pakfire CGI, using a POST request
@@ -192,10 +272,11 @@ class PakfireJS {
// Update status flags
this.running = (data['running'] != '0');
this.reboot = (data['reboot'] != '0');
+ this.failure = (data['failure'] != '0');
// 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 += `${line}\n`;
});
$('#pflog-messages').text(messages);
+
+ // Pakfire finished without errors, return to main screen
+ if((! this.running) && (! this.failure)) {
+ this._pageReload_trigger();
+ }
}
}
}
@@ -1974,7 +1974,8 @@
'pakfire configuration' => 'Pakfire Konfiguration',
'pakfire core update auto' => 'Core- und Addon-Updates automatisch installieren:',
'pakfire core update level' => 'Core-Update-Level',
-'pakfire finished' => 'Pakfire ist fertig!. Bitte überprüfen Sie die Log Ausgabe.',
+'pakfire finished' => 'Pakfire ist fertig! Kehre zurück...',
+'pakfire finished error' => 'Pakfire ist fertig! Fehler sind aufgetreten, bitte überprüfen Sie die Log-Ausgabe, bevor Sie fortfahren.',
'pakfire health check' => 'Mirrors auf Erreichbarkeit prüfen (Ping):',
'pakfire install description' => 'Wählen Sie ein oder mehrere Pakete zur Installation aus und drücken Sie auf das plus-Symbol.',
'pakfire install package' => 'Sie möchten folgende Pakete installieren: ',
@@ -2009,7 +2009,8 @@
'pakfire configuration' => 'Pakfire Configuration',
'pakfire core update auto' => 'Install core and addon updates automatically:',
'pakfire core update level' => 'Core-Update-Level',
-'pakfire finished' => 'Pakfire is finished! Please check the log output.',
+'pakfire finished' => 'Pakfire has finished! Returning...',
+'pakfire finished error' => 'Pakfire has finished! Errors occurred, please check the log output before proceeding.',
'pakfire health check' => 'Check if mirror is reachable (ping):',
'pakfire install description' => 'Please choose one or more items from the list below and click the plus to install.',
'pakfire install package' => 'You want to install the following packages: ',