Re: [PATCH] pakfire: Better errorhandling on downloads

[Adolf Belka <adolf.belka@ipfire.org>]

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

Hi Robin,

On 07/03/2022 14:33, Robin Roevens wrote:
> Hi Adolf
> 
> Thanks for testing.
> 
> Adolf Belka schreef op vr 04-03-2022 om 16:08 [+0100]:
>> Hi All,
>>
>> I have run a test with this patch. I set up a CU 165 vm and ran
>> pakfire
>> with and without internet access to try and download 4 different
>> addons.
>> I then patched the pakfire functions.pl file and redid the test.
>>
>> In both cases with internet access the four addons were successfully
>> downloaded.
>>
>> In both cases with no internet access I got the same error message
>> that
>> I needed Internet access to install addons.
>>
>>   From the point of view of a pakfire user there is no difference
>> between
>> with and without the patch.
> 
> The main pain points are when no meta-data could be downloaded (and is
> not already in cache) or when any of the lists is missing or could only
> be partly downloaded.
> 
> For my tests, I disabled internet by stopping unbound:
> [root(a)ipfire-test lists]# /etc/init.d/unbound stop
> Stopping Unbound DNS Proxy...                                  [  OK  ]
> 
> Before/after differences:
> - Installing a package with missing server-list.db:
> [root(a)ipfire-test lists]# mv server-list.db server-list.db.old
> 
> BEFORE:
> ---
> [root(a)ipfire-test lists]# pakfire install wio
> 
> Giving up: There was no chance to get the file "2.27.2-
> x86_64/lists/server-list.db" from any available server.
> There was an error on the way. Please fix it.
> 
> Giving up: There was no chance to get the file "2.27.2-
> x86_64/lists/server-list.db" from any available server.
> There was an error on the way. Please fix it.
> 
> Giving up: There was no chance to get the file "2.27.2-
> x86_64/lists/server-list.db" from any available server.
> There was an error on the way. Please fix it.
> 
> Giving up: There was no chance to get the file "2.27.2-
> x86_64/lists/server-list.db" from any available server.
> There was an error on the way. Please fix it.
> 
> Giving up: There was no chance to get the file "2.27.2-
> x86_64/lists/server-list.db" from any available server.
> There was an error on the way. Please fix it.
> MIRROR ERROR: Could not find or download a server list
> ---
> 
> AFTER: (less clutter)
> ---
> [root(a)ipfire-test lists]# pakfire install wio
> PAKFIRE RESV: wio: Resolving dependencies...
> 
> Giving up: There was no chance to get the file "2.27.2-
> x86_64/lists/server-list.db" from any available server.
> There was an error on the way. Please fix it.
> MIRROR ERROR: Could not find or download a server list
> ---
> 
> - Installing a package with meta-data already in cache:
> BEFORE:
> ---
> [root(a)ipfire-test lists]# pakfire install fping
> PAKFIRE RESV: fping: Resolving dependencies...
> 
> 
> PAKFIRE INFO: Packages to install:
> PAKFIRE INFO: fping 	 - 30.00 KB
> 
> PAKFIRE INFO: Total size: 	 ~ 30.00 KB
> 
> PAKFIRE INFO: Is this okay? [y/N]
> y
> 
> Giving up: There was no chance to get the file "paks/fping-5.0-
> 6.ipfire" from any available server.
> There was an error on the way. Please fix it.
> 
> PAKFIRE INST: fping: Decrypting...
> 
> Giving up: There was no chance to get the file "paks/fping-5.0-
> 6.ipfire" from any available server.
> There was an error on the way. Please fix it.
> sh: line 1: /opt/pakfire/cache/fping-5.0-6.ipfire: No such file or
> directory
> tar: This does not look like a tar archive
> tar: Exiting with failure status due to previous errors
> ---
> 
> AFTER: (clean error message, no attempted untar of non-existent file)
> ---
> [root(a)ipfire-test lists]# pakfire install fping
> PAKFIRE RESV: fping: Resolving dependencies...
> 
> 
> PAKFIRE INFO: Packages to install:
> PAKFIRE INFO: fping 	 - 30.00 KB
> 
> PAKFIRE INFO: Total size: 	 ~ 30.00 KB
> 
> PAKFIRE INFO: Is this okay? [y/N]
> y
> 
> Giving up: There was no chance to get the file "paks/fping-5.0-
> 6.ipfire" from any available server.
> There was an error on the way. Please fix it.
> PAKFIRE ERROR: Unable to download fping.
> ---
> 
> - Installing a package without meta-data already in cache:
> 
> BEFORE:
> ---
> [root(a)ipfire-test lists]# pakfire install fping
> 
> Giving up: There was no chance to get the file "meta/meta-fping" from
> any available server.
> There was an error on the way. Please fix it.
> PAKFIRE RESV: fping: Resolving dependencies...
> 
> 
> PAKFIRE INFO: Packages to install:
> 
> Giving up: There was no chance to get the file "meta/meta-fping" from
> any available server.
> There was an error on the way. Please fix it.
> PAKFIRE INFO: fping 	 - 0.00 B
> 
> Giving up: There was no chance to get the file "meta/meta-fping" from
> any available server.
> There was an error on the way. Please fix it.
> 
> PAKFIRE INFO: Total size: 	 ~ 0.00 B
> 
> PAKFIRE INFO: Is this okay? [y/N]
> y
> 
> Giving up: There was no chance to get the file "meta/meta-fping" from
> any available server.
> There was an error on the way. Please fix it.
> No filename given in meta-file.
> ---
> 
> --> Resulted in empty meta-file for package:
> [root(a)ipfire-test lists]# ls -lia /opt/pakfire/db/meta/meta-fping
> 24871 -rw-r--r-- 1 root root 0 Mar  6 21:52 /opt/pakfire/db/meta/meta-
> fping
> 
> AFTER: (clean error without asking user to confirm installation which
> is already doomed to fail, or maybe possibly succeed if package is
> already in cache but without installing dependencies since meta-data is
> missing)
> ---
> [root(a)ipfire-test lists]# pakfire install fping
> PAKFIRE RESV: fping: Resolving dependencies...
> 
> Giving up: There was no chance to get the file "meta/meta-fping" from
> any available server.
> There was an error on the way. Please fix it.
> PAKFIRE ERROR: Error retrieving dependency information on fping. Unable
> to resolve dependencies.
> ---
> 
> --> no empty meta-file is generated:
> [root(a)ipfire-test lists]# ls -lia /opt/pakfire/db/meta/meta-fping
> ls: cannot access '/opt/pakfire/db/meta/meta-fping': No such file or
> directory
> 
> - Install core upgrade without meta-data already cached:
> 
> BEFORE:
> ---
> [root(a)ipfire-test lists]# pakfire upgrade
> CORE UPGR: Upgrading from release 164 to 165
> 
> Giving up: There was no chance to get the file "meta/meta-core-upgrade-
> 165" from any available server.
> There was an error on the way. Please fix it.
> No filename given in meta-file.
> ---
> 
> --> Resulted in empty meta-file for core:
> [root(a)ipfire-test lists]# ls -lia /opt/pakfire/db/meta/meta-core-
> upgrade-165
> 24855 -rw-r--r-- 1 root root 0 Mar  6 21:57 /opt/pakfire/db/meta/meta-
> core-upgrade-165
> 
> 
> AFTER: (clean error instead of 'no filename given..')
> ---
> [root(a)ipfire-test lists]# pakfire upgrade
> CORE UPGR: Upgrading from release 164 to 165
> 
> Giving up: There was no chance to get the file "meta/meta-core-upgrade-
> 165" from any available server.
> There was an error on the way. Please fix it.
> PAKFIRE ERROR: Unable to retrieve core-upgrade-165 metadata.
> ---
> 
> --> no empty meta-file for core
> [root(a)ipfire-test lists]# ls -lia /opt/pakfire/db/meta/meta-core-
> upgrade-165
> ls: cannot access '/opt/pakfire/db/meta/meta-core-upgrade-165': No such
> file or directory
> 
> When unbound is started and internet is available, there is no
> difference to the user before or after the patch.
> 
> Not sure if any of the above errors messages would reach the user when
> pakfire wui is used, but at least there is less chance that meta-files
> or list.db's are getting corrupted or empty.
I suspect that the error messages on the WUI are simplified.

