On Thu, May 31, 2012 at 08:39:42AM -0500, Travis P wrote:
> Here's what I learned this morning: it appears to work when I don't
> close STDIN.
>
> #close $_ for *STDIN, *STDOUT, *STDERR; # What I was doing. Fails.
> close $_ for *STDOUT, *STDERR; # Tried this, it works.
> *STDOUT = $log_fh;
> *STDERR = $log_fh;
Yeah, don't do that. This can cause subtle bugs in subprocesses. For
example:
1. You don't have a descriptor 0, because it is closed.
2. Some part of the program opens a new descriptor (e.g., to read a
file, making a pipe, etc). This becomes descriptor 0, because it is
the lowest unused descriptor.
3. The program wants to redirect its stdin (e.g., because it is
forking and exec'ing a child). So it calls dup2(fd, 0), closing
what was at stdin previously, which might have been valuable.
The right thing to do is to redirect stdin from /dev/null, not close it
entirely.
> > 2. We check isatty(1) for starting a pager, auto-selecting color,
> > and in recent versions of git, for column support. But none of
> > those things should be in use by git-pull anyway.
>
> Ahh, this could be it: when the pull does receive an output and I'm
> running the command in the shell, I get output with a "+-" where the
> plus is green and the minus red. So, I think that git may be trying
> to check whether color (and columns?) is supported to output.
> However, it appears that this check is sensitive to stdin being
> connected (based on test mentioned earlier here), which is surprising.
>
> Is the code that calls isatty, calling it on all 3 descriptors, even
> when STDIN is not relevant?
No, the color code just checks isatty(1). And even if it checked some
random descriptor, the worst case should be that it affects the color
flag. So I think the terminal thing is a red herring, and it is more
likely you are seeing some subtle issue like the one I described above.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html