Re: [PATCH] pakfire: Better errorhandling on downloads

["Peter Müller" <peter.mueller@ipfire.org>]

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

Hello Robin,

many thanks for your mail.

Yes, this was indeed an accident, as I went through Patchwork the other day to
gather all remaining patches for Core Update 169 - we are a bit behind schedule,
unfortunately. On this occasion, I marked a bunch of orphaned patches as "dropped"
or "superseded", yours included.

That was wrong, as this patch of yours has nothing to do with
https://patchwork.ipfire.org/project/ipfire/list/?series=2650, but did not occur
to me in this moment.

I will revert this, and you should receive an e-mail from Patchwork soon. Sorry
for the delay and negligence, I will keep this in mind for Core Update 170.

Again, thanks for letting us know.

All the best,
Peter Müller


> Hi all
> 
> Today I received a patchwork notice that this patch was Superseded 
> http://patchwork.ipfire.org/project/ipfire/patch/20220223202130.6104-2-robin.roevens(a)disroot.org/
> 
> So I was wondering which patch is superseding this one as I didn't
> supersede it myself, nor did I see a patch on the mailinglist tackling
> the same problem and it still looks valid against current next.
> I know there where improvements to pakfire.cgi, but this patch improves
> the errorhandling in the pakfire CLI command itself. So those
> pakfire.cgi patches should not be considered to supersede this one. 
> 
> Is this possibly marked as superseded by accident?
> 
> Regards
> Robin
> 
> Robin Roevens schreef op wo 23-02-2022 om 21:21 [+0100]:
>> - Add true/false return codes to fetchfile, getmetafile and
>> getmirrors
>>   indicating succes or failure.
>> - Check on those return codes and fail gracefully with clean
>>   error message(s) when downloads fail.
>> - Replace duplicate meta-file fetching code in dbgetlist with
>>   getmetafile function (fixing possibly missed cariage return
>>   conversion in meta-files).
>> - Remove pointless 5 retries to download server-list.db in
>>   selectmirror as fetchfile already retries 5 times.
>> ---
>>  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
>>
>>
>