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

[Michael Tremer <michael.tremer@ipfire.org>]

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

Hey,

> On 22 Mar 2022, at 12:58, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> 
> 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.

Okay. Thank you.

> 
>> 
>>> +       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' ?

No, this is a little bit similar to apt-get which has an update and an upgrade command. That was the intention at least.

I would just change the name “Core Upgrade” to “Core Update” because that is what we call it. It is only an aesthetic change.

> 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)

This would break people’s scripts, so I wouldn’t say this is worth it.

> 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.