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 741666 - should make gnome-shell dbus activatable
should make gnome-shell dbus activatable
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2014-12-17 17:30 UTC by Ray Strode [halfline]
Modified: 2021-07-05 14:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
core: move backend setting to helper function (3.53 KB, patch)
2014-12-17 18:34 UTC, Ray Strode [halfline]
reviewed Details | Review
core: start as wayland display server if run when there's a wayland session registered (5.83 KB, patch)
2014-12-17 18:34 UTC, Ray Strode [halfline]
reviewed Details | Review
data: drop mutter-wayland.desktop (2.21 KB, patch)
2014-12-17 18:34 UTC, Ray Strode [halfline]
none Details | Review
data: drop gnome-shell-wayland.desktop (4.02 KB, patch)
2014-12-17 18:34 UTC, Ray Strode [halfline]
needs-work Details | Review
ui: disable GL in gdk backend (2.23 KB, patch)
2014-12-19 20:38 UTC, Ray Strode [halfline]
committed Details | Review
core: move backend setting to helper function (3.52 KB, patch)
2014-12-19 20:39 UTC, Ray Strode [halfline]
committed Details | Review
core: start as wayland display server if run when there's a wayland session registered (5.51 KB, patch)
2014-12-19 20:39 UTC, Ray Strode [halfline]
committed Details | Review
data: drop mutter-wayland.desktop (2.20 KB, patch)
2014-12-19 20:39 UTC, Ray Strode [halfline]
committed Details | Review
data: merge gnome-shell.desktop and gnome-shell-wayland.desktop into one file (9.20 KB, patch)
2014-12-19 20:47 UTC, Ray Strode [halfline]
committed Details | Review
data: rename gnome-shell required component to org.gnome.Shell (1.35 KB, patch)
2015-11-07 04:38 UTC, Ray Strode [halfline]
committed Details | Review

Description Ray Strode [halfline] 2014-12-17 17:30:31 UTC
this will require 4 things as i understand it

1) renaming gnome-shell.desktop to org.gnome.Shell.desktop
2) adding DBusActivatable=true
3) Dropping gnome-shell-classic.desktop (already done see bug 741660 )
4) Dropping gnome-shell-wayland.desktop .  This requires "doing the right thing" based on the logind session type (see bug 741658 for the GDM work that was required)
Comment 1 Jasper St. Pierre (not reading bugmail) 2014-12-17 17:42:15 UTC
Why?
Comment 2 Ray Strode [halfline] 2014-12-17 18:25:06 UTC
For more details see: https://mail.gnome.org/archives/desktop-devel-list/2014-January/msg00081.html

tl;dr 
the plan is to make all gnome components dbus activatable, then use the glib APIs for activating those components instead of spawning them directly.  They'll get run inside the systemd user session this way and we'll be one step closer to our utopian future of flying cars.
Comment 3 Ray Strode [halfline] 2014-12-17 18:33:42 UTC
i need to test these, but here's my first draft...
Comment 4 Ray Strode [halfline] 2014-12-17 18:34:09 UTC
Created attachment 292921 [details] [review]
core: move backend setting to helper function

This paves the way for making the backend setting
be more automatic.
Comment 5 Ray Strode [halfline] 2014-12-17 18:34:13 UTC
Created attachment 292922 [details] [review]
core: start as wayland display server if run when there's a wayland session registered

This commit makes gets rid of the need for --display-server and
--wayland when mutter detects that a wayland session is registered.
Comment 6 Ray Strode [halfline] 2014-12-17 18:34:17 UTC
Created attachment 292924 [details] [review]
data: drop mutter-wayland.desktop

It's not needed anymore, now that we can automatically figure things out
based on logind.
Comment 7 Ray Strode [halfline] 2014-12-17 18:34:34 UTC
Created attachment 292925 [details] [review]
data: drop gnome-shell-wayland.desktop

We can autodetect what to do, based on the user session, so drop
the separate desktop file
Comment 8 Ray Strode [halfline] 2014-12-17 21:06:13 UTC
Review of attachment 292925 [details] [review]:

::: data/gnome-shell-wayland.desktop.in.in
@@ -11,3 @@
-OnlyShowIn=GNOME;
-NoDisplay=true
-X-GNOME-Autostart-Phase=DisplayServer

