Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request: Wayland Support #2046

Closed
lorenzleutgeb opened this issue May 6, 2021 · 33 comments
Closed

Request: Wayland Support #2046

lorenzleutgeb opened this issue May 6, 2021 · 33 comments

Comments

@lorenzleutgeb
Copy link

lorenzleutgeb commented May 6, 2021

Currently, some vimtex Features, e.g. cursor sync with Zathura, are not compatible with Wayland. I'd like to request making it compatible. This involves finding out which features are not compatible, first.

The topic has come up in (at least) the two issues linked below. I create this request to allow tracking of Wayland support and to invite other Wayland users to give their "thumbs up" to show its importance.

Is your feature request related to a problem?
Please refer to
#1775
#1719

Describe the solution you'd like
vimtex should support interaction with Wayland, with minimal user-configuration/-interaction.

Describe alternatives you've considered
None. Note that I don't think use "Use X11." is an alternative worth considering.

Additional context
None.

@lervag
Copy link
Owner

lervag commented May 6, 2021

Currently, some vimtex Features, e.g. cursor sync with Zathura, are not compatible with Wayland. I'd like to request making it compatible. This involves finding out which features are not compatible, first.

I don't mind having Wayland support. Or, more correctly: I would be very happy if I could claim VimTeX works perfectly well on Wayland.

As you say, the first step is to know exactly what is lacking on Wayland. As far as I know, the main hurdle is that the Zathura and MuPDF viewers rely on the xdotool command to avoid multiple windows (both) and for some concenient controls (MuPDF).

Another obvious hurdle: I don't use Wayland yet myself. The main, and big, implication: I won't actively work to solve this issue. But I will be more than interested in helping anyone who is interested and able to suggest a solution!

The topic has come up in (at least) the two issues linked below. I create this request to allow tracking of Wayland support and to invite other Wayland users to give their "thumbs up" to show it's importance.

Since I don't use Wayland myself, I think the main impact the thumbs up's may give here is attention to the issue, and perhaps motivation for someone who can help me solve this.

@lervag
Copy link
Owner

lervag commented Oct 9, 2021

I'm closing this now; not because I'm not interested in progress, but because I won't act on it myself. I would be very happy to see PRs or concrete suggestions, but as long as I don't use Wayland I will require active users who can help both with implementing and testing.

@thetic
Copy link

thetic commented Jan 8, 2022

Can we reopen this? I think its still valuable even if it isn't a priority right now.

@lervag
Copy link
Owner

lervag commented Jan 8, 2022

It's not about priority - instead, it is due to me not having any plan of working on it. I need (competent) users to help with this. I agree it is valuable!

I can reopen and tag this as "help wanted" and "won't fix", though.

@lervag lervag reopened this Jan 8, 2022
@mvasi90
Copy link

mvasi90 commented Apr 16, 2022

Hello everyone.

I'm using Wayland and I don't have the xdotool.
Accessing events of another process like injecting or reading keys is a security and privacy leak.

To auto reload Mupdf you can send a HUP signal like this pkill -HUP mupdf when the compilation process ends. But where?

Update
Works well using zathura compiled with synctex flag and zathura-pdf-mupdf.

@lervag
Copy link
Owner

lervag commented Apr 16, 2022

Accessing events of another process like injecting or reading keys is a security and privacy leak.

Yes, that's probably right. Still, I'm not aware of any actual issues with this in my ~20 year experience with Linux and I am quite sure that this issue is not such a big deal. I'm no expert and of course, I can be wrong.

In any case, I'm glad you got it working!

@krishnakumarg1984
Copy link
Contributor

krishnakumarg1984 commented May 9, 2022

My University's CS lab have migrated to a wayland-only login session (with fallback to linux console) on the shared lab computers.

Have other users tried and tested vimtex on Wayland? What's the experience so far? xclip, xsel, xdotool are not available on Wayland, right?

@lervag
Copy link
Owner

lervag commented May 10, 2022

I believe xdotool is the most important obstacle here. But I'm not sure if it is a very big deal, really. Just a minor nuisance.

@thetic
Copy link

