Red Hat Bugzilla – Full Text Bug Listing
|Summary:||Review Request: ConsoleKit - System daemon for tracking seats and sessions|
|Product:||[Fedora] Fedora||Reporter:||Matthias Clasen <mclasen>|
|Component:||Package Review||Assignee:||Ray Strode [halfline] <rstrode>|
|Status:||CLOSED RAWHIDE||QA Contact:||Fedora Extras Quality Assurance <extras-qa>|
|Fixed In Version:||Doc Type:||Bug Fix|
|Doc Text:||Story Points:||---|
|Last Closed:||2007-01-12 15:38:12 EST||Type:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
|Bug Depends On:|
Description Matthias Clasen 2007-01-11 23:00:30 EST
Spec URL: http://people.redhat.com/mclasen/ConsoleKit/ConsoleKit.spec SRPM URL: http://people.redhat.com/mclasen/ConsoleKit/ConsoleKit-0.1.0-1.src.rpm Description: ConsoleKit is a system daemon for tracking seats and sessions. It provides asynchronous notification via the system message bus. ConsoleKit is a central part of the effort to make fast user switching work well for FC7.
Comment 1 Matthias Clasen 2007-01-11 23:19:14 EST
I should have mentioned that still needs to go in core, since gdm will depend on it.
Comment 2 Ray Strode [halfline] 2007-01-11 23:23:40 EST
the description is a little terse. the words "seats" and "sessions" have a specific undefined meaning for instance.
Comment 3 Ray Strode [halfline] 2007-01-11 23:25:05 EST
in the last package i reviewed you said you didn't think having specific version for the build requires was a good idea.
Comment 4 Ray Strode [halfline] 2007-01-11 23:26:20 EST
If the tarball name is console-kit, why are we calling the package ConsoleKit ? why are we specifically passing -n to %setup?
Comment 5 Ray Strode [halfline] 2007-01-11 23:29:12 EST
you need a Requires(preun): service i think
Comment 6 Ray Strode [halfline] 2007-01-11 23:37:27 EST
do we really want to be shipping a binary that upstream explicitly made noinst? and giving it our own name nontheless?
Comment 7 Ray Strode [halfline] 2007-01-11 23:39:38 EST
The init scripts have bogus priority numbers. why is it getting started so late?
Comment 8 Matthias Clasen 2007-01-11 23:41:45 EST
Re comment 2, from the README: What is a seat? 8 =============== 9 10 A seat is a collection of sessions and a set of hardware (usually at 11 least a keyboard and mouse). Only one session may be active on a 12 seat at a time. 13 14 What is a session? 15 ================== 16 17 A session is a collection of all processes that originate from a single 18 common ancestor and retain knowledge of a secret. As an implementation 19 detail, this secret may be stored in the process environment by the 20 login manager under the name XDG_SESSION_COOKIE. That reminds me that I should include the README ChangeLog, etc in the rpm Re comment 3: True. This spec was stolen from davidz... Re comment 4: I don't know. This is the way William Jon McCann shipped it. The project itself is clearly called ConsoleKit (see e.g. the README), but the tarball is named console-kit. David decided to go with the project name for the rpm, which is explicitly allowed by the package naming guidelines. Re comment 5: /sbin/service is part of initscripts, which is on the exceptions list.
Comment 9 Ray Strode [halfline] 2007-01-11 23:47:54 EST
can you put a complete url in the source line?
Comment 10 Matthias Clasen 2007-01-11 23:49:19 EST
Comment 11 Ray Strode [halfline] 2007-01-11 23:51:12 EST
ConsoleKit seems sort of low level. If we start it at the end, it means no other services will be able to use it... We made this same mistake with hal and networkmanager for what its worth. We shouldn't repeat it again.
Comment 12 Matthias Clasen 2007-01-12 00:13:27 EST
Well, its currently started at the same time as hal and NetworkManager... Do we really want to deviate from upstream for that, without a concrete problem that would be solved by starting it earlier ? Updated: Spec URL: http://people.redhat.com/mclasen/ConsoleKit/ConsoleKit.spec SRPM URL: http://people.redhat.com/mclasen/ConsoleKit/ConsoleKit-0.1.0-2.src.rpm
Comment 13 Ray Strode [halfline] 2007-01-12 00:15:14 EST
it guess is shouldn't hold up the review, but it's something we need to fix for all three services.
Comment 14 Ray Strode [halfline] 2007-01-12 00:18:38 EST
We should fix the description to say what consolekit does in primitive terms. But with that change and assuming this passes rpmlint and succeeds in mock, it seems like it's good to go.
Comment 15 Mamoru TASAKA 2007-01-12 01:00:19 EST
* Commands under paths only used by previledged users (like /sbin/service) must be written with full path in scriptlets.
Comment 16 Matthias Clasen 2007-01-12 01:11:24 EST
We should probably require dbus explicitly, for the /etc/dbus-1/system.d directory
Comment 17 Matthias Clasen 2007-01-12 08:31:31 EST
New versions including the last 3 comments: Spec URL: http://people.redhat.com/mclasen/ConsoleKit/ConsoleKit.spec SRPM URL: http://people.redhat.com/mclasen/ConsoleKit/ConsoleKit-0.1.0-3.src.rpm
Comment 18 Jesse Keating 2007-01-12 11:34:17 EST
rpmlint has some interesting things to say: W: ConsoleKit conffile-without-noreplace-flag /etc/rc.d/init.d/ConsoleKit Are these really config files? W: ConsoleKit non-conffile-in-etc /etc/dbus-1/system.d/console-kit.conf This WOULD be a config, and should get (noreplace) maybe? E: ConsoleKit executable-marked-as-config-file /etc/rc.d/init.d/ConsoleKit Another dupe of above. W: ConsoleKit service-default-enabled /etc/rc.d/init.d/ConsoleKit I think this is fine yes? We _always_ want ConsoleKit running? W: ConsoleKit incoherent-subsys /etc/rc.d/init.d/ConsoleKit $servicename W: ConsoleKit incoherent-subsys /etc/rc.d/init.d/ConsoleKit $servicename W: ConsoleKit incoherent-subsys /etc/rc.d/init.d/ConsoleKit $servicename This is because "ConsoleKit" != the binary name. I think thats OK in this case right? W: ConsoleKit no-reload-entry /etc/rc.d/init.d/ConsoleKit Does 'reload' vs restart make any sense in this daemon? W: ConsoleKit incoherent-init-script-name ConsoleKit Dupe from above.
Comment 19 Ray Strode [halfline] 2007-01-12 11:45:29 EST
Note, despite its location /etc/dbus-1/system.d doesn't really house config files so much as policy files. i.e., it would be very rare for a sysadmin to want to change anything in that directory.
Comment 20 Bill Nottingham 2007-01-12 11:53:54 EST
The 'incoherent' messages are real errors. The lock file name in subsys *must* match the script file name.
Comment 21 Matthias Clasen 2007-01-12 12:23:00 EST
New versions including the last 3 comments: Spec URL: http://people.redhat.com/mclasen/ConsoleKit/ConsoleKit.spec SRPM URL: http://people.redhat.com/mclasen/ConsoleKit/ConsoleKit-0.1.0-4.src.rpm
Comment 22 Jesse Keating 2007-01-12 14:29:22 EST
Added to brew. Close when built for rawhide.
Comment 23 Matthias Clasen 2007-01-12 15:38:12 EST