So one problem is gnome-shell.desktop uses the Autostart-Phase=WindowManager not DisplayServer.
Comment 9 Ray Strode [halfline] 2014-12-17 21:07:52 UTC
Given we already start the window manager before the initialization phase in wayland, i guess it's probably okay to start the window manager before the initialization phase in X too (since any ordering problems would crop up in wayland as well), so we can probably use DisplayServer for both.
Comment 10 Ray Strode [halfline] 2014-12-17 21:11:30 UTC
another idea is we could have two bus names... org.gnome.DisplayServer and org.gnome.Shell.  the former would be required for the wayland session definition and the latter would be required by the non-wayland session definition.
Comment 11 Jasper St. Pierre (not reading bugmail) 2014-12-18 20:40:58 UTC
Review of attachment 292922 [details] [review]:

::: src/core/main.c
@@ +303,3 @@
+  char **sessions = NULL;
+  char *session_id = NULL;
+  int result, i;

I usually use "ret" for this variable. Style preference.

@@ +307,3 @@
+  result = sd_pid_get_session (0, &session_id);
+
+  if (result == 0 && session_id != NULL)

I usually prefer:

    if (ret < 0)
      goto out;

@@ +354,3 @@
+    }
+
+  free (sessions);

g_strfreev ?

@@ +369,3 @@
+
+#ifdef HAVE_NATIVE_BACKEND
+  session_id = find_logind_session (&session_type);

Do we actually need both the session ID and the session type? You could make find_logind_session much simpler if you don't.

@@ +376,3 @@
+      is_wayland = g_strcmp0 (session_type, "wayland") == 0;
+      free (session_id);
+      free (session_type);

You should free in every case, in case it's possible to fill in the session type without filling in the session ID.

@@ +389,3 @@
+
+#if defined(HAVE_WAYLAND) && defined(HAVE_NATIVE_BACKEND)
+  session_type_is_wayland = check_for_wayland_session_type ();

Hm. This is guarded with HAVE_WAYLAND && HAVE_NATIVE_BACKEND, but check_for_wayland_session_type(); is only guarded with HAVE_WAYLAND_BACKEND, leading to an unused warning in one case.

You should be able to simplify by using the same guards in both places.
Comment 12 Jasper St. Pierre (not reading bugmail) 2014-12-18 20:42:08 UTC
Review of attachment 292921 [details] [review]:

::: src/core/main.c
@@ +305,3 @@
+  meta_set_is_wayland_compositor (opt_wayland);
+#endif
+

Extra whitespace.
Comment 13 Ray Strode [halfline] 2014-12-19 17:54:58 UTC
(In reply to comment #11)
> Review of attachment 292922 [details] [review]:
> 
> ::: src/core/main.c
> @@ +303,3 @@
> +  char **sessions = NULL;
> +  char *session_id = NULL;
> +  int result, i;
> 
> I usually use "ret" for this variable. Style preference.
sure, I actually usually use ret too, and even wrote ret this time and changed it to result at the last minute...not sure why.

> 
> @@ +307,3 @@
> +  result = sd_pid_get_session (0, &session_id);
> +
> +  if (result == 0 && session_id != NULL)
> 
> I usually prefer:
> 
>     if (ret < 0)
>       goto out;

This isn't actually a case of "if fail, quit".  the code first looks if you're in a session and if so uses it. Otherwise, if you're running outside of a logind session (as would be the case with systemd --user) then it looks for the session.  what i could do is

if (ret == 0)
  goto out;

and then do the sd_session_get_type code in the out: path.

> 
> @@ +354,3 @@
> +    }
> +
> +  free (sessions);
> 
> g_strfreev ?
systemd doesn't use g_malloc.  not that it matters in practice, since g_malloc by default calls malloc, but we generally try to pretend like it matters.

> Do we actually need both the session ID and the session type? You could make
> find_logind_session much simpler if you don't.
we need the session id to get the session type.  We don't need to return the session id. I can stop doing that.  I'm a little worried we're going to need the session id down the line and this hairy code will get duplicated, but we can cross that bridge when we get to it.
> 
> @@ +376,3 @@
> +      is_wayland = g_strcmp0 (session_type, "wayland") == 0;
> +      free (session_id);
> +      free (session_type);
> 
> You should free in every case, in case it's possible to fill in the session
> type without filling in the session ID.
That would be non-conventional.  most apis that have out args, leave those out args "undefined" if the call fails.

Of course this is all moot, since i'm going to change it to

session_type = find_logind_session_type ();
Comment 14 Ray Strode [halfline] 2014-12-19 20:38:51 UTC
Created attachment 293093 [details] [review]
ui: disable GL in gdk backend

initializing GL leads to deadlock, since it calls into xwayland,
which calls back into mutter.

