Re: [PATCH 9/9] pakfire: Redesign update output and workflow

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

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

Hello,

> On 22 Mar 2022, at 18:32, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> 
> Michael Tremer schreef op ma 21-03-2022 om 16:36 [+0000]:
>> Hello,
>> 
>>> On 9 Mar 2022, at 22:56, Robin Roevens <robin.roevens(a)disroot.org>
>>> wrote:
>>> 
>>> - Revamped 'pakfire update' more in the style of 'pakfire
>>>  install' for a more consistent look and feel.
>>> - Add core update details to output
>>> - Add new dependencies to install to output
>>> - First collect all upgrade info, then perform core update
>>>  together with available package updates and after user
>>>  confirmation (without -y).
>> 
>> I am sorry to disappoint Tom, but this is quite likely a no-can-do.
>> 
>> The problem is that we are repeatedly tight on disk space which makes
>> downloading everything first a bit of a problem.
> 
> I'm not sure what you mean? Originally the upgrade function forced a
> core upgrade first, then retrieved info about pak updates and requested
> confirmation to install them.
> I changed this into first retrieving all pak and core update info (not
> the paks or core themselves), presenting the info to the user and
> asking confirmation. After that the core is downloaded and installed,
> then the paks are downloaded and installed.
> So in terms of disk space usage, nothing should have changed except
> that pak metadata is now downloaded before the core update is
> performed. But those few text files should not be the problem I assume?

Oh okay. I read this differently then. The text files do not matter when it comes to disk space, but:

We sometimes change the tree in a core update (2.27 -> 2.29 or something), which this will break. Pakfire will then not find the correct new packages. The meta data needs to be updated after the core update is being installed and then pakfire can decide what packages to install. Otherwise this would become a two-step update now.

> 
>> 
>> We do not have any mechanism that checks whether enough space for
>> either the downloads or for extracting the update is available, which
>> is why the old way was more of a step-by-step system.
>> 
>> I don’t like the old system at all, but I would propose to add a
>> function that checks if enough disk space is available before it asks
>> the user whether they want to continue.
>> 
>> That should consider:
>> 
>> * The download size of the updates
>> * The download size of all add-ons
>> * The extracted size (for simplicity probably assuming that the core
>> update is only adding new files)
>> * The extracted size of all add-ons
> 
> Great minds think alike, I suppose :-) I was already in the middle of
> trying to implement size checking. But I didn't want the current
> patchset submission to be postponed for that. (The last patch was
> already some kind of 'extra' and not really that related to the purpose
> of that patchset (the cleanup of dblist and using extra metadata))

Yes, we should approach things one by one.

I have a massive work load at the moment and so reviewing large changes becomes a little bit difficult for me.

> I already did want to check some things about the size calculation here
> on the mailing list, but I figured that this patchset should first need
> to be accepted, as the size calculation would depend on it.

We can think about it and decide to implement it later...

> But now that we are on this topic.. 
> I wanted to verify here if the size recorded in the meta-files, is the
> size of the tar-files? 

This is the download size.

> And since everything is tarred without compression (or so I think?)
> this should more or less be equal to the installed size ? Or am I
> mistaken?

No, we compress the data. There are two tarballs inside each other.

The outer tarball has the scripts and the payload and is not compressed. The payload is compressed using XZ.

> If we ever decide to actually compress those we will need to record
> uncompressed size in the metadata.

So, the download size is usually quite far away from the installed size.

ms(a)rice-oxley ~ % curl https://mirror1.ipfire.org/pakfire2/2.27/paks/samba-4.14.6-82.ipfire | gpg --decrypt | tar xO files.tar.xz | xz -d | wc -c
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 11.4M  100 11.4M    0     0  11.5M      0 --:--:-- --:--:-- --:--:-- 11.8M
gpg: Signature made Tue 21 Dec 11:21:18 2021 GMT
gpg:                using RSA key 3ECA8AA4478208B924BB96206FEF7A8ED713594B
gpg: Can't check signature: No public key
 67164160

Samba is about 64MiB extracted (as tar) and 11 MiB downloaded.

We would have to add this to the metadata so that you can use the installed size in your calculations.

> 
> Then for calculation, I was thinking about separating temporary space
> and installed space:
> * core update:
> -- Required tempspace: size of new core (tar-file)
> -- required installspace: size of new core

Agreed.

> * new dependencies:
> -- required tempspace: sum of size of all required deps (tar files)
> -- required installspace: sum of size of all required deps

Agreed.

> * pak updates:
> -- required tempspace: sum of size of all required paks (tar files)
> -- required installspace: for each pak: size of installed pak version -
> size of new pak version

Technically you are correct. I am just unsure how much margin of error we should add to this.

> in the end total required space is tempspace + installspace, if
> installspace is positive. It can in theory also be negative (no core
> update and size of updated paks is less than previous versions), then
> required space = tempspace..

True. Again, rather be safe than sorry.

