That's probably okay.

Maybe a better approach, though, would be to get away from DM specific code and make accounts service manage its own configuration that the DMs use instead of their own configuration.

thoughts?

Created attachment 50137 [details] [review]
Proposed patch

One notable thing about this patch is how it checks whether GDM is running.  It currently checks if org.gnome.DisplayManager is owned.  Is there a better way?

(In reply to comment #2)
> Created an attachment (id=50137) [details]
> Proposed patch
> 
> One notable thing about this patch is how it checks whether GDM is running.  It
> currently checks if org.gnome.DisplayManager is owned.  Is there a better way?

org.gnome.DisplayManager seems reasonable to me.

Other than that, are you happy with your patch?

As for the patch, I have done some quick tests and it seems to work for me.  I'll likely play with it another day or so and make sure I don't hit any issues.  So if you want to hold off until I give a more resounding vote of QA confidence, that's reasonable.

As for config files, I'm fine with either way (accountsservice tweaking others' files or having its own), but I'm not a main LightDM or GDM developer so can't really speak to how easy a central config file would be to adopt.

This is just one config value (autologin user).  If that's all that's envisioned, I'm not sure it's worth having a special config file.  But if you plan to add more display manager settings to accountsservice, maybe it starts to make sense.

alright, let's go with this approach for now, but reconsider if we end up moving more options (like include/exclude lists?) or gain interest from other DMs

Fwiw, I've always considered the current 'gdm support' a hack that should go away when gdm starts talking to accountsservice. It makes a lot more sense for the accountsservice to store this information itself, and for interested DMs to query it.

Hi,

As the LightDM maintainer I very much support the idea of having LightDM reading configuration from the accounts service.

I am currently gauging interest on if other desktops will adopt AccountsService:
http://lists.freedesktop.org/archives/lightdm/2011-August/000051.html

Mike, the patch doesn't apply any more to 0.6.14. Do you mind porting it? Are you happy with how well it works?

Created attachment 51002 [details] [review]
Patch #2

Here's an updated patch for 0.6.14 (and it fixes a bug with detecting LightDM's autologin status).  Works for me now.

Comment on attachment 51002 [details] [review]
Patch #2

Review of attachment 51002 [details] [review]:
-----------------------------------------------------------------

::: src/daemon.c
@@ +1446,5 @@
> +{
> +        if (!save_autologin_gdm (daemon, name, enabled, error))
> +                return FALSE;
> +        if (!save_autologin_lightdm (daemon, name, enabled, error))
> +                return FALSE;

Hmm, I feel like this should always try both save functions, if we don't want to look for which dm is actually in use.

ie it should be something like

result = TRUE;
result &= save_autologin_gdm(...);
result &= save_autologin_lightdm(...);
return result;

Hi all!

Any news on this?

Comment #6 still stands - this needs to be flipped around. Make 'autologin=yes' per-user information that is provided by the accountsservice, and make interested display managers use that information.

Adding code for reading and writing all the display managers config file formats is not a sane or scalable approach.

LightDM now uses the AccountsService extension mechanism which obsoletes the majority of this bug.

https://github.com/CanonicalLtd/lightdm/commit/6015bce25f241e7580c03594d846769f8236232f