This commit disables it. Longer term we should probably stop using
gtk inside mutter.
Comment 15 Ray Strode [halfline] 2014-12-19 20:39:04 UTC
Created attachment 293094 [details] [review]
core: move backend setting to helper function

This paves the way for making the backend setting
be more automatic.
Comment 16 Ray Strode [halfline] 2014-12-19 20:39:12 UTC
Created attachment 293095 [details] [review]
core: start as wayland display server if run when there's a wayland session registered

This commit makes gets rid of the need for --display-server and
--wayland when mutter detects that a wayland session is registered.
Comment 17 Ray Strode [halfline] 2014-12-19 20:39:20 UTC
Created attachment 293096 [details] [review]
data: drop mutter-wayland.desktop

It's not needed since we can automatically figure things out
based on logind.
Comment 18 Ray Strode [halfline] 2014-12-19 20:47:48 UTC
Created attachment 293097 [details] [review]
data: merge gnome-shell.desktop and gnome-shell-wayland.desktop into one file

We can autodetect what to do based on the user's session, so drop
the separate desktop files.

This, along with some future changes, will enable us to be bus
activatable.
Comment 19 Ray Strode [halfline] 2015-11-06 14:36:00 UTC
(In reply to Ray Strode [halfline] from comment #9)
> Given we already start the window manager before the initialization phase in
> wayland, i guess it's probably okay to start the window manager before the
> initialization phase in X too (since any ordering problems would crop up in
> wayland as well), so we can probably use DisplayServer for both.

So there actually are some ordering issues with wayland that we need to tackle.  see bug 738205
Comment 20 Ray Strode [halfline] 2015-11-06 19:25:58 UTC
Comment on attachment 293093 [details] [review]
ui: disable GL in gdk backend

this was committed some time ago.
Comment 21 Ray Strode [halfline] 2015-11-06 21:24:32 UTC
Comment on attachment 293094 [details] [review]
core: move backend setting to helper function

(getting this one in now because it's just a refactor)

Attachment 293094 [details] pushed as db4355b - core: move backend setting to helper function
Comment 22 Ray Strode [halfline] 2015-11-07 04:38:37 UTC
Created attachment 315022 [details] [review]
data: rename gnome-shell required component to org.gnome.Shell

gnome-shell and gnome-shell-wayland have been consolidated, and
now get dbus activated.  The new name is org.gnome.Shell.

This commit changes the session files to reference org.gnome.Shell.
Comment 23 Ray Strode [halfline] 2015-11-07 04:39:41 UTC
unless there's any complaints in the next couple days I'll probably push these out on monday sometime
Comment 24 Ray Strode [halfline] 2015-11-09 18:02:56 UTC
Attachment 293095 [details] pushed as f251f38 - core: start as wayland display server if run when there's a wayland session registered
Attachment 293096 [details] pushed as 7b20d15 - data: drop mutter-wayland.desktop
Comment 25 Ray Strode [halfline] 2015-11-09 18:03:49 UTC
Comment on attachment 293097 [details] [review]
data: merge gnome-shell.desktop and gnome-shell-wayland.desktop into one file

Attachment 293097 [details] pushed as b8e29ae - data: merge gnome-shell.desktop and gnome-shell-wayland.desktop into one file
Comment 26 Ray Strode [halfline] 2015-11-09 18:04:31 UTC
Attachment 315022 [details] pushed as 95cc979 - data: rename gnome-shell required component to org.gnome.Shell
Comment 27 Jonas Ådahl 2015-11-25 13:53:59 UTC
A patch from this series caused a regression. A filed it as bug 758658.
Comment 28 bluescreen_avenger 2015-11-28 05:32:09 UTC
It seems that this changes the way to start gnome-shell wayland from the command line? What's the new way?
Comment 29 Ray Strode [halfline] 2016-05-04 15:29:32 UTC
So I never actually did 2) in comment 0.

Reopening.
Comment 30 Ray Strode [halfline] 2016-05-04 19:09:46 UTC
note it's not enough to merely do 2) from comment 0 .  We have a few dependencies on $XDG_SESSION_ID in mutter and gnome-shell
Comment 31 Debarshi Ray 2016-08-29 09:16:04 UTC
Review of attachment 293097 [details] [review]:

::: data/gnome-shell.desktop.in.in
@@ +11,3 @@
 OnlyShowIn=GNOME;
 NoDisplay=true
+X-GNOME-Autostart-Phase=DisplayServer

This caused a HiDpi regression on X. See bug 765196
Comment 32 GNOME Infrastructure Team 2021-07-05 14:41:47 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of  gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new ticket at
  https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/

Thank you for your understanding and your help.