> I'm not sure if downloaded pakfiles/core-updates are deleted after
> install..? Or after some time ? I did not immediately find it in the
> code. We could implement some cache pruning ('pakfire clear' or
> 'pakfire clear-cache' or so?)

Update.sh removes the downloaded core update before it starts extracting everything. So technically, it is on disk multiple times:

1) the downloaded file in /var/cache/pakfire
2) The extracted download file in /opt/pakfire/tmp
3) Then we extract the payload

Worst case is that we need the downloaded space twice + the installed size.

Pruning the cache is a bit difficult. Pakfire does not store the uninstall script which means that we need the package file when we uninstall a package.

That is why we usually leave everything in the cache. An extra command would be helpful and pakfire should ideally delete the downloaded core update as soon as it got extracted (before calling update.sh).

OMG this is complicated :)

> this size diff at least for pak updates should be quite accurate, as
> previous pak is uninstalled and new pak is installed during update.

Any opened files will still remain on disk. Sometimes we replace things while they are running which won’t free disk space immediately.

That is why I am suggestion some margin of error here of maybe 20%? I wouldn’t calculate this to the last byte.

> I have been trying to calculate the actual size of a core update.. but
> only just now I figured that a core update file is probably not a full
> ipfire fs, but only the diff in files against previous core update :-)

Yes it is.

> For the actual size of a core update, we would need to have a diff of
> the size of a clean ipfire on previous core and a clean ipfire on next
> core.. 

I would just assume that all files are new files and will need extra space.

We can’t easily compute the diff here.

> But I don't think that is data we have already available or could
> easily have available ? So assuming core update only adds files is
> probably indeed the safest straight-forward method. Other method could
> be to assume a percentage of new files-size in a core update..? 

We could, but what would that be? Sometimes we might extract 100M and only replace existing stuff. The we might add a new kernel which will 100M of modules on its own. I don’t think there is an easy formula. However, many systems have a / partition of only 2G. We need to be able to install updates to those systems cannot overshoot too much and hope for a gigabyte of space that we can use.

My gut tells me that maybe taking the install size of the core update by 3/4 might be a good number?

-Michael

