🔨 Wayland support for electron apps #23

noonsleeper · 2024-02-22T19:12:54Z

if socket=wayland is active the parameters to handle wayland decorations is set also ozone-platform-hint=auto is set

if you override with nosocket=wayland force ozone-platform=x11

daiyam · 2024-05-08T07:36:24Z

Hello @gasinvein, can you give us a timeline for this PR? Thx

gasinvein · 2024-05-08T09:30:08Z

Hello, sorry for the delay and thanks for the contribution. Sadly, I cannot test this myself atm. Did you try to build VSCod[ium] flatpak with this wrapper version and verify that it works as intended?

editor.sh

gasinvein · 2024-05-08T09:57:33Z

editor.sh

+  # See https://github.com/flathub/im.riot.Riot/blob/3fdd41c84f40fa1e8e186bade5d832d79045600c/element.sh
+  # See also https://gaultier.github.io/blog/wayland_from_scratch.html 
+  # and https://github.com/flathub/com.vscodium.codium/issues/321
+  if [[ ${WAYLAND_DISPLAY} == "/run/flatpak/wayland-"* ]] || [[ -e "${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}" ]] && [[ "wayland" == "${XDG_SESSION_TYPE}" ]]


Isn't checking if XDG_SESSION_TYPE is set to wayland is enough?

Suggested change

if [[ ${WAYLAND_DISPLAY} == "/run/flatpak/wayland-"* ]] || [[ -e "${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}" ]] && [[ "wayland" == "${XDG_SESSION_TYPE}" ]]

if [[ "wayland" == "${XDG_SESSION_TYPE}" ]]

Isn't checking if XDG_SESSION_TYPE is set to wayland is enough?

No, under KDE, $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY will give /run/user/1000//run/flatpak/wayland-0 which is incorrect. That's why @noonsleeper is testing for the prefix /run/flatpak/wayland-.

Isn't checking if XDG_SESSION_TYPE is set to wayland is enough?

Checking for the socket is correct, $XDG_SESSION_TYPE can stay on wayland even if the finish arg is removed (ie. with --nosocket) as it is inherited from the environment and has no connection with the active finish args.

/run/user/1000//run/flatpak/wayland-0

Those two should not overlap like that, it should always be /run/flatpak/wayland-{0, 1,...}

You should check for only one of them - preferably for the one /run/flatpak/ only.

If you want to make it more sound you can check if stat -c %F /run/flatpak/wayland-0 reports socket or if it fails (ie. when nosocket=wayland or not on wayland session).

hi @bbhtt, to sum up, I only need to check if $WAYLAND_DISPLAY contains wayland- and also $XDG_SESSION_TYPE == wayland, because flatpak already check if wayland-* is a socket, is that so?

You can do if [[ -e "$(echo "/run/flatpak/wayland-"*)" ]]; then echo "yes"; else echo "no"; fi but it might match multiple files wayland-0, wayland-0.lock too (fortunately there is only one wayland-0 in /run/flatpak/)

@bbhtt, @gasinvein

I'm still on Silverblue F39 and WAYLAND_DISPLAY is reporting only wayland-0 not the full path, I don't check if F40 change that behaviour within gnome 46, but I think other distros like Debian or Ubuntu maybe has the same behaviour for Gnome 45.
Then, if we need to check both at the same time, I come with this:

It is not perfect like you can see, but it comes handy because I don't use or pass that variable to anything, also this check that at least the variable is filled with something that resembles what we are looking for,

Also, I can go with a more accurate regex that only *wayland-? (to me this will add more complexity to something that doesn't require that, but maybe I'm wrong, I'm open to any advice), what do you think?

Btw, sorry for this wall of text

I think it would be easier to check if WAYLAND_DISPLAY is an absolute path. If not, then construct the path to the socket using the implicit path. Like:

local display_type=x11 local display_server_args local wayland_socket if [ "$XDG_DISPLAY_TYPE" = wayland ] && [ -n "$WAYLAND_DISPLAY" ]; then if [[ $WAYLAND_DISPLAY =~ ^/ ]]; then wayland_socket=$WAYLAND_DISPLAY else wayland_socket="${XDG_RUNTIME_DIR:-/run/user/${UID}}/${WAYLAND_DISPLAY}" fi [ -e "$wayland_socket" ] && display_type=wayland fi if [ "$display_type" = wayland ]; then # Set display_server_args for wayland else # Set display_server_args for x11 fi echo $display_server_args

Another way without using a regex would be [ "${WAYLAND_DISPLAY::1}" = / ].

I think it would be easier to check if WAYLAND_DISPLAY is an absolute path. If not, then construct the path to the socket using the implicit path. Like:

This also works I think.

daiyam · 2024-05-08T10:45:15Z

Hello, sorry for the delay and thanks for the contribution. Sadly, I cannot test this myself atm. Did you try to build VSCod[ium] flatpak with this wrapper version and verify that it works as intended?

No problem. :)

The latest version should include it with the fix for KDE but I haven't test it myself...
But VSCodium has been using with the PR since the March releases.

noonsleeper · 2024-06-08T18:54:25Z

Hi @gasinvein, Can you give me an update about this?

gasinvein

LGTM.

gasinvein · 2024-06-08T19:17:58Z

