Re: [PATCH 5/9] pakfire: Optimize upgradecore function

[Robin Roevens <robin.roevens@disroot.org>]

[-- Attachment #1: Type: text/plain, Size: 6929 bytes --]


Michael Tremer schreef op ma 21-03-2022 om 16:24 [+0000]:
> Hello,
> 
> > On 9 Mar 2022, at 22:56, Robin Roevens <robin.roevens(a)disroot.org>
> > wrote:
> > 
> > upgradecore function should just upgrade the core:
> > Moved check if upgrade is necessary to pakfire upgrade code,
> > removing
> > code from upgradecore function duplicating codedbinfo workings.
> > Also adding more vebosity to pakfire upgrade.
> > 
> > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
> > ---
> > src/pakfire/lib/functions.pl | 47 +++++++++++++++------------------
> > ---
> > src/pakfire/pakfire          | 16 +++++++++++-
> > 2 files changed, 35 insertions(+), 28 deletions(-)
> > 
> > diff --git a/src/pakfire/lib/functions.pl
> > b/src/pakfire/lib/functions.pl
> > index 1e2729485..6287367f5 100644
> > --- a/src/pakfire/lib/functions.pl
> > +++ b/src/pakfire/lib/functions.pl
> > @@ -735,35 +735,28 @@ sub setuppak {
> > }
> > 
> > sub upgradecore {
> > -       getcoredb("noforce");
> > -       eval(`grep "core_" $Conf::dbdir/lists/core-list.db`);
> > -       if ("$core_release" > "$Conf::core_mine") {
> > -               # Safety check for lazy testers:
> > -               # Before we upgrade to the latest release, we re-
> > install the previous release
> > -               # to make sure that the tester has always been on
> > the latest version.
> > -               my $tree = &get_tree();
> > -               $Conf::core_mine-- if ($tree eq "testing" || $tree
> > eq "unstable");
> > -
> > -               message("CORE UPGR: Upgrading from release
> > $Conf::core_mine to $core_release");
> > -
> > -               my @seq = `seq $Conf::core_mine $core_release`;
> > -               shift @seq;
> > -               my $release;
> > -               foreach $release (@seq) {
> > -                       chomp($release);
> > -                       getpak("core-upgrade-$release");
> > -               }
> > -
> > -               foreach $release (@seq) {
> > -                       chomp($release);
> > -                       upgradepak("core-upgrade-$release");
> > -               }
> > -
> > -               system("echo $core_release > $Conf::coredir/mine");
> > +       # Safety check for lazy testers:
> > +       # Before we upgrade to the latest release, we re-install
> > the previous release
> > +       # to make sure that the tester has always been on the
> > latest version.
> > +       my $tree = &get_tree();
> > +       $Conf::core_mine-- if ($tree eq "testing" || $tree eq
> > "unstable");
> > 
> > -       } else {
> > -               message("CORE ERROR: No new upgrades available. You
> > are on release $Conf::core_mine.");
> > +       message("CORE UPGR: Upgrading from release $Conf::core_mine
> > to $core_release");
> > +       
> > +       my @seq = `seq $Conf::core_mine $core_release`;
> 
> Can we remove this shell command call? This should be easily replaced
> with native Perl.
Strange.. I can remember I changed that to
($Conf::core_mine..$core_release) .. must have gone lost somewhere in
my drafts.
I will change it.

> 
> > +       shift @seq;
> > +       my $release;
> > +       foreach $release (@seq) {
> > +               chomp($release);
> > +               getpak("core-upgrade-$release");
> >         }
> > +       
> > +       foreach $release (@seq) {
> > +               chomp($release);
> > +               upgradepak("core-upgrade-$release");
> > +       }
> > +       
> > +       system("echo $core_release > $Conf::coredir/mine");
> > }
> > 
> > sub isinstalled {
> > diff --git a/src/pakfire/pakfire b/src/pakfire/pakfire
> > index f23110cf5..2fb9adce7 100644
> > --- a/src/pakfire/pakfire
> > +++ b/src/pakfire/pakfire
> > @@ -266,7 +266,21 @@
> >                         $use_color =
> > "$Pakfire::color{'lightpurple'}";
> >                 }
> > 
> > -               &Pakfire::upgradecore();
> > +               &Pakfire::message("CORE INFO: Checking for Core
> > updates...");
> > +
> > +               ### Make sure that the core db is not outdated. 
> > +               &Pakfire::getcoredb("noforce");
> > +               my %coredb = &Pakfire::coredbinfo();
> > +
> > +               if (defined $coredb{'AvailableRelease'}) {
> > +                       &Pakfire::upgradecore();
> > +               } else {
> > +                       &Pakfire::message("CORE WARN: No new Core
> > upgrades available. You are on release ".$coredb{'Release'});
> 
> Here, the Core Update is being called Core Upgrade. That might be
> coming from somewhere else. In the end we should probably reword lots
> of the error messages to make them consistent and easy to understand.
Yes, also "pakfire update" vs "pakfire upgrade", possibly the reason
why it is called "core upgrade" here.
Should I change everything to 'update' ?
But then I think it would also be more clear if 'pakfire update' would
update the system instead of only the pakfire db?
So maybe we can 'rename' 'pakfire update' to 'pakfire refresh' and then
rename 'pakfire upgrade' to 'pakfire update'... (possibly keeping
'pakfire upgrade' as an alias for 'pakfire update' for some backward
compatibility)
I'm a bit reticent about this as it changes the existing meaning of
'pakfire update' but I think that it would be more consistent in the
end ?

> 
> > +               }
> > +
> > +               &Pakfire::message("PAKFIRE INFO: Checking for
> > package updates...");
> > +               ### Make sure that the package list is not
> > outdated. 
> > +               &Pakfire::dbgetlist("noforce");
> >                 
> >                 my @deps = ();
> >                 if (my %upgradepaks = &Pakfire::dblist("upgrade"))
> > {
> > -- 
> > 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.