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

DelayedProgressPopup for File Conflicts Progress during Package Installation [master] #1252

Merged
merged 15 commits into from Apr 7, 2022

Conversation

shundhammer
Copy link
Contributor

Target Branch / Product

This is for master / Factory / Tumbleweed.

This merges #1250 to master / Factory / Tumbleweed.

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 the start() callback of the file conflicts progress, to update it in the progress() callback and to close and delete it (assigning nil to the variable) in the finish() 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 the register() method which is always called first. The start() callback just starts the timer, and the finish() callback just closes the DelayedProgressPopup (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, ...).

Related PR

yast/yast-packager#612

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.009%) to 41.651% when pulling 450a254 on huha-dl-progress-master into 769d508 on master.

Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

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

LGTM

@shundhammer shundhammer merged commit 161f0cc into master Apr 7, 2022
@shundhammer shundhammer deleted the huha-dl-progress-master branch April 7, 2022 14:00
@yast-bot
Copy link
Contributor

yast-bot commented Apr 7, 2022

✔️ Public Jenkins job #364 successfully finished
✔️ Created OBS submit request #967512

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.

None yet

4 participants