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 686650 - Move session tracking code into gnome-session, depend on it
Move session tracking code into gnome-session, depend on it
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
: 671861 (view as bug list)
Depends on: 687821
Blocks:
 
 
Reported: 2012-10-22 15:00 UTC by Colin Walters
Modified: 2012-11-12 14:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Move session tracking code into gnome-desktop, depend on it (34.19 KB, patch)
2012-10-22 15:00 UTC, Colin Walters
rejected Details | Review
Port to gnome-session's SessionIsActive property (18.51 KB, patch)
2012-11-07 12:58 UTC, Colin Walters
needs-work Details | Review
color: Use gnome-session's SessionIsActive property (4.02 KB, patch)
2012-11-12 09:15 UTC, Bastien Nocera
none Details | Review
color: Use gnome-session's SessionIsActive property (3.14 KB, patch)
2012-11-12 09:15 UTC, Bastien Nocera
committed Details | Review
main: Remove libsystemd-login specific code (9.87 KB, patch)
2012-11-12 09:17 UTC, Bastien Nocera
committed Details | Review
main: remove a systemd header that was missed (846 bytes, patch)
2012-11-12 13:54 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Colin Walters 2012-10-22 15:00:30 UTC
The gnome-settings-session.c file also (rather confusingly) had a
utility method to create a DBus proxy for gnome-session.  Keep that,
but rename the file to gnome-settings-util.
Comment 1 Colin Walters 2012-10-22 15:00:33 UTC
Created attachment 227005 [details] [review]
Move session tracking code into gnome-desktop, depend on it
Comment 2 Matthias Clasen 2012-10-22 15:05:47 UTC
The utility method just appeared there this weekend - it was just the most convenient place to put it.
Comment 3 Bastien Nocera 2012-10-23 05:41:07 UTC
Comment on attachment 227005 [details] [review]
Move session tracking code into gnome-desktop, depend on it

As per discussion on the mailing-list.
Comment 4 Colin Walters 2012-11-07 12:58:18 UTC
Created attachment 228363 [details] [review]
Port to gnome-session's SessionIsActive property

Rather than maintaining the systemd code here, monitor gnome-session's
SessionIsActive property.  This allows us to drop the compile-time
dependency on systemd.

The power plugin is still declared to be fully dependent on systemd at
runtime, but the rest of the code should operate in more "basic
functionality" mode.
Comment 5 Bastien Nocera 2012-11-10 15:03:32 UTC
*** Bug 671861 has been marked as a duplicate of this bug. ***
Comment 6 Bastien Nocera 2012-11-10 15:08:27 UTC
Review of attachment 228363 [details] [review]:

gsd-automount-manager.c is gone.

Rest looks mostly right, but can you split out the plugins changes from the function removal.
(plugins first, function removal second).

::: plugins/color/gsd-color-manager.c
@@ +2268,3 @@
         /* track the active session */
+        priv->session = gnome_settings_session_get_session_proxy ();
+        g_signal_connect (priv->session, "g-properties-changed",

You'll need to disconnect this, as the session proxy is a singleton.

::: plugins/power/gsd-power-manager.c
@@ +3961,3 @@
         /* track the active session */
+        manager->priv->session = gnome_settings_session_get_session_proxy ();
+        g_signal_connect (manager->priv->session, "g-properties-changed",

Ditto.
Comment 7 Bastien Nocera 2012-11-12 09:15:44 UTC
Created attachment 228761 [details] [review]
color: Use gnome-session's SessionIsActive property

Rather than gnome-settings-daemon's libsystemd-login
dependent code.
Comment 8 Bastien Nocera 2012-11-12 09:15:56 UTC
Created attachment 228762 [details] [review]
color: Use gnome-session's SessionIsActive property

Rather than gnome-settings-daemon's libsystemd-login
dependent code.
Comment 9 Bastien Nocera 2012-11-12 09:17:42 UTC
Created attachment 228763 [details] [review]
main: Remove libsystemd-login specific code

After porting both users of the helper code (the power and
color plugins) to use gnome-session's SessionIsActive property,
remove the libsystemd-login dependent code.

As originally intended, gnome-settings-daemon requires
systemd for the power plugin to work at run-time, but does
not hard depend on libsystemd-login itself at compile-time.
Comment 10 Bastien Nocera 2012-11-12 09:23:27 UTC
Attachment 228762 [details] pushed as bc9b2af - color: Use gnome-session's SessionIsActive property
Attachment 228763 [details] pushed as f7928b5 - main: Remove libsystemd-login specific code
Comment 11 Allison Karlitskaya (desrt) 2012-11-12 13:15:17 UTC
This patch removes libsystemd-login from one PKG_CHECK_MODULES line in configure.ac and adds it to another.

The result is the same: it's still not possible to build gnome-settings-daemon on Ubuntu.
Comment 12 Bastien Nocera 2012-11-12 13:28:24 UTC
(In reply to comment #11)
> This patch removes libsystemd-login from one PKG_CHECK_MODULES line in
> configure.ac and adds it to another.
> 
> The result is the same: it's still not possible to build gnome-settings-daemon
> on Ubuntu.

I'm going to start throwing my arms up the air like that as well. It couldn't possibly be because there's a bug.

commit f39a2f6859106e82d622732ca2953336092170d3
Author: Bastien Nocera <hadess@hadess.net>
Date:   Mon Nov 12 14:25:06 2012 +0100

    power: Remove unneeded libsystemd-login dep
    
    I wrongly added a libsystemd-login build dependency. We actually
    only have a run-time one for the power plugin.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=686650#c11
Comment 13 Allison Karlitskaya (desrt) 2012-11-12 13:54:18 UTC
Created attachment 228776 [details] [review]
main: remove a systemd header that was missed

Remove an #include that was missed by Colin's patch to remove the
systemd dependency.
Comment 14 Bastien Nocera 2012-11-12 13:58:08 UTC
Review of attachment 228776 [details] [review]:

++