Bug 228111

See bug #228079 comment #10 for an explanation of the segfault.

Created attachment 152864 [details]
KDM patch to support ConsoleKit

The patch I'm attaching adds ConsoleKit support to KDM. I lifted some code
(most of it, actually, especially most of consolekit.[ch]) from GDM, but that
should be OK thanks to the GPL. :-)

WARNING: This hasn't tested beyond "it compiles" (which it does, at least on my
FC6 system; note that it ConsoleKit-devel is not required to build, only
dbus-devel). I hope some of the brave Rawhide users can help out with the
testing.

I'm also going to attach a specfile patch.

Created attachment 152865 [details]
kdebase specfile patch to include and apply the KDM ConsoleKit patch

This is the patch to apply to kdebase.spec.

NOTE: To test-build this on FC6, I also changed:
BuildRequires: libsmbclient-devel
to:
BuildRequires: samba >= 2.2.3a-5
but as this would actually be incorrect for F7, I reverted this before taking
the diff.

Than, could you apply/test this patch asap?  I'd *really* like to see this in
f7t4 if at all possible.

it's now included in kdebase-3.5.6-6.fc7, which will be built today. Kevin, 
thanks for the patch, great work!

Kevin, have you tested kdm built with this patch?  I have built it on my test 
machine, it just crashed when i tried to login. 

Prolly not, per comment #2 : WARNING: This hasn't tested beyond "it compiles" 
:(

So let's reopen this bug. :-( (I can't do it, I don't have fedorabugs 
permissions.)
I guess what's wrong is that D-Bus needs to be integrated with the main loop 
somehow (using 
the "dbus_connection_set_watch_functions", "dbus_connection_set_timeout_functions" 
and "dbus_connection_set_wakeup_main_function" functions and callbacks), 
there's a comment where those functions could be used. Unfortunately, KDM uses 
a custom main loop, not any of the standard ones (glib, Qt, ...) where 
it's "obvious" what the callbacks should do. :-( Someone needs to look into 
this further, but the bad news is that I certainly won't be able to do that 
before Thursday (i.e. 2 days from now).

Comment on attachment 152864 [details]
KDM patch to support ConsoleKit

