process_spawn: Return status code (#6075)

src/nvim/eval.c

@@ -23046,8 +23046,9 @@ static inline bool common_job_start(TerminalJobData *data, typval_T *rettv)
 
   data->refcount++;
   char *cmd = xstrdup(proc->argv[0]);
-  if (!process_spawn(proc)) {
+  int status = process_spawn(proc);
-    EMSG2(_(e_jobspawn), cmd);
+  if (status) {
+    EMSG3(_(e_jobspawn), os_strerror(status), cmd);
     xfree(cmd);
     if (proc->type == kProcessTypePty) {
       xfree(data->proc.pty.term_name);

src/nvim/event/libuv_process.c

@@ -13,7 +13,8 @@
 # include "event/libuv_process.c.generated.h"
 #endif
 
-bool libuv_process_spawn(LibuvProcess *uvproc)
+/// @returns zero on success, or negative error code
+int libuv_process_spawn(LibuvProcess *uvproc)
   FUNC_ATTR_NONNULL_ALL
 {
   Process *proc = (Process *)uvproc;
@@ -51,11 +52,11 @@ bool libuv_process_spawn(LibuvProcess *uvproc)
   int status;
   if ((status = uv_spawn(&proc->loop->uv, &uvproc->uv, &uvproc->uvopts))) {
     ELOG("uv_spawn failed: %s", uv_strerror(status));
-    return false;
+    return status;
   }
 
   proc->pid = uvproc->uv.pid;
-  return true;
+  return status;
 }
 
 void libuv_process_close(LibuvProcess *uvproc)

src/nvim/event/process.c

@@ -30,7 +30,8 @@
 
 static bool process_is_tearing_down = false;
 
-bool process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
+/// @returns zero on success, or negative error code
+int process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
 {
   if (proc->in) {
     uv_pipe_init(&proc->loop->uv, &proc->in->uv.pipe, 0);
@@ -44,19 +45,19 @@ bool process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
     uv_pipe_init(&proc->loop->uv, &proc->err->uv.pipe, 0);
   }
 
-  bool success;
+  int status;
   switch (proc->type) {
     case kProcessTypeUv:
-      success = libuv_process_spawn((LibuvProcess *)proc);
+      status = libuv_process_spawn((LibuvProcess *)proc);
       break;
     case kProcessTypePty:
-      success = pty_process_spawn((PtyProcess *)proc);
+      status = pty_process_spawn((PtyProcess *)proc);
       break;
     default:
       abort();
   }
 
-  if (!success) {
+  if (status) {
     if (proc->in) {
       uv_close((uv_handle_t *)&proc->in->uv.pipe, NULL);
     }
@@ -74,7 +75,7 @@ bool process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
     }
     shell_free_argv(proc->argv);
     proc->status = -1;
-    return false;
+    return status;
   }
 
   if (proc->in) {
@@ -105,7 +106,7 @@ bool process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
   proc->internal_close_cb = decref;
   proc->refcount++;
   kl_push(WatcherPtr, proc->loop->children, proc);
-  return true;
+  return 0;
 }
 
 void process_teardown(Loop *loop) FUNC_ATTR_NONNULL_ALL
@@ -155,19 +156,16 @@ void process_close_err(Process *proc) FUNC_ATTR_NONNULL_ALL
 
 /// Synchronously wait for a process to finish
 ///
-/// @param process The Process instance
+/// @param process  Process instance
-/// @param ms Number of milliseconds to wait, 0 for not waiting, -1 for
+/// @param ms       Time in milliseconds to wait for the process.
-///        waiting until the process quits.
+///                 0 for no wait. -1 to wait until the process quits.
-/// @return returns the status code of the exited process. -1 if the process is
+/// @return Exit code of the process.
-///         still running and the `timeout` has expired. Note that this is
+///         -1 if the timeout expired while the process is still running.
-///         indistinguishable from the process returning -1 by itself. Which
+///         -2 if the user interruped the wait.
-///         is possible on some OS. Returns -2 if an user has interruped the
-///         wait.
 int process_wait(Process *proc, int ms, MultiQueue *events)
   FUNC_ATTR_NONNULL_ARG(1)
 {
-  // The default status is -1, which represents a timeout
+  int status = -1;  // default
-  int status = -1;
   bool interrupted = false;
   if (!proc->refcount) {
     status = proc->status;
@@ -179,8 +177,8 @@ int process_wait(Process *proc, int ms, MultiQueue *events)
     events = proc->events;
   }
 
-  // Increase refcount to stop the exit callback from being called(and possibly
+  // Increase refcount to stop the exit callback from being called (and possibly
-  // being freed) before we have a chance to get the status.
+  // freed) before we have a chance to get the status.
   proc->refcount++;
   LOOP_PROCESS_EVENTS_UNTIL(proc->loop, events, ms,
                             // Until...

src/nvim/globals.h

@@ -1127,7 +1127,7 @@ EXTERN char_u e_isadir2[] INIT(= N_("E17: \"%s\" is a directory"));
 EXTERN char_u e_invjob[] INIT(= N_("E900: Invalid job id"));
 EXTERN char_u e_jobtblfull[] INIT(= N_("E901: Job table is full"));
 EXTERN char_u e_jobspawn[] INIT(= N_(
-      "E903: Process for command \"%s\" could not be spawned"));
+        "E903: Process failed to start: %s: \"%s\""));
 EXTERN char_u e_jobnotpty[] INIT(= N_("E904: Job is not connected to a pty"));
 EXTERN char_u e_libcall[] INIT(= N_("E364: Library call failed for \"%s()\""));
 EXTERN char_u e_mkdir[] INIT(= N_("E739: Cannot create directory %s: %s"));

src/nvim/os/pty_process_unix.c

@@ -33,7 +33,8 @@
 # include "os/pty_process_unix.c.generated.h"
 #endif
 
-bool pty_process_spawn(PtyProcess *ptyproc)
+/// @returns zero on success, or negative error code
+int pty_process_spawn(PtyProcess *ptyproc)
   FUNC_ATTR_NONNULL_ALL
 {
   static struct termios termios;
@@ -41,6 +42,7 @@ bool pty_process_spawn(PtyProcess *ptyproc)
     init_termios(&termios);
   }
 
+  int status = 0;  // zero or negative error code (libuv convention)
   Process *proc = (Process *)ptyproc;
   assert(!proc->err);
   uv_signal_start(&proc->loop->children_watcher, chld_handler, SIGCHLD);
@@ -50,8 +52,9 @@ bool pty_process_spawn(PtyProcess *ptyproc)
   int pid = forkpty(&master, NULL, &termios, &ptyproc->winsize);
 
   if (pid < 0) {
+    status = -errno;
     ELOG("forkpty failed: %s", strerror(errno));
-    return false;
+    return status;
   } else if (pid == 0) {
     init_child(ptyproc);
     abort();
@@ -60,30 +63,34 @@ bool pty_process_spawn(PtyProcess *ptyproc)
   // make sure the master file descriptor is non blocking
   int master_status_flags = fcntl(master, F_GETFL);
   if (master_status_flags == -1) {
+    status = -errno;
     ELOG("Failed to get master descriptor status flags: %s", strerror(errno));
     goto error;
   }
   if (fcntl(master, F_SETFL, master_status_flags | O_NONBLOCK) == -1) {
+    status = -errno;
     ELOG("Failed to make master descriptor non-blocking: %s", strerror(errno));
     goto error;
   }
 
-  if (proc->in && !set_duplicating_descriptor(master, &proc->in->uv.pipe)) {
+  if (proc->in
+      && (status = set_duplicating_descriptor(master, &proc->in->uv.pipe))) {
     goto error;
   }
-  if (proc->out && !set_duplicating_descriptor(master, &proc->out->uv.pipe)) {
+  if (proc->out
+      && (status = set_duplicating_descriptor(master, &proc->out->uv.pipe))) {
     goto error;
   }
 
   ptyproc->tty_fd = master;
   proc->pid = pid;
-  return true;
+  return 0;
 
 error:
   close(master);
   kill(pid, SIGKILL);
   waitpid(pid, NULL, 0);
-  return false;
+  return status;
 }
 
 void pty_process_resize(PtyProcess *ptyproc, uint16_t width, uint16_t height)
@@ -137,9 +144,10 @@ static void init_child(PtyProcess *ptyproc) FUNC_ATTR_NONNULL_ALL
     return;
   }
 
+  char *prog = ptyproc->process.argv[0];
   setenv("TERM", ptyproc->term_name ? ptyproc->term_name : "ansi", 1);
-  execvp(ptyproc->process.argv[0], ptyproc->process.argv);
+  execvp(prog, ptyproc->process.argv);
-  fprintf(stderr, "execvp failed: %s\n", strerror(errno));
+  fprintf(stderr, "execvp failed: %s: %s\n", strerror(errno), prog);
 }
 
 static void init_termios(struct termios *termios) FUNC_ATTR_NONNULL_ALL
@@ -197,22 +205,24 @@ static void init_termios(struct termios *termios) FUNC_ATTR_NONNULL_ALL
   termios->c_cc[VTIME]    = 0;
 }
 
-static bool set_duplicating_descriptor(int fd, uv_pipe_t *pipe)
+static int set_duplicating_descriptor(int fd, uv_pipe_t *pipe)
   FUNC_ATTR_NONNULL_ALL
 {
+  int status = 0;  // zero or negative error code (libuv convention)
   int fd_dup = dup(fd);
   if (fd_dup < 0) {
+    status = -errno;
     ELOG("Failed to dup descriptor %d: %s", fd, strerror(errno));
-    return false;
+    return status;
   }
-  int uv_result = uv_pipe_open(pipe, fd_dup);
+  status = uv_pipe_open(pipe, fd_dup);
-  if (uv_result) {
+  if (status) {
     ELOG("Failed to set pipe to descriptor %d: %s",
-         fd_dup, uv_strerror(uv_result));
+         fd_dup, uv_strerror(status));
     close(fd_dup);
-    return false;
+    return status;
   }
-  return true;
+  return status;
 }
 
 static void chld_handler(uv_signal_t *handle, int signum)

src/nvim/os/shell.c

@@ -124,13 +124,9 @@ int os_call_shell(char_u *cmd, ShellOpts opts, char_u *extra_args)
 
   size_t nread;
 
-  int status = do_os_system(shell_build_argv((char *)cmd, (char *)extra_args),
+  int exitcode = do_os_system(shell_build_argv((char *)cmd, (char *)extra_args),
-                            input.data,
+                              input.data, input.len, output_ptr, &nread,
-                            input.len,
+                              emsg_silent, forward_output);
-                            output_ptr,
-                            &nread,
-                            emsg_silent,
-                            forward_output);
 
   xfree(input.data);
 
@@ -139,16 +135,16 @@ int os_call_shell(char_u *cmd, ShellOpts opts, char_u *extra_args)
     xfree(output);
   }
 
-  if (!emsg_silent && status != 0 && !(opts & kShellOptSilent)) {
+  if (!emsg_silent && exitcode != 0 && !(opts & kShellOptSilent)) {
     MSG_PUTS(_("\nshell returned "));
-    msg_outnum(status);
+    msg_outnum(exitcode);
     msg_putchar('\n');
   }
 
   State = current_state;
   signal_accept_deadly();
 
-  return status;
+  return exitcode;
 }
 
 /// os_system - synchronously execute a command in the shell
@@ -157,7 +153,7 @@ int os_call_shell(char_u *cmd, ShellOpts opts, char_u *extra_args)
 ///   char *output = NULL;
 ///   size_t nread = 0;
 ///   char *argv[] = {"ls", "-la", NULL};
-///   int status = os_sytem(argv, NULL, 0, &output, &nread);
+///   int exitcode = os_sytem(argv, NULL, 0, &output, &nread);
 ///
 /// @param argv The commandline arguments to be passed to the shell. `argv`
 ///             will be consumed.
@@ -218,11 +214,14 @@ static int do_os_system(char **argv,
   proc->in = input != NULL ? &in : NULL;
   proc->out = &out;
   proc->err = &err;
-  if (!process_spawn(proc)) {
+  int status = process_spawn(proc);
+  if (status) {
     loop_poll_events(&main_loop, 0);
-    // Failed, probably due to 'sh' not being executable
+    // Failed, probably 'shell' is not executable.
     if (!silent) {
-      MSG_PUTS(_("\nCannot execute "));
+      MSG_PUTS(_("\nshell failed to start: "));
+      msg_outtrans((char_u *)os_strerror(status));
+      MSG_PUTS(": ");
       msg_outtrans((char_u *)prog);
       msg_putchar('\n');
     }
@@ -262,7 +261,7 @@ static int do_os_system(char **argv,
   // busy state.
   ui_busy_start();
   ui_flush();
-  int status = process_wait(proc, -1, NULL);
+  int exitcode = process_wait(proc, -1, NULL);
   if (!got_int && out_data_decide_throttle(0)) {
     // Last chunk of output was skipped; display it now.
     out_data_ring(NULL, SIZE_MAX);
@@ -289,7 +288,7 @@ static int do_os_system(char **argv,
   assert(multiqueue_empty(events));
   multiqueue_free(events);
 
-  return status;
+  return exitcode;
 }
 
 ///  - ensures at least `desired` bytes in buffer
@@ -321,7 +320,7 @@ static void system_data_cb(Stream *stream, RBuffer *buf, size_t count,
 /// Tracks output received for the current executing shell command, and displays
 /// a pulsing "..." when output should be skipped. Tracking depends on the
 /// synchronous/blocking nature of ":!".
-//
+///
 /// Purpose:
 ///   1. CTRL-C is more responsive. #1234 #5396
 ///   2. Improves performance of :! (UI, esp. TUI, is the bottleneck).