D15512: [startplasmacompositor] Add Wayland socket argument

[Roman Gilg]

romangg abandoned this revision.
romangg added a comment.


  Superseded by D17122 .

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: zzag, graesslin, fvogt, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Vlad Zagorodniy]

zzag added a comment.


  In D15512#359309 , @fvogt wrote:
  
  > 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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: zzag, graesslin, fvogt, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Fabian Vogt]

fvogt added a comment.


  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).
  
  In D15512#359308 , @graesslin 
wrote:
  
  > In D15512#359253 , @fvogt wrote:
  >
  > > 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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: graesslin, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Martin Flöser]

graesslin added a comment.


  In D15512#359253 , @fvogt wrote:
  
  > > 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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: graesslin, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Fabian Vogt]

fvogt added a comment.


  > 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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: graesslin, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Martin Flöser]

graesslin added a comment.


  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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: graesslin, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Roman Gilg]

romangg updated this revision to Diff 45431.
romangg added a comment.


  - Use SOCKET_OPTION variable

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15512?vs=41660=45431

BRANCH
  kwinWaylandSocketArg

REVISION DETAIL
  https://phabricator.kde.org/D15512

AFFECTED FILES
  startkde/startplasmacompositor.cmake

To: romangg, #kwin, #plasma
Cc: fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Roman Gilg]

romangg added a comment.


  On the other side does it sometimes make sense to specify a port 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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Fabian Vogt]

fvogt added a comment.


  In D15512#335264 , @romangg wrote:
  
  > 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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Roman Gilg]

romangg added a comment.


  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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Fabian Vogt]

fvogt added a comment.


  In D15512#326093 , @romangg wrote:
  
  > In D15512#326091 , @fvogt wrote:
  >
  > > 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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Roman Gilg]

romangg added a comment.


  In D15512#326091 , @fvogt wrote:
  
  > 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?

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Fabian Vogt]

fvogt added a comment.


  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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15512

To: romangg, #kwin, #plasma
Cc: fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15512: [startplasmacompositor] Add Wayland socket argument

[Roman Gilg]

romangg created this revision.
romangg added reviewers: KWin, Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
romangg requested review of this revision.

REVISION 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.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  kwinWaylandSocketArg

REVISION DETAIL
  https://phabricator.kde.org/D15512

AFFECTED FILES
  startkde/startplasmacompositor.cmake

To: romangg, #kwin, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart