[7/9] pakfire: Refactor status function separating UI and logic
Commit Message
- 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(-)
Comments
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/colors.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?
> <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.
>
> - 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.
> } else {
> &Pakfire::usage;
> }
> --
> 2.34.1
>
>
> --
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
>
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.
> >
>
>
Hello,
> On 23 Mar 2022, at 19:56, Robin Roevens <robin.roevens@disroot.org> wrote:
>
> 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.
Let’s talk about that plan before you start coding.
>>
>>> <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.
>>>
>>
>>
>
> --
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
@@ -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/colors.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"
);
@@ -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>
<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";
}
- 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' />
@@ -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() {
@@ -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";
} else {
&Pakfire::usage;
}