Re: [PATCH] pakfire: Better errorhandling on downloads

Hello,

On 3 Mar 2022, at 21:20, Robin Roevens robin.roevens@disroot.org wrote:

Hi

Michael Tremer schreef op di 01-03-2022 om 13:31 [+0000]:

...

Hello,

...

On 23 Feb 2022, at 20:21, Robin Roevens robin.roevens@disroot.org wrote:

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

It would have been useful to have these four things in four different patches, because that makes reviewing things a lot easier.

Altogether, the patch isn’t that large, but it is easier to spot any problems if changes are bundled into smaller logical sets.

I understand, but I considered this (or at least the first two items) already as a single logical set. But you are right I could have split the patch in 4. Should I resubmit it as a v2 patchset of 4 ?

No, I don’t want to create any extra work for you now.

I just wanted to note this for future reference.

-Michael

Robin

...

Does anyone want to test this? I am just a little bit more concerned about breaking the package management system, because then we can’t fix things easily any more.

...

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@ipfire.org # +# Copyright (C) 2007-2022 IPFire Team info@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.