thetic commented May 11, 2022

The Sway window manager lists a few potential alternatives for Wayland: https://github.com/swaywm/sway/wiki/i3-Migration-Guide#common-x11-apps-used-on-i3-with-wayland-alternatives

@krishnakumarg1984
Copy link
Contributor

The Sway window manager lists a few potential alternatives for Wayland: https://github.com/swaywm/sway/wiki/i3-Migration-Guide#common-x11-apps-used-on-i3-with-wayland-alternatives

So, one of these tools can be incorporated as a backend for automating the viewer interaction in VimTeX, right? There might be some complexity in detecting whether we are in X or Wayland and making the appropriate backend calls, all in vimscript!

@lervag
Copy link
Owner

lervag commented May 12, 2022

So, one of these tools can be incorporated as a backend for automating the viewer interaction in VimTeX, right?

Yes, I would believe so. I already noticed ydotool some time ago, but I notice there are now more alternatives. I would not be surprised if we could make a Wayland version with one of these tools.

Specifically, we would need to implement something like these lines, except with Wayland options instead of xdotool:

function! s:viewer.xdo_check() dict abort " {{{1
return executable('xdotool') && has_key(self, 'xwin_id')
endfunction
" }}}1
function! s:viewer.xdo_get_id() dict abort " {{{1
if !self.xdo_check() | return 0 | endif
if self.xwin_id <= 0
" Allow some time for the viewer to start properly
sleep 500m
let l:xwin_ids = vimtex#jobs#capture('xdotool search --class ' . self.name)
if len(l:xwin_ids) == 0
call vimtex#log#warning('Viewer cannot find ' . self.name . ' window ID!')
let self.xwin_id = 0
else
let self.xwin_id = l:xwin_ids[-1]
endif
endif
return self.xwin_id
endfunction
" }}}1
function! s:viewer.xdo_exists() dict abort " {{{1
if !self.xdo_check() | return v:false | endif
" If xwin_id is already set, check if it still exists
if self.xwin_id > 0
let xwin_ids = vimtex#jobs#capture('xdotool search --class ' . self.name)
if index(xwin_ids, self.xwin_id) < 0
let self.xwin_id = 0
endif
endif
" If xwin_id is unset, check if matching viewer windows exist
if self.xwin_id == 0
let l:pid = has_key(self, 'get_pid') ? self.get_pid() : 0
if l:pid > 0
let xwin_ids = vimtex#jobs#capture(
\ 'xdotool search --all --pid ' . l:pid
\ . ' --name ' . fnamemodify(self.out(), ':t'))
let self.xwin_id = get(xwin_ids, 0)
else
let xwin_ids = vimtex#jobs#capture(
\ 'xdotool search --name ' . fnamemodify(self.out(), ':t'))
let ids_already_used = filter(map(
\ deepcopy(vimtex#state#list_all()),
\ {_, x -> get(get(x, 'viewer', {}), 'xwin_id')}),
\ 'v:val > 0')
for id in xwin_ids
if index(ids_already_used, id) < 0
let self.xwin_id = id
break
endif
endfor
endif
endif
return self.xwin_id > 0
endfunction
" }}}1

Then one could copy zathura.vim and make a zathura_wayland.vim where the .xdo_* calls is changed with the Wayland alternatives.

There might be some complexity in detecting whether we are in X or Wayland and making the appropriate backend calls, all in vimscript!

Well, I wouldn't make it too complicated. As a first proof of concept, I would simply copy an existing viewer and make a new one (e.g. zathura_wayland), then we could look into ways to make it more seamless in a stage 2. If we don't find a good way, then having two viewer flavours (one for X and one for Wayland) would not be a big problem.

I would be more than happy to assist someone to make a PR! :)

@evesdropper
Copy link

I have some time this week and I would like to help out. What would I need to do in a PR?

@lervag
Copy link
Owner

lervag commented Aug 15, 2022

What would I need to do in a PR?

As a first step, I would modify the existing xdotool based code. It is specifically the code below this point:

" Methods that rely on xdotool. These are made available to all viewers, but
" they are only relevant for those that has the "xwin_id" attribute.

