Bug 58493 - user: Fix x-session -> xsession property notification
Summary: user: Fix x-session -> xsession property notification
Status: RESOLVED FIXED
Alias: None
Product: accountsservice
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Matthias Clasen
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-19 00:20 UTC by Colin Walters
Modified: 2012-12-19 17:36 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
user: Fix x-session -> xsession property notification (854 bytes, patch)
2012-12-19 00:20 UTC, Colin Walters
Details | Splinter Review

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.