Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

daemon: New API to get download size of pkg including deps #1626

Closed
wants to merge 2 commits into from

Conversation

m-blaha
Copy link
Member

@m-blaha m-blaha commented Aug 13, 2024

The new API simulates install operation on given pkg returning all dependencies that are going to be installed with their sizes.

Changes:

be3c8b6 (Marek Blaha, 49 seconds ago)
dnfdaemon: Example of Rpm.check_install() usage

be9411b (Marek Blaha, 4 minutes ago)
dnfdaemon: API to get download size including dependencies

This can be used to get the "cost" of installation of a package - how much
data needs to be downloaded and how much disk space will be used.

Resolves: #1593

This can be used to get the "cost" of installation of a package - how
much data needs to be downloaded and how much disk space will be used.
@m-blaha
Copy link
Member Author

m-blaha commented Aug 13, 2024

This is only a POC. @mcrha, @jan-kolarik, do you consider this worth having?
The other solution could be enhancing Goal.resolve() return values to also contain packages sizes. Then the same result as Rpm.check_install(pkg_spec) can be achieved using two calls - Rpm.install(pkg_spec) and Goal.resolve().

@jan-kolarik jan-kolarik self-requested a review August 19, 2024 13:29
@mcrha
Copy link
Contributor

mcrha commented Aug 19, 2024

Having something like this would work for me. Being able to verify whether the install will work and how much it'll "cost" is a good idea.

Having there pkg_attrs as another option would be nice to have, thus one can ask what it wants to see.

The name check_install is fine, I'm only thinking, what about simulate_install? I thought there was some sort of --simulate or --dry-run or some such argument for the command line tool, but it looks like there is no such thing, thus I might see it with another software/tool.

By the way, it can sometimes fail with the GPG key not being installed/updated yet. Will that break the check/simulation completely, like the call returning failure/nothing?

The call can take a long time, due to dependency resolution, slow connection to update local caches and such. That can easily cause timeout on the D-Bus call. I set indefinite timeout in some cases in the gnome-software, thus not a big deal, I'm only mentioning it as a possible bottleneck for any archive reader.

@jan-kolarik
Copy link
Member

The other solution could be enhancing Goal.resolve() return values to also contain packages sizes.

Hmm, but there are already download_size and install_size attributes for packages or not?

Anyway it seems to me that both resolve and this new check_install are hugely overlapping so it would be better to have it at a single place. Either the same (possibly extended) call or keeping the new check_install call, but sharing the logic inside the code.

@mcrha
Copy link
Contributor

mcrha commented Aug 20, 2024

Aha, it's what Marek meant, there is already a way to do this, it's only hidden with two D-Bus calls. I do not like code duplication, thus ideally share the code. While a dedicated function makes it clearer how to achieve that, that not-so-obvious-two-calls works for me too.

Looking on what Jan pointed to, that part does a lot more things.

My comment about customizable pkg_attrs still applies.

@m-blaha
Copy link
Member Author

m-blaha commented Aug 20, 2024

I considered extending the Rpm.list() method with new pkg_attrs (like download_size_with_deps), but ultimately decided against it. These attributes are quite time-consuming to calculate, and if a user were to run Rpm.list('*') with them, it could take an exceptionally long time to complete.
But I will consider extending the resolve call.

@mcrha
Copy link
Contributor

mcrha commented Aug 20, 2024

I considered extending the Rpm.list() method with new pkg_attrs (like download_size_with_deps), but ultimately decided against it.

I agree, that would be a giant performance hit. Better to let the API user decide which package(s) it should check and which not, which is perfectly fine to do with a "simulate" of the install call.

@mcrha
Copy link
Contributor

mcrha commented Aug 21, 2024

I tried the resolve call on a hedgewars package and the transaction output variant does contain the sizes for the packages, as Honza pointed to above.