The zypak branch was in a WiP state, but it seems like several apps on Flathub are using it now. Do you folks consider it ready? If so, should we merge it into the main branch (see #13)?

noonsleeper · 2024-06-08T19:57:37Z

The zypak branch was in a WiP state, but it seems like several apps on Flathub are using it now. Do you folks consider it ready? If so, should we merge it into the main branch (see #13)?

I was using this since nearly the creation of the branch, and at least for codium and codium-insiders it works like a charm.

gasinvein · 2024-06-09T07:52:31Z

editor.sh

 function exec_editor() {
  @EXPORT_ENVS@
-  exec "@WRAPPER_PATH@" @EDITOR_ARGS@ "$@"
+  # shellcheck disable=SC2046,SC2086
+  exec "@WRAPPER_PATH@" @EDITOR_ARGS@ $(display_server_args) ${EDITOR_RUNTIME_ARGS} "$@"


@noonsleeper Ok, there seems to be one thing I've missed. The wrapper isn't solely for Electron apps, thus electron-specific cli args shouldn't be added unconditionally.
Adding the display server args for electron should be gated by Zypak feature enabled.

noonsleeper mentioned this pull request May 8, 2024

[Discussion] General question about the wrapper repo flathub/com.vscodium.codium#336

Closed

gasinvein force-pushed the zypak branch from 188d214 to b090f37 Compare May 8, 2024 09:46

noonsleeper added 2 commits May 8, 2024 12:52

🔨 Wayland support for electron apps

e9916ff

🐛 Fix KDE wayland check

08e2b43

gasinvein requested changes May 8, 2024

View reviewed changes

noonsleeper added 4 commits May 26, 2024 02:30

🔧 Add EDITOR_RUNTIME_ARGS environment variable usage example

86de662

🔨 Check for wayland socket

fc64d54

🐛 Fix wrong variable name

138f9a3

🔧 match the existing format

75d37e1

gasinvein approved these changes Jun 8, 2024

View reviewed changes

gasinvein merged commit 7b72347 into flathub-infra:zypak Jun 8, 2024
1 check passed

gasinvein reviewed Jun 9, 2024

View reviewed changes

gasinvein mentioned this pull request Jun 9, 2024

Gate appending electron by meson feature #24

Merged

noonsleeper deleted the wayland-support branch June 9, 2024 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🔨 Wayland support for electron apps #23

🔨 Wayland support for electron apps #23

noonsleeper commented Feb 22, 2024 •

edited

Loading

daiyam commented May 8, 2024

gasinvein commented May 8, 2024

gasinvein May 8, 2024

daiyam May 8, 2024

bbhtt May 8, 2024 •

edited

Loading

bbhtt May 8, 2024 •

edited

Loading

noonsleeper May 8, 2024

bbhtt May 8, 2024 •

edited

Loading

noonsleeper May 9, 2024

dbnicholson May 9, 2024

dbnicholson May 9, 2024

bbhtt May 9, 2024

daiyam commented May 8, 2024

noonsleeper commented Jun 8, 2024

gasinvein left a comment

gasinvein commented Jun 8, 2024 •

edited

Loading

noonsleeper commented Jun 8, 2024

gasinvein Jun 9, 2024 •

edited

Loading

	if [[ ${WAYLAND_DISPLAY} == "/run/flatpak/wayland-"* ]] \|\| [[ -e "${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}" ]] && [[ "wayland" == "${XDG_SESSION_TYPE}" ]]
	if [[ "wayland" == "${XDG_SESSION_TYPE}" ]]

🔨 Wayland support for electron apps #23

🔨 Wayland support for electron apps #23

Conversation

noonsleeper commented Feb 22, 2024 • edited Loading

daiyam commented May 8, 2024

gasinvein commented May 8, 2024

gasinvein May 8, 2024

Choose a reason for hiding this comment

daiyam May 8, 2024

Choose a reason for hiding this comment

bbhtt May 8, 2024 • edited Loading

Choose a reason for hiding this comment

bbhtt May 8, 2024 • edited Loading

Choose a reason for hiding this comment

noonsleeper May 8, 2024

Choose a reason for hiding this comment

bbhtt May 8, 2024 • edited Loading

Choose a reason for hiding this comment

noonsleeper May 9, 2024

Choose a reason for hiding this comment

dbnicholson May 9, 2024

Choose a reason for hiding this comment

dbnicholson May 9, 2024

Choose a reason for hiding this comment

bbhtt May 9, 2024

Choose a reason for hiding this comment

daiyam commented May 8, 2024

noonsleeper commented Jun 8, 2024

gasinvein left a comment

Choose a reason for hiding this comment

gasinvein commented Jun 8, 2024 • edited Loading

noonsleeper commented Jun 8, 2024

gasinvein Jun 9, 2024 • edited Loading

Choose a reason for hiding this comment

noonsleeper commented Feb 22, 2024 •

edited

Loading

bbhtt May 8, 2024 •

edited

Loading

bbhtt May 8, 2024 •

edited

Loading

bbhtt May 8, 2024 •

edited

Loading

gasinvein commented Jun 8, 2024 •

edited

Loading

gasinvein Jun 9, 2024 •

edited

Loading