[4/9] pakfire: Replace coreupdate_available duplicate code

Message ID 20220309225655.4472-5-robin.roevens@disroot.org
State Superseded
Headers
Series pakfire: remove dup. code + seperate ui/logic |

Commit Message

Robin Roevens March 9, 2022, 10:56 p.m. UTC
  Replace coreupdate_available code duplicating coredbinfo
workings with call to actual coredbinfo function.

Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
---
 src/pakfire/lib/functions.pl | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
  

Comments

Michael Tremer March 21, 2022, 4:21 p.m. UTC | #1
This is a lot nicer without eval().

> On 9 Mar 2022, at 22:56, Robin Roevens <robin.roevens@disroot.org> wrote:
> 
> Replace coreupdate_available code duplicating coredbinfo
> workings with call to actual coredbinfo function.
> 
> Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
> ---
> src/pakfire/lib/functions.pl | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/pakfire/lib/functions.pl b/src/pakfire/lib/functions.pl
> index 0caa4787e..1e2729485 100644
> --- a/src/pakfire/lib/functions.pl
> +++ b/src/pakfire/lib/functions.pl
> @@ -884,9 +884,10 @@ sub updates_available {
> }
> 
> sub coreupdate_available {
> -	eval(`grep "core_" $Conf::dbdir/lists/core-list.db`);
> -	if ("$core_release" > "$Conf::core_mine") {
> -		return "yes ($core_release)";
> +	my %coredb = &Pakfire::coredbinfo();
> +
> +	if ("$coredb{'AvailableRelease'}" > "$coredb{'Release'}") {
> +		return "yes ($coredb{'AvailableRelease'})";
> 	}
> 	else {
> 		return "no";

Is returning a string what we want here?

> -- 
> 2.34.1
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
>
  
Robin Roevens March 22, 2022, 12:42 p.m. UTC | #2
Hi Michael

Michael Tremer schreef op ma 21-03-2022 om 16:21 [+0000]:
> This is a lot nicer without eval().
> 
> > On 9 Mar 2022, at 22:56, Robin Roevens <robin.roevens@disroot.org>
> > wrote:
> > 
> > Replace coreupdate_available code duplicating coredbinfo
> > workings with call to actual coredbinfo function.
> > 
> > Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
> > ---
> > src/pakfire/lib/functions.pl | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/pakfire/lib/functions.pl
> > b/src/pakfire/lib/functions.pl
> > index 0caa4787e..1e2729485 100644
> > --- a/src/pakfire/lib/functions.pl
> > +++ b/src/pakfire/lib/functions.pl
> > @@ -884,9 +884,10 @@ sub updates_available {
> > }
> > 
> > sub coreupdate_available {
> > -       eval(`grep "core_" $Conf::dbdir/lists/core-list.db`);
> > -       if ("$core_release" > "$Conf::core_mine") {
> > -               return "yes ($core_release)";
> > +       my %coredb = &Pakfire::coredbinfo();
> > +
> > +       if ("$coredb{'AvailableRelease'}" > "$coredb{'Release'}") {
> > +               return "yes ($coredb{'AvailableRelease'})";
> >         }
> >         else {
> >                 return "no";
> 
> Is returning a string what we want here?
Valid question.. I will look into it. In the light of moving UI out of
the library functions that would certainly be the right thing to do
(not returning strings here).

> 
> > -- 
> > 2.34.1
> > 
> > 
> > -- 
> > Dit bericht is gescanned op virussen en andere gevaarlijke
> > inhoud door MailScanner en lijkt schoon te zijn.
> > 
> 
>
  
Robin Roevens March 23, 2022, 9:50 p.m. UTC | #3
Hi

Robin Roevens schreef op di 22-03-2022 om 13:42 [+0100]:
> Hi Michael
> 
> Michael Tremer schreef op ma 21-03-2022 om 16:21 [+0000]:
> > This is a lot nicer without eval().
> > 
> > > On 9 Mar 2022, at 22:56, Robin Roevens
> > > <robin.roevens@disroot.org>
> > > wrote:
> > > 
> > > Replace coreupdate_available code duplicating coredbinfo
> > > workings with call to actual coredbinfo function.
> > > 
> > > Signed-off-by: Robin Roevens <robin.roevens@disroot.org>
> > > ---
> > > src/pakfire/lib/functions.pl | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/src/pakfire/lib/functions.pl
> > > b/src/pakfire/lib/functions.pl
> > > index 0caa4787e..1e2729485 100644
> > > --- a/src/pakfire/lib/functions.pl
> > > +++ b/src/pakfire/lib/functions.pl
> > > @@ -884,9 +884,10 @@ sub updates_available {
> > > }
> > > 
> > > sub coreupdate_available {
> > > -       eval(`grep "core_" $Conf::dbdir/lists/core-list.db`);
> > > -       if ("$core_release" > "$Conf::core_mine") {
> > > -               return "yes ($core_release)";
> > > +       my %coredb = &Pakfire::coredbinfo();
> > > +
> > > +       if ("$coredb{'AvailableRelease'}" > "$coredb{'Release'}")
> > > {
> > > +               return "yes ($coredb{'AvailableRelease'})";
> > >         }
> > >         else {
> > >                 return "no";
> > 
> > Is returning a string what we want here?
> Valid question.. I will look into it. In the light of moving UI out
> of
> the library functions that would certainly be the right thing to do
> (not returning strings here).

I looked into it and found out that the only place where this function
is actually used is in Pakfire::status which I rewrite a few patches
later and this function becomes obsolete.
So this patch is actually pointless :-).. I will remove it and remove
the function coreupdate_available.
This information is available from either function coredbinfo (wether
key 'AvailableRelease' exists) of from the new status function (returns
hash with a key 'CoreUpdateAvailable' = yes/no)

Robin
> 
> > 
> > > -- 
> > > 2.34.1
> > > 
> > > 
> > > -- 
> > > Dit bericht is gescanned op virussen en andere gevaarlijke
> > > inhoud door MailScanner en lijkt schoon te zijn.
> > > 
> > 
> > 
>
  

Patch

diff --git a/src/pakfire/lib/functions.pl b/src/pakfire/lib/functions.pl
index 0caa4787e..1e2729485 100644
--- a/src/pakfire/lib/functions.pl
+++ b/src/pakfire/lib/functions.pl
@@ -884,9 +884,10 @@  sub updates_available {
 }
 
 sub coreupdate_available {
-	eval(`grep "core_" $Conf::dbdir/lists/core-list.db`);
-	if ("$core_release" > "$Conf::core_mine") {
-		return "yes ($core_release)";
+	my %coredb = &Pakfire::coredbinfo();
+
+	if ("$coredb{'AvailableRelease'}" > "$coredb{'Release'}") {
+		return "yes ($coredb{'AvailableRelease'})";
 	}
 	else {
 		return "no";