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 741688 - Start plugins as independent processes
Start plugins as independent processes
Status: RESOLVED OBSOLETE
Product: gnome-settings-daemon
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on: 772370
Blocks: 690866
 
 
Reported: 2014-12-17 23:14 UTC by Matthias Clasen
Modified: 2019-03-20 11:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't use install an autostart file (3.29 KB, patch)
2014-12-17 23:14 UTC, Matthias Clasen
none Details | Review
main: Remove ability to start/stop individual plugins (25.32 KB, patch)
2016-09-27 09:55 UTC, Bastien Nocera
none Details | Review
main: Remove GSettings override for plugin priority (30.76 KB, patch)
2016-09-27 09:55 UTC, Bastien Nocera
none Details | Review
dummy: Remove dummy plugin (12.60 KB, patch)
2016-09-27 09:56 UTC, Bastien Nocera
none Details | Review
clipboard: Add test application (1.88 KB, patch)
2016-09-27 09:56 UTC, Bastien Nocera
none Details | Review
color: Add test application (1.89 KB, patch)
2016-09-27 09:56 UTC, Bastien Nocera
none Details | Review
plugins: Remove "test" from the stand-alone program names (24.85 KB, patch)
2016-09-27 09:56 UTC, Bastien Nocera
none Details | Review
common: Add verbose option to test-plugin.h (1.49 KB, patch)
2016-09-27 09:56 UTC, Bastien Nocera
none Details | Review
plugins: Rename sources of all test applications (22.04 KB, patch)
2016-09-27 09:56 UTC, Bastien Nocera
none Details | Review
sound: Rename the sound plugin for PulseAudio (1.12 KB, patch)
2016-09-27 09:56 UTC, Bastien Nocera
none Details | Review
housekeeping: Fix typo in DISTCLEANFILES (730 bytes, patch)
2016-09-27 09:56 UTC, Bastien Nocera
none Details | Review
plugins: Add systemd user service files for all the plugins (31.10 KB, patch)
2016-09-27 09:56 UTC, Bastien Nocera
none Details | Review
sharing: Add missing generated file to the daemon sources (745 bytes, patch)
2016-09-27 09:57 UTC, Bastien Nocera
none Details | Review
plugins: Stop building helpers as plugins (56.74 KB, patch)
2016-09-27 09:57 UTC, Bastien Nocera
none Details | Review
power: Add missing source to stand-alone program (703 bytes, patch)
2016-09-27 09:57 UTC, Bastien Nocera
none Details | Review
main: Remove gnome-settings-daemon binary (38.38 KB, patch)
2016-09-27 09:57 UTC, Bastien Nocera
none Details | Review
data: Remove pkg-config files (2.98 KB, patch)
2016-09-27 09:57 UTC, Bastien Nocera
none Details | Review
common: Remove use of SCHEMA_NAME (3.63 KB, patch)
2016-09-27 09:57 UTC, Bastien Nocera
none Details | Review
build: Bump version to current development version (721 bytes, patch)
2016-09-30 11:14 UTC, Bastien Nocera
committed Details | Review
main: Remove ability to start/stop individual plugins (25.32 KB, patch)
2016-09-30 11:14 UTC, Bastien Nocera
none Details | Review
main: Remove GSettings override for plugin priority (30.76 KB, patch)
2016-09-30 11:14 UTC, Bastien Nocera
none Details | Review
dummy: Remove dummy plugin (12.60 KB, patch)
2016-09-30 11:14 UTC, Bastien Nocera
needs-work Details | Review
clipboard: Add test application (1.88 KB, patch)
2016-09-30 11:14 UTC, Bastien Nocera
committed Details | Review
color: Add test application (1.89 KB, patch)
2016-09-30 11:14 UTC, Bastien Nocera
committed Details | Review
plugins: Remove "test" from the stand-alone program names (23.89 KB, patch)
2016-09-30 11:14 UTC, Bastien Nocera
none Details | Review
common: Add verbose option to test-plugin.h (1.49 KB, patch)
2016-09-30 11:15 UTC, Bastien Nocera
none Details | Review
plugins: Rename sources of all test applications (22.24 KB, patch)
2016-09-30 11:15 UTC, Bastien Nocera
none Details | Review
sound: Rename the sound plugin for PulseAudio (1.12 KB, patch)
2016-09-30 11:15 UTC, Bastien Nocera
none Details | Review
housekeeping: Fix typo in DISTCLEANFILES (730 bytes, patch)
2016-09-30 11:15 UTC, Bastien Nocera
committed Details | Review
plugins: Add systemd user service files for all the plugins (31.10 KB, patch)
2016-09-30 11:15 UTC, Bastien Nocera
needs-work Details | Review
sharing: Add missing generated file to the daemon sources (745 bytes, patch)
2016-09-30 11:15 UTC, Bastien Nocera
none Details | Review
plugins: Stop building helpers as plugins (84.17 KB, patch)
2016-09-30 11:16 UTC, Bastien Nocera
none Details | Review
power: Add missing source to stand-alone program (703 bytes, patch)
2016-09-30 11:16 UTC, Bastien Nocera
committed Details | Review
main: Remove gnome-settings-daemon binary (47.42 KB, patch)
2016-09-30 11:16 UTC, Bastien Nocera
none Details | Review
data: Adapt pkg-config file for API removal (2.45 KB, patch)
2016-09-30 11:16 UTC, Bastien Nocera
none Details | Review
common: Remove use of SCHEMA_NAME (3.81 KB, patch)
2016-09-30 11:16 UTC, Bastien Nocera
none Details | Review
common: Report errors on startup in the helper skeleton (924 bytes, patch)
2016-09-30 11:16 UTC, Bastien Nocera
none Details | Review
common: Remove obsolete key parsing test program (1.88 KB, patch)
2016-09-30 11:17 UTC, Bastien Nocera
committed Details | Review
build: Add separate GTK+ skeleton (48.62 KB, patch)
2016-09-30 11:17 UTC, Bastien Nocera
none Details | Review
build: Require lcms2 version 2.2 minimum (2.50 KB, patch)
2016-09-30 11:17 UTC, Bastien Nocera
committed Details | Review
build: Remove separate check for gnome-desktop (1.47 KB, patch)
2016-09-30 11:17 UTC, Bastien Nocera
none Details | Review
common: Don't run when not under GNOME (2.37 KB, patch)
2016-09-30 11:17 UTC, Bastien Nocera
needs-work Details | Review
main: Remove gnome-settings-plugin.h (21.38 KB, patch)
2016-09-30 11:17 UTC, Bastien Nocera
none Details | Review
power: Re-add "mock" support to gpm-common.c (4.52 KB, patch)
2016-09-30 11:18 UTC, Bastien Nocera
none Details | Review
color: Fix self-test (1.12 KB, patch)
2016-09-30 11:18 UTC, Bastien Nocera
committed Details | Review
plugins: Use LIBM macro instead of hard-coding it (4.15 KB, patch)
2016-09-30 11:18 UTC, Bastien Nocera
none Details | Review
sharing: Install stand-alone gsd-sharing (716 bytes, patch)
2016-09-30 11:18 UTC, Bastien Nocera
none Details | Review
power: Install stand-alone gsd-power (1.13 KB, patch)
2016-09-30 11:18 UTC, Bastien Nocera
none Details | Review
data: Adapt pkg-config file for API removal (2.45 KB, patch)
2016-09-30 12:09 UTC, Bastien Nocera
none Details | Review
dummy: fix build (836 bytes, patch)
2016-10-13 15:58 UTC, Alberts Muktupāvels
none Details | Review

Description Matthias Clasen 2014-12-17 23:14:16 UTC
We are not using autostart for gnome-settings-daemon, but make
it a required component of the session. Instead, install a regular
desktop file. At the same time, install a D-Bus service file
and mark the application D-Bus activatable in the desktop file.
Comment 1 Matthias Clasen 2014-12-17 23:14:24 UTC
Created attachment 292944 [details] [review]
Don't use install an autostart file
Comment 2 Giovanni Campagna 2014-12-18 09:52:24 UTC
Wasn't it the case at some point that some GNOME apps were using gnome-settings-daemon's DBus names (maybe Rhythmbox for media keys), which caused g-s-d to start outside of GNOME, prompting to remove DBus activation?
Comment 3 Bastien Nocera 2014-12-18 10:22:50 UTC
(In reply to comment #2)
> Wasn't it the case at some point that some GNOME apps were using
> gnome-settings-daemon's DBus names (maybe Rhythmbox for media keys), which
> caused g-s-d to start outside of GNOME, prompting to remove DBus activation?

Yep, see https://bugzilla.gnome.org/show_bug.cgi?id=673586
Comment 4 Ray Strode [halfline] 2014-12-18 15:29:35 UTC
I think we can have our cake and eat it too.  If the session is activated with a org.gtk.Application.Activate() call then we can start it up in a full blown all things go sort of way.  If it's activated when using some api provided by a module, we can either exit with an error or load just that module.
Comment 5 Bastien Nocera 2014-12-18 15:54:06 UTC
I really don't want to do that, because what I would actually want to do in gnome-settings-daemon, is split each plugin into a systemd service, so we don't have I/O blockers, we can start the housekeeping plugin through timers instead of keeping it always there, etc.
Comment 6 Ray Strode [halfline] 2014-12-18 16:34:12 UTC
well that fixes the problem too.
Comment 7 Bastien Nocera 2016-09-27 09:55:18 UTC
Patch set coming. A couple of things aren't particularly set though:
- how to we make the start of the daemons conditional on gnome-session being present? Should we exit as soon as possible if we're not in a GNOME session?
- how do we mark the .service files as to be auto-started (is that a post-install script/trigger? Do we need to print instructions after "make install")

Additional changes could include:
- make xrandr plugin exit when unused
- simplify plugin start (do we need separate init/finalize and start/stop)
- make housekeeping use a timer
- merge gsd-printer and gsd-print-notifications
Comment 8 Bastien Nocera 2016-09-27 09:55:49 UTC
Created attachment 336326 [details] [review]
main: Remove ability to start/stop individual plugins

All the plugins are now enabled by default, until we use another way to
start them individually.
Comment 9 Bastien Nocera 2016-09-27 09:55:56 UTC
Created attachment 336327 [details] [review]
main: Remove GSettings override for plugin priority

There's a default value already available in the plugin file itself.
Comment 10 Bastien Nocera 2016-09-27 09:56:01 UTC
Created attachment 336328 [details] [review]
dummy: Remove dummy plugin

We soon won't need an example plugin.
Comment 11 Bastien Nocera 2016-09-27 09:56:08 UTC
Created attachment 336329 [details] [review]
clipboard: Add test application
Comment 12 Bastien Nocera 2016-09-27 09:56:14 UTC
Created attachment 336330 [details] [review]
color: Add test application
Comment 13 Bastien Nocera 2016-09-27 09:56:21 UTC
Created attachment 336331 [details] [review]
plugins: Remove "test" from the stand-alone program names
Comment 14 Bastien Nocera 2016-09-27 09:56:27 UTC
Created attachment 336332 [details] [review]
common: Add verbose option to test-plugin.h
Comment 15 Bastien Nocera 2016-09-27 09:56:35 UTC
Created attachment 336333 [details] [review]
plugins: Rename sources of all test applications
Comment 16 Bastien Nocera 2016-09-27 09:56:41 UTC
Created attachment 336334 [details] [review]
sound: Rename the sound plugin for PulseAudio

org.gnome.SettingsDaemon.Sound not simply org.gnome.SettingsDaemon
Comment 17 Bastien Nocera 2016-09-27 09:56:48 UTC
Created attachment 336335 [details] [review]
housekeeping: Fix typo in DISTCLEANFILES
Comment 18 Bastien Nocera 2016-09-27 09:56:54 UTC
Created attachment 336336 [details] [review]
plugins: Add systemd user service files for all the plugins

Most of the plugins are either D-Bus services, or simple background
services. Special cases are:
- the media-keys and sound plugins waiting for PulseAudio to start
- the xrandr plugin is not started on boot, but auto-started when
  the D-Bus service is requested
Comment 19 Bastien Nocera 2016-09-27 09:57:01 UTC
Created attachment 336337 [details] [review]
sharing: Add missing generated file to the daemon sources
Comment 20 Bastien Nocera 2016-09-27 09:57:07 UTC
Created attachment 336338 [details] [review]
plugins: Stop building helpers as plugins
Comment 21 Bastien Nocera 2016-09-27 09:57:14 UTC
Created attachment 336339 [details] [review]
power: Add missing source to stand-alone program
Comment 22 Bastien Nocera 2016-09-27 09:57:20 UTC
Created attachment 336340 [details] [review]
main: Remove gnome-settings-daemon binary
Comment 23 Bastien Nocera 2016-09-27 09:57:26 UTC
Created attachment 336341 [details] [review]
data: Remove pkg-config files

We don't have an API anymore, plugins are now stand-alone daemons.
Comment 24 Bastien Nocera 2016-09-27 09:57:33 UTC
Created attachment 336342 [details] [review]
common: Remove use of SCHEMA_NAME
Comment 25 Bastien Nocera 2016-09-27 10:03:49 UTC
And a small list of the benefits:
- don't start plugins that aren't needed at a particular point (housekeeping, xrandr)
- avoid side-effects with some X11 APIs (see bug 760288)
- avoid crashes restarting all of gnome-settings-daemon (including loss of theme setting, with fonts and themes changing)
- avoid side-effects of hangs or synchronous I/O (like the recent CUPS problems)
- easier to debug crashes and hangs, better logging
Comment 26 Bastien Nocera 2016-09-30 11:14:10 UTC
Created attachment 336631 [details] [review]
build: Bump version to current development version
Comment 27 Bastien Nocera 2016-09-30 11:14:17 UTC
Created attachment 336632 [details] [review]
main: Remove ability to start/stop individual plugins

All the plugins are now enabled by default, until we use another way to
start them individually.
Comment 28 Bastien Nocera 2016-09-30 11:14:24 UTC
Created attachment 336633 [details] [review]
main: Remove GSettings override for plugin priority

There's a default value already available in the plugin file itself.
Comment 29 Bastien Nocera 2016-09-30 11:14:35 UTC
Created attachment 336634 [details] [review]
dummy: Remove dummy plugin

We soon won't need an example plugin.
Comment 30 Bastien Nocera 2016-09-30 11:14:43 UTC
Created attachment 336635 [details] [review]
clipboard: Add test application
Comment 31 Bastien Nocera 2016-09-30 11:14:50 UTC
Created attachment 336636 [details] [review]
color: Add test application
Comment 32 Bastien Nocera 2016-09-30 11:14:57 UTC
Created attachment 336637 [details] [review]
plugins: Remove "test" from the stand-alone program names
Comment 33 Bastien Nocera 2016-09-30 11:15:04 UTC
Created attachment 336638 [details] [review]
common: Add verbose option to test-plugin.h
Comment 34 Bastien Nocera 2016-09-30 11:15:11 UTC
Created attachment 336639 [details] [review]
plugins: Rename sources of all test applications
Comment 35 Bastien Nocera 2016-09-30 11:15:18 UTC
Created attachment 336640 [details] [review]
sound: Rename the sound plugin for PulseAudio

org.gnome.SettingsDaemon.Sound not simply org.gnome.SettingsDaemon
Comment 36 Bastien Nocera 2016-09-30 11:15:25 UTC
Created attachment 336641 [details] [review]
housekeeping: Fix typo in DISTCLEANFILES
Comment 37 Bastien Nocera 2016-09-30 11:15:38 UTC
Created attachment 336642 [details] [review]
plugins: Add systemd user service files for all the plugins

Most of the plugins are either D-Bus services, or simple background
services. Special cases are:
- the media-keys and sound plugins waiting for PulseAudio to start
- the xrandr plugin is not started on boot, but auto-started when
  the D-Bus service is requested
Comment 38 Bastien Nocera 2016-09-30 11:15:52 UTC
Created attachment 336643 [details] [review]
sharing: Add missing generated file to the daemon sources
Comment 39 Bastien Nocera 2016-09-30 11:16:00 UTC
Created attachment 336644 [details] [review]
plugins: Stop building helpers as plugins
Comment 40 Bastien Nocera 2016-09-30 11:16:14 UTC
Created attachment 336645 [details] [review]
power: Add missing source to stand-alone program
Comment 41 Bastien Nocera 2016-09-30 11:16:21 UTC
Created attachment 336646 [details] [review]
main: Remove gnome-settings-daemon binary
Comment 42 Bastien Nocera 2016-09-30 11:16:33 UTC
Created attachment 336647 [details] [review]
data: Adapt pkg-config file for API removal

We don't have an API anymore, plugins are now stand-alone daemons, but
we still need to install enumeration headers.
Comment 43 Bastien Nocera 2016-09-30 11:16:40 UTC
Created attachment 336648 [details] [review]
common: Remove use of SCHEMA_NAME
Comment 44 Bastien Nocera 2016-09-30 11:16:55 UTC
Created attachment 336649 [details] [review]
common: Report errors on startup in the helper skeleton
Comment 45 Bastien Nocera 2016-09-30 11:17:01 UTC
Created attachment 336650 [details] [review]
common: Remove obsolete key parsing test program
Comment 46 Bastien Nocera 2016-09-30 11:17:09 UTC
Created attachment 336651 [details] [review]
build: Add separate GTK+ skeleton

And simplify library requirements for each helper. This should allow
us to trim the number of dependencies for each one of the helpers. This
also moves the libnotify initialisation directly into the plugins that
need it.
Comment 47 Bastien Nocera 2016-09-30 11:17:27 UTC
Created attachment 336652 [details] [review]
build: Require lcms2 version 2.2 minimum
Comment 48 Bastien Nocera 2016-09-30 11:17:37 UTC
Created attachment 336653 [details] [review]
build: Remove separate check for gnome-desktop
Comment 49 Bastien Nocera 2016-09-30 11:17:45 UTC
Created attachment 336654 [details] [review]
common: Don't run when not under GNOME
Comment 50 Bastien Nocera 2016-09-30 11:17:58 UTC
Created attachment 336655 [details] [review]
main: Remove gnome-settings-plugin.h

The plugin system is not used any more, and wasn't dist'ed, which broke
building from tarballs.
Comment 51 Bastien Nocera 2016-09-30 11:18:09 UTC
Created attachment 336656 [details] [review]
power: Re-add "mock" support to gpm-common.c

It was removed when we removed support for building plugins, with the
stand-alone programs taking over.

This adds runtime mock support, rather than the compile-time mock
support we used before.
Comment 52 Bastien Nocera 2016-09-30 11:18:21 UTC
Created attachment 336657 [details] [review]
color: Fix self-test

The data now comes from hwdb via gnome-desktop, and the name of the
vendor changed.
Comment 53 Bastien Nocera 2016-09-30 11:18:28 UTC
Created attachment 336658 [details] [review]
plugins: Use LIBM macro instead of hard-coding it

And fix gsd-color linkage by the same token.
Comment 54 Bastien Nocera 2016-09-30 11:18:35 UTC
Created attachment 336659 [details] [review]
sharing: Install stand-alone gsd-sharing
Comment 55 Bastien Nocera 2016-09-30 11:18:46 UTC
Created attachment 336660 [details] [review]
power: Install stand-alone gsd-power
Comment 56 Bastien Nocera 2016-09-30 11:43:31 UTC
I ran into a bunch of problems trying to make use of this:
- we'll need to install a preset file, and run "systemctl --user preset ..." to set it up to run by default
- attachment 336654 [details] [review] doesn't work because we're started so early that 1) XDG_CURRENT_DESKTOP isn't set 2) XDG_SESSION_DESKTOP isn't set either even though pam_systemd is supposed to set it up when logging in!
- if we revert attachment 336654 [details] [review], we are being run too early, and X isn't available yet.
- I added a gnome-session.service file, but systemd cannot attach it to the running gnome-session (which isn't started via D-Bus)
Comment 57 Bastien Nocera 2016-09-30 12:09:02 UTC
Created attachment 336662 [details] [review]
data: Adapt pkg-config file for API removal

We don't have an API anymore, plugins are now stand-alone daemons, but
we still need to install enumeration headers.
Comment 58 Ray Strode [halfline] 2016-09-30 13:36:27 UTC
So I want to dive in and try these soon and do some review, but let me just chime in on this first:

(In reply to Bastien Nocera from comment #56)
> - we'll need to install a preset file, and run "systemctl --user preset ..."
> to set it up to run by default
So Martin Pitt is actually doing some work in this area, and he added some new targets we should make use of.  See

https://github.com/systemd/systemd/pull/3678
https://github.com/systemd/systemd/pull/3848

Basically we need to put in each of the service files:

PartOf=graphical-session.target

Then the gnome-session.service should have:

BindsTo=graphical-session.target

Finally, we'll need to change how gnome-session is started.  Right now /usr/bin/gnome-session is a shell script that starts /usr/libexec/gnome-session-binary.  We'll need to change it to instead:

systemctl --user start graphical-session-pre.target
systemctl --user start gnome-session.target

> - attachment 336654 [details] [review] [review] doesn't work because we're started so
> early that 1) XDG_CURRENT_DESKTOP isn't set 2) XDG_SESSION_DESKTOP isn't set
> either even though pam_systemd is supposed to set it up when logging in!
Those things are actually set by GDM.  The problem is they are set in the session environment, not the systemd --user environment. But /usr/bin/gnome-session pushes them up to the systemd --user environment with its dbus-update-activation-environment call, so if the session is kicked off like I outlined above, they should be set.

> - if we revert attachment 336654 [details] [review] [review], we are being run too
> early, and X isn't available yet.
So I think the right way to associate the target with the gnome session would be to do:

systemctl --user add-wants gnome-session.target foo.service

(of course we'll have to have a gnome-classic-session.target too)

> - I added a gnome-session.service file, but systemd cannot attach it to the
> running gnome-session (which isn't started via D-Bus)
Yea, we just need to change how we're starting gnome-session.
Comment 59 Bastien Nocera 2016-10-03 21:52:53 UTC
Comment 58 along with https://www.youtube.com/watch?v=hq18daxTkLA tells me that we're not quite ready for using systemd to start gnome-settings-daemon bits.

But, what we can do, is commit only some of those patches, and start X different .desktop targets from gnome-session instead of a single one. I've filed bug 772370 about this.
Comment 60 Martin Pitt 2016-10-04 07:13:16 UTC
(In reply to Ray Strode [halfline] from comment #58)
> So Martin Pitt is actually doing some work in this area, and he added some
> new targets we should make use of.  See
> 
> https://github.com/systemd/systemd/pull/3678
> https://github.com/systemd/systemd/pull/3848

I haven't announced this widely yet as currently the required startup script is still quite horrible, and this should be worked out first before we start upstreaming this at a large scale. After systemd.conf I actually cleaned that up quite a bit, and the only major outstanding thing is 
https://github.com/systemd/systemd/issues/3750 -- however, this has a relatively easy workaround (just add After=graphical-session-pre.target to every session service), and that workaround is now done in our run-systemd-session wrapper in Ubuntu.

> Basically we need to put in each of the service files:
> PartOf=graphical-session.target

Yes.

> Then the gnome-session.service should have:
> 
> BindsTo=graphical-session.target

No, I think gnome-session.target should have that -- i. e. the top-level thing (the GNOME session target) should define what its process leader is, services should IMHO not declare themselves to be that.


> We'll need to change it to instead:
> 
> systemctl --user start graphical-session-pre.target
> systemctl --user start gnome-session.target

Explicitly starting -pre.target is now not actually required any more (that was one of the workarounds). However, you need to add --wait so that the systemctl command waits until the .target is stopped again (the Exec= line is the process leader, if it exits the whole  session is stopped and gdm tosses you back to the login screen).
Comment 61 Bastien Nocera 2016-10-04 08:58:17 UTC
Rebased versions of those patches are now in.

Attachment 336631 [details] pushed as f95dc22 - build: Bump version to current development version
Attachment 336641 [details] pushed as 61f8c51 - housekeeping: Fix typo in DISTCLEANFILES
Attachment 336645 [details] pushed as a41174d - power: Add missing source to stand-alone program
Attachment 336650 [details] pushed as 8fdc5b1 - common: Remove obsolete key parsing test program
Attachment 336652 [details] pushed as c4ae361 - build: Require lcms2 version 2.2 minimum
Attachment 336657 [details] pushed as dd0dba2 - color: Fix self-test
Comment 62 Bastien Nocera 2016-10-04 09:50:40 UTC
Attachment 336635 [details] pushed as 32b8dd0 - clipboard: Add test application
Attachment 336636 [details] pushed as e29a17f - color: Add test application
Comment 63 Ray Strode [halfline] 2016-10-04 19:53:01 UTC
(In reply to Martin Pitt from comment #60)
> > Then the gnome-session.service should have:
> > 
> > BindsTo=graphical-session.target
> 
> No, I think gnome-session.target should have that
oops, that's actually what I meant. I was transcribing from the systemd.special man page, and somehow introduced a thinko.

> Explicitly starting -pre.target is now not actually required any more (that
> was one of the workarounds). However, you need to add --wait so that the
> systemctl command waits until the .target is stopped again (the Exec= line
> is the process leader, if it exits the whole  session is stopped and gdm
> tosses you back to the login screen).
Oh, good.  So since this is turning into one command, we should probably augment the /usr/share/xsession file format to allow specifying a systemd target, then GDM can just do it itself, and we can ditch the wrapper script altogether.  We'll still need to do environment setup, but we can do that during graphical-session-pre instead of from the gnome-session wrapper. 

or maybe pam_systemd could set the $XDG_RUNTIME_DIR/systemd/user/default.target symlink itself based on the XDG_SESSION_DESKTOP that GDM passes to it?
Comment 64 Alan Jenkins 2016-10-04 20:06:08 UTC
Hi, n00b jhbuild user here.

These commits broke my build.  I manually applied the LIBM and dummy patches, then the build succeeded.
Comment 65 Bastien Nocera 2016-10-04 20:27:52 UTC
(In reply to Alan Jenkins from comment #64)
> Hi, n00b jhbuild user here.
> 
> These commits broke my build.  I manually applied the LIBM and dummy
> patches, then the build succeeded.

They worked fine here, so you might want to attach a build log. I'd probably have got yelled at if it broke the automated builds as well.
Comment 66 Alberts Muktupāvels 2016-10-05 11:55:59 UTC
Making all in color
make[3]: Entering directory '/mnt/wd20ezrx-00dc0b0/JHBuild/3.22/checkout/gnome-settings-daemon/plugins/color'
  CC       libcolor_la-gcm-edid.lo
  CC       libcolor_la-gsd-color-calibrate.lo
  CC       libcolor_la-gsd-color-manager.lo
  CC       libcolor_la-gsd-color-profiles.lo
  CC       gsd_test_color-test-color.o
  CC       gsd_test_color-gcm-edid.o
  CC       libcolor_la-gsd-color-state.lo
  CC       libcolor_la-gsd-color-plugin.lo
  CC       gsd_test_color-gsd-color-calibrate.o
  CC       gsd_test_color-gsd-color-manager.o
  CC       gsd_test_color-gsd-color-profiles.o
  CC       gsd_test_color-gsd-color-state.o
LC_ALL=C /usr/bin/intltool-merge -d -u -c ../../po/.intltool-merge-cache ../../po color.gnome-settings-plugin.in color.gnome-settings-plugin
Found cached translation database
Merging translations into color.gnome-settings-plugin.
  CCLD     gsd-test-color
  CCLD     libcolor.la
/usr/bin/ld: gsd_test_color-gcm-edid.o: undefined reference to symbol 'pow@@GLIBC_2.2.5'
//lib/x86_64-linux-gnu/libm.so.6: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
Makefile:876: recipe for target 'gsd-test-color' failed
make[3]: *** [gsd-test-color] Error 1
make[3]: *** Waiting for unfinished jobs....
ar: `u' modifier ignored since `D' is the default (see `U')
make[3]: Leaving directory '/mnt/wd20ezrx-00dc0b0/JHBuild/3.22/checkout/gnome-settings-daemon/plugins/color'
Comment 67 Alberts Muktupāvels 2016-10-05 12:08:20 UTC
Making all in dummy
make[3]: Entering directory '/mnt/wd20ezrx-00dc0b0/JHBuild/3.22/checkout/gnome-settings-daemon/plugins/dummy'
make[3]: *** No rule to make target 'test-dummy.c', needed by 'gsd_test_dummy-test-dummy.o'.  Stop.
make[3]: *** Waiting for unfinished jobs....
  CC       libdummy_la-gsd-dummy-manager.lo
  CC       libdummy_la-gsd-dummy-plugin.lo
make[3]: Leaving directory '/mnt/wd20ezrx-00dc0b0/JHBuild/3.22/checkout/gnome-settings-daemon/plugins/dummy'
Comment 68 Bastien Nocera 2016-10-05 12:30:48 UTC
Both of those are fixed in master:
commit 818a6e99a51a18c01b50a661a641bd009136922d
Author: Bastien Nocera <hadess@hadess.net>
Date:   Wed Oct 5 14:29:54 2016 +0200

    color: Fix gsd-test-color linkage
    
    https://bugzilla.gnome.org/show_bug.cgi?id=741688#c66

commit 70e90f817bff29eedd3d649460048404cfcf66f1
Author: Bastien Nocera <hadess@hadess.net>
Date:   Wed Oct 5 14:26:09 2016 +0200

    dummy: Add missing test-dummy.c source
    
    https://bugzilla.gnome.org/show_bug.cgi?id=741688#c67
Comment 69 Alan Jenkins 2016-10-05 13:38:56 UTC
lol, apparently I'm also a n00b at gnome bugzilla.  There was a conflict when I commented, & I chose a resolution which left me off the bug CC list.

To confirm, the above commits fix the problem I was having.  Thanks all.
Comment 70 Alberts Muktupāvels 2016-10-13 15:58:45 UTC
Created attachment 337620 [details] [review]
dummy: fix build

/usr/bin/ld: gsd_dummy-main.o: undefined reference to symbol 'g_dbus_proxy_call'
/mnt/wd20ezrx-00dc0b0/JHBuild/3.24/install/lib/libgio-2.0.so.0: error adding symbols: DSO missing from command line
Comment 71 Alberts Muktupāvels 2016-10-13 16:40:21 UTC
Also gsd-color segfaults...

Problem comes from gcm_session_get_output_edid in gsd-color-state.c. When edid is found in cache NULL is returned not edid. That leads to segfault when trying to access error->message.
Comment 72 Bastien Nocera 2016-10-13 16:43:32 UTC
(In reply to Alberts Muktupāvels from comment #71)
> Also gsd-color segfaults...
> 
> Problem comes from gcm_session_get_output_edid in gsd-color-state.c. When
> edid is found in cache NULL is returned not edid. That leads to segfault
> when trying to access error->message.

Please file new bugs. This bug is about using systemd to handle the g-s-d daemons.
Comment 73 Bastien Nocera 2017-02-14 16:27:03 UTC
Comment on attachment 337620 [details] [review]
dummy: fix build

Already fixed in bug 772900
Comment 74 Iain Lane 2017-12-06 17:24:56 UTC
Just doing some first research.

(In reply to Ray Strode [halfline] from comment #58)
> (In reply to Bastien Nocera from comment #56)
> > - attachment 336654 [details] [review] [review] [review] doesn't work because we're started so
> > early that 1) XDG_CURRENT_DESKTOP isn't set 2) XDG_SESSION_DESKTOP isn't set
> > either even though pam_systemd is supposed to set it up when logging in!
> Those things are actually set by GDM.  The problem is they are set in the
> session environment, not the systemd --user environment. But
> /usr/bin/gnome-session pushes them up to the systemd --user environment with
> its dbus-update-activation-environment call, so if the session is kicked off
> like I outlined above, they should be set.

Since 52a3c15a1d756c559402cc9505926a9b9d6cf3a7 this comment isn't true any more. /usr/bin/gnome-session-binary is now uploading the environment into systemd itself.

I think maybe split this out into a separate binary that's called early (Before=graphical-session-pre.target or from /usr/bin/gnome-session again)?
Comment 75 GNOME Infrastructure Team 2019-03-20 11:25:15 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-settings-daemon/issues/262.