Skip to content

Commit

Permalink
run_command(): handle missing command errors more gracefully
Browse files Browse the repository at this point in the history
When run_command() was asked to run a non-existant command, its behavior
varied depending on the platform:

  - on POSIX systems, we would fork, and then after the execvp call
    failed, we could call die(), which prints a message to stderr and
    exits with code 128.

  - on Windows, we do a PATH lookup, realize the program isn't there, and
    then return ERR_RUN_COMMAND_FORK

The goal of this patch is to make it clear to callers that the specific
error was a missing command. To do this, we will return the error code
ERR_RUN_COMMAND_EXEC, which is already defined in run-command.h, checked
for in several places, but never actually gets set.

The new behavior is:

  - on POSIX systems, we exit the forked process with code 127 (the same
    as the shell uses to report missing commands). The parent process
    recognizes this code and returns an EXEC error. The stderr message is
    silenced, since the caller may be speculatively trying to run a
    command. Instead, we use trace_printf so that somebody interested in
    debugging can see the error that occured.

  - on Windows, we check errno, which is already set correctly by
    mingw_spawnvpe, and report an EXEC error instead of a FORK error

Thus it is safe to speculatively run a command:

  int r = run_command_v_opt(argv, 0);
  if (r == -ERR_RUN_COMMAND_EXEC)
	  /* oops, it wasn't found; try something else */
  else
	  /* we failed for some other reason, error is in r */

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Jan 28, 2009
1 parent f172f33 commit 45c0961
Showing 1 changed file with 14 additions and 4 deletions.
18 changes: 14 additions & 4 deletions run-command.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ int start_command(struct child_process *cmd)
} else {
execvp(cmd->argv[0], (char *const*) cmd->argv);
}
die("exec %s failed.", cmd->argv[0]);
trace_printf("trace: exec '%s' failed: %s\n", cmd->argv[0],
strerror(errno));
exit(127);
}
#else
int s0 = -1, s1 = -1, s2 = -1; /* backups of stdin, stdout, stderr */
Expand Down Expand Up @@ -187,6 +189,7 @@ int start_command(struct child_process *cmd)
#endif

if (cmd->pid < 0) {
int err = errno;
if (need_in)
close_pair(fdin);
else if (cmd->in)
Expand All @@ -197,7 +200,9 @@ int start_command(struct child_process *cmd)
close(cmd->out);
if (need_err)
close_pair(fderr);
return -ERR_RUN_COMMAND_FORK;
return err == ENOENT ?
-ERR_RUN_COMMAND_EXEC :
-ERR_RUN_COMMAND_FORK;
}

if (need_in)
Expand Down Expand Up @@ -236,9 +241,14 @@ static int wait_or_whine(pid_t pid)
if (!WIFEXITED(status))
return -ERR_RUN_COMMAND_WAITPID_NOEXIT;
code = WEXITSTATUS(status);
if (code)
switch (code) {
case 127:
return -ERR_RUN_COMMAND_EXEC;
case 0:
return 0;
default:
return -code;
return 0;
}
}
}

Expand Down

0 comments on commit 45c0961

Please sign in to comment.