From eb7f24b4ae369086a2bb26966758069e63edeee1 Mon Sep 17 00:00:00 2001 From: Gregory Anders Date: Mon, 14 Jun 2021 11:56:43 -0600 Subject: [PATCH 1/4] feat(job): add parameter to close stdin Some programs behave differently when they detect that stdin is being piped. This can be problematic when these programs are used with the job control API where stdin is attached, but not typically used. It is possible to run the job using a PTY which circumvents this problem, but that includes a lot of overhead when simply closing the stdin pipe would suffice. To enable this behavior, add a new parameter to the jobstart options dict called "stdin" with two valid values: "pipe" (the default) implements the existing behavior of opening a channel for stdin and "null" which disconnects stdin (or, if you prefer, connects it to /dev/null). This is extensible so that other modes can be added in the future. --- runtime/doc/eval.txt | 3 +++ src/nvim/channel.c | 23 +++++++++++++++++++---- src/nvim/channel.h | 4 ++++ src/nvim/eval/funcs.c | 24 +++++++++++++++++++----- 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt index 9e9b828c5a..8c360b1778 100644 --- a/runtime/doc/eval.txt +++ b/runtime/doc/eval.txt @@ -5637,6 +5637,9 @@ jobstart({cmd}[, {opts}]) *jobstart()* before invoking `on_stderr`. |channel-buffered| stdout_buffered: (boolean) Collect data until EOF (stream closed) before invoking `on_stdout`. |channel-buffered| + stdin: (string) Either "pipe" (default) to connect the + job's stdin to a channel or "null" to disconnect + stdin. width: (number) Width of the `pty` terminal. {opts} is passed as |self| dictionary to the callback; the diff --git a/src/nvim/channel.c b/src/nvim/channel.c index 60af11e94b..ffa4d12d11 100644 --- a/src/nvim/channel.c +++ b/src/nvim/channel.c @@ -289,6 +289,9 @@ static void close_cb(Stream *stream, void *data) /// `on_stdout` is ignored /// @param[in] detach True if the job should not be killed when nvim exits, /// ignored if `pty` is true +/// @param[in] stdin Stdin mode. Either kChannelStdinPipe to open a channel +/// for stdin or kChannelStdinNull to leave stdin +/// disconnected. /// @param[in] cwd Initial working directory for the job. Nvim's working /// directory if `cwd` is NULL /// @param[in] pty_width Width of the pty, ignored if `pty` is false @@ -302,7 +305,7 @@ static void close_cb(Stream *stream, void *data) Channel *channel_job_start(char **argv, CallbackReader on_stdout, CallbackReader on_stderr, Callback on_exit, bool pty, bool rpc, bool overlapped, bool detach, - const char *cwd, + ChannelStdinMode stdin, const char *cwd, uint16_t pty_width, uint16_t pty_height, dict_T *env, varnumber_T *status_out) { @@ -345,7 +348,7 @@ Channel *channel_job_start(char **argv, CallbackReader on_stdout, proc->overlapped = overlapped; char *cmd = xstrdup(proc->argv[0]); - bool has_out, has_err; + bool has_in, has_out, has_err; if (proc->type == kProcessTypePty) { has_out = true; has_err = false; @@ -353,7 +356,17 @@ Channel *channel_job_start(char **argv, CallbackReader on_stdout, has_out = rpc || callback_reader_set(chan->on_data); has_err = callback_reader_set(chan->on_stderr); } - int status = process_spawn(proc, true, has_out, has_err); + + switch (stdin) { + case kChannelStdinPipe: + has_in = true; + break; + case kChannelStdinNull: + has_in = false; + break; + } + + int status = process_spawn(proc, has_in, has_out, has_err); if (status) { EMSG3(_(e_jobspawn), os_strerror(status), cmd); xfree(cmd); @@ -369,7 +382,9 @@ Channel *channel_job_start(char **argv, CallbackReader on_stdout, tv_dict_free(proc->env); } - wstream_init(&proc->in, 0); + if (has_in) { + wstream_init(&proc->in, 0); + } if (has_out) { rstream_init(&proc->out, 0); } diff --git a/src/nvim/channel.h b/src/nvim/channel.h index 9d26852ce5..df858e1602 100644 --- a/src/nvim/channel.h +++ b/src/nvim/channel.h @@ -28,6 +28,10 @@ typedef enum { kChannelPartAll } ChannelPart; +typedef enum { + kChannelStdinPipe, + kChannelStdinNull, +} ChannelStdinMode; typedef struct { Stream in; diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 3cda7ec5d4..20434a937e 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -5182,6 +5182,7 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, FunPtr fptr) bool pty = false; bool clear_env = false; bool overlapped = false; + ChannelStdinMode stdin = kChannelStdinPipe; CallbackReader on_stdout = CALLBACK_READER_INIT, on_stderr = CALLBACK_READER_INIT; Callback on_exit = CALLBACK_NONE; @@ -5196,6 +5197,17 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, FunPtr fptr) clear_env = tv_dict_get_number(job_opts, "clear_env") != 0; overlapped = tv_dict_get_number(job_opts, "overlapped") != 0; + char *s = tv_dict_get_string(job_opts, "stdin", false); + if (s) { + if (!strncmp(s, "null", NUMBUFLEN)) { + stdin = kChannelStdinNull; + } else if (!strncmp(s, "pipe", NUMBUFLEN)) { + // Nothing to do, default value + } else { + EMSG3(_(e_invargNval), "stdin", s); + } + } + if (pty && rpc) { EMSG2(_(e_invarg2), "job cannot have both 'pty' and 'rpc' options set"); shell_free_argv(argv); @@ -5252,8 +5264,8 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, FunPtr fptr) env = create_environment(job_env, clear_env, pty, term_name); Channel *chan = channel_job_start(argv, on_stdout, on_stderr, on_exit, pty, - rpc, overlapped, detach, cwd, width, height, - env, &rettv->vval.v_number); + rpc, overlapped, detach, stdin, cwd, width, + height, env, &rettv->vval.v_number); if (chan) { channel_create_event(chan, NULL); } @@ -7733,8 +7745,9 @@ static void f_rpcstart(typval_T *argvars, typval_T *rettv, FunPtr fptr) Channel *chan = channel_job_start(argv, CALLBACK_READER_INIT, CALLBACK_READER_INIT, CALLBACK_NONE, - false, true, false, false, NULL, 0, 0, - NULL, &rettv->vval.v_number); + false, true, false, false, + kChannelStdinPipe, NULL, 0, 0, NULL, + &rettv->vval.v_number); if (chan) { channel_create_event(chan, NULL); } @@ -10850,9 +10863,10 @@ static void f_termopen(typval_T *argvars, typval_T *rettv, FunPtr fptr) const bool rpc = false; const bool overlapped = false; const bool detach = false; + ChannelStdinMode stdin = kChannelStdinPipe; uint16_t term_width = MAX(0, curwin->w_width_inner - win_col_off(curwin)); Channel *chan = channel_job_start(argv, on_stdout, on_stderr, on_exit, - pty, rpc, overlapped, detach, cwd, + pty, rpc, overlapped, detach, stdin, cwd, term_width, curwin->w_height_inner, env, &rettv->vval.v_number); if (rettv->vval.v_number <= 0) { From 35e13df3c871e993f4d9b332985d9a9525f4ca00 Mon Sep 17 00:00:00 2001 From: Gregory Anders Date: Mon, 12 Jul 2021 11:55:20 -0600 Subject: [PATCH 2/4] Rename stdin to stdin_mode stdin is a macro in Windows builds. --- src/nvim/channel.c | 10 +++++----- src/nvim/eval/funcs.c | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/nvim/channel.c b/src/nvim/channel.c index ffa4d12d11..a0db1bcdfd 100644 --- a/src/nvim/channel.c +++ b/src/nvim/channel.c @@ -289,9 +289,9 @@ static void close_cb(Stream *stream, void *data) /// `on_stdout` is ignored /// @param[in] detach True if the job should not be killed when nvim exits, /// ignored if `pty` is true -/// @param[in] stdin Stdin mode. Either kChannelStdinPipe to open a channel -/// for stdin or kChannelStdinNull to leave stdin -/// disconnected. +/// @param[in] stdin_mode Stdin mode. Either kChannelStdinPipe to open a +/// channel for stdin or kChannelStdinNull to leave +/// stdin disconnected. /// @param[in] cwd Initial working directory for the job. Nvim's working /// directory if `cwd` is NULL /// @param[in] pty_width Width of the pty, ignored if `pty` is false @@ -305,7 +305,7 @@ static void close_cb(Stream *stream, void *data) Channel *channel_job_start(char **argv, CallbackReader on_stdout, CallbackReader on_stderr, Callback on_exit, bool pty, bool rpc, bool overlapped, bool detach, - ChannelStdinMode stdin, const char *cwd, + ChannelStdinMode stdin_mode, const char *cwd, uint16_t pty_width, uint16_t pty_height, dict_T *env, varnumber_T *status_out) { @@ -357,7 +357,7 @@ Channel *channel_job_start(char **argv, CallbackReader on_stdout, has_err = callback_reader_set(chan->on_stderr); } - switch (stdin) { + switch (stdin_mode) { case kChannelStdinPipe: has_in = true; break; diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 20434a937e..0c9b717b0b 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -5182,7 +5182,7 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, FunPtr fptr) bool pty = false; bool clear_env = false; bool overlapped = false; - ChannelStdinMode stdin = kChannelStdinPipe; + ChannelStdinMode stdin_mode = kChannelStdinPipe; CallbackReader on_stdout = CALLBACK_READER_INIT, on_stderr = CALLBACK_READER_INIT; Callback on_exit = CALLBACK_NONE; @@ -5200,7 +5200,7 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, FunPtr fptr) char *s = tv_dict_get_string(job_opts, "stdin", false); if (s) { if (!strncmp(s, "null", NUMBUFLEN)) { - stdin = kChannelStdinNull; + stdin_mode = kChannelStdinNull; } else if (!strncmp(s, "pipe", NUMBUFLEN)) { // Nothing to do, default value } else { @@ -5264,7 +5264,7 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, FunPtr fptr) env = create_environment(job_env, clear_env, pty, term_name); Channel *chan = channel_job_start(argv, on_stdout, on_stderr, on_exit, pty, - rpc, overlapped, detach, stdin, cwd, width, + rpc, overlapped, detach, stdin_mode, cwd, width, height, env, &rettv->vval.v_number); if (chan) { channel_create_event(chan, NULL); @@ -10863,11 +10863,11 @@ static void f_termopen(typval_T *argvars, typval_T *rettv, FunPtr fptr) const bool rpc = false; const bool overlapped = false; const bool detach = false; - ChannelStdinMode stdin = kChannelStdinPipe; + ChannelStdinMode stdin_mode = kChannelStdinPipe; uint16_t term_width = MAX(0, curwin->w_width_inner - win_col_off(curwin)); Channel *chan = channel_job_start(argv, on_stdout, on_stderr, on_exit, - pty, rpc, overlapped, detach, stdin, cwd, - term_width, curwin->w_height_inner, + pty, rpc, overlapped, detach, stdin_mode, + cwd, term_width, curwin->w_height_inner, env, &rettv->vval.v_number); if (rettv->vval.v_number <= 0) { return; From d7382475b31cb7b130aa9c7ce17d42adc16ac0bb Mon Sep 17 00:00:00 2001 From: Gregory Anders Date: Mon, 12 Jul 2021 12:05:25 -0600 Subject: [PATCH 3/4] Fix line length clint error --- src/nvim/eval/funcs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 0c9b717b0b..8960cac9c5 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -5264,8 +5264,8 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, FunPtr fptr) env = create_environment(job_env, clear_env, pty, term_name); Channel *chan = channel_job_start(argv, on_stdout, on_stderr, on_exit, pty, - rpc, overlapped, detach, stdin_mode, cwd, width, - height, env, &rettv->vval.v_number); + rpc, overlapped, detach, stdin_mode, cwd, + width, height, env, &rettv->vval.v_number); if (chan) { channel_create_event(chan, NULL); } From f83763b2615e272d2187ae042a968672c37a5764 Mon Sep 17 00:00:00 2001 From: Gregory Anders Date: Mon, 12 Jul 2021 13:22:56 -0600 Subject: [PATCH 4/4] Add test case for 'null' stdin mode --- test/functional/core/job_spec.lua | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/functional/core/job_spec.lua b/test/functional/core/job_spec.lua index 34ab90d760..c4745e636f 100644 --- a/test/functional/core/job_spec.lua +++ b/test/functional/core/job_spec.lua @@ -348,6 +348,12 @@ describe('jobs', function() eq(false, pcall(function() nvim('command', 'call jobsend(j, ["some data"])') end)) + + command("let g:job_opts.stdin = 'null'") + nvim('command', "let j = jobstart(['cat', '-'], g:job_opts)") + eq(false, pcall(function() + nvim('command', 'call jobsend(j, ["some data"])') + end)) end) it('disallows jobsend on a non-existent job', function()