How to handle terminal detection in a daemon calling git?

I've got a script that runs in the background without a terminal. It actually does have STDOUT and STDERR set to a rotating log file.

When it runs:
  /bin/sh -c 'cd /to/my/wc; git pull --ff-only'
the git command fails (rc 32768).

When it runs
 /bin/sh -c 'cd /to/my/wc; git pull --ff-only > /to/a/file 2>&1'

or even
 /bin/sh -c 'cd /to/my/wc; git pull --ff-only | cat'

then all is well. The command succeeds (rc 0, and I see the expected results).

Piping through 'cat' is okay, but I'd rather avoid the 'trick'. Is there some way to communicate to git that it should operate just as if output were redirected?

-Travis

On Wed, May 30, 2012 at 04:16:47PM -0500, Travis P wrote:

If your stdout and stderr are not a terminal in the first place (you say they go to a rotating log file), then that should not be making a difference. Are they connected by a pty or something odd?

Can you describe the failure in the first case more? Does git produce any output?

Git cares about terminals in only a few cases:

  1. We check isatty(2) to enable progress reporting by default. You can
     use --no-progress to disable this. However, in your final example
     you only redirect stdout, which makes me think that stderr is not
     relevant.

  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.

  3. Merge was changed recently to open an editor when we have a
     terminal. That can be changed by setting GIT_MERGE_AUTOEDIT=no in
     the environment. However, since you pass --ff-only, we shouldn't be
     running merge at all.

So I'm confused. Could it be not related to a terminal at all, but that there is a problem writing to the original stdout? Something that might give git a SIGPIPE? Can you describe the original stdout destination more?

-Peff

Jeff King <peff@peff.net> writes:

A more likely failure case is when fd 0, 1 and 2 are _closed_.

I vaguely recall we once saw a failure report for that particular case, and then audited the code several years ago, but I do not offhand know if we have regressed over time.

On Wed, May 30, 2012, at 11:22 PM, Junio C Hamano wrote:

Thanks all for your suggestions about where to look. I've been doing some more experiments, and I see that Junio's comment is very relevant.

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;

Jeff King wrote:

Writing to stdout, stderr are both fine.  I've been doing
  print STDOUT "Stdout test.\n";
  print STDERR "Stderr test.\n";
right before the git commands to ensure that STDOUT, STDERR are hooked
up fine to the log file.

But, you gave me a hint that led to an error message.

In a test rig where I've been looking at this just now, I was using
backticks and losing
a message to stderr:
 'cd /local/test_web; git pull --ff-only | cat'   rc=0 out={{}}
 'cd /local/test_web; git pull --ff-only 2>&1 | cat'   rc=0 out={{fatal:
 The remote end hung up unexpectedly
 
Note however:  despite the "fatal", I am seeing rc=0 and I was observing
my working
copy update as expected, so I wasn't looking for error output.

I don't see "--no-progress" making any difference.

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?

Yes, I tried the env switch just to be sure, and this doesn't appear relevant.

-Travis

On Thu, May 31, 2012 at 08:39:42AM -0500, Travis P wrote:

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.

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

On Jun 1, 2012, at 4:53 AM, Jeff King wrote:

Ah, yes, that's a normal thing I would think about in C. In Perl, I imagined that those details were handled by Perl.

With that in mind, I'm still seeing strange behavior when I do this, where it looks to me like I'm closing and then immediately assigning STDIN:

  my $null_in_fh;
  open($null_in_fh, '<', '/dev/null') or die;
  close *STDIN;  # this appears to mess things up, even with the  
following assignment
  *STDIN = $null_in_fh;

But if I don't do the close STDIN (with or without the glob), then things work. Maybe that assignment doesn't work for some Perl-ish reason.

Because, if I just do this
   open(STDIN, '<', '/dev/null') or die;
even after the close and/or assignment, then all appears okay.

I'll stick with this last technique and just chalk it up to something to something about Perl I don't understand. (I had the idea that maybe I wanted to redirect /dev/null to STDIN at some point anyway.)

Thanks for your comments.

-Travis

On Fri, Jun 01, 2012 at 08:52:04AM -0500, Travis wrote:

Keep in mind that STDIN is a perl filehandle, not a file descriptor. When you close it, you close the filehandle and its underlying descriptor. Then you assign another filehandle to STDIN, which has its own descriptor.

Try this:

  strace perl -e '
    open($null, "</dev/null");
    close *STDIN;
    *STDIN = $null;
    <STDIN>;
  '

you'll see that it is doing something like:

  open("/dev/null", O_RDONLY)             = 3
  close(0)                                = 0
  read(3, "", 8192)                       = 0

The final read comes from the new descriptor, and we never re-opened descriptor 0. You are basically just pointing the name STDIN to a new handle, not doing anything with the underlying descriptor.

Yeah, this is the right way to re-open a descriptor in perl, because open() does the magic to dup2 the newly opened descriptor onto STDIN's descriptor.

No problem. Glad it is working now.

-Peff

Oops, I meant to mention that I'm using git 1.7.6 on RedHat. Someone else did the git installation.

On Thu, May 31, 2012 at 2:46 AM, Travis P <git@castle.fastmail.fm> wrote:

would "git --no-pager" help? Honestly I don't know why your first error occurred at all -- they should all have worked, IMO.