GNOME Bugzilla – Bug 525157
With new gnome-session window manager might not be started...
Last modified: 2008-04-10 21:53:47 UTC
The new gnome-session autostart magic is totally awesome. However when testing todays's trunk I see that no window manager is started. I thought, oh yes all that needs to happen is that metacity install its .desktop file into the gnome/default-session directory. But, oh, not everyone want to use the wonderful metacity (they want fish in their cubes instead.) Okay, fair enough, everyone should really have their window manager's .desktop file in the per-user autostart directory. Oh. But this breaks in the case where you drop into the failsafe session, because as http://live.gnome.org/SessionManagement/NewGnomeSession says and also the code implements when in failsafe mode we only read from the default-session directory. Ahaha. Those dastardly dangerous window managers won't be started in our nice failsafe GNOME environment, ah, hhhm. So. My suggestion is also add a failsafe-session directory that is also read in failsafe mode and in here we can have the metacity.desktop file. Or, we could be a bit more ugly and in the GSM_SESSION_PHASE_WINDOW_MANAGER check to see if we have actually started anything in this phase and if not, use 'gnome-wm' to start something. So, boss(es), how should we do this?
There's a plan for handling non-default window managers and stuff, and the session-manager-side at least is already implemented: http://live.gnome.org/SessionManagement/RequiredComponents So why aren't you getting a window manager? Because metacity doesn't install its desktop file in any of the standard directories, so even though gnome-session knows it wants to launch "metacity.desktop", it can't find it. Oops. (It's possible there are other problems in the code too of course, but even if there weren't, it still wouldn't work because of this.) Your point about failsafe is still relevant, because the session-setup code doesn't do the RequiredComponents handling in failsafe mode, because the user may have accidentally messed up the RequiredComponents-related keys in GConf, which is exactly the sort of failing that failsafe is supposed to be safe against. So the default session is expected to include the required defaults (but it's possible for the user to override them in the non-failsafe case). [That said, there may still be arguments for having separate default-session and failsafe-session...] But anyway, according to the current Plan, your first suggestion was basically correct: there should be a metacity.desktop file in the default-session directory. However, I don't think that *metacity* should be installing it; the fact that metacity is the default window manager is not a fact about metacity, it's a fact about GNOME. Eg, if we decided in the future to make metacity *not* be the default window manager any more, we should be able to do that without having to put out a new release of metacity. Of course, we don't want to have our own copy of metacity.desktop in the gnome-session source code either, because it would get out-of-sync... So maybe we should revert having the default session be a directory, and instead make it a file, or a list-valued gconf key, which just gives the names of the .desktop files of the default session components ("metacity, gnome-panel, nautilus" or something like that), and then gnome-session will look for those .desktop files in the standard autostart and applications dirs, and launch them.
Some quick and straight-to-the-point comments: - I agree that the current directory-based default session has its drawbacks (well explained by danw already). - I prefer to use gconf instead of files because it feels more consistent with the existing required-components thing and for other practical reasons. - I prefer to have a separate registry for the failsafe session because it brings more flexibility on defining it and tends to simplify the code. Well, by default it will be the same as the default session but distros might want to define it differently. Working on patch now.
Created attachment 108805 [details] [review] First tentative patch. Review is welcome.
Yeah, looks good. The fact that it's possible for the user to override the failsafe session makes it a little less... failsafe. Is it possible for us to install a default value for it in the "mandatory" config source so the user can't override it? (But the admin still could via pessulus.) The comment above the call to append_required_apps() is now sort of wrong. You might be able to simplify append_default_apps() using g_key_file_load_from_data_dirs()? And do we really want to be looking into the wm-properties dir?
(In reply to comment #4) > Yeah, looks good. > > The fact that it's possible for the user to override the failsafe session makes > it a little less... failsafe. Is it possible for us to install a default value > for it in the "mandatory" config source so the user can't override it? (But the > admin still could via pessulus.) Hmm, you're right. I thought that providing ways to customize the failsafe session (independently of the default one) would be useful for admins. Let's keep the sane and simpler current aproach then (failsafe = only default apps). > You might be able to simplify append_default_apps() using > g_key_file_load_from_data_dirs()? And do we really want to be looking into the > wm-properties dir? Nice tip. Didn't know about this function. I added this directory (wm-properties) because, for some reason, this is where metacity installs its .desktop file. Maybe it should be installed in applications directory too?
(In reply to comment #5) > Hmm, you're right. I thought that providing ways to customize the failsafe > session (independently of the default one) would be useful for admins. Let's > keep the sane and simpler current aproach then (failsafe = only default apps). Well, that has the same problem; if the user changes the default session settings in gconf, then failsafe is no longer failsafe. That's why I was suggesting using the "mandatory" thing; because then the admin can change it, like you say, but the user can't mess it up. > I added this directory > (wm-properties) because, for some reason, this is where metacity installs its > .desktop file. Maybe it should be installed in applications directory too? I think so. But maybe there are arguments for doing it the other way too.
(In reply to comment #6) > (In reply to comment #5) > > Hmm, you're right. I thought that providing ways to customize the failsafe > > session (independently of the default one) would be useful for admins. Let's > > keep the sane and simpler current aproach then (failsafe = only default apps). > > Well, that has the same problem; if the user changes the default session > settings in gconf, then failsafe is no longer failsafe. That's why I was > suggesting using the "mandatory" thing; because then the admin can change it, > like you say, but the user can't mess it up. Yes but isn't the default session supposed to be failsafe? I mean, if you can't run a window manager, panel and nautilus, there's something really wrong. Maybe "default" is not a good name... > > I added this directory > > (wm-properties) because, for some reason, this is where metacity installs its > > .desktop file. Maybe it should be installed in applications directory too? > > I think so. But maybe there are arguments for doing it the other way too. I'll check that with the metacity guys.
Created attachment 108871 [details] [review] New version with general improvements I refactored the whole patch to make gnome_session_new() and append_default_apps() simpler. Also, I removed the special failsafe stuff for now. We can discuss this separately. I kept the wm-properties for now too.
yeah, that all looks about right
Ok, commited in trunk: 2008-04-10 Lucas Rocha <lucasr@gnome.org> Re-implemented the way we define the default session. Instead of a directory full of .desktop files, we now get the list of default apps from a gconf key. Then session manager then looks for those apps in the standard applications and autostart directories. This way we don't require default apps to export their .desktop files in a special directory. #525157, Rob Bradford. * data/gnome-session.schemas.in: added a new gconf key /desktop/gnome/session/default-session which stores the list of default session apps. * gnome-session/Makefile.am: no need to define default-session directory anymore. * gnome-session/gsm.h: added new constant called GSM_GCONF_DEFAULT_SESSION_KEY refering to new gconf key. * gnome-session/session.c (gsm_session_new, append_default_apps): new function to load default apps from gconf key. The gconf key only stores the application names and GsmSession looks for a respective .desktop file in some standard application and autostart directories. (get_autostart_dirs, get_app_dirs): new utility functions which return all autostart and applications directories respectively. Thanks for the reviews Dan! Rob, do you think all your issues are fixed with those changes?
Lucas, I'm in the process of jhbuilding up. Having looked at the patch I think everything will be hunky. Thanks dudes!