I will retry the patch and look in the log file directly just to confirm that I get the same as you did.

Regards,
Adolf.
> 
>>
>> One thing I did notice with the current CU 165 is that after
>> installing
>> the selected addons the wui goes back to the original pakfire screen
>> and
>> does not show the addons having been installed in the right hand box.
>> If
>> I select the pakfire menu item again then the additional addons are
>> shown. It looks like the wui page is not being updated after the
>> addons
>> have been installed. Should I raise this as a bug or is this an
>> interim
>> status as CU 165 is still a work in progress.
> 
> I see this behavior too in cu 165, both with original functions.pl and
> my patched version. So I don't think it is related with my changes.
> 
> Regards
> 
> Robin
> 
>>
>> Regards,
>> Adolf.
>>>>> Best,
>>>>> -Michael
>>>>>
>>>>>> ---
>>>>>> src/pakfire/lib/functions.pl | 84 +++++++++++++++++++++------
>>>>>> ------
>>>>>> ---
>>>>>> 1 file changed, 49 insertions(+), 35 deletions(-)
>>>>>>
>>>>>> diff --git a/src/pakfire/lib/functions.pl
>>>>>> b/src/pakfire/lib/functions.pl
>>>>>> index d4e338f23..24c55fd4a 100644
>>>>>> --- a/src/pakfire/lib/functions.pl
>>>>>> +++ b/src/pakfire/lib/functions.pl
>>>>>> @@ -2,7 +2,7 @@
>>>>>> #############################################################
>>>>>> ######
>>>>>> ############
>>>>>> #
>>>>>>              #
>>>>>> # IPFire.org - A linux based
>>>>>> firewall                                         #
>>>>>> -# Copyright (C) 2007-2021   IPFire Team
>>>>>> <info(a)ipfire.org>                   #
>>>>>> +# Copyright (C) 2007-2022   IPFire Team
>>>>>> <info(a)ipfire.org>                   #
>>>>>> #
>>>>>>              #
>>>>>> # This program is free software: you can redistribute it
>>>>>> and/or
>>>>>> modify        #
>>>>>> # it under the terms of the GNU General Public License as
>>>>>> published
>>>>>> by        #
>>>>>> @@ -206,7 +206,7 @@ sub fetchfile {
>>>>>>
>>>>>>                   if ( $code eq "500" ) {
>>>>>>                           message("Giving up: There was no
>>>>>> chance to
>>>>>> get the file \"$getfile\" from any available server.\nThere
>>>>>> was an
>>>>>> error on the way. Please fix it.");
>>>>>> -                       return 1;
>>>>>> +                       return 0;
>>>>>>                   }
>>>>>>
>>>>>>                   if ($response->is_success) {
>>>>>> @@ -226,7 +226,7 @@ sub fetchfile {
>>>>>>                                   }
>>>>>>                                   logger("DOWNLOAD FINISHED:
>>>>>> $file");
>>>>>>                                   $allok = 1;
>>>>>> -                               return 0;
>>>>>> +                               return 1;
>>>>>>                           } else {
>>>>>>                                   logger("DOWNLOAD ERROR:
>>>>>> Could not
>>>>>> open $Conf::tmpdir/$bfile for writing.");
>>>>>>                           }
>>>>>> @@ -235,7 +235,7 @@ sub fetchfile {
>>>>>>                   }
>>>>>>           }
>>>>>>           message("DOWNLOAD ERROR: There was no chance to get
>>>>>> the
>>>>>> file \"$getfile\" from any available server.\nMay be you
>>>>>> should run
>>>>>> \"pakfire update\" to get some new servers.");
>>>>>> -       return 1;
>>>>>> +       return 0;
>>>>>> }
>>>>>>
>>>>>> sub getmirrors {
>>>>>> @@ -256,9 +256,14 @@ sub getmirrors {
>>>>>>           }
>>>>>>
>>>>>>           if ("$force" eq "force") {
>>>>>> -               fetchfile("$Conf::version/lists/server-
>>>>>> list.db",
>>>>>> "$Conf::mainserver");
>>>>>> -               move("$Conf::cachedir/server-list.db",
>>>>>> "$Conf::dbdir/lists/server-list.db");
>>>>>> +               if (fetchfile("$Conf::version/lists/server-
>>>>>> list.db", "$Conf::mainserver")) {
>>>>>> +                       move("$Conf::cachedir/server-
>>>>>> list.db",
>>>>>> "$Conf::dbdir/lists/server-list.db");
>>>>>> +               } elsif (! -e "$Conf::dbdir/lists/server-
>>>>>> list.db" )
>>>>>> {
>>>>>> +                       # if we end up with no server-list at
>>>>>> all,
>>>>>> return failure
>>>>>> +                       return 0;
>>>>>> +               }
>>>>>>           }
>>>>>> +       return 1;
>>>>>> }
>>>>>>
>>>>>> sub getcoredb {
>>>>>> @@ -279,8 +284,9 @@ sub getcoredb {
>>>>>>           }
>>>>>>
>>>>>>           if ("$force" eq "force") {
>>>>>> -               fetchfile("lists/core-list.db", "");
>>>>>> -               move("$Conf::cachedir/core-list.db",
>>>>>> "$Conf::dbdir/lists/core-list.db");
>>>>>> +               if (fetchfile("lists/core-list.db", "")) {
>>>>>> +                       move("$Conf::cachedir/core-list.db",
>>>>>> "$Conf::dbdir/lists/core-list.db");
>>>>>> +               }
>>>>>>           }
>>>>>> }
>>>>>>
>>>>>> @@ -318,15 +324,13 @@ sub selectmirror {
>>>>>>
>>>>>>           ### Check if there is a current server list and read
>>>>>> it.
>>>>>>           #   If there is no list try to get one.
>>>>>> -       my $count = 0;
>>>>>> -       while (!(open(FILE, "<$Conf::dbdir/lists/server-
>>>>>> list.db"))
>>>>>> && ($count lt 5)) {
>>>>>> -               $count++;
>>>>>> -               getmirrors("noforce");
>>>>>> -       }
>>>>>> -       if ($count == 5) {
>>>>>> -               message("MIRROR ERROR: Could not find or
>>>>>> download a
>>>>>> server list");
>>>>>> -               exit 1;
>>>>>> +       unless (open(FILE, "<$Conf::dbdir/lists/server-
>>>>>> list.db")) {
>>>>>> +               unless (getmirrors("noforce")) {
>>>>>> +                       message("MIRROR ERROR: Could not find
>>>>>> or
>>>>>> download a server list");
>>>>>> +                       exit 1;
>>>>>> +               }
>>>>>>           }
>>>>>> +
>>>>>>           my @lines = <FILE>;
>>>>>>           close(FILE);
>>>>>>
>>>>>> @@ -390,8 +394,13 @@ sub dbgetlist {
>>>>>>           }
>>>>>>
>>>>>>           if ("$force" eq "force") {
>>>>>> -               fetchfile("lists/packages_list.db", "");
>>>>>> -               move("$Conf::cachedir/packages_list.db",
>>>>>> "$Conf::dbdir/lists/packages_list.db");
>>>>>> +               if (fetchfile("lists/packages_list.db", ""))
>>>>>> {
>>>>>> +
>>>>>> move("$Conf::cachedir/packages_list.db",
>>>>>> "$Conf::dbdir/lists/packages_list.db");
>>>>>> +               } elsif ( -e
>>>>>> "$Conf::dbdir/lists/packages_list.db"
>>>>>> ) {
>>>>>> +                       # If we end up with no db file after
>>>>>> download error there
>>>>>> +                       # is nothing more we can do here.
>>>>>> +                       return 0;
>>>>>> +               }
>>>>>>           }
>>>>>>
>>>>>>           # Update the meta database if new packages was in
>>>>>> the
>>>>>> package list
>>>>>> @@ -419,8 +428,7 @@ sub dbgetlist {
>>>>>>                           @templine = split(/\;/,$prog);
>>>>>>                           if (("$metadata{'Name'}" eq
>>>>>> "$templine[0]")
>>>>>> && ("$metadata{'Release'}" ne "$templine[2]")) {
>>>>>>                                  
>>>>>> move("$Conf::dbdir/meta/meta-
>>>>>> $metadata{'Name'}","$Conf::dbdir/meta/old_meta-
>>>>>> $metadata{'Name'}");
>>>>>> -                               fetchfile("meta/meta-
>>>>>> $metadata{'Name'}", "");
>>>>>> -                               move("$Conf::cachedir/meta-
>>>>>> $metadata{'Name'}", "$Conf::dbdir/meta/meta-
>>>>>> $metadata{'Name'}");
>>>>>> +
>>>>>> getmetafile($metadata{'Name'});
>>>>>>                           }
>>>>>>                   }
>>>>>>           }
>>>>>> @@ -532,11 +540,14 @@ sub dblist {
>>>>>>
>>>>>> sub resolvedeps_one {
>>>>>>           my $pak = shift;
>>>>>> -
>>>>>> -       getmetafile("$pak");
>>>>>> -
>>>>>> +
>>>>>>           message("PAKFIRE RESV: $pak: Resolving
>>>>>> dependencies...");
>>>>>>
>>>>>> +       unless (getmetafile("$pak")) {
>>>>>> +               message("PAKFIRE ERROR: Error retrieving
>>>>>> dependency
>>>>>> information on $pak. Unable to resolve dependencies.");
>>>>>> +               exit 1;
>>>>>> +       };
>>>>>> +
>>>>>>           my %metadata =
>>>>>> parsemetafile("$Conf::dbdir/meta/meta-
>>>>>> $pak");
>>>>>>           my @all;
>>>>>>           my @deps = split(/ /, $metadata{'Dependencies'});
>>>>>> @@ -629,14 +640,10 @@ sub cleanup {
>>>>>>
>>>>>> sub getmetafile {
>>>>>>           my $pak = shift;
>>>>>> -
>>>>>> -       unless ( -e "$Conf::dbdir/meta/meta-$pak" ) {
>>>>>> -               fetchfile("meta/meta-$pak", "");
>>>>>> -               move("$Conf::cachedir/meta-$pak",
>>>>>> "$Conf::dbdir/meta/meta-$pak");
>>>>>> -       }
>>>>>> -
>>>>>> -       if ( -z "$Conf::dbdir/meta/meta-$pak" ) {
>>>>>> -               fetchfile("meta/meta-$pak", "");
>>>>>> +
>>>>>> +       # Try to download meta-file if we don't have one yet,
>>>>>> or it
>>>>>> is empty for some reason
>>>>>> +       if ((! -e "$Conf::dbdir/meta/meta-$pak" ) || ( -z
>>>>>> "$Conf::dbdir/meta/meta-$pak" )) {
>>>>>> +               return 0 unless (fetchfile("meta/meta-$pak",
>>>>>> ""));
>>>>>>                   move("$Conf::cachedir/meta-$pak",
>>>>>> "$Conf::dbdir/meta/meta-$pak");
>>>>>>           }
>>>>>>
>>>>>> @@ -651,6 +658,7 @@ sub getmetafile {
>>>>>>                   print FILE $string;
>>>>>>           }
>>>>>>           close(FILE);
>>>>>> +
>>>>>>           return 1;
>>>>>> }
>>>>>>
>>>>>> @@ -713,8 +721,11 @@ sub getpak {
>>>>>>           my $pak = shift;
>>>>>>           my $force = shift;
>>>>>>
>>>>>> -       getmetafile("$pak");
>>>>>> -
>>>>>> +       unless (getmetafile("$pak")) {
>>>>>> +               message("PAKFIRE ERROR: Unable to retrieve
>>>>>> $pak
>>>>>> metadata.");
>>>>>> +               exit 1;
>>>>>> +       }
>>>>>> +
>>>>>>           my %metadata =
>>>>>> parsemetafile("$Conf::dbdir/meta/meta-
>>>>>> $pak");
>>>>>>           my $file = $metadata{'File'};
>>>>>>
>>>>>> @@ -728,8 +739,11 @@ sub getpak {
>>>>>>                           return $file;
>>>>>>                   }
>>>>>>           }
>>>>>> -
>>>>>> -       fetchfile("paks/$file", "");
>>>>>> +
>>>>>> +       unless (fetchfile("paks/$file", "")) {
>>>>>> +               message("PAKFIRE ERROR: Unable to download
>>>>>> $pak.");
>>>>>> +               exit 1;
>>>>>> +       }
>>>>>>           return $file;
>>>>>> }
>>>>>>
>>>>>> -- 
>>>>>> 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.
>>
>> -- 
>> Sent from my laptop
>>
>>
>