pakfire: Better errorhandling on downloads

Message ID 20220223202130.6104-2-robin.roevens@disroot.org
State Accepted
Commit 8ce729457d736aee35fc5f731cc6aa17ff549c73
Headers
Series pakfire: Better errorhandling on downloads |

Commit Message

Robin Roevens Feb. 23, 2022, 8:21 p.m. UTC
  - 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(-)
  

Comments

Michael Tremer March 1, 2022, 1:31 p.m. UTC | #1
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.

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.
>
  
Robin Roevens March 3, 2022, 9:20 p.m. UTC | #2
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 ?

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.
> > 
> 
>
  
Michael Tremer March 4, 2022, 11:18 a.m. UTC | #3
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.
  
Adolf Belka March 4, 2022, 3:08 p.m. UTC | #4
Hi All,

On 04/03/2022 12:18, Michael Tremer wrote:
> 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.
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.

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.

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@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.
  
Robin Roevens March 7, 2022, 1:33 p.m. UTC | #5
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@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@ipfire-test lists]# mv server-list.db server-list.db.old

BEFORE:
---
[root@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@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@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@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@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@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@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@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@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@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@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@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.

> 
> 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@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.
> 
> -- 
> Sent from my laptop
> 
>
  
Adolf Belka March 7, 2022, 2:11 p.m. UTC | #6
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@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@ipfire-test lists]# mv server-list.db server-list.db.old
> 
> BEFORE:
> ---
> [root@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@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@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@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@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@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@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@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@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@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@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@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@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.
>>
>> -- 
>> Sent from my laptop
>>
>>
>
  
Robin Roevens June 23, 2022, 9:20 p.m. UTC | #7
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@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@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
> 
>
  
Peter Müller June 25, 2022, 10:11 a.m. UTC | #8
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@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@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
>>
>>
>
  

Patch

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;
 }