D17122: Add option to use wl_display_add_socket_auto
This revision was automatically updated to reflect the committed changes. Closed by commit R127:d671fcd0c09d: Add option to use wl_display_add_socket_auto (authored by fvogt). REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17122?vs=50238=50241 REVISION DETAIL https://phabricator.kde.org/D17122 AFFECTED FILES autotests/server/test_display.cpp src/server/display.cpp src/server/display.h To: fvogt, #kwin, #plasma, romangg Cc: davidedmundson, zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
romangg accepted this revision. This revision is now accepted and ready to land. REPOSITORY R127 KWayland BRANCH master REVISION DETAIL https://phabricator.kde.org/D17122 To: fvogt, #kwin, #plasma, romangg Cc: davidedmundson, zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
romangg added a dependent revision: D18522: Name Wayland socket automatically when no socket name was specified. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D17122 To: fvogt, #kwin, #plasma, romangg Cc: davidedmundson, zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
fvogt updated this revision to Diff 50238. fvogt added a comment. Replace XDG_RUNTIME_DIR, test still passes REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17122?vs=49806=50238 BRANCH master REVISION DETAIL https://phabricator.kde.org/D17122 AFFECTED FILES autotests/server/test_display.cpp src/server/display.cpp src/server/display.h To: fvogt, #kwin, #plasma, romangg Cc: davidedmundson, zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
fvogt marked 4 inline comments as done. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D17122 To: fvogt, #kwin, #plasma, romangg Cc: davidedmundson, zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
romangg added inline comments.
INLINE COMMENTS
> fvogt wrote in test_display.cpp:223
> That won't work reliably either though - if wayland-0 is free, but wayland-1
> is used, it would pick wayland-0 and wayland-2. Maybe it should just check
> that starting both displays at the same time succeeds and that their socket
> names are not equal.
That's why I said "do it one more time from there until you find the second
wayland-y".
For example (semi-pseudo):
QString name1, name2;
int cnt = -1;
while(true) {
cnt++;
const QString name = "wayland-" + str(cnt);
if (!runtimeDir.exists(name)) {
name1 = name;
break;
}
}
while(true) {
cnt++;
const QString name = "wayland-" + str(cnt);
if (!runtimeDir.exists(name)) {
name2 = name;
break;
}
}
David's solution is nicer though.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D17122
To: fvogt, #kwin, #plasma, romangg
Cc: davidedmundson, zzag, romangg, kde-frameworks-devel, michaelh, ngraham,
bruns
D17122: Add option to use wl_display_add_socket_auto
davidedmundson added inline comments. INLINE COMMENTS > fvogt wrote in test_display.cpp:223 > That won't work reliably either though - if wayland-0 is free, but wayland-1 > is used, it would pick wayland-0 and wayland-2. Maybe it should just check > that starting both displays at the same time succeeds and that their socket > names are not equal. I would suggest changing the XDG_RUNTIME_DIR for the duration of the test. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D17122 To: fvogt, #kwin, #plasma, romangg Cc: davidedmundson, zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
fvogt added inline comments. INLINE COMMENTS > romangg wrote in test_display.cpp:223 > This fails when one tries to run the test inside another Wayland session, > which already uses the wayland-0 socket name. > > What you could do here is loop until you find the first non-existing > `wayland-x` in the runtime dir. Then do it one more time from there until you > find the second `wayland-y`. Remember x,y and compare these below against the > created socket names. That won't work reliably either though - if wayland-0 is free, but wayland-1 is used, it would pick wayland-0 and wayland-2. Maybe it should just check that starting both displays at the same time succeeds and that their socket names are not equal. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D17122 To: fvogt, #kwin, #plasma, romangg Cc: zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
romangg requested changes to this revision.
romangg added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> test_display.cpp:223
> +QVERIFY(runtimeDir.exists());
> +QVERIFY(!runtimeDir.exists("wayland-0"));
> +QVERIFY(!runtimeDir.exists("wayland-1"));
This fails when one tries to run the test inside another Wayland session, which
already uses the wayland-0 socket name.
What you could do here is loop until you find the first non-existing
`wayland-x` in the runtime dir. Then do it one more time from there until you
find the second `wayland-y`. Remember x,y and compare these below against the
created socket names.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D17122
To: fvogt, #kwin, #plasma, romangg
Cc: zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
fvogt updated this revision to Diff 49806. fvogt marked 2 inline comments as done. fvogt added a comment. Add some style REPOSITORY R127 KWayland CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17122?vs=49802=49806 BRANCH master REVISION DETAIL https://phabricator.kde.org/D17122 AFFECTED FILES autotests/server/test_display.cpp src/server/display.cpp src/server/display.h To: fvogt, #kwin, #plasma Cc: zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
fvogt added a comment. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D17122 To: fvogt, #kwin, #plasma Cc: zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
romangg added inline comments. INLINE COMMENTS > display.cpp:87 > +bool running = false, > + automaticSocketNaming = false; > QList outputs; bool running = false; bool automaticSocketNaming = false; REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D17122 To: fvogt, #kwin, #plasma Cc: zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
zzag added inline comments.
INLINE COMMENTS
> display.cpp:86-87
> QString socketName = QStringLiteral("wayland-0");
> -bool running = false;
> +bool running = false,
> + automaticSocketNaming = false;
> QList outputs;
Style:
https://techbase.kde.org/Policies/Frameworks_Coding_Style#Variable_Declarations
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D17122
To: fvogt, #kwin, #plasma
Cc: zzag, romangg, kde-frameworks-devel, michaelh, ngraham, bruns
D17122: Add option to use wl_display_add_socket_auto
fvogt retitled this revision from "RFC: Use wl_display_add_socket_auto by default" to "Add option to use wl_display_add_socket_auto". fvogt edited the summary of this revision. fvogt edited the test plan for this revision. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D17122 To: fvogt, #kwin, #plasma Cc: romangg, kde-frameworks-devel, michaelh, ngraham, bruns
