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 696262 - gsm_systemd_new() checks for systemd, not for logind
gsm_systemd_new() checks for systemd, not for logind
Status: RESOLVED FIXED
Product: gnome-session
Classification: Core
Component: gnome-session
unspecified
Other Linux
: Normal minor
: ---
Assigned To: Session Maintainers
Session Maintainers
3.8.1
Depends on:
Blocks:
 
 
Reported: 2013-03-21 10:14 UTC by Martin Pitt
Modified: 2013-04-04 13:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gsm_systemd_new: Check for logind, not for systemd (1.82 KB, patch)
2013-03-21 10:16 UTC, Martin Pitt
none Details | Review
gsm_systemd_new: Check for logind, not for systemd (1.81 KB, patch)
2013-03-21 11:45 UTC, Martin Pitt
none Details | Review
gsm_systemd_new: Check for logind, not for systemd (1.82 KB, patch)
2013-03-21 12:48 UTC, Martin Pitt
committed Details | Review
0001-build-Link-against-libsystemd-daemon-again.patch (1.05 KB, patch)
2013-04-01 13:32 UTC, Colin Walters
committed Details | Review

Description Martin Pitt 2013-03-21 10:14:29 UTC
gnome-session wants to talk to logind, but actually checks for systemd. It is
possible to build systemd without logind, in which case /sys/fs/cgroup/systemd
would still exist. For that reason, sd_booted() was recently fixed upstream to only be true if systemd init is active [1]

For running systemd init without logind, or for running logind without systemd init we need to change the check to test for logind.

For details, see [2] if you are interested.

[1] http://cgit.freedesktop.org/systemd/systemd/commit/?id=66e411811b8090
[2] https://mail.gnome.org/archives/desktop-devel-list/2013-March/msg00092.html
Comment 1 Martin Pitt 2013-03-21 10:16:03 UTC
Created attachment 239445 [details] [review]
gsm_systemd_new: Check for logind, not for systemd
Comment 2 Martin Pitt 2013-03-21 11:45:13 UTC
Created attachment 239456 [details] [review]
gsm_systemd_new: Check for logind, not for systemd

Sorry, the previous patch was wrong (reversed check).
Comment 3 Martin Pitt 2013-03-21 12:48:26 UTC
Created attachment 239462 [details] [review]
gsm_systemd_new: Check for logind, not for systemd

The previous patch had a confusing changelog. I think this is better.
Comment 4 Bastien Nocera 2013-03-27 11:35:12 UTC
Review of attachment 239462 [details] [review]:

> It is possible to build systemd without logind, in which case sd_booted() would
> still succeed.

Sure, but it's the opposite you're actually trying to achieve. I doubt that system integrators using systemd would want to use something other than logind for seat tracking.
Comment 5 Martin Pitt 2013-03-27 14:00:50 UTC
Ah, we could change the commit message to

  It is possible to build systemd without logind or run logind without systemd init, in both cases testing for systemd init is wrong. Check for /run/systemd/seats instead, as recommended by systemd upstream.

Sounds better?
Comment 6 Matthias Clasen 2013-03-28 02:22:07 UTC
Review of attachment 239462 [details] [review]:

sure
Comment 7 Martin Pitt 2013-03-29 09:58:34 UTC
Comment on attachment 239462 [details] [review]
gsm_systemd_new: Check for logind, not for systemd

Pushed to master with updated changelog. There is no gnome-3-8 branch yet, so I assume this is sufficient for going into 3.8.1.
Comment 8 Colin Walters 2013-04-01 13:11:25 UTC
This broke the gnome-ostree builder; 

"Drop the now unnecessary linking against libsystemd-daemon."

Linking now fails with:
/usr/lib/gcc/x86_64-gnomeos-linux/4.7.2/../../../../lib/libsystemd-id128.so: undefined reference to `sd_listen_fds'

I suspect this is actually a bug in systemd, but if you don't mind keeping this apparent overlinking for now, I'd appreciate it.
Comment 9 Colin Walters 2013-04-01 13:32:04 UTC
Created attachment 240306 [details] [review]
0001-build-Link-against-libsystemd-daemon-again.patch
Comment 10 Colin Walters 2013-04-01 13:37:16 UTC
Review of attachment 240306 [details] [review]:

Eh, no one online now, I'd like to get the gnome-ostree build green again.  Will investigate this for real later.
Comment 11 Martin Pitt 2013-04-04 05:43:09 UTC
Ah, thanks Colin. Indeed, I was away in the last few days.
Comment 12 Martin Pitt 2013-04-04 05:45:46 UTC
> At least some toolchains generate a libsystemd-login.so which doesn't
> have a DT_NEEDED on libsystemd-daemon, but it does actually need it.

Hm, which symbol in libsystemd-login (or -id128) needs libsystemd-daemon? I don't see any symbol, and neither library links against libsystemd-daemon here?
Comment 13 Colin Walters 2013-04-04 13:02:34 UTC
(In reply to comment #12)
> > At least some toolchains generate a libsystemd-login.so which doesn't
> > have a DT_NEEDED on libsystemd-daemon, but it does actually need it.
> 
> Hm, which symbol in libsystemd-login (or -id128) needs libsystemd-daemon? I
> don't see any symbol, and neither library links against libsystemd-daemon here?

This is likely fixed by:

http://cgit.freedesktop.org/systemd/systemd/commit/?id=d3b9e0ff4e9f1b0bb328dc57ca5507bac48a6615

But the overlinking here is pretty harmless, so if you don't mind let's keep it in a while?
Comment 14 Martin Pitt 2013-04-04 13:13:44 UTC
No, I don't mind at all. I just changed it initially to avoid unnecessary linking, but it doesn't really hurt of course.