[startplasmacompositor] Add Wayland socket argument
AbandonedPublic

Authored by romangg on Sep 14 2018, 7:05 PM.

Details

Reviewers
None
Group Reviewers
KWin
Plasma
Summary

Argument will be forwarded to KWin.

Allows to start a full Wayland Plasma session from a second
VT while another Wayland session is active on the first one.

Diff Detail

Repository
R120 Plasma Workspace
Branch
kwinWaylandSocketArg
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 4915
Build 4933: arc lint + arc unit
romangg created this revision.Sep 14 2018, 7:05 PM
Restricted Application added a project: Plasma. · View Herald TranscriptSep 14 2018, 7:05 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
romangg requested review of this revision.Sep 14 2018, 7:05 PM
fvogt added a subscriber: fvogt.Sep 14 2018, 7:08 PM

I'm against this - kwin_wayland should just pick the next available socket if wayland-0 is not available. That's also what Xorg does and is IMO the expected behaviour.

I'm against this - kwin_wayland should just pick the next available socket if wayland-0 is not available. That's also what Xorg does and is IMO the expected behaviour.

Maybe this would be nice, haven't thought about it yet. But what speaks in any case against a manual override?

fvogt added a comment.Sep 14 2018, 8:11 PM

I'm against this - kwin_wayland should just pick the next available socket if wayland-0 is not available. That's also what Xorg does and is IMO the expected behaviour.

Maybe this would be nice, haven't thought about it yet. But what speaks in any case against a manual override?

It's not necessary AFAICT. Either a socket exists and can't be used, or it does not exist and does not matter.

Is there a real reason against it besides "not necessary"? Maybe that we can't remove the manual override again without breaking some API promise once we have the grand solution of automatically taking the next free Wayland socket?

Otherwise there is no damage in adding this parameter and I can avoid having my local git checkout diverge from master.

fvogt added a comment.Oct 2 2018, 1:09 PM

Is there a real reason against it besides "not necessary"? Maybe that we can't remove the manual override again without breaking some API promise once we have the grand solution of automatically taking the next free Wayland socket?

IMO "not necessary" is a real reason.
That this adds public API is also a reason not to add this, especially because it will be fully superseded by the next free socket search.

Otherwise there is no damage in adding this parameter and I can avoid having my local git checkout diverge from master.

The use case for this is to not conflict with an already existing wayland display. As this only works if launched manually with a special parameter, it only works for the minority of users, which makes this a workaround or even hack.
AFAICT (but I might be wrong) implementing the next free socket function shouldn't be more lines than this diff is and work in all cases and without user interaction.

romangg added a comment.EditedOct 2 2018, 2:16 PM

On the other side does it sometimes make sense to specify a socket manually even if KWin could take the next free one? I would say yes if I have a client to test against, which I can provide with a socket argument (I don't need to query which free socket actually was taken by KWin). So in the end we want to have both methods anyway.

romangg updated this revision to Diff 45431.Nov 13 2018, 7:12 PM
  • Use SOCKET_OPTION variable

KWin does not have functionality to generate the socket. KWin doesn't need this functionality and shouldn't have this functionality and it would be difficult to implement. Given that I fully support this change.

fvogt added a comment.Nov 14 2018, 8:03 AM

KWin doesn't need this functionality and shouldn't have this functionality and it would be difficult to implement.

Why? That goes entirely against what I and probably many others expect from a kwin_wayland call without explicit socket argument.

kwin_wayland already checks whether a socket is used, it can't be that hard to extend that to iterate a few socket names.

If you suggest to implement that in a layer higher than kwin, that's not possible as kwin has to create the .lock file itself.

I'd be fine with this change if it were more generic and allowed to add arbitrary arguments to kwin_wayland.

KWin doesn't need this functionality and shouldn't have this functionality and it would be difficult to implement.

Why? That goes entirely against what I and probably many others expect from a kwin_wayland call without explicit socket argument.

This was a deliberate design decision when writing the code. One reason is that code performing the check and then trying to create the socket is racy. Such a race does not belong into KWin.

Also for me the reason "it was like that in X" is not valid. The only valid question is: what makes sense from a KWin perspective.

kwin_wayland already checks whether a socket is used, it can't be that hard to extend that to iterate a few socket names.

No kwin_wayland does not check whether a socket is used.

If you suggest to implement that in a layer higher than kwin, that's not possible as kwin has to create the .lock file itself.

The lock file is not created by KWin, but by the wayland library. That is a layer higher than KWin. If such functionality should exist it should be in libwayland-server and from there available to all compositors. I refuse to accept that every compositor has to implement racy code.

I'd be fine with this change if it were more generic and allowed to add arbitrary arguments to kwin_wayland.

No other option makes sense to pass to kwin from startplasmacompositor.

It looks like all of that is already implemented in libwayland-server.
kwin_wayland just needs to make use of wl_display_add_socket_auto (other compositors do as well).

I'd be fine with this change if it were more generic and allowed to add arbitrary arguments to kwin_wayland.

No other option makes sense to pass to kwin from startplasmacompositor.

I would say most of the options used for kwin_wayland make sense except for "--help" and such. Let's say dbus-run-session startplasmacompositor --output-count 2 to start a windowed plasma session with two windows for example.

zzag added a subscriber: zzag.Feb 8 2019, 2:49 PM

kwin_wayland just needs to make use of wl_display_add_socket_auto (other compositors do as well).

It looks like now it does that.

romangg abandoned this revision.Feb 8 2019, 3:30 PM

Superseded by D17122.