Marking patch obsolete as it doesn't work, even without having a replacement
yet. :-(

(In reply to comment #8)
> I guess what's wrong is that D-Bus needs to be integrated with the main loop 
> somehow

You don't need D-Bus mainloop integration if you just use blocking calls. You
may also want to check bug 237621 where there is talk of achieving the same
thing using the PAM module and setting a variable. HTH.

FYI, pam_ck_connector , bug #228110 comment #15

If it's not mainloop integration that's needed, then why is it locking up?

Created attachment 153524 [details]
Diff between GDM ConsoleKit code and my KDM one

I don't think there's any logic bugs in my code because I'm using the exact
same algorithm as the known-good GDM. I've attached the diff between GDM's
ConsoleKit code and mine which makes this obvious. The only thing which is in
GDM's code and not in mine is mainloop integration: GDM calls
dbus_connection_setup_with_g_main, which I can't do in KDM because it doesn't
use GLib. That's why if mainloop integration isn't it, then I don't know why
GDM works and my patched KDM doesn't. Unfortunately, I don't have a Rawhide
machine here to do any debugging on this. :-(

Removing "blocks F7Test4" as it's obviously too late for that now. ;-( Also a 
nice way to try out my newly-acquired fedorabugs powers. ;-) Hopefully we can 
get this sorted out for the final.

I'm currently trying hard to debug this in QEMU. I know the problem is a 
segfault, but I don't know yet where exactly.

Than, could you please install that kdebase RPM on your test machine again, 
along with the corresponding -debuginfo RPM and Valgrind, and then run:
valgrind --trace-children=yes kdm -nodaemon
from runlevel 3? This is probably the only way we'll ever figure out where the 
segfault is happening, and Valgrind doesn't appear to work properly under QEMU, 
so I'm stuck.

Here's an e-mail from S.Çağlar Onur from Pardus. Than, I'm going to update my 
patch with his fix, so no debugging is needed, just fixing the patch in CVS and 
rebuilding.

===

Hi all;

While searching ConsoleKit support of KDM i found that [1] so i want to give 
it a try and i also reproduced the problem mentioned there, i had some free 
time to debug and here is the solution, it works perfectly for me :). 

I mailed you directly cause i don't have a bugzilla account @ 
bugzilla.redhat.com. I hope this wont create a problem.

--- kdebase-3.5.6-consolekit-kdm.patch.orig     2007-04-28 05:38:01.098664155 
+0300
+++ kdebase-3.5.6-consolekit-kdm.patch  2007-04-28 05:38:12.596910026 +0300
@@ -795,7 +795,7 @@
                SetupDisplay( td_setup );
  
 +#ifdef WITH_CONSOLE_KIT
-+      ck_session_cookie = open_ck_session (getpwnam(d->userName), d);
++      ck_session_cookie = open_ck_session (getpwnam(curuser), d);
 +      if (!(clientPid = StartClient(ck_session_cookie))) {
 +#else
        if (!(clientPid = StartClient())) {


[caglar@zangetsu][~/svk/devel/desktop/kde/base/kdebase/files]> 
ck-list-sessions
Session9:
        uid = '1000'
        realname = 'S.Çağlar Onur'
        seat = 'Seat1'
        session-type = ''
        active = TRUE
        x11-display = ':0'
        x11-display-device = '/dev/tty7'
        display-device = ''
        remote-host-name = ''
        is-local = TRUE
        on-since = '2007-04-28T02:37:13Z'

[1] https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228111

Cheers
-- 
S.Çağlar Onur <caglar.tr>
http://cekirdek.pardus.org.tr/~caglar/

Linux is like living in a teepee. No Windows, no Gates and an Apache in house!

Created attachment 153710 [details]
KDM patch to support ConsoleKit (fixed)

Attaching the patch with S.Çağlar Onur's fix. Thanks!

Sebastian Vahl, once the updated kdebase has been built by Than Ngo and tagged 
for Rawhide by releng, it would be nice if you could make sure a KDE live CD 
snapshot with the new kdebase and KDM set as default is torrented for wider 
testing.

(In reply to comment #19)
> Sebastian Vahl, once the updated kdebase has been built by Than Ngo and tagged 
> for Rawhide by releng, it would be nice if you could make sure a KDE live CD 
> snapshot with the new kdebase and KDM set as default is torrented for wider 
> testing.

I will test this tomorrow (or when I'm getting the new kdebase). If it's working
for me I'll ask Jeremy if he's planing to release new rawhide-livecds. If not
I'll ask Rex if he will provide them as a torrent.

We should also get redhat-artwork-kde (#236792, built, waiting for signature) 
into the spin, KDM warns about a missing background image without it.

Shouldn't this be required by kdebase? But I can include this manually into the
spin when it' getting into rahide (and this bug is getting OT :)

Not possible before either kdebase moves to Extras or the merge happens (but in 
the latter case, redhat-artwork-kde should probably be merged back into 
redhat-artwork anyway).

Ok. right. I'll will include redhat-artwork-kde manually then.

Actually, there are currently packages in core which depend on packages in
extras (namely cdrkit, which many things depend on now that cdrwtools are gone
from Core).  So it's not an absolute prohibition, but I don't know what hacks
are required to make it work.  If you have a legitimate need to do that before
the merge, talk to releng.

If any other things BuildRequire such a package, it will choke the build system 
though, I don't know off hand if really nothing in Core BuildRequires kdebase.

it's non built in rawhide

it's now built in rawhide

And redhat-artwork-kde has now been pushed to extras-development, so it looks 
like everything should be in place for a new KDE Live CD snapshot with the next 
Rawhide push.

No, it's not in Rawhide:
http://www.redhat.com/archives/fedora-devel-list/2007-May/msg00019.html
Due to the freeze, it's not enough to build for dist-fc7 now, this needs to be 
explicitly pushed to f7final by Release Engineering.

It's "f7-final" actually, but the name doesn't matter because releng has to do 
it anyway. See:
https://www.redhat.com/archives/fedora-maintainers/2007-April/msg00582.html

*** Bug 228151 has been marked as a duplicate of this bug. ***

Now pushed to Rawhide by release engineering, thanks!
http://www.redhat.com/archives/fedora-devel-list/2007-May/msg00097.html

Created attachment 158502 [details]
First attempt at KDE 4 port of the patch

Here's a first attempt at porting this patch to KDE 3.91.0 (4.0 alpha 2).

WARNING: This comes "straight from the presses", I haven't tried compiling it
yet, let alone running the result. In particular, the CMake stuff may need more
work. (KDE only had checks for QtDBus and I needed the low-level D-Bus, so I
lifted the check from libzypp, edited it a bit to handle LIB_SUFFIX in a better
way, and added the FindPackage to workspace/kdm/backend/CMakeLists.txt and
${DBUS_LIBRARY} to the target_link_libraries in the same file.)

Moreover, I think this will need to be fully conditionalized before we can send
it upstream, right now #define WITH_CONSOLE_KIT is still hardcoded in dm.h.

Created attachment 158925 [details]
kdebase4 specfile patch to include and apply the KDM ConsoleKit patch

Here's the specfile patch against the current kdebase4.spec in kde-redhat
unstable. I'm currently testing compilation.

This fails to build.
.../workspace/kdm/backend/consolekit.c:31:23: error: dbus/dbus.h: No such file 
or directory
I need to tweak my CMake stuff. :-(

Kevin, please open a separate bug for kde4 support (and/or file the rfe upstream
at bugs.kde.org).

Created attachment 158937 [details]
KDE 4 port of the patch (actually compiling)

This version of the patch actually compiles. (I added the missing
include_directories line.)

separate bug: Well, I was unsure against what to file it against as KDE 4 isn't
in Fedora yet, and I was more concerned about actually getting work done. ;-)

upstream: I'm going to move this upstream now that it compiles, but I think it
needs more work to be acceptable upstream: as I said further up this thread,
the ConsoleKit support isn't fully conditionalized (there's #ifdef
WITH_CONSOLE_KIT everywhere in the code, but the #define is currently hardcoded
rather than being set with CMake magic; this was the same in the KDE 3 version
of the patch) and I don't think upstream will want to make ConsoleKit a hard
requirement for KDE 4.