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 729575 - Classic mode is broken on latest master
Classic mode is broken on latest master
Status: RESOLVED FIXED
Product: gnome-session
Classification: Core
Component: gnome-session
unspecified
Other Linux
: Normal major
: ---
Assigned To: Session Maintainers
Session Maintainers
: 733376 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-05-05 14:23 UTC by Vadim Rutkovsky
Modified: 2014-07-18 20:33 UTC
See Also:
GNOME target: 3.14
GNOME version: ---


Attachments
autostart: Stop messing with XDG_CURRENT_DESKTOP (3.72 KB, patch)
2014-05-06 14:31 UTC, Florian Müllner
committed Details | Review
autostart: Temporarily add back call to set_desktop_env() (1.06 KB, patch)
2014-05-08 06:08 UTC, Florian Müllner
none Details | Review
autostart: Replace should_show() with get_show_in() again (1.25 KB, patch)
2014-05-09 11:05 UTC, Florian Müllner
committed Details | Review
Set XDG_CURRENT_DESKTOP if unset (1.24 KB, patch)
2014-05-19 17:41 UTC, Owen Taylor
committed Details | Review

Description Vadim Rutkovsky 2014-05-05 14:23:09 UTC
Classic mode is broken in latest master - gdm displays the black window (http://build.gnome.org/continuous/buildmaster/builds/2014/05/05/18/smoketest-classic/work-gnome-continuous-x86_64-runtime/screenshot-10.png), the log doesn't have 'GNOME Shell started at' message (http://build.gnome.org/continuous/buildmaster/builds/2014/05/05/18/smoketest-classic/work-gnome-continuous-x86_64-runtime/journal.txt)

Commit https://git.gnome.org/browse/gnome-shell-extensions/commit/data?id=110e747e0440265535c8df09017f43b300c6da7e triggers the issue, so the actual problem might be in commit https://git.gnome.org/browse/gdm/commit/?id=af384da3c72efee40dcba48bb0e3e2ad6a08f6e4

This also happens on rawhide's gnome-classic-session-3.13.1-1.fc21.noarch
Comment 1 Florian Müllner 2014-05-06 14:31:50 UTC
Created attachment 275986 [details] [review]
autostart: Stop messing with XDG_CURRENT_DESKTOP

The use of the XDG_CURRENT_DESKTOP variable predates the actual
standardization that only happened recently, and indeed does not
work when the variable contains a colon-separated list as allowed
by the new standard. Stop using it altogether, so GIO can handle it
for us (which it will soon).
Comment 2 Matthias Clasen 2014-05-07 02:14:28 UTC
Review of attachment 275986 [details] [review]:

ok
Comment 3 Florian Müllner 2014-05-07 07:12:09 UTC
Attachment 275986 [details] pushed as 35044a5 - autostart: Stop messing with XDG_CURRENT_DESKTOP
Comment 4 Vadim Rutkovsky 2014-05-08 00:56:40 UTC
This commit doesn't fix the issue and also breaks the gnome session (at least the timed login, see recent builds on gnome-continuous - gnome-shell gets stuck in gdm mode)
Comment 5 Florian Müllner 2014-05-08 06:07:52 UTC
(In reply to comment #4)
> This commit doesn't fix the issue

As expected:

(In reply to comment #1)
> Stop using it altogether, so GIO can handle it for us (which it will soon).

So the change depends on yet-to-happen GIO changes Ryan is working on. I'll attach an (untested) patch that should work while we are waiting for the GIO part to land.
Comment 6 Florian Müllner 2014-05-08 06:08:29 UTC
Created attachment 276127 [details] [review]
autostart: Temporarily add back call to set_desktop_env()

XDG_CURRENT_DESKTOP support in GIO has not landed yet, so return
to setting the desktop environment manually to avoid a broken
session in the meantime.
Comment 7 Vadim Rutkovsky 2014-05-08 12:24:54 UTC
Last patch didn't solve it for me - regular, timed and automatic login are broken, initial setup didn't startup
Comment 8 Allison Karlitskaya (desrt) 2014-05-08 13:08:36 UTC
See bug 729813 for the GIO side of things.

Vadim: are you sure that your issues were caused by this and not the fallout that happened with (nullable) and (optional) at approximately the same time?
Comment 9 Vadim Rutkovsky 2014-05-08 13:20:48 UTC
(In reply to comment #8)
> Vadim: are you sure that your issues were caused by this and not the fallout
> that happened with (nullable) and (optional) at approximately the same time?
Yes, reverted patch from comment 1 and the session logs in correctly.
Comment 10 Florian Müllner 2014-05-09 11:05:27 UTC
Created attachment 276230 [details] [review]
autostart: Replace should_show() with get_show_in() again

GDesktopAppInfo's should_show() implementation also considers the
NoDisplay flag, which is generally used by core components like
window managers. Return to use get_show_in(), which just checks
checks ShowOnlyIn and friends based on XDG_CURRENT_DESKTOP to
get the session back working.
Comment 11 Florian Müllner 2014-05-09 11:13:40 UTC
Comment on attachment 276230 [details] [review]
autostart: Replace should_show() with get_show_in() again

Attachment 276230 [details] pushed as 8c73ddc - autostart: Replace should_show() with get_show_in() again

Pushing, as obviously correct. The GIO change has landed as well, so in theory the issue should be fixed. In practice, GDM(rawhide)+session(jhbuild) still fails to start here due to XDG_CURRENT_DESKTOP not being set/passed on - might be just the rawhide-git mix of course, so could you please check if continuous works now?
Comment 12 Vadim Rutkovsky 2014-05-10 00:21:05 UTC
(In reply to comment #11)
> could you please check if continuous works now?

Yes, works now (see http://build.gnome.org/#/build/20140509.45), thanks!
Comment 13 Owen Taylor 2014-05-19 17:41:30 UTC
Created attachment 276795 [details] [review]
Set XDG_CURRENT_DESKTOP if unset

Here's a patch I'm pushing (tested + email review from fmuellner +
irc review from rstrode)

It would be slightly more complete to do:

 if (g_getenv ("XDG_CURRENT_DESKTOP") == NULL))
   {
     if (strcmp (session_name, "gnome-classic.session") == 0)
       /* set environment variable to GNOME-Classic:GNOME */
     else
       /* set environment variable to GNOME */
   }

(needs moving lower in the file.) But don't want to spend the time
at the moment to get GNOME-Classic installed in my VM and test,
so pushing the simpler version in hopes that nobody will ever
notice the lack :-)

====

From 3.14 GDM sets XDG_CURRENT_DESKTOP. For compatibility with
older versions of GDM,  other display managers, and startx,
set a fallback value if we don't find it set.
Comment 14 Owen Taylor 2014-05-19 17:45:29 UTC
Comment on attachment 276795 [details] [review]
Set XDG_CURRENT_DESKTOP if unset

Attachment 276795 [details] pushed as 1db085c - Set XDG_CURRENT_DESKTOP if unset
Comment 15 Florian Müllner 2014-07-18 20:33:40 UTC
*** Bug 733376 has been marked as a duplicate of this bug. ***