Hi
Michael Tremer schreef op ma 21-03-2022 om 16:28 [+0000]:
Hey,
On 9 Mar 2022, at 22:56, Robin Roevens robin.roevens@disroot.org wrote:
- removed UI related code from status function and refactor it
to returning hash representing pakfire status.
- Add UI part to pakfire script
- Use pakfire status function in pakfire.cgi, removing duplicate
code
Signed-off-by: Robin Roevens robin.roevens@disroot.org
html/cgi-bin/pakfire.cgi | 39 ++++++++++++++++-----------------
src/pakfire/lib/functions.pl | 40 ++++++++++++++++++---------------
src/pakfire/pakfire | 13 +++++++++++- 3 files changed, 50 insertions(+), 42 deletions(-)
diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi index 30a23128b..d62b6058f 100644 --- a/html/cgi-bin/pakfire.cgi +++ b/html/cgi-bin/pakfire.cgi @@ -37,6 +37,9 @@ my %color = (); my %pakfiresettings = (); my %mainsettings = ();
+# Get Pakfire status +my %pakfire_status = &Pakfire::status();
# Load general settings &General::readhash("${General::swroot}/main/settings", %mainsettings); &General::readhash("/srv/web/ipfire/html/themes/ipfire/include/colo rs.txt", %color); @@ -66,7 +69,7 @@ if($cgiparams{'ACTION'} eq 'json-getstatus') { my %status = ( 'running' => &_is_pakfire_busy() || "0", 'running_since' => &General::age("$Pakfire::lockfile") || "0s", - 'reboot' => (-e "/var/run/need_reboot") || "0", + 'reboot' => ("$pakfire_status{'RebootRequired'}" eq "yes") || "0", 'failure' => $failure || "0" );
Okay.
@@ -333,32 +336,26 @@ END exit; }
-my $core_release = `cat /opt/pakfire/db/core/mine 2>/dev/null`; -chomp($core_release); -my $core_update_age = &General::age("/opt/pakfire/db/core/mine"); -my $corelist_update_age = &General::age("/opt/pakfire/db/lists/core-list.db"); -my $server_update_age = &General::age("/opt/pakfire/db/lists/server-list.db"); -my $packages_update_age = &General::age("/opt/pakfire/db/lists/packages_list.db");
&Header::openbox("100%", "center", "Pakfire");
print <<END; <table id="pfmain"> END -if ( -e "/var/run/need_reboot") { +if ("$pakfire_status{'RebootRequired'}" eq "yes") { print "\t\t<tr><td colspan='2'><a href='/cgi- bin/shutdown.cgi'>$Lang::tr{'needreboot'}!</a></td></tr>\n"; }
print <<END; <tr><td class="heading">$Lang::tr{'pakfire system state'}:</td> <td class="heading">$Lang::tr{'available updates'}:</td></tr>
- <tr><td><strong>$Lang::tr{'pakfire core update level'}: $core_release</strong> + <tr><td><strong>$Lang::tr{'pakfire core update level'}: $pakfire_status{'Release'}</strong> <hr> <div class="pflist"> - $Lang::tr{'pakfire last update'} $core_update_age $Lang::tr{'pakfire ago'}<br> - $Lang::tr{'pakfire last serverlist update'} $server_update_age $Lang::tr{'pakfire ago'}<br> - $Lang::tr{'pakfire last core list update'} $corelist_update_age $Lang::tr{'pakfire ago'}<br> - $Lang::tr{'pakfire last package update'} $packages_update_age $Lang::tr{'pakfire ago'} + $Lang::tr{'pakfire last update'} $pakfire_status{'LastUpdate'} $Lang::tr{'pakfire ago'}<br> + $Lang::tr{'pakfire last serverlist update'} $pakfire_status{'LastServerListUpdate'} $Lang::tr{'pakfire ago'}<br> + $Lang::tr{'pakfire last core list update'} $pakfire_status{'LastCoreListUpdate'} $Lang::tr{'pakfire ago'}<br> + $Lang::tr{'pakfire last package update'} $pakfire_status{'LastPakListUpdate'} $Lang::tr{'pakfire ago'} </div>
Is it actually useful to show those timestamps? If anything, the last update of the lists (all together) would be interesting.
I do not think that it is important to know when the last update was installed. How do you feel about this?
I gave the current pakfire wui a good long stare.. and indeed I have to conclude that there is not much added value to show these. Last lists update is indeed the only timestamp that is interesting as there is also a button to refresh them now. But as I already plan to overhaul that page completely, I will integrate the removal of all those timestamps there.
<form method='post' action='$ENV{'SCRIPT_NAME'}'> <input type='hidden' name='ACTION' value='update' /> @@ -370,17 +367,17 @@ print <<END; <select name="UPDPAKS" class="pflist" size="5" disabled> END
- my %coredb = &Pakfire::coredbinfo(); - if (defined $coredb{'AvailableRelease'}) { - print "<option value="core">Core-Update -- $coredb{'CoreVersion'} -- Release: $coredb{'Release'} -> $coredb{'AvailableRelease'}</option>\n"; + if ("$pakfire_status{'CoreUpdateAvailable'}" eq "yes") { + print "<option value="core">Core-Update -- $pakfire_status{'CoreVersion'} -- Release: $pakfire_status{'Release'} -> $pakfire_status{'AvailableRelease'}</option>\n"; }
See my remarks on localisation in my other email.
Noted, will change it.
- my %upgradelist = &Pakfire::dblist("upgrade"); - foreach my $pak (sort keys %upgradelist) { - print "<option value="$pak">Update: $pak -- Version: $upgradelist{$pak}{'ProgVersion'} -> $upgradelist{$pak}{'AvailableProgVersion'} -- Release: $upgradelist{$pak}{'Release'} -> $upgradelist{$pak}{'AvailableRelease'}</option>\n"; + if ($pakfire_status{'PakUpdatesAvailable'} > 0) { + my %upgradelist = &Pakfire::dblist("upgrade"); + foreach my $pak (sort keys %upgradelist) { + print "<option value="$pak">Update: $pak -- Version: $upgradelist{$pak}{'ProgVersion'} -> $upgradelist{$pak}{'AvailableProgVersion'} -- Release: $upgradelist{$pak}{'Release'} -> $upgradelist{$pak}{'AvailableRelease'}</option>\n"; + } }
print <<END; </select> <input type='hidden' name='ACTION' value='upgrade' /> diff --git a/src/pakfire/lib/functions.pl b/src/pakfire/lib/functions.pl index b35aed6a3..028a0277e 100644 --- a/src/pakfire/lib/functions.pl +++ b/src/pakfire/lib/functions.pl @@ -897,26 +897,26 @@ sub reboot_required { }
sub status { - # General info - my $return = "Core-Version: $Conf::version\n"; - $return .= "Core-Update-Level: $Conf::core_mine\n"; - $return .= "Last update: " . &General::age("/opt/pakfire/db/core/mine") . " ago\n"; - $return .= "Last core-list update: " . &General::age("/opt/pakfire/db/lists/core-list.db") . " ago\n"; - $return .= "Last server-list update: " . &General::age("/opt/pakfire/db/lists/server-list.db") . " ago\n"; - $return .= "Last packages-list update: " . &General::age("/opt/pakfire/db/lists/packages_list.db") . " ago\n";
- # Get availability of core updates - $return .= "Core-Update available: " . &Pakfire::coreupdate_available() . "\n";
- # Get availability of package updates - $return .= "Package-Updates available: " . &Pakfire::updates_available() . "\n";
- # Test if reboot is required - $return .= "Reboot required: " . &Pakfire::reboot_required() . "\n";
- # Return status text - print "$return"; - exit 1; + ### This subroutine returns pakfire status information in a hash. + # Usage is without arguments
+ # Add core version info + my %status = &Pakfire::coredbinfo();
+ # Add last update info + $status{'LastUpdate'} = &General::age("/opt/pakfire/db/core/mine"); + $status{'LastCoreListUpdate'} = &General::age("/opt/pakfire/db/lists/core-list.db"); + $status{'LastServerListUpdate'} = &General::age("/opt/pakfire/db/lists/server-list.db"); + $status{'LastPakListUpdate'} = &General::age("/opt/pakfire/db/lists/packages_list.db");
+ # Add number of available package updates + $status{'CoreUpdateAvailable'} = (defined $status{'AvailableRelease'}) ? "yes" : "no"; + $status{'PakUpdatesAvailable'} = &Pakfire::updates_available();
+ # Add if reboot is required + $status{'RebootRequired'} = &Pakfire::reboot_required();
+ return %status; }
sub get_arch() { diff --git a/src/pakfire/pakfire b/src/pakfire/pakfire index b529db77a..0ed8aacd4 100644 --- a/src/pakfire/pakfire +++ b/src/pakfire/pakfire @@ -406,7 +406,18 @@ system("rm -f /etc/fcron.daily/pakfire- upgrade"); } } elsif ("$ARGV[0]" eq "status") { - &Pakfire::status; + my %status = &Pakfire::status;
+ print "Core-Version: $status{'CoreVersion'}\n"; + print "Core-Update-Level: $status{'Release'}\n"; + print "Last update: $status{'LastUpdate'} ago\n"; + print "Last core-list update: $status{'LastCoreListUpdate'} ago\n"; + print "Last server-list update: $status{'LastServerListUpdate'} ago\n"; + print "Last packages-list update: $status{'LastPakListUpdate'} ago\n"; + print "Core-Update available: $status{'CoreUpdateAvailable'}"; + print " ($status{'AvailableRelease'})" if ("$status{'CoreUpdateAvailable'}" eq "yes"); + print "\nPackage-Updates available: $status{'PakUpdatesAvailable'}\n"; + print "Reboot required: $status{'RebootRequired'}\n";
The return status was lost here. I do not know why we would always return 1, but I would say that it makes sense to indicate whether updates are available or all is good.
Indeed, I missed the return value.. Will add one.
Regards
Robin
} else { &Pakfire::usage; } -- 2.34.1
-- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn.