Re: [Announce] GIT v1.5.0-rc2

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Jan 24, 2007, 19:21 UTC
Message-ID: <Pine.LNX.4.64.0701241108370.3606@woody.linux-foundation.org>
In-Reply-To: <Pine.LNX.4.64.0701231157430.32200@woody.linux-foundation.org>

On Tue, 23 Jan 2007, Linus Torvalds wrote:

Show 14 quoted lines

> 
> I think "less" is actually seriously buggy with -F.
> 
> There are two bugs:
> 
>  - it will always screw up the screen and move to the end. It does this 
>    even if you use -FX which should disable any init sequences, so it's 
>    not about that problem.
> 
>  - if you resize the terminal while less is waiting for input, less
>    will exit entirely without even showing the output. This is very
>    noticeable if you do something like "git diff" on a big and cold-cache 
>    tree and git takes a few seconds to think, and then you resize the 
>    window while it's preparing. Boom. No output AT ALL.

Heh. This extremely hacky patch works around the second bug.

It does so by simply adding a "select()" on the input before even starting "less", which will mean that by the time less starts, it always has something to read, and that in turn hides the bug with resizing the terminal window while less is waiting for input.

I'm sure there's still a window for the bug to trigger, but I can no longer trivially reproduce the problem any more.

(The way to reproduce the problem is to do some pager operation that takes a while in git, and resizing the window while git is thinking about the output. I use

	git diff --stat v2.6.12..

in the kernel tree to do something where it takes a while for git to start outputting information)

Without this patch, I can easily just resize the window while git is thinking, and the end result is that "less" will exit early and indeed leave the tty in a broken state with no echo etc. With this patch, less doesn't get confused.

NOTE! To see this problem, you must use the LESS environment that git provides by default (LESS=FRSX) and not have your own environment set that overrides the git ones (or if you do, it must have -F set).

Is it ugly? Yes. Does it work? Yes. Do we want to apply it? You decide.

		Linus

---

diff --git a/pager.c b/pager.c
index 4587fbb..5f280ab 100644
--- a/pager.c
+++ b/pager.c
@@ -1,5 +1,7 @@
 #include "cache.h"
 
+#include <sys/select.h>
+
 /*
  * This is split up from the rest of git so that we might do
  * something different on Windows, for example.
@@ -7,6 +9,16 @@
 
 static void run_pager(const char *pager)
 {
+	/*
+	 * Work around bug in "less" by not starting it until we
+	 * have real input
+	 */
+	fd_set in;
+
+	FD_ZERO(&in);
+	FD_SET(0, &in);
+	select(1, &in, NULL, &in, NULL);
+
 	execlp(pager, pager, NULL);
 	execl("/bin/sh", "sh", "-c", pager, NULL);
 }