In total, it is about 140 lines of code, so my suggestion would be to first read that code and try and understand what it does. The most important parts are the ones that concern finding the window ID, because it is the part that we use to ensure we only open a single instance. The last three functions are not so important, but nice to have.

So: We need to know if there is something similar to the xwindow ID that we get from xdotool search, and if e.g. ydotool would allow searching for it with something similar (ydotool search).

I would start with something like this:

  • In a terminal, start one or more Zathura instances.
  • In another terminal, play around with ydotool or other similar tools - the more general, the better. Attempt to reproduce the calls made by VimTeX.

@evesdropper
Copy link

So: We need to know if there is something similar to the xwindow ID that we get from xdotool search, and if e.g. ydotool would allow searching for it with something similar (ydotool search).

Hmm, I noticed that there seems to be an issue - the three main xdotool alternatives (as listed by the Sway Migration Guide don't seem to have the search feature - ydotool only supports a small subset of the features and those mostly pertain to keyboard/mouse rather than window features.

@lervag
Copy link
Owner

lervag commented Aug 16, 2022

Yes, that seems to be an issue. As mentioned, the main point of xdotool is to avoid duplicate instances by detecting existing instances. We need something similar on Wayland. There may be different ways to do this, though! So, perhaps, time to be creative? In fact, it may be possible to find a different approach that works on both systems? (I don't know, I'm just thinking out loud.)

@evesdropper
Copy link

evesdropper commented Aug 16, 2022

I agree; that would probably be the best approach (vs poking those who run the xdotool alternatives to work on window functions). My current thoughts for a cross-platform solution would probably be something that implements the needed functionality from xdotool for VimTeX and is compatible on both platforms, ditching the need for the dependency.

I looked at the source code for xdotool and I think I have a general idea/outline of a course of action that theoretically seems to make sense:

  • Implement something like xdotool's search/other window commands (probably fork theirs or write the same logic in another language)
  • Test on X-based Linux
  • Look for changes on Wayland and account for them (creating some options for Wayland, and then adding a conditional to run the correct functions based on which platform you are on)
  • Woops, I forgot to look at ydotool earlier - my goal would probably try to do something similar (something that's platform agnostic) but for window functions.
  • Integrate to VimTeX.

A few considerations/thoughts against this approach:

  • The current functions for forward search are in Vim Script. Perhaps this re-implementation could be in Lua for easy integration?
  • Looking through this again, the idea does seem a bit overkill - implementing a handful of xdotool features for the sake of a plugin is a bit much in my opinion. This goes back to the poking developers of xdotool alternatives point a bit. However, the implementations could a bit more customized for VimTeX if this ends up being a feasible approach?

Thoughts?

@lervag
Copy link
Owner

lervag commented Aug 16, 2022

  • Looking through this again, the idea does seem a bit overkill

Agreed. Of course, I won't mind if anyone did it and it works, but I would hope and more or less guess that it could be solved in a simpler way. For instance, perhaps it is sufficient to instead rely on inspecting processes with ps, pgrep, /proc/PID? It should be possible to look for Zathura instances that has a specified file open, perhaps - something like fuser or lsof does that? Of course, we want this to also work on MacOS and Windows (but let's assume WSL!).

I mean, again, the main point is to avoid window duplication, which essentially means process duplication, right?

@evesdropper
Copy link

evesdropper commented Aug 16, 2022

Ah yes, that would probably be a bit more efficient. Perhaps I can start with testing something that if I can find duplicates using pgrep.

I think something like this is probably helpful as inspiration, so I'll probably try to take a similar approach with this, perhaps in Vim Script.

What I have so far:

  • I have the contents that pgrep -fa returns: PID progname filename for each process. From there, I can probably do something such as create something that parses only the PID and filename and maps them as pairs, then we can check for duplicates. This seems (at a glance) to be sufficient for the search function.
  • proc/pid/environ(?) to get window ID from PID perhaps? I can't test it out though; Linux complains and denies me permission - this is normal, and I think it could be a bit of a roadblock.

@lervag
Copy link
Owner

lervag commented Aug 17, 2022

Ah yes, that would probably be a bit more efficient. Perhaps I can start with testing something that if I can find duplicates using pgrep.

Notice, FYI, I already use pgrep for Zathura. So with this line of thought the idea is to go further.

function! s:viewer.get_pid() dict abort " {{{1
" First try to match full output file name
let l:outfile = fnamemodify(get(self, 'outfile', self.out()), ':t')
let l:output = vimtex#jobs#capture(
\ 'pgrep -nf "^zathura.*' . escape(l:outfile, '~\%.') . '"')
let l:pid = str2nr(join(l:output, ''))
if !empty(l:pid) | return l:pid | endif
" Now try to match correct servername as fallback
let l:output = vimtex#jobs#capture(
\ 'pgrep -nf "^zathura.+--servername ' . v:servername . '"')
return str2nr(join(l:output, ''))
endfunction
" }}}1

I think something like this is probably helpful as inspiration, so I'll probably try to take a similar approach with this, perhaps in Vim Script.

Yes, great. That seems relevant. The less dependencies the better!

  • I have the contents that pgrep -fa returns: PID progname filename for each process. From there, I can probably do something such as create something that parses only the PID and filename and maps them as pairs, then we can check for duplicates. This seems (at a glance) to be sufficient for the search function.

This is similar to what I already do for Zathura, specifically.

  • proc/pid/environ(?) to get window ID from PID perhaps? I can't test it out though; Linux complains and denies me permission - this is normal, and I think it could be a bit of a roadblock.

You will only have permissions if you are the user who owns the process. I think that should be more than sufficient.

Also, I don't think we need to care about the window IDs anymore. Instead, we need to detect e.g. Zathura processes that has a specified file open. I'm thinking something like this:

procs = ps find zathura pids
for pid in procs
  files = lsof for pid
  if current pdf in files
    return pid
  endif
endfor

return 0

@evesdropper
Copy link

Also, I don't think we need to care about the window IDs anymore. Instead, we need to detect e.g. Zathura processes that has a specified file open. I'm thinking something like this:

for pid in procs
 files = lsof for pid
 if current pdf in files
   return pid
 endif
endfor

return 0

Looks good to me! I think I have an idea of what could work then with inspiration from the psuedocode; however, when I was working withlsof -p PID, it seems like only using sudo returns the correct files, otherwise Linux complains about permission being denied (similar to /proc/pid/environ. I'm not sure if that's a problem that only I face or if it'll be a global issue - I did have an issue where the zathura executable path got reset, but I'm not sure if that affects this at all.

Other than that, I think I could try implementing the function in Vim Script as a standalone function first, then test this with the VimTeX plugin. On that note, do you have any insight on updating and testing code in VimTeX/plugins in general? Would it be just fork VimTeX, update code for Zathura, visual test using minimal.vim (not sure which features to test specifically - I believe it would be forward/inverse search for sure, but the confounding factor could then be me not setting up inverse search correctly), write and run tests (a bit unsure about how to write in Vim Script - I've done JUnit for Java and Python doctests), then merge request (?) and then full functionality for VimTeX users.

@lervag
Copy link
Owner

lervag commented Aug 17, 2022

otherwise Linux complains about permission being denied (similar to /proc/pid/environ. I'm not sure if that's a problem that only I face or if it'll be a global issue - I did have an issue where the zathura executable path got reset, but I'm not sure if that affects this at all.

Strange. When I do ls -l /proc I see that I have access to processes I own. I can verify that - I'm able to read e.g. environ files for processes I own. Are you not?

Other than that, I think I could try implementing the function in Vim Script as a standalone function first, then test this with the VimTeX plugin.

Sounds good. It could still be smart to rely on e.g. vimtex#job#capture for job control.

On that note, do you have any insight on updating and testing code in VimTeX/plugins in general? Would it be just fork VimTeX, update code for Zathura, visual test using minimal.vim (not sure which features to test specifically - I believe it would be forward/inverse search for sure, but the confounding factor could then be me not setting up inverse search correctly), write and run tests (a bit unsure about how to write in Vim Script - I've done JUnit for Java and Python doctests), then merge request (?) and then full functionality for VimTeX users.

In general, I use simple vimscript files for testing VimTeX combined with Makefiles for automatic testing. Most of VimTeX is under automatic tests in this way. You'll find the various tests under VIMTEX/test/test-*.

However, VimTeX + Zathura is hard to test automatically. So I haven't done that. Instead, I have simple examples or test cases that I can easily use for quickly doing manual testing.


For this specific feature, the main idea is really to update the s:viewer._exists() function for Zathura (on line 41) and (when it works) remove the call self.xdo_get_id() on line 51 in zathura.vim. If you can do this, even in a coarse manner, such that it works on Wayland, then it is more than OK to open a merge/pull request. We'll discuss further how to combine it properly. I believe I would want to generalize/abstract so we have shared code with MuPDF as well. But the hard part is really to get the proper system calls such that it works reliably.

@evesdropper
Copy link

Strange. When I do ls -l /proc I see that I have access to processes I own. I can verify that - I'm able to read e.g. environ files for processes I own. Are you not?

I can see the list and that I have permissions; however, when using any /proc commands, it seems like I magically lose access, which is strange. This kind of impedes my progress as in my script I would have to use sudo.

However, VimTeX + Zathura is hard to test automatically. So I haven't done that. Instead, I have simple examples or test cases that I can easily use for quickly doing manual testing.

Sounds good - what are some possible tests that I could do?

@lervag
Copy link
Owner

lervag commented Aug 21, 2022

Strange. When I do ls -l /proc I see that I have access to processes I own. I can verify that - I'm able to read e.g. environ files for processes I own. Are you not?

I can see the list and that I have permissions; however, when using any /proc commands, it seems like I magically lose access, which is strange. This kind of impedes my progress as in my script I would have to use sudo.

Strange! I have no idea why this is happening on your side.. :\

However, VimTeX + Zathura is hard to test automatically. So I haven't done that. Instead, I have simple examples or test cases that I can easily use for quickly doing manual testing.

Sounds good - what are some possible tests that I could do?

Well, what we want is to avoid duplicate viewer instances. In this case, I would do something like this:

  1. Write a simple vimscript file that automates as much as possible (example below).
  2. Run nvim --clean -u test.vim to run the script.
  3. Inspect the result to determine if things work or not.
" test.vim
set nocompatible
set runtimepath^=~/.local/plugged/vimtex
set runtimepath+=~/.local/plugged/vimtex/after
filetype plugin indent on
syntax enable

nnoremap q :qall!<cr>

let g:vimtex_view_method = 'zathura'
let g:vimtex_view_forward_search_on_start = 0

silent edit test.tex

VimtexCompile
" give time to compile!
sleep 2

" If it works, then the following will not open multiple viewer windows
VimtexView
sleep 200m
VimtexView
sleep 200m
VimtexView

quitall

The let g:vimtex_view_forward_search_on_start = 0 is necessary for this test. With the current VimTeX, the above will only create a single window. When I changed the zathura._exists() function to always return false, then the above will open multiple viewers.

@krillin666
Copy link

I see this issue has a won't fix tag. Is this because @lervag won't fix it himself but is open to PRs, or you don't want this feature added into VimTex ?
Thanks!

@krillin666
Copy link

Another question: Is this something that is limited by the inherent security permissions of the Wayland composer, or is there some way that a specific PDF reader could bypass ?

@evesdropper
Copy link

Is this because lervag won't fix it himself but is open to PRs, or you don't want this feature added into VimTex ?

Former. I did start drafting some stuff up but then I ended up moving to Sioyek where this isn't as relevant for me + lack the time to dedicate to it during the academic year.

Can't speak as much to your other question but from what I gathered before some problems came from xdotool which does not have a good replacement in Wayland.

@lervag
Copy link
Owner

lervag commented Jan 25, 2023

I see this issue has a won't fix tag. Is this because @lervag won't fix it himself but is open to PRs, or you don't want this feature added into VimTex ? Thanks!

@evesdropper answered well; I (still) don't use Wayland, and so working on this particular feature has little value to me personally. And it is hard to work on without using Wayland. Thus, I don't close the issue as someone else may be interested and have the time and skills necessary - I'll be glad to help by reviewing code and discussing.

Another question: Is this something that is limited by the inherent security permissions of the Wayland composer, or is there some way that a specific PDF reader could bypass ?

The problem in this particular case is that the tool xdotool that is used by VimTeX to detect and recognize Zathura windows to avoid opening duplicate windows does not work on Wayland. But there are alternatives and I believe it should be possible to resolve the issue. I've outlined my thoughts in my earlier comment and I believe there may be a useful discussion between me and @evesdropper if someone should want to give it a new go.

PS! Is it ever correct to have a space before a question or exclamation mark?

@rrueger
Copy link

rrueger commented Feb 18, 2023

I spend quite some time trying to understand the existing code and writing a fix. I tried using swaymsg to get window settings directly from sway (the Wayland Compositor in my case), using lsof and pgrep.

However, for some reason the replacing the body of s:viewer.xdo_find_win_id_by_pid() with return 1 fixes most of the functionality using Zathura under Sway.

Tested:

  • @lervag's test using test.vim
  • Opening two documents. There is no interference
  • Synctex
  • Changing the document open in Zathura
    • vim document.tex
    • Switch focus to Zathura
    • Open a different pdf in Zathura
    • Switch focus back to Vim
    • :VimtexView (correctly) opens a new Zathura window

What doesn't work is programmatically changing focus from Vim to Zathura within Vim (after :VimtexView, focus stays on Vim)

I suppose this is all true, because testing for existence is easily done using pgrep, and synctex is independent of the viewer window anyway.

lervag added a commit that referenced this issue Feb 19, 2023
This is more a test than anything else. I'm not sure it will actually
work well yet.

refer: #2046
lervag added a commit that referenced this issue Feb 19, 2023
This is a simpler version of the Zathura viewer that does not depend on
xdotool. It should work better on systems that do not use Xorg, e.g. on
Wayland.

refer: #2046
@lervag
Copy link
Owner

lervag commented Feb 19, 2023

First, thank you @rrueger for the effort of looking into this! I've closed your PR because I don't agree with the solution, but the work you did was useful because it seems to indicate that a nice solution for Zathura on Wayland is simply to ignore the xdotool dependency. I've therefore pushed a branch where I'm adding a zathura_wayland viewer that does this. I would be glad to have someone test it (I don't use Wayland). So, potential testers, please see #2639 for information on how to test.

@lervag
Copy link
Owner

lervag commented Feb 19, 2023

I am proposing that #2639 is a solution for this issue. Please let me know if you think it is not a sufficient solution. @lorenzleutgeb

@lervag lervag closed this as completed Feb 21, 2023
@lervag
Copy link
Owner

lervag commented Feb 21, 2023

Closing this as I think #2639 should suffice. Of course, feel free to object!

@rrueger
Copy link

rrueger commented Feb 25, 2023

it seems to indicate that a nice solution for Zathura on Wayland is simply to ignore the xdotool dependency

Indeed. The reason is, that even once we get a "window id", we do nothing with it. Simply, because we can't. At the moment there are no tools for sending keys to specific windows in Wayland. Another issue, is that the notion of a window id in Wayland, appears to be compositor dependent. This means a xdotool --search would need to be implemented in every compositor.

In Sway, this can be easily done with swaymsg. For example

swaymsg -rt get_tree  | jq ".. | objects | select(.pid == $(pgrep zathura)) | .id"

will return the sway specific window id. A cursory internet search shows that GNOME has no interface for listing open windows.

@lervag
Copy link
Owner

lervag commented Feb 25, 2023

Thanks for the info!

lervag added a commit that referenced this issue Feb 28, 2023
This is a simpler version of the Zathura viewer that does not depend on
xdotool. It should work better on systems that do not use Xorg, e.g. on
Wayland.

refer: #2046
lervag added a commit that referenced this issue Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants