GNOME Bugzilla – Bug 676401
Update for libgdmgreeter changes
Last modified: 2012-07-17 15:39:44 UTC
I expect that some of the changes in gdm I'm working on will affect gnome-shell. Consider this a dumpbin for such changes. Don't review until library is actually updated, of course. :)
Created attachment 214471 [details] [review] Login dialog: update for GDM port to GDBus libgdmgreeter changed interface slightly due to the port to GDBus. Update for that.
Created attachment 215971 [details] [review] Login dialog: update for GDM port to GDBus libgdmgreeter changed interface slightly due to the port to GDBus. Update for that.
Review of attachment 215971 [details] [review]: This was fine, but the library has changed a bit since then. Will post updated patch.
Created attachment 218997 [details] [review] gdm: port from libgdmgreeter to libgdm When GDM was moved over to GDBus it dropped the libgdmgreeter library and introduced a new libgdm library with a somewhat different API. The main differences in the API are: 1) open_connection is now implicit and automatic 2) conversations don't need to be started explicitly, they're started just-in-time when verification is requested 3) The functions are split up between the client, and new helper objects that correspond to the dbus interfaces they were generated from (one for user verification, one for greeter specific operations, and a couple more that aren't used by gnome-shell). 4) libgdm supports reauthenticating in an already running session, so user switching should now affect the users session more like screen unlocking does. This commit moves the shell over to the new library. Based on work by Giovanni Campagna <gcampagna@src.gnome.org>
Review of attachment 218997 [details] [review]: Just one comment: was it necessary to rename GdmGreeter to Gdm? Doing so makes it necessary to build gdm in jhbuild environments. Or otherwise, it should be possible to make the dependency in userMenu optional with a try-catch around the Gdm import.
(In reply to comment #5) > Review of attachment 218997 [details] [review]: > Or otherwise, it should be possible to make the dependency in userMenu optional > with a try-catch around the Gdm import. The conclusion in bug 677118 was that we don't consider gdm optional.
To be "right" we would need to change the gobject-introspection interface version number when we change the so name, anyway (of course we could be a little fast and loose with it since it's not a stable library). Out of curiousity, what's the problem with building GDM in jhbuild? The extra time? GDM should be a drop in the bucket compared webkit. I guess there's an argument to be made that it wastes space, which is fair. From that point of view, the patch in bug 677118 sort of makes sense. Though diskspace isn't a huge argument in and of its own. Ideally being able to usefully run GDM in jhbuild would be nice, though figuring out the logistics of integrating it into the OS would be interesting.
Review of attachment 218997 [details] [review]: Just minor comments here...I read the code and didn't see anything obviously badly wrong, but I didn't trace through all of the API use. ::: js/gdm/loginDialog.js @@ +925,2 @@ _onReset: function(client, serviceName) { + this._userVerifier = null Missing semicolon. @@ +1366,3 @@ + id = this._userVerifier.connect('reset', + Lang.bind(this, function() { + let i; Duplicate "let i" ::: js/ui/userMenu.js @@ +563,3 @@ let allowSwitch = !this._lockdownSettings.get_boolean(DISABLE_USER_SWITCH_KEY); let multiUser = this._userManager.can_switch() && this._userManager.has_multiple_users; + let multiSession = Gdm.get_session_ids().length > 1; This looks like a bugfix for an earlier patch, since we were already testing multiSession?
Hey thanks! The last hunk isn't a bugfix, just a rename from GdmGreeter to Gdm.
Attachment 218997 [details] pushed as e06ecb8 - gdm: port from libgdmgreeter to libgdm