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

dnf5: Transaction progress: Fix total number of actions, hide total transaction progress bar #302

Merged

Conversation

jrohel
Copy link
Contributor

@jrohel jrohel commented Feb 16, 2023

  • dnf5: Hides total transaction progress bar - It was showing nonsensical information.
  • dnf5: Fixes total number of actions (steps) in transaction progress - It shows [x/max], before fix [x/x+1].

Solves: https://bugzilla.redhat.com/show_bug.cgi?id=2180229

@jrohel jrohel force-pushed the transaction_table_total branch 3 times, most recently from 1390a44 to 213d6f8 Compare March 9, 2023 13:03
@jrohel jrohel force-pushed the transaction_table_total branch from 213d6f8 to acb1a2f Compare March 27, 2023 17:21
@jrohel jrohel changed the title Improve transaction table dnf5: Transaction progress: Fix total number of actions, hide total transaction progress bar Mar 27, 2023
@jrohel jrohel force-pushed the transaction_table_total branch from acb1a2f to 53ed765 Compare March 27, 2023 17:38
@jan-kolarik jan-kolarik self-assigned this Mar 28, 2023
dnf5/context.cpp Outdated Show resolved Hide resolved
@jan-kolarik
Copy link
Member

Today I was using dnf5 in container rebased to this PR and I've found the following bug.

First I've installed the nudoku package:

[root@b97345e505af dnf5]# dnf5 install nudoku
Updating and loading repositories:
Repositories loaded.
Package                  Arch    Version                  Repository        Size
Installing:                                                                     
 nudoku                  x86_64  2.1.0-4.fc36             fedora        78.4 KiB

Transaction Summary:
 Installing:        1 packages

Is this ok [y/N]: y
[1/1] nudoku-0:2.1.0-4.fc36.x86_64      100% |   0.0   B/s |   0.0   B |  00m00s
>>> Already downloaded
--------------------------------------------------------------------------------
[1/1] Total                             100% |   0.0   B/s |   0.0   B |  00m00s
Verifying PGP signatures

Running transaction
[1/3] Verify package files              100% |   0.0   B/s |   1.0   B |  00m00s
[2/3] Prepare transaction               100% |  21.0   B/s |   1.0   B |  00m00s
[3/3] Installing nudoku-0:2.1.0-4.fc36. 100% | 540.9 KiB/s |  80.6 KiB |  00m00s
>>> Running trigger-install scriptlet: glibc-common-0:2.35-15.fc36.x86_64
>>> Stop trigger-install scriptlet: glibc-common-0:2.35-15.fc36.x86_64

And then I've removed it, but incorrect number of steps was shown in progress bar there:

[root@b97345e505af dnf5]# dnf5 remove nudoku
Package                  Arch    Version                  Repository        Size
Removing:                                                                       
 nudoku                  x86_64  2.1.0-4.fc36             fedora        78.4 KiB

Transaction Summary:
 Removing:          1 packages

Is this ok [y/N]: y

Verifying PGP signatures

Running transaction
[1/3] Prepare transaction               100% |  31.0   B/s |   1.0   B |  00m00s
[2/3] Erasing nudoku-0:2.1.0-4.fc36.x86 100% | 112.0   B/s |  14.0   B |  00m00s
[root@b97345e505af dnf5]#

Tried that repeatedly with the same results.

jrohel added 3 commits March 30, 2023 09:13
It is much more intuitive for the user to use NEVER_VISIBLE_LIMIT
instead of using the maximum value of std::size_t.
It was showing nonsensical information.
@jrohel jrohel force-pushed the transaction_table_total branch from 53ed765 to 390bdeb Compare March 30, 2023 07:20
@jrohel jrohel force-pushed the transaction_table_total branch from 390bdeb to 8b905ce Compare March 30, 2023 08:49
Copy link
Member

@jan-kolarik jan-kolarik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks great and the issue seems to be resolved.

@jan-kolarik jan-kolarik merged commit 1da192b into rpm-software-management:main Mar 30, 2023
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.

2 participants