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

clipboard provider can no longer access /dev/tty in 0.3.0 #8450

Closed
agriffis opened this issue May 29, 2018 · 8 comments
Closed

clipboard provider can no longer access /dev/tty in 0.3.0 #8450

agriffis opened this issue May 29, 2018 · 8 comments

Comments

@agriffis
Copy link

@agriffis agriffis commented May 29, 2018

  • nvim --version: v0.3.0-1304-gf711b6351
  • nvim v0.2.2 behaves differently
  • Operating system/version: Fedora 28
  • Terminal name/version: tmux/mosh/hterm.js
  • $TERM: tmux-256color

The clipboard provider has lost the ability to access /dev/tty which is needed for OSC 52. So my clipboard provider worked in 0.2.2 and no longer works in 0.3.0

Steps to reproduce using nvim -u NORC

This is just a demonstration of losing /dev/tty not really the clipboard provider which is more involved.

nvim -u NORC
!!echo hi > /dev/tty

Actual behaviour

/bin/bash: /dev/tty: No such device or address

Expected behaviour

Well, this reproducer actually makes some garbage in the terminal in 0.2.2, and no error message. But the important part is that the clipboard provider doesn't have access to /dev/tty either.

@justinmk

This comment has been minimized.

Copy link
Member

@justinmk justinmk commented May 29, 2018

Might be the same root cause as #8217 .

Does commenting-out this line fix your issue:

uvproc->uvopts.flags |= UV_PROCESS_DETACHED;

@justinmk justinmk added this to the 0.3.1 milestone May 29, 2018
@agriffis

This comment has been minimized.

Copy link
Author

@agriffis agriffis commented May 29, 2018

@justinmk I've just been running with the appimage, not building my own. Not sure I'll have time this week, but I'll let you know if I do. That looks promising, though.

@justinmk justinmk modified the milestones: 0.3.1, 0.3 May 29, 2018
@justinmk justinmk added the bug label May 29, 2018
@kristofferhagen

This comment has been minimized.

Copy link

@kristofferhagen kristofferhagen commented Jun 8, 2018

On neovim commit c7e6bb2 (current master)

:!echo hi > /dev/tty
/bin/bash: /dev/tty: No such device or address

shell returned 1

Press ENTER or type command to continue

After commenting out uvproc->uvopts.flags |= UV_PROCESS_DETACHED;

:!echo hi > /dev/tty
hi

Press ENTER or type command to continue
@justinmk justinmk modified the milestones: 0.3, 0.3.1 Jun 11, 2018
@justinmk

This comment has been minimized.

Copy link
Member

@justinmk justinmk commented Jul 2, 2018

The clipboard provider has lost the ability to access /dev/tty which is needed for OSC 52. So my clipboard provider worked in 0.2.2 and no longer works in 0.3.0

Slightly off-topic: @agriffis are you speaking of the Nvim default clipboard provider, or did you set g:clipboard to something different? We don't use OSC 52 directly, I'm wondering if there's some improvement we can make for the default provider there.

justinmk added a commit to justinmk/neovim that referenced this issue Jul 3, 2018
closes neovim#8217
closes neovim#8450

system() and :! are expected to run the in same process-group as Nvim.

NB: jobstart() does NOT run in the same process-group (neovim#8107):
    :call jobstart('ps', {'on_stdout':{j,d,e->append(0,d)}})

Background:
8d90171 changed ALL child-spawn utilities to do setsid().

Q: If we don't create a new session/process-group for :! and system(),
   how to avoid zombie descendants (e.g. process_wait() calls
   process_stop(), which only kills the root process)?
A: Send signal to process-group, but ignore the signal in our own
   process (signal_reject_deadly()). Vim does something similar:
   https://github.com/vim/vim/blob/e7499ddc33508d3d341e96f84a0e7b95b2d6927c/src/os_unix.c#L4834-L4841
   https://github.com/vim/vim/blob/e7499ddc33508d3d341e96f84a0e7b95b2d6927c/src/os_unix.c#L5122-L5134

Vim does setsid() in some cases of mch_call_shell_fork() (analogous to
Nvim's os_system()), but check the logic carefully--it's only for some
(irrelevant) GUI scenarios.
justinmk added a commit to justinmk/neovim that referenced this issue Jul 3, 2018
closes neovim#8217
closes neovim#8450

system() and :! are expected to run the in same process-group as Nvim.

NB: jobstart() does NOT run in the same process-group (neovim#8107):
    :call jobstart('ps', {'on_stdout':{j,d,e->append(0,d)}})

Background:
8d90171 changed ALL child-spawn utilities to do setsid().

Q: If we don't create a new session/process-group for :! and system(),
   how to avoid zombie descendants (e.g. process_wait() calls
   process_stop(), which only kills the root process)?
A: Send signal to process-group, but ignore the signal in our own
   process (signal_reject_deadly()). Vim does something similar:
   https://github.com/vim/vim/blob/e7499ddc33508d3d341e96f84a0e7b95b2d6927c/src/os_unix.c#L4834-L4841
   https://github.com/vim/vim/blob/e7499ddc33508d3d341e96f84a0e7b95b2d6927c/src/os_unix.c#L5122-L5134

Vim does setsid() in some cases of mch_call_shell_fork() (analogous to
Nvim's os_system()), but check the logic carefully--it's only for some
(irrelevant) GUI scenarios.
@agriffis

This comment has been minimized.

Copy link
Author

@agriffis agriffis commented Jul 3, 2018

@justinmk I define y to yank to tmux, so that I can yank and paste between vims, and Y additionally yanks to the system clipboard via OSC 52.

let g:clipboard = {
      \ 'name': 'myClipboard',
      \     'copy': {
      \         '+': 'clipboard-provider copy',
      \         '*': 'env COPY_PROVIDERS=tmux clipboard-provider copy',
      \     },
      \     'paste': {
      \         '+': 'clipboard-provider paste',
      \         '*': 'env COPY_PROVIDERS=tmux clipboard-provider paste',
      \     },
      \ }

" Yank to tmux normally
set clipboard=unnamed

" Yank to system clipboard with Y
nmap YY "+yy
nmap Y "+y
vmap Y "+y

The script copy-provider looks like this:

#!/bin/bash
#
# clipboard provider for neovim
#
# :help provider-clipboard

#exec 2>> ~/clipboard-provider.out
#set -x

: ${COPY_PROVIDERS:=tmux osc52}
: ${PASTE_PROVIDERS:=tmux}

main() {
    declare buffer status=1

    case $1 in
        copy)
            buffer=$(base64)
            internal() { base64 -d <<<"$buffer"; }
            for p in $COPY_PROVIDERS; do
                internal | $p-provider copy && status=0
            done ;;
        paste)
            for p in $PASTE_PROVIDERS; do
                $p-provider paste && status=0 && break
            done ;;
    esac

    exit $status
}

tmux-provider() {
    [[ -n $TMUX ]] || return
    case $1 in
        copy) internal | tmux load-buffer - ;;
        paste) tmux save-buffer - ;;
    esac
}

