After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 631888 - Update gnome-shell to latest gdm user manager code
Update gnome-shell to latest gdm user manager code
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Dan Winship
gnome-shell-maint
: 622090 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-10-11 15:22 UTC by Ray Strode [halfline]
Modified: 2010-10-13 23:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gdm: resync cut-and-paste code from gdm tree. (176.42 KB, patch)
2010-10-11 15:22 UTC, Ray Strode [halfline]
committed Details | Review
statusMenu: port to new gdmuser api (2.58 KB, patch)
2010-10-11 15:22 UTC, Ray Strode [halfline]
committed Details | Review

Description Ray Strode [halfline] 2010-10-11 15:22:38 UTC
The user manager code in GDM upstream has been changed to
talk to the freedesktop accounts service.  It's also been
changed to be more asynchronous and overall have better
performance than the old code.

It's probably a good idea to update gnome-shell to this code.
One thing that's a little yucky about the gdmuser code, is that it
supports fallback if the accounts service is unavailable.  That fallback
support really dirties the code up.

I think this code should be treated as a stop-gap.  In the near term,
I'm going to move the gdm user manager code to the accounts service
repository and provide a proper library.  At that point, it's going to lose
the dirty fallback code mentioned above and we should move the shell to
use the library.
Comment 1 Ray Strode [halfline] 2010-10-11 15:22:41 UTC
Created attachment 172104 [details] [review]
gdm: resync cut-and-paste code from gdm tree.

The GDM code upstream talks to the account service now,
has better introspection annotations, and is more
asynchronous.

This commit updates the shell's copy to the latest
upstream.

Note, the API changed somewhat and so the callers will
need to be fixed up subsequently.
Comment 2 Ray Strode [halfline] 2010-10-11 15:22:46 UTC
Created attachment 172105 [details] [review]
statusMenu: port to new gdmuser api
Comment 3 Dan Winship 2010-10-11 19:08:16 UTC
*** Bug 622090 has been marked as a duplicate of this bug. ***
Comment 4 Ray Strode [halfline] 2010-10-12 16:04:24 UTC
I've put the start of a client library here:

http://cgit.freedesktop.org/accountsservice/commit/?id=8c031bce3f6fe67bd5f0f782a457aebd6af0ceba

It's just the above code moved into accounts service with the fallback stuff quickly stripped out (and a few other minor changes).

Once accounts dialog is in control center we should switch to using this library I think.
Comment 5 Dan Winship 2010-10-13 17:43:22 UTC
Comment on attachment 172104 [details] [review]
gdm: resync cut-and-paste code from gdm tree.

mostly not reviewing this this time, but...

>+	-DGDM_CACHE_DIR=\""$(localstatedir)/cache/gdm"\"	\

Shouldn't this point to gdm's localstatedir, not gnome-shell's?
Comment 6 Dan Winship 2010-10-13 17:46:09 UTC
Comment on attachment 172105 [details] [review]
statusMenu: port to new gdmuser api

>-        this._userNameChangedId = this._user.connect('notify::display-name', Lang.bind(this, this._updateUserName));
>+        this._userLoadedId = this._user.connect('notify::is-loaded', Lang.bind(this, this._updateUserName));
>+        this._userChangedId = this._user.connect('changed', Lang.bind(this, this._updateUserName));

Any reason you can't just connect to 'notify::real-name'? If the answer is "because the C code isn't calling g_object_notify() everywhere it should", then...

otherwise looks good
Comment 7 Ray Strode [halfline] 2010-10-13 23:27:31 UTC
(In reply to comment #5)
> (From update of attachment 172104 [details] [review])
> mostly not reviewing this this time, but...
> 
> >+	-DGDM_CACHE_DIR=\""$(localstatedir)/cache/gdm"\"	\
> 
> Shouldn't this point to gdm's localstatedir, not gnome-shell's?
Yea, but gdm's cache dir isn't really queryable (one of the pitfalls of copy and paste code i guess).  If it's wrong it just means a user's icon won't show up in a fallback case that's getting removed soon anyway.  I'm not too worried about it.

(In reply to comment #6)
> (From update of attachment 172105 [details] [review])
> >-        this._userNameChangedId = this._user.connect('notify::display-name',  Lang.bind(this, this._updateUserName));
> >+        this._userLoadedId = this._user.connect('notify::is-loaded', Lang.bind(this, this._updateUserName));
> >+        this._userChangedId = this._user.connect('changed', Lang.bind(this, this._updateUserName));

> Any reason you can't just connect to 'notify::real-name'? If the answer is
> "because the C code isn't calling g_object_notify() everywhere it should",
> then...
"because the C code doesn't export real-name as a gobject property anymore", which I agree is wrong, and I fixed when making the library mentioned in comment 4, but isn't fixed in this stop gap code.