Re: [PATCH/RFC] notice error exit from pager
From: Johannes Sixt <hidden>
Date: 2016-06-15 22:51:40
Am 26.07.2011 23:04, schrieb Clemens Buchacher:
If the pager fails to run, git produces no output, e.g.:
$ GIT_PAGER=not-a-command git log
The error reporting fails for two reasons:
(1) start_command: There is a mechanism that detects errors during
execvp introduced in 2b541bf8 (start_command: detect execvp
failures early). ... This mechanism is
incompatible with the workaround introduced in 35ce8622
(pager: Work around window resizing bug in 'less')You analysis is correct. I think the bug in less was fixed shortly after this workaround was introduced. I would like to remove this workaround for a different reason, but it seems there are people who are quite fond of it ;)
quoted hunk ↗ jump to hunk
diff --git a/run-command.c b/run-command.c index 70e8a24..944a882 100644 --- a/run-command.c +++ b/run-command.c@@ -127,9 +127,6 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure) if (code == 127) { code = -1; failed_errno = ENOENT; - if (!silent_exec_failure) - error("cannot run %s: %s", argv0, - strerror(ENOENT)); } } else { error("waitpid is confused (%s)", argv0);@@ -287,10 +284,14 @@ fail_pipe: * Do not check for cmd->silent_exec_failure; the parent * process will check it when it sees this exit code. */ - if (errno == ENOENT) + if (errno == ENOENT) { + if (!cmd->silent_exec_failure) + error("cannot run %s: %s", cmd->argv[0], + strerror(ENOENT));
This change is not good enough: There is no guarantee that this message goes to the right channel (stderr in the child can be redirected). Look carefully: We set a special die routine in the forked child that ensures that the die() messages are written to the right channel; we do not have a similar facility for error() messages (or do we?).
quoted hunk ↗ jump to hunk
exit(127); - else + } else { die_errno("cannot exec '%s'", cmd->argv[0]); + } } if (cmd->pid < 0) error("cannot fork() for %s: %s", cmd->argv[0],
-- Hannes