git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Clemens Buchacher <drizzd@aon.at>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH] notice error exit from pager
Date: Mon, 1 Aug 2011 19:59:21 +0200	[thread overview]
Message-ID: <20110801175921.GA17092@toss> (raw)
In-Reply-To: <20110726210401.GA25207@toss.lan>

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). The child writes one byte to a pipe only if
     execvp fails.  The parent waits for either EOF, when the
     successful execvp automatically closes the pipe (see
     FD_CLOEXEC in fcntl(1)), or it reads a single byte, in which
     case it knows that the execvp failed. This mechanism is
     incompatible with the workaround introduced in 35ce8622
     (pager: Work around window resizing bug in 'less'), which
     waits for input from the parent before the exec. Since both
     the parent and the child are waiting for input from each
     other, that would result in a deadlock. In order to avoid
     that, the mechanism is disabled by closing the child_notifier
     file descriptor.

 (2) finish_command: The parent correctly detects the 127 exit
     status from the child, but the error output goes nowhere,
     since by that time it is already being redirected to the
     child.

No simple solution for (1) comes to mind.

Number (2) can be solved by not sending error output to the pager.
Not redirecting error output to the pager can result in the pager
overwriting error output with standard output, however.

Since there is no reliable way to handle error reporting in the
parent, produce the output in the child instead.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---

Nothing new compared to the RFC, just a slightly trimmed commit
message.

 run-command.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/run-command.c b/run-command.c
index 5c91f37..a2796c4 100644
--- a/run-command.c
+++ b/run-command.c
@@ -125,9 +125,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);
@@ -282,14 +279,14 @@ fail_pipe:
 		} else {
 			execvp(cmd->argv[0], (char *const*) cmd->argv);
 		}
-		/*
-		 * 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));
 			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],
-- 
1.7.3.1.105.g84915

  parent reply	other threads:[~2011-08-01 17:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-26 21:04 [PATCH/RFC] notice error exit from pager Clemens Buchacher
2011-07-26 21:35 ` Linus Torvalds
2011-07-27 19:36 ` Johannes Sixt
2011-07-27 21:32   ` [PATCH] error_routine: use parent's stderr if exec fails Clemens Buchacher
2011-08-01 17:59 ` Clemens Buchacher [this message]
2011-08-01 20:17   ` [PATCH] notice error exit from pager Junio C Hamano
2011-08-01 20:35     ` Clemens Buchacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110801175921.GA17092@toss \
    --to=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).