Merge lp://staging/~pitti/software-properties/pygi into lp://staging/software-properties

Proposed by Martin Pitt
Status: Merged
Merged at revision: 643
Proposed branch: lp://staging/~pitti/software-properties/pygi
Merge into: lp://staging/software-properties
Diff against target: 952 lines (+205/-173)
15 files modified
debian/changelog (+28/-0)
debian/control (+1/-1)
software-properties-gtk (+6/-11)
softwareproperties/MirrorTest.py (+18/-5)
softwareproperties/gtk/CdromProgress.py (+16/-16)
softwareproperties/gtk/DialogAdd.py (+2/-2)
softwareproperties/gtk/DialogAddSourcesList.py (+7/-6)
softwareproperties/gtk/DialogCacheOutdated.py (+9/-5)
softwareproperties/gtk/DialogEdit.py (+2/-2)
softwareproperties/gtk/DialogMirror.py (+36/-55)
softwareproperties/gtk/SimpleGtkbuilderApp.py (+7/-7)
softwareproperties/gtk/SoftwarePropertiesGtk.py (+65/-55)
softwareproperties/gtk/dialogs.py (+3/-3)
softwareproperties/gtk/utils.py (+4/-4)
softwareproperties/kde/DialogMirror.py (+1/-1)
To merge this branch: bzr merge lp://staging/~pitti/software-properties/pygi
Reviewer Review Type Date Requested Status
Martin Pitt Needs Resubmitting
Michael Vogt Pending
Review via email: mp+47463@code.staging.launchpad.net

Description of the change

This ports the GTK UI from pygtk2 to gobject-introspection. It's mostly working now, except for the Drag & Drop parts; they work fine when using GTK3, but since in GTK 2 GtkTargetEntry is not a class and thus not constructable.

To post a comment you must log in.
Revision history for this message
Martin Pitt (pitti) wrote :

Note that with our current GTK package I get crashes when clicking wildly in the mirror selection tree view. I just debugged that and fixed it in upstream GTK:

  http://git.gnome.org/browse/gtk+/commit/?h=gtk-2-24&id=461d71f6aa7f93a7a57a1d19053cd79597a09070

So we'll get this fix into Natty.

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks, code looks good. However when I run it on my box I get:
sudo ./software-properties-gtk

** (software-properties-gtk:17374): WARNING **: Out argument 0 in get_cursor returns a struct with a transfer mode of "full". Transfer mode should be set to "none" for struct type returns as there is no way to free them safely. Ignoring transfer mode to prevent a potential invalid free. This may cause a leak in your application.

I guess that is ok for now?

Revision history for this message
Michael Vogt (mvo) wrote :

The other thing that does not work is the ping test on:
"Download from: Other…", "Select best server"
Its supposed to open a window with a lot of pings. That does not work currently, it hangs instead.

Revision history for this message
Martin Pitt (pitti) wrote :

Right, the get_cursor() warning is expected; that eventually needs to be fixed in the GTK overrides or pygobject. It doesn't hurt, though.

I'll look at the ping window, thanks for the review!

Revision history for this message
Martin Pitt (pitti) wrote :

So the problem is that this uses GTK from two different threads: the main one, and the MirrorTestGtk class, which is instantiated and run in a parallel one. Apparently pygtk had some magic to have that work, but with plain GTK it has never worked well. In particular, the worker thread doesn't seem to have a main loop, so there are no updates.

It would be a lot better if the GTK bits could be in the main thread only, and the worker thread would just do the pings and update callbacks, and the main thread would receive the callbacks and continue to run the main loop. Michael, would you mind if I reorganized the code accordingly?

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for looking into this Martin! I am absolutely in favour for this reorganizing.

649. By Martin Pitt

software-properties-gtk, softwareproperties/gtk/DialogMirror.py: Drop
calls to Gdk.threads_*(). They were insufficient and causing lockups, and
since multi-threaded Gtk is a pain to get right, we'll rather change the
code structure to only use Gtk from the main thread.

650. By Martin Pitt

softwareproperties/gtk/DialogMirror.py: Run GTK operations in main thread,
to avoid thread lockups.

651. By Martin Pitt

softwareproperties/MirrorTest.py: Move additional MirrorTestGtk
functionality (setting threading.Event flag and storing current
action/progress into main MirrorTest class, so that we can entirely drop
MirrorTestGtk from softwareproperties/gtk/DialogMirror.py. KDE still uses
its own implementation with real threads, as this is still easier to do.

Revision history for this message
Martin Pitt (pitti) wrote :

r650 moved all GTK operations to the main thread, to avoid the thread lockups. r649 removed all the Gdk.thread_* stuff, as this is now obsolete.

r651 is not a functional change, but simplifies the code a bit.

This now works fine, so I resubmit this. I verified that the kde frontend still works well, too.

review: Needs Resubmitting

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: