Re: [gdm-list] A review of patches in Debian packaging

[Ray Strode <halfline gmail com>]

Hi,
>> 01_minimal_uid.patch
>>         Debian-specific. Non-system UIDs start at 1000 for us.
>>         What would be nice would be a way to specify this information as
>>         a configure option.
>
> The old GDM allowed sysadmins or distros to configure this in the GDM
> daemon configuration.  Perhaps it makes sense to add this configuration
> option back so distros can set it as needed in their default
> configuration file.
since it's distro specific, a configure option seems better than a
config file option.  Note there's an open bug about parsing login.defs
which probably makes sense here.

>> 05_stop_welcome_session.patch
>>         Since the gnome-session and dbus-daemon processes for the login
>>         manager are killed before starting the user session,
>>         --exit-with-session is not appropriate. On the contrary it will
>>         start a watcher process that remains after the user has logged
>>         on.
>
> +1 to get this patch upstream.  I'd recommend filing a bug in
> bugzilla.gnome.org in the "gdm" category.
right leaking the watcher processes sounds bad.  If that's happening
we should get the fix upstream. It's probably fall out from dropping
the "kill the whole process group" stuff we used to have in gdm.
>
>> 06_first_vt.patch
>>         This patch reintroduces the TTY manager from GDM 2.20 instead of
>>         letting X decide on which VT to run. Letting X decide makes it
>>         start on tty1 and crash as soon as getty does the same. There
>>         was broad consensus among Debian developers that neither GDM nor
>>         graphical boot thingies should not interfere with tty1 to tty6.
>
> Ray and Jon are more familiar with how the new GDM works with display
> switching, so I'll defer to their thoughts about this.
why is gdm getting started before the gettys?
>
>> 07_libexec-paths.patch
>>         Various things in the build system suppose that $libexecdir is
>>         the same for GDM and other packages: ConsoleKit, g-s-d,
>>         PolicyKit. This excludes FHS-compliant systems, which don’t have
>>         a /usr/libexec. Currently we hardcode the paths instead, but
>>         this should really be detected with pkg-config.
>
> Yes, this should be detected with pkg-config.  Using configure options
> should also be acceptable if the information about an install location
> is not available via pkg-config.  I'd think that if this were made
> appropriately configurable, it should go upstream.
I believe there's already a bug about this.

>> 08_frequent-users_greeter.patch
>>         To be sure the greeter doesn’t show users that shouldn’t be here
>>         for one reason or another, this patch sets the session type to
>>         "gdm" for user sessions, and requests to CK only sessions
>>         corresponding to this type.
>
> I'm not clear about what this patch does, or what problem it is trying
> to solve.  Is there a bug report that we could review, or any additional
> information you could provide?
So you don't want users that login at a getty to show up in the gdm user list?
>
>> 09_default_session.patch
>>         Use a default session set through alternatives instead of
>>         hardcoding gnome.session. See GNOME #594733
>
If we can't find gnome.desktop then we pick the first available
session in the directory (sorted), so you should be able to create a
symlink using alternatives in there with a name like 00-blah.desktop




>> 18_switch_kill_greeter.patch
>> 20_endsession_respawn.patch
>>         As explained in GNOME #618047, user switching leaves a X server
>>         behind, which is not acceptable. It also has inconsistencies in
>>         terms of interface. All of it is built on the premise that X
>>         switches VTs by itself when dying, which is not right when
>>         launched from a display manager. Luckily there is a X server
>>         flag to disable this behavior (-novtswitch) so we have reworked
>>         user experience around it.
>
> I think Jon or Ray is the best person to look over this.
Last time I looked at -novtswitch it didn't work properly, and when I
talked to X people about it they said it wasn't likely to work
properly.  That was a long time ago though, maybe things are better
now.

>> 21_schemas_usr.patch
>>         Put gdm.schemas in /usr, it has nothing to do in /etc. The patch
>>         is probably missing the corresponding documentation update.
>
> If there is a good reason to move the schemas to /usr, then I'd file a
> bug report and explain the reasoning.
This change makes a ton of sense. I have no idea why we put it in /etc
in the first place.  It's not a config file so it doesn't belong in
/etc.

--Ray