-
Notifications
You must be signed in to change notification settings - Fork 44
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
DelayedProgressPopup for File Conflicts Progress during Package Installation #1250
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shundhammer
changed the title
Added DelayedProgressPopup
DelayedProgressPopup for File Conflicts Progress during Package Installation
Mar 31, 2022
This was referenced Mar 31, 2022
lslezak
approved these changes
Apr 6, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! 👍
shundhammer
force-pushed
the
huha-dl-progress
branch
from
April 6, 2022 13:50
d7fd16c
to
935d566
Compare
lslezak
approved these changes
Apr 6, 2022
✔️ Internal Jenkins job #3 successfully finished |
This was referenced Apr 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Target Branch / Product
This is for SLE-15-SP4.
Version for master: #1252
Bugzilla
https://bugzilla.suse.com/show_bug.cgi?id=1195608
Trello
https://trello.com/c/38oLzang/
Problem
During package installation with the new unified single progress bar that handles downloads as well as installing the packages, after the download phase, there is a very disruptive new progress bar that overlays the previous one (in the main window wizard) and usually just flashes by.
Only when watching an installation video in slow motion, it becomes apparent that this is the progress of libzypp checking for file conflicts. Usually that happens so fast it just flickers shortly, i.e. this whole file conflict check could simply be ignored; but even then the flickering is disruptive.
But it doesn't get better one bit if it takes longer, so the user can actually see that file conflicts progress; because it (visually) replaces the previous global progress bar, and it goes from 0 to 100 again. This is very annoying. Technically, it's a wizard pop-up, but to the user it looks as if the progress bar jumps to 0 again and then quickly progresses to 100; only to be back at the old previous value (somewhere in the middle). This is very irritating.
Video of the Problem
pkg-dl-progress-2022-03-29-1700.mp4
Solution
Immediately changing the label text on the global progress bar to "Checking file conflicts" for instant feedback. That text remains static (no added numbers) while that check is going on.
Also, added a new class
DelayedProgressPopup
which opens the progress bar in a pop-up dialog with a slight delay (by default 4 seconds), i.e. only when it even makes sense to bother the user with that operation that is usually over so quickly that we really don't need to confuse him with that.Added that class to the file conflicts callbacks so it's no longer a full-screen progress bar that appears to replace the previous global progress bar.
Videos
delayed-progress-qt.mp4
DelayedProgressPopup in the Qt UI in the standalone new example
delayed-progress-ncurses.mp4
DelayedProgressPopup in the NCurses UI in the standalone new example
no-popup.mp4
Smooth installation: No disruption due to a progress bar for file conflicts overlaying the global progress. If it doesn't take too long, you don't see anything at all about that file conflicts check; like here.
immediate-text-and-delayed-popup.mp4
If it does take longer, the file conflict checking progress is displayed in a delayed progress popup.
(Added an artificial
sleep()
to make it slow enough to show this popup here)'Invalid Object' Problem and Workaround
The natural thing to do was of course to create an instance of the
DelayedProgressPopup
in thestart()
callback of the file conflicts progress, to update it in theprogress()
callback and to close and delete it (assigningnil
to the variable) in thefinish()
callback.But that didn't work: It resulted in a very weird error message Invalid object about that instance. We (@lslezak and @shundhammer) suspected that it had something to do with transferring objects between the Ruby layer, the C++ layer, the package bindings, and libzypp.
So after a while of experimenting, a workaround turned out to be to create that
DelayedProgressPopup
instance in advance, before the callbacks were used: In theregister()
method which is always called first. Thestart()
callback just starts the timer, and thefinish()
callback just closes theDelayedProgressPopup
(but doesn't attempt to deallocate it, leaving it for further use, if needed). That's not exactly elegant, but it works.It is still unclear what exactly happened; it might be something about object lifetime in those different software layers. But that's only a wild guess.
Testing
Manually tested the
DelayedProgressPopup
with the new example script that allows to use different sets of parameters.Manually tested the file conflicts progress with (many times!) upgrading only a handful of packages at a time.
Automated Testing?
That would be really complicated since it depends on timing behavior, on actually installing or upgrading some packages, and on how long libzypp takes for checking file conflicts. Without using the real thing, the testing wouldn't make much sense since the problem lies in the integration of all the various parts (Ruby, C++ / package bindings, libzypp callbacks, ...).
Installer Self-Update?
This affects only the installed system, not the system installation or upgrade, so an installer self-update is not needed.
Related PR
yast/yast-packager#609