{"thread":{"id":"30662","subject":"How to handle terminal detection in a daemon calling git?","startedAt":"2012-05-30T21:16:47Z","lastAt":"2012-06-02T16:51:32Z","messageCount":9,"participants":["Travis P","Jeff King","Sitaram Chamarty","Junio C Hamano","Travis"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"192511","messageId":"1338412607.9452.140661082904349.02F677C3@webmail.messagingengine.com","threadId":"30662","inReplyTo":null,"subject":"How to handle terminal detection in a daemon calling git?","fromName":"Travis P","fromEmail":"git@castle.fastmail.fm","sentAt":"2012-05-30T21:16:47Z","receivedAt":"2012-05-30T21:16:47Z","isPatch":false,"sender":{"key":"git@castle.fastmail.fm","avatar":null},"body":"\nI've got a script that runs in the background without a terminal.\nIt actually does have STDOUT and STDERR set to a rotating log file.\n\nWhen it runs:\n /bin/sh -c 'cd /to/my/wc; git pull --ff-only'\nthe git command fails (rc 32768).\n\nWhen it runs\n /bin/sh -c 'cd /to/my/wc; git pull --ff-only > /to/a/file 2>&1'\n\nor even\n /bin/sh -c 'cd /to/my/wc; git pull --ff-only | cat'\n\nthen all is well. The command succeeds (rc 0, and I see the expected\nresults).\n\nPiping through 'cat' is okay, but I'd rather avoid the 'trick'. Is\nthere\nsome way to communicate to git that it should operate just as if output\nwere redirected?\n\n-Travis\n"},{"id":"192531","messageId":"20120531012908.GD5488@sigill.intra.peff.net","threadId":"30662","inReplyTo":"1338412607.9452.140661082904349.02F677C3@webmail.messagingengine.com","subject":"Re: How to handle terminal detection in a daemon calling git?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-31T01:29:09Z","receivedAt":"2012-05-31T01:29:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 30, 2012 at 04:16:47PM -0500, Travis P wrote:\n\n> I've got a script that runs in the background without a terminal.\n> It actually does have STDOUT and STDERR set to a rotating log file.\n> \n> When it runs:\n> /bin/sh -c 'cd /to/my/wc; git pull --ff-only'\n> the git command fails (rc 32768).\n> \n> When it runs\n> /bin/sh -c 'cd /to/my/wc; git pull --ff-only > /to/a/file 2>&1'\n> \n> or even\n> /bin/sh -c 'cd /to/my/wc; git pull --ff-only | cat'\n> \n> then all is well. The command succeeds (rc 0, and I see the expected\n> results).\n\nIf your stdout and stderr are not a terminal in the first place (you say\nthey go to a rotating log file), then that should not be making a\ndifference. Are they connected by a pty or something odd?\n\nCan you describe the failure in the first case more? Does git produce\nany output?\n\n> Piping through 'cat' is okay, but I'd rather avoid the 'trick'. Is\n> there some way to communicate to git that it should operate just as if\n> output were redirected?\n\nGit cares about terminals in only a few cases:\n\n 1. We check isatty(2) to enable progress reporting by default. You can\n use --no-progress to disable this. However, in your final example\n you only redirect stdout, which makes me think that stderr is not\n relevant.\n\n 2. We check isatty(1) for starting a pager, auto-selecting color, and\n in recent versions of git, for column support. But none of those\n things should be in use by git-pull anyway.\n\n 3. Merge was changed recently to open an editor when we have a\n terminal. That can be changed by setting GIT_MERGE_AUTOEDIT=no in\n the environment. However, since you pass --ff-only, we shouldn't be\n running merge at all.\n\nSo I'm confused. Could it be not related to a terminal at all, but that\nthere is a problem writing to the original stdout? Something that might\ngive git a SIGPIPE? Can you describe the original stdout destination\nmore?\n\n-Peff\n"},{"id":"192532","messageId":"CAMK1S_gmzgNZeJgywFaghYHH1enG=eoEnmiFhdWB0nq0iKUk3Q@mail.gmail.com","threadId":"30662","inReplyTo":"1338412607.9452.140661082904349.02F677C3@webmail.messagingengine.com","subject":"Re: How to handle terminal detection in a daemon calling git?","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2012-05-31T03:14:25Z","receivedAt":"2012-05-31T03:14:25Z","isPatch":false,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"On Thu, May 31, 2012 at 2:46 AM, Travis P wrote:\n>\n> I've got a script that runs in the background without a terminal.\n> It actually does have STDOUT and STDERR set to a rotating log file.\n>\n> When it runs:\n>  /bin/sh -c 'cd /to/my/wc; git pull --ff-only'\n> the git command fails (rc 32768).\n>\n> When it runs\n>  /bin/sh -c 'cd /to/my/wc; git pull --ff-only > /to/a/file 2>&1'\n>\n> or even\n>  /bin/sh -c 'cd /to/my/wc; git pull --ff-only | cat'\n>\n> then all is well.  The command succeeds (rc 0, and I see the expected\n> results).\n>\n> Piping through 'cat' is okay, but I'd rather avoid the 'trick'.  Is\n> there\n> some way to communicate to git that it should operate just as if output\n> were redirected?\n\nwould \"git --no-pager\" help? Honestly I don't know why your first\nerror occurred at all -- they should all have worked, IMO.\n"},{"id":"192535","messageId":"7vaa0o98e5.fsf@alter.siamese.dyndns.org","threadId":"30662","inReplyTo":"20120531012908.GD5488@sigill.intra.peff.net","subject":"Re: How to handle terminal detection in a daemon calling git?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-31T06:22:58Z","receivedAt":"2012-05-31T06:22:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, May 30, 2012 at 04:16:47PM -0500, Travis P wrote:\n>\n>> I've got a script that runs in the background without a terminal.\n>> It actually does have STDOUT and STDERR set to a rotating log file.\n>> \n>> When it runs:\n>> /bin/sh -c 'cd /to/my/wc; git pull --ff-only'\n>> the git command fails (rc 32768).\n>> \n>> When it runs\n>> /bin/sh -c 'cd /to/my/wc; git pull --ff-only > /to/a/file 2>&1'\n>> \n>> or even\n>> /bin/sh -c 'cd /to/my/wc; git pull --ff-only | cat'\n>> \n>> then all is well. The command succeeds (rc 0, and I see the expected\n>> results).\n>\n> If your stdout and stderr are not a terminal in the first place (you say\n> they go to a rotating log file), then that should not be making a\n> difference. Are they connected by a pty or something odd?\n\nA more likely failure case is when fd 0, 1 and 2 are _closed_.\n\nI vaguely recall we once saw a failure report for that particular\ncase, and then audited the code several years ago, but I do not\noffhand know if we have regressed over time.\n"},{"id":"192572","messageId":"1338471582.12573.140661083191693.32060E78@webmail.messagingengine.com","threadId":"30662","inReplyTo":"7vaa0o98e5.fsf@alter.siamese.dyndns.org","subject":"Re: How to handle terminal detection in a daemon calling git?","fromName":"Travis P","fromEmail":"git@castle.fastmail.fm","sentAt":"2012-05-31T13:39:42Z","receivedAt":"2012-05-31T13:39:42Z","isPatch":false,"sender":{"key":"git@castle.fastmail.fm","avatar":null},"body":"\nOn Wed, May 30, 2012, at 11:22 PM, Junio C Hamano wrote:\n> \n> A more likely failure case is when fd 0, 1 and 2 are _closed_.\n> \n> I vaguely recall we once saw a failure report for that particular\n> case, and then audited the code several years ago, but I do not\n> offhand know if we have regressed over time.\n\nThanks all for your suggestions about where to look. I've been doing\nsome more experiments, and I see that Junio's comment is very relevant.\n\nHere's what I learned this morning: it appears to work when I don't\nclose STDIN.\n\n #close $_ for *STDIN, *STDOUT, *STDERR; # What I was doing. Fails.\n close $_ for *STDOUT, *STDERR; # Tried this, it works.\n *STDOUT = $log_fh;\n *STDERR = $log_fh;\n\nJeff King wrote:\n> Could it be not related to a terminal at all, but that\n> there is a problem writing to the original stdout?\n\nWriting to stdout, stderr are both fine. I've been doing\n print STDOUT \"Stdout test.\\n\";\n print STDERR \"Stderr test.\\n\";\nright before the git commands to ensure that STDOUT, STDERR are hooked\nup fine to the log file.\n\nBut, you gave me a hint that led to an error message.\n\nIn a test rig where I've been looking at this just now, I was using\nbackticks and losing\na message to stderr:\n 'cd /local/test_web; git pull --ff-only | cat' rc=0 out={{}}\n 'cd /local/test_web; git pull --ff-only 2>&1 | cat' rc=0 out={{fatal:\n The remote end hung up unexpectedly\n \nNote however: despite the \"fatal\", I am seeing rc=0 and I was observing\nmy working\ncopy update as expected, so I wasn't looking for error output.\n\n> 1. We check isatty(2) to enable progress reporting by default. You can\n> use --no-progress to disable this.\n\nI don't see \"--no-progress\" making any difference.\n\n> 2. We check isatty(1) for starting a pager, auto-selecting color, and\n> in recent versions of git, for column support. But none of those\n> things should be in use by git-pull anyway.\n\nAhh, this could be it: when the pull does receive an output and I'm\nrunning\nthe command in the shell, I get output with a \"+-\" where the plus is\ngreen\nand the minus red. So, I think that git may be trying to check whether\ncolor\n (and columns?) is supported to output. However, it appears that this\n check\nis sensitive to stdin being connected (based on test mentioned earlier\nhere),\nwhich is surprising.\n\nIs the code that calls isatty, calling it on all 3 descriptors, even\nwhen\nSTDIN is not relevant?\n\n> 3. Merge was changed recently to open an editor when we have a\n> terminal. That can be changed by setting GIT_MERGE_AUTOEDIT=no in\n> the environment. However, since you pass --ff-only, we shouldn't be\n> running merge at all.\n\nYes, I tried the env switch just to be sure, and this doesn't appear\nrelevant.\n\n-Travis\n"},{"id":"192573","messageId":"1338471817.15272.140661083202725.1D34BB07@webmail.messagingengine.com","threadId":"30662","inReplyTo":"7vaa0o98e5.fsf@alter.siamese.dyndns.org","subject":"Re: How to handle terminal detection in a daemon calling git?","fromName":"Travis P","fromEmail":"git@castle.fastmail.fm","sentAt":"2012-05-31T13:43:37Z","receivedAt":"2012-05-31T13:43:37Z","isPatch":false,"sender":{"key":"git@castle.fastmail.fm","avatar":null},"body":"\nOops, I meant to mention that I'm using git 1.7.6 on RedHat.\nSomeone else did the git installation.\n"},{"id":"192644","messageId":"20120601095333.GF32340@sigill.intra.peff.net","threadId":"30662","inReplyTo":"1338471582.12573.140661083191693.32060E78@webmail.messagingengine.com","subject":"Re: How to handle terminal detection in a daemon calling git?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-01T09:53:33Z","receivedAt":"2012-06-01T09:53:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 31, 2012 at 08:39:42AM -0500, Travis P wrote:\n\n> Here's what I learned this morning: it appears to work when I don't\n> close STDIN.\n> \n> #close $_ for *STDIN, *STDOUT, *STDERR; # What I was doing. Fails.\n> close $_ for *STDOUT, *STDERR; # Tried this, it works.\n> *STDOUT = $log_fh;\n> *STDERR = $log_fh;\n\nYeah, don't do that. This can cause subtle bugs in subprocesses. For\nexample:\n\n 1. You don't have a descriptor 0, because it is closed.\n\n 2. Some part of the program opens a new descriptor (e.g., to read a\n file, making a pipe, etc). This becomes descriptor 0, because it is\n the lowest unused descriptor.\n\n 3. The program wants to redirect its stdin (e.g., because it is\n forking and exec'ing a child). So it calls dup2(fd, 0), closing\n what was at stdin previously, which might have been valuable.\n\nThe right thing to do is to redirect stdin from /dev/null, not close it\nentirely.\n\n> > 2. We check isatty(1) for starting a pager, auto-selecting color,\n> > and in recent versions of git, for column support. But none of\n> > those things should be in use by git-pull anyway.\n> \n> Ahh, this could be it: when the pull does receive an output and I'm\n> running the command in the shell, I get output with a \"+-\" where the\n> plus is green and the minus red. So, I think that git may be trying\n> to check whether color (and columns?) is supported to output.\n> However, it appears that this check is sensitive to stdin being\n> connected (based on test mentioned earlier here), which is surprising.\n>\n> Is the code that calls isatty, calling it on all 3 descriptors, even\n> when STDIN is not relevant?\n\nNo, the color code just checks isatty(1). And even if it checked some\nrandom descriptor, the worst case should be that it affects the color\nflag. So I think the terminal thing is a red herring, and it is more\nlikely you are seeing some subtle issue like the one I described above.\n\n-Peff\n"},{"id":"192661","messageId":"827737B4-F5EE-4FD3-9B1A-710047836080@castle.fastmail.fm","threadId":"30662","inReplyTo":"20120601095333.GF32340@sigill.intra.peff.net","subject":"Re: How to handle terminal detection in a daemon calling git?","fromName":"Travis","fromEmail":"git@castle.fastmail.fm","sentAt":"2012-06-01T13:52:04Z","receivedAt":"2012-06-01T13:52:04Z","isPatch":false,"sender":{"key":"git@castle.fastmail.fm","avatar":null},"body":"\nOn Jun 1, 2012, at 4:53 AM, Jeff King wrote:\n\n> On Thu, May 31, 2012 at 08:39:42AM -0500, Travis P wrote:\n>\n>> Here's what I learned this morning: it appears to work when I don't\n>> close STDIN.\n>>\n>> #close $_ for *STDIN, *STDOUT, *STDERR; # What I was doing. Fails.\n>> close $_ for *STDOUT, *STDERR; # Tried this, it works.\n>> *STDOUT = $log_fh;\n>> *STDERR = $log_fh;\n>\n> Yeah, don't do that. This can cause subtle bugs in subprocesses. For\n> example:\n>\n> 1. You don't have a descriptor 0, because it is closed.\n>\n> 2. Some part of the program opens a new descriptor (e.g., to read a\n> file, making a pipe, etc). This becomes descriptor 0, because it \n> is\n> the lowest unused descriptor.\n>\n> 3. The program wants to redirect its stdin (e.g., because it is\n> forking and exec'ing a child). So it calls dup2(fd, 0), closing\n> what was at stdin previously, which might have been valuable.\n>\n> The right thing to do is to redirect stdin from /dev/null, not close \n> it\n> entirely.\n\nAh, yes, that's a normal thing I would think about in C. In Perl,\nI imagined that those details were handled by Perl.\n\nWith that in mind, I'm still seeing strange behavior when I do this,\nwhere it looks to me like I'm closing and then immediately assigning\nSTDIN:\n\n my $null_in_fh;\n open($null_in_fh, '<', '/dev/null') or die;\n close *STDIN; # this appears to mess things up, even with the \nfollowing assignment\n *STDIN = $null_in_fh;\n\nBut if I don't do the close STDIN (with or without the glob), then \nthings work.\nMaybe that assignment doesn't work for some Perl-ish reason.\n\nBecause, if I just do this\n open(STDIN, '<', '/dev/null') or die;\neven after the close and/or assignment, then all appears okay.\n\nI'll stick with this last technique and just chalk it up to something\nto something about Perl I don't understand. (I had the idea that\nmaybe I wanted to redirect /dev/null to STDIN at some point anyway.)\n\nThanks for your comments.\n\n-Travis\n"},{"id":"192720","messageId":"20120602165132.GE15017@sigill.intra.peff.net","threadId":"30662","inReplyTo":"827737B4-F5EE-4FD3-9B1A-710047836080@castle.fastmail.fm","subject":"Re: How to handle terminal detection in a daemon calling git?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-02T16:51:32Z","receivedAt":"2012-06-02T16:51:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 01, 2012 at 08:52:04AM -0500, Travis wrote:\n\n> With that in mind, I'm still seeing strange behavior when I do this,\n> where it looks to me like I'm closing and then immediately assigning\n> STDIN:\n> \n> my $null_in_fh;\n> open($null_in_fh, '<', '/dev/null') or die;\n> close *STDIN; # this appears to mess things up, even with the\n> following assignment\n> *STDIN = $null_in_fh;\n\nKeep in mind that STDIN is a perl filehandle, not a file descriptor.\nWhen you close it, you close the filehandle and its underlying\ndescriptor. Then you assign another filehandle to STDIN, which has its\nown descriptor.\n\nTry this:\n\n strace perl -e '\n open($null, \";\n '\n\nyou'll see that it is doing something like:\n\n open(\"/dev/null\", O_RDONLY) = 3\n close(0) = 0\n read(3, \"\", 8192) = 0\n\nThe final read comes from the new descriptor, and we never re-opened\ndescriptor 0. You are basically just pointing the name STDIN to a new\nhandle, not doing anything with the underlying descriptor.\n\n> Because, if I just do this\n> open(STDIN, '<', '/dev/null') or die;\n> even after the close and/or assignment, then all appears okay.\n\nYeah, this is the right way to re-open a descriptor in perl, because\nopen() does the magic to dup2 the newly opened descriptor onto STDIN's\ndescriptor.\n\n> Thanks for your comments.\n\nNo problem. Glad it is working now.\n\n-Peff\n"}]}