Bug 58493

Summary: user: Fix x-session -> xsession property notification
Product: accountsservice Reporter: Colin Walters <walters>
Component: generalAssignee: Matthias Clasen <mclasen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: rstrode, walters
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: user: Fix x-session -> xsession property notification

Description Colin Walters 2012-12-19 00:20:41 UTC
Created attachment 71777 [details] [review]
user: Fix x-session -> xsession property notification

Subject: [PATCH] user: Fix x-session -> xsession property notification

Introduced by commit acb79f31770318f461f2dec95d0ba971cbfbc177.
---
 src/user.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Comment 1 Ray Strode [halfline] 2012-12-19 14:47:09 UTC
Comment on attachment 71777 [details] [review]
user: Fix x-session -> xsession property notification

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

::: src/user.c
@@ +315,4 @@
>          if (s != NULL) {
>                  g_free (user->x_session);
>                  user->x_session = s;
> +                g_object_notify (G_OBJECT (user), "xsession");

can you describe a little more the reasoning behind this?

we do g_object_notify (..., "x-session") elsewhere in the code and the class_init function has:

        g_object_class_install_property (gobject_class,•
                                         PROP_X_SESSION,•
                                         g_param_spec_string ("x-session",•
                                                              "X session",•
                                                              "User's X session.",•
                                                              NULL,•
                                                              G_PARAM_READABLE));•

I'm assuming this patch was written to address a specific problem you ran into, but I think if there's a problem, the fix might be more elaborate then just that change.
Comment 2 Colin Walters 2012-12-19 17:10:08 UTC
(In reply to comment #1)
> Comment on attachment 71777 [details] [review] [review]
> user: Fix x-session -> xsession property notification
> 
> Review of attachment 71777 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: src/user.c
> @@ +315,4 @@
> >          if (s != NULL) {
> >                  g_free (user->x_session);
> >                  user->x_session = s;
> > +                g_object_notify (G_OBJECT (user), "xsession");
> 
> can you describe a little more the reasoning behind this?

Booted latest gnome-ostree, saw a g_warning from accountsservice.  Created patch.  No more warning.

> we do g_object_notify (..., "x-session") elsewhere in the code

User != ActUser

It looks like src/user.c has a lot of leftover bits from before it was switched to using gdbus-codegen.

This whole thing though is really a mess.  Right now gdbus-codegen is being run twice.  Probably a side effect of your use of recursive automake (as opposed to nonrecursive) - recursive easily creates broken cross-folder dependency ordering problems.
Comment 3 Ray Strode [halfline] 2012-12-19 17:33:00 UTC
So why did you say "Introduced by commit acb79f31770318f461f2dec95d0ba971cbfbc177" if we're not talking about act-user.c?
Comment 4 Ray Strode [halfline] 2012-12-19 17:35:30 UTC
oh nevermind, I get it, just wrong commit id. pushed.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.