> 
> Robin
> 
>> 
>> Then, we can even drop some last checks in the update scripts that
>> aborted the update process if disk space was running low.
>> 
>> How does that sound?
>> 
>> -Michael
>> 
>>> 
>>> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
>>> ---
>>> src/pakfire/pakfire | 79 +++++++++++++++++++++++++++++-------------
>>> ---
>>> 1 file changed, 51 insertions(+), 28 deletions(-)
>>> 
>>> diff --git a/src/pakfire/pakfire b/src/pakfire/pakfire
>>> index 9935481a5..0a144c517 100644
>>> --- a/src/pakfire/pakfire
>>> +++ b/src/pakfire/pakfire
>>> @@ -258,51 +258,76 @@
>>>                 &Pakfire::getcoredb("$force");
>>> 
>>>         } elsif ("$ARGV[0]" eq "upgrade") {
>>> -               my $use_color = "";
>>> -               my $reset_color = "";
>>> -
>>> -               if ("$Pakfire::enable_colors" eq "1") {
>>> -                       $reset_color = "$Pakfire::color{'normal'}";
>>> -                       $use_color =
>>> "$Pakfire::color{'lightpurple'}";
>>> -               }
>>> -
>>>                 &Pakfire::message("CORE INFO: Checking for Core
>>> updates...");
>>> -
>>>                 ### Make sure that the core db is not outdated. 
>>>                 &Pakfire::getcoredb("noforce");
>>> -               my %coredb = &Pakfire::coredbinfo();
>>> 
>>> -               if (defined $coredb{'AvailableRelease'}) {
>>> -                       &Pakfire::upgradecore();
>>> -               } else {
>>> -                       &Pakfire::message("CORE WARN: No new Core
>>> upgrades available. You are on release ".$coredb{'Release'});
>>> -               }
>>> +               my %coredb = &Pakfire::coredbinfo();
>>> +               &Pakfire::message("CORE WARN: No new Core upgrades
>>> available. You are on release ".$coredb{'Release'}) unless (defined
>>> $coredb{'AvailableRelease'});
>>> 
>>>                 &Pakfire::message("PAKFIRE INFO: Checking for
>>> package updates...");
>>>                 ### Make sure that the package list is not
>>> outdated. 
>>>                 &Pakfire::dbgetlist("noforce");
>>>                 
>>>                 my @deps = ();
>>> -               if (my %upgradepaks = &Pakfire::dblist("upgrade"))
>>> {
>>> -                       # Resolve the dependencies of the to be
>>> upgraded packages
>>> -                       @deps =
>>> &Pakfire::resolvedeps_recursive(keys %upgradepaks);
>>> +               my %upgradepaks = &Pakfire::dblist("upgrade");
>>> 
>>> -                       foreach $pak (sort keys %upgradepaks) {
>>> -                               print "${use_color}Update:
>>> $pak\nVersion: $upgradepaks{$pak}{'ProgVersion'} ->
>>> $upgradepaks{$pak}{'AvailableProgVersion'}\n";
>>> -                               print "Release:
>>> $upgradepaks{$pak}{'Release'} ->
>>> $upgradepaks{$pak}{'AvailableRelease'}${reset_color}\n";
>>> -                       }
>>> +               # Resolve the dependencies of the to be upgraded
>>> packages
>>> +               @deps = &Pakfire::resolvedeps_recursive(keys
>>> %upgradepaks) if (%upgradepaks);
>>> +               &Pakfire::message("PAKFIRE WARN: No new package
>>> upgrades available.") unless (@deps);
>>> +
>>> +               if (defined $coredb{'AvailableRelease'} ||
>>> %upgradepaks) {
>>> +                       &Pakfire::message("");
>>>                         &Pakfire::message("");
>>> -                       &Pakfire::message("PAKFIRE UPGR: We are
>>> going to install all packages listed above.");
>>> +                       &Pakfire::message("PAKFIRE INFO: Upgrade
>>> summary:");
>>> +                       &Pakfire::message("");
>>> +
>>> +                       if (defined $coredb{'AvailableRelease'}) {
>>> +                               &Pakfire::message("CORE INFO: Core-
>>> Update $coredb{'CoreVersion'} to install:");
>>> +                               &Pakfire::message("CORE UPGR: 
>>> Release: $coredb{'Release'} -> $coredb{'AvailableRelease'}");
>>> +                               &Pakfire::message("");
>>> +                       }
>>> +
>>> +                       if (@deps) {
>>> +                               &Pakfire::message("PAKFIRE INFO:
>>> New dependencies to install:");
>>> +                               my $totalsize = 0;
>>> +                               foreach $pak (@deps) {
>>> +                                       unless (defined
>>> $upgradepaks{$pak} || &Pakfire::isinstalled($pak) == 0) {
>>> +                                               my $size =
>>> &Pakfire::getsize("$pak");
>>> +                                               $totalsize +=
>>> $size;
>>> +                                               $size =
>>> &Pakfire::beautifysize($size);
>>> +                                               &Pakfire::message("
>>> PAKFIRE INFO:  $pak \t - $size");
>>> +                                       }
>>> +                               }
>>> +                               $totalsize =
>>> &Pakfire::beautifysize($totalsize);
>>> +                               &Pakfire::message("");
>>> +                               &Pakfire::message("PAKFIRE INFO:
>>> Total size: \t ~ $totalsize");
>>> +                               &Pakfire::message("");
>>> +                       }
>>> +
>>> +                       if (%upgradepaks) {
>>> +                               &Pakfire::message("PAKFIRE INFO:
>>> Packages to upgrade:");
>>> +                               foreach $pak (sort keys
>>> %upgradepaks) {
>>> +                                       &Pakfire::message("PAKFIRE
>>> UPGR:  $pak\t$upgradepaks{$pak}{'ProgVersion'}-
>>> $upgradepaks{$pak}{'Release'} ->
>>> $upgradepaks{$pak}{'AvailableProgVersion'}-
>>> $upgradepaks{$pak}{'AvailableRelease'}");
>>> +                               }
>>> +                               &Pakfire::message("");
>>> +                       }
>>> +
>>>                         if ($interactive) {
>>> -                         &Pakfire::message("PAKFIRE INFO: Is this
>>> okay? [y/N]");
>>> +                               &Pakfire::message("PAKFIRE INFO: Is
>>> this okay? [y/N]");
>>>                                 my $ret = <STDIN>;
>>>                                 chomp($ret);
>>>                                 &Pakfire::logger("PAKFIRE INFO:
>>> Answer: $ret");need
>>>                                 if ( $ret ne "y" ) {
>>> -                                 &Pakfire::message("PAKFIRE ERROR:
>>> Installation aborted.");
>>> -                                 exit 1;
>>> +                                       &Pakfire::message("PAKFIRE
>>> ERROR: Installation aborted.");
>>> +                                       exit 1;
>>>                                 }
>>>                         }
>>> +
>>> +                       # Perform core update
>>> +                       if (defined $coredb{'AvailableRelease'}) {
>>> +                               &Pakfire::upgradecore();
>>> +                       }
>>>                 
>>>                         # Download packages
>>>                         foreach $pak (sort keys %upgradepaks) {
>>> @@ -323,8 +348,6 @@
>>>                         foreach $pak (sort keys %upgradepaks) {
>>>                                 &Pakfire::upgradepak("$pak");
>>>                         }
>>> -               } else {
>>> -                       &Pakfire::message("PAKFIRE WARN: No new
>>> package upgrades available.");
>>>                 }
>>> 
>>>         } elsif ("$ARGV[0]" eq "list") {
>>> -- 
>>> 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.