resolved with result:0 and transaction:'[
   ('Package', 'Install', 'User', @a{sv} {}, {'arch': <'x86_64'>, 'download_size': <uint64 2844164>, 'epoch': <'0'>,
    'evr': <'1.0.2-7.fc40'>, 'from_repo_id': <''>, 'full_nevra': <'hedgewars-0:1.0.2-7.fc40.x86_64'>, 'id': <8073>,
    'install_size': <uint64 5784790>, 'name': <'hedgewars'>, 'reason': <'None'>, 'release': <'7.fc40'>, 'repo_id': <'updates'>,
    'version': <'1.0.2'>}),
    ('Package', 'Install', 'Dependency', {}, {'arch': <'noarch'>, 'download_size': <uint64 145588917>, 'epoch': <'0'>,
     'evr': <'1.0.2-7.fc40'>, 'from_repo_id': <''>, 'full_nevra': <'hedgewars-data-0:1.0.2-7.fc40.noarch'>, 'id': <8074>,
     'install_size': <uint64 161683968>, 'name': <'hedgewars-data'>, 'reason': <'None'>, 'release': <'7.fc40'>,
     'repo_id': <'updates'>, 'version': <'1.0.2'>}),
    ('Package', 'Install', 'Dependency', {}, {'arch': <'x86_64'>, 'download_size': <uint64 20076>, 'epoch': <'0'>,
      'evr': <'2.2.0-5.fc40'>, 'from_repo_id': <''>, 'full_nevra': <'SDL2_net-0:2.2.0-5.fc40.x86_64'>, 'id': <29954>,
      'install_size': <uint64 27818>, 'name': <'SDL2_net'>, 'reason': <'None'>, 'release': <'5.fc40'>, 'repo_id': <'fedora'>,
      'version': <'2.2.0'>}),
    ('Package', 'Install', 'Dependency', {}, {'arch': <'x86_64'>, 'download_size': <uint64 86028>, 'epoch': <'0'>,
      'evr': <'3.0.2-14.fc40'>, 'from_repo_id': <''>, 'full_nevra': <'physfs-0:3.0.2-14.fc40.x86_64'>, 'id': <68521>,
      'install_size': <uint64 171340>, 'name': <'physfs'>, 'reason': <'None'>, 'release': <'14.fc40'>, 'repo_id': <'fedora'>,
      'version': <'3.0.2'>}),
    ('Package', 'Install', 'Dependency', {}, {'arch': <'noarch'>, 'download_size': <uint64 7946036>, 'epoch': <'0'>,
      'evr': <'0.9.46-31.fc40'>, 'from_repo_id': <''>, 'full_nevra': <'wqy-zenhei-fonts-0:0.9.46-31.fc40.noarch'>, 'id': <102476>,
      'install_size': <uint64 17157417>, 'name': <'wqy-zenhei-fonts'>, 'reason': <'None'>, 'release': <'31.fc40'>,
      'repo_id': <'fedora'>, 'version': <'0.9.46'>}),
    ('Package', 'Install', 'Dependency', {}, {'arch': <'x86_64'>, 'download_size': <uint64 166484>, 'epoch': <'0'>,
      'evr': <'2.8.0-1.fc40'>, 'from_repo_id': <''>, 'full_nevra': <'SDL2_mixer-0:2.8.0-1.fc40.x86_64'>, 'id': <3859>,
      'install_size': <uint64 353616>, 'name': <'SDL2_mixer'>, 'reason': <'None'>, 'release': <'1.fc40'>, 'repo_id': <'updates'>,
      'version': <'2.8.0'>})]'

Thus, from my point of view, this change is not needed.

What would be good to have are two things:

  1. extend the API to provide the pkg_attrs myself, thus I can read for the transaction only those bits I'm interested in; the default value can the attributes used right now, thus any current API users will not need changing anything;
  2. be able to cancel the transaction, or better run it as "simulate", thus it's not saved for later when I only want to know how much the install would cost. The resolve function does have an options argument, thus adding there a boolean simulate or dry-run or whichever name any other common tools use, to indicate to the dnf5daemon to discard the result as soon as it's resolved, would be ideal, both because the daemon can do it straight away and for the client not needing to do two D-Bus calls.

@mcrha
Copy link
Contributor

mcrha commented Sep 6, 2024

From my previous comment, the 1. is a feature request, not belonging here. The 2. is covered by the #1678 . I propose to close this, because the extra API is not needed (proved by being able to get to the values without this change too).

@m-blaha m-blaha closed this Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dnf5daemon: Get dependencies to be downloaded (sizes are good enough)
3 participants