osc52-provider() {
    # HACK: this ignores stdin and looks directly at the base64 buffer
    case $1 in
        copy) printf $'\e]52;c;%s\a' "$buffer" > /dev/tty ;;
        paste) return 1 ;;
    esac
}

main "$@"
@agriffis

This comment has been minimized.

Copy link
Author

@agriffis agriffis commented Jul 3, 2018

@justinmk Also I have this in .tmux.conf

# Ms modifies OSC 52 clipboard handling to work with mosh, see
# https://gist.github.com/yudai/95b20e3da66df1b066531997f982b57b
set -ag terminal-overrides "vte*:XT:Ms=\\E]52;c;%p2%s\\7,xterm*:XT:Ms=\\E]52;c;%p2%s\\7"

# enable OSC 52 clipboard
# https://medium.freecodecamp.org/tmux-in-practice-integration-with-system-clipboard-bcd72c62ff7b
set -g set-clipboard on

There are drawbacks to OSC 52. Generally terminal emulators don't implement reading the system clipboard for security reasons. And writing is sometimes capped, for example mosh caps at 4 kB (and mosh doesn't support OSC 52 in a released version, only in git). Still, it's quite a nice solution for yanking text from vim over mosh and tmux, and pasting it in the browser, for example all the snippets I provided above ;-)

@justinmk

This comment has been minimized.

Copy link
Member

@justinmk justinmk commented Jul 3, 2018

@agriffis Good info, thanks!

lost the ability to access /dev/tty which is needed for OSC 52

Nvim 0.3.0 provides v:stderr which is available as a channel, so you can write to it. For example, if you do this in Nvim TUI, you should see some blinking text:

:call chansend(v:stderr, '^[[5mxxx')

That's equivalent to writing to /dev/tty.

I'm tagging this as wontfix for the reasons mentioned in #8217 (comment) , but #8678 outlines a potential path to restoring the Vim-compatible behavior.

@justinmk justinmk closed this Jul 3, 2018
justinmk added a commit to justinmk/neovim that referenced this issue Jul 4, 2018
(This commit is for reference; the functional change will be reverted.)

ref neovim#8217
ref neovim#8450
ref neovim#8678

In terminal-Vim, system() and :! run in Vim's process-group. But
8d90171 changed all of Nvim's process-spawn utilities to do
setsid(), which conflicts with that expected terminal-Vim behavior.

To "fix" that, this commit defines Process.detach as a TriState, then
handles the kNone case such that system() and :! do not do setsid() in
the spawned child.

But this commit REGRESSES 8d90171 (neovim#8107), so for example the
following code causes orphan processes:
    :echo system('sleep 30|sleep 30|sleep 30')

Q: If we don't create a new session/process-group, how to avoid zombie
   descendants (e.g. process_wait() calls process_stop(), which only
   kills the root process)?
A: Vim's approach in mch_call_shell_fork() is:
   1. BLOCK_SIGNALS (ignores deadly)
   2. fork()
   3. unblock signals in the child
   4. On CTRL-C, send SIGINT to the process-group id: kill(-pid, SIGINT)
   5. Parent (vim) ignores the signal. Child (and descendants) do not.
   https://github.com/vim/vim/blob/e7499ddc33508d3d341e96f84a0e7b95b2d6927c/src/os_unix.c#L4834-L4841
   https://github.com/vim/vim/blob/e7499ddc33508d3d341e96f84a0e7b95b2d6927c/src/os_unix.c#L5122-L5134

But we can't do that if we want to use the existing (libuv-based) form
of process_spawn().
@agriffis

This comment has been minimized.

Copy link
Author

@agriffis agriffis commented Jul 23, 2018

@justinmk Thanks for the tip about v:stderr. Unfortunately that doesn't play well with g:clipboard because it refers to external commands rather than internal functions/commands. So I'm using this hack for now in my clipboard-provider script:

TTY=$(tty < /proc/$PPID/fd/0)

and then sending OSC 52 commands to $TTY

agriffis added a commit to agriffis/skel that referenced this issue Aug 2, 2018
ShikChen added a commit to ShikChen/osc52.vim that referenced this issue May 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.