Skip Menu |

This queue is for tickets about the Git-Repository CPAN distribution.

Report information
The Basics
Id: 80610
Status: resolved
Priority: 0/
Queue: Git-Repository

People
Owner: Nobody in particular
Requestors: tlinden [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 1.27
Fixed in: (no value)



Subject: wierd error reporting when run without controlling terminal
Hello, I'm using the module in http://search.cpan.org/dist/Quietly-Confident/ and it works very well. However, I observed some strange behavior. The script I'm using works on a git repository but it is also able to do fallback on simple filesystem operations. For this to work, I catch errors returned by Git::Repository using eval{}: sub watch { my ($git) = @_; my $gitmode = 1; if(! $git) { eval { $git = Git::Repository->new( work_tree => $config{repository} ) or die "Could not init git: $!\n"; }; if($@ || ! -d "$config{repository}/.git") { print STDERR "$config{repository} is not a git repository!\n"; print STDERR "Watching in filesystem mode.\n"; $gitmode = 0; } } if($gitmode) { &log("gitwatch"); &gitwatch($git); } else { &log("fswatch"); &fswatch(); } } This routine works as expected when run in a controlling terminal session. But the script is also able to run as a daemon using fork() and this is what happens then: - if STDERR is closed (what I usually do when in daemon mode), Git::Repository causes the script to terminate. - if I leave STDERR open, it prints the following message to STDERR: fatal: Not a git repository (or any parent up to mount parent /home) Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set). at /usr/local/lib/perl5/site_perl/5.10.1/Git/Repository.pm line 195 but keeps running. This is the very same message I'm catching in the eval{} block above (and ignoring it to go to the fallback mode). And as extra bonus the eval{} block doesn't catch the error in daemon mode. $@ is empty and instead Git::Repository prints it itself to STDERR, if available, or exits if not. I'm not sure wether this is caused by Git::Repository itself or by behavior of the underlying git command. But it would be great if I could catch errors using eval{} as expected when not connected to a controlling terminal session. best regards, Tom
PS: I added ! -d "$config{repository}/.git" as a workaround to the problem. So the method posted works in daemon mode as well as long as I keep STDERR open. It doesn't catch $@ but looks itself for the .git/ subdir. But I'd find it better without it.
Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Sun, 4 Nov 2012 20:22:44 +0100
To: "T. Linden via RT" <bug-Git-Repository [...] rt.cpan.org>
From: "Philippe Bruhat (BooK)" <book [...] cpan.org>
On Sun, Nov 04, 2012 at 04:33:35AM -0500, T. Linden via RT wrote: Show quoted text
> > I'm using the module in http://search.cpan.org/dist/Quietly-Confident/ > and it works very well. However, I observed some strange behavior.
I've tried to reproduce the problem, but haven't managed to yet. Maybe replying to your email will help me focus on the details. :-) Show quoted text
> The > script I'm using works on a git repository but it is also able to do > fallback on simple filesystem operations. For this to work, I catch > errors returned by Git::Repository using eval{}: > > sub watch { > my ($git) = @_; > my $gitmode = 1; > if(! $git) { > eval { > $git = Git::Repository->new( work_tree => $config{repository} ) > or die "Could not init git: $!\n"; > };
You don't need the "or die" here. Git::Repository dies whenever git exits with a status code of 128 (fatal error) or 129 (usage error). Wrapping an eval {} around Git::Repository->new() is enough to catch any git error. Also, Git::Repository doesn't set $!, which is for errors in system calls. Show quoted text
> if($@ || ! -d "$config{repository}/.git") { > print STDERR "$config{repository} is not a git repository!\n"; > print STDERR "Watching in filesystem mode.\n"; > $gitmode = 0; > } > } > if($gitmode) { > &log("gitwatch"); > &gitwatch($git); > } > else { > &log("fswatch"); > &fswatch(); > } > } > > This routine works as expected when run in a controlling terminal > session.
As explained above, Git::Repository dies (croaks actually) when git exits with status 128 or 129. When running Git::Repository->new in a non-repository, the results are: $ git rev-parse --git-dir ; echo $? fatal: Not a git repository (or any of the parent directories): .git 128 in a "normal" directory, or $ git rev-parse --git-dir ; echo $? fatal: Not a git repository (or any parent up to mount parent /home) Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set). 128 in a directory that is mounted somewhere below / (as in your case). Show quoted text
> But the script is also able to run as a daemon using fork() and > this is what happens then:
Show quoted text
> - if STDERR is closed (what I usually do when in daemon mode), > Git::Repository causes the script to terminate. > - if I leave STDERR open, it prints the following message to STDERR: > > fatal: Not a git repository (or any parent up to mount parent /home) > Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not > set). at /usr/local/lib/perl5/site_perl/5.10.1/Git/Repository.pm line 195 > > but keeps running.
Indeed, the expected behaviour for Git::Repository would be to die, and if in an eval block, $@ should be set to the message that git outputs. Show quoted text
> This is the very same message I'm catching in the eval{} block above > (and ignoring it to go to the fallback mode). And as extra bonus the > eval{} block doesn't catch the error in daemon mode. $@ is empty and > instead Git::Repository prints it itself to STDERR, if available, or > exits if not. > > I'm not sure wether this is caused by Git::Repository itself or by > behavior of the underlying git command. But it would be great if I could > catch errors using eval{} as expected when not connected to a > controlling terminal session.
Git::Repository is using System::Command to run external commands, which itself depends on IPC::Open3. Given that the program works as expected in "normal" mode, I suppose the issue is caused by the loss of a controlling terminal. I would be interested in seeing the commands you perform to go in daemon mode, so that I could try to reproduce the problem more accurately. -- Philippe Bruhat (BooK) Only a fool follows orders without knowing why. (Moral from Groo The Wanderer #86 (Epic))
Show quoted text
> You don't need the "or die" here. Git::Repository dies whenever git > exits with a status code of 128 (fatal error) or 129 (usage error). > Wrapping an eval {} around Git::Repository->new() is enough to > catch any git error. > > Also, Git::Repository doesn't set $!, which is for errors in system > calls.
Fascinating. I removed the "or die" clause and now everything works as expected. Thanks for the hint! best regards, Tom
Ahm, sorry. Don't know what I did, but the problem remains. I removed the "or die" code, but $@ is empty. I dumped it out: eval { $git = Git::Repository->new( work_tree => $config{repository} ); }; print STDERR Dumper ($@); fatal: Not a git repository (or any parent up to mount parent /home) Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set). at /usr/local/lib/perl5/site_perl/5.10.1/Git/Repository.pm line 195 fatal: Not a git repository: '/home/leo/website/source' at /usr/local/lib/perl5/site_perl/5.10.1/Git/Repository.pm line 195 $VAR1 = ''; As you can see, $@ is empty. But I took a look into System::Command and I think the source is there. It's using eval{} as well, in fact two times. Including my eval{} block this makes 3 nested eval's. Somewhere in between (without controlling terminal) $@ gets lost. I'll file an issue at System::Command.
Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Mon, 5 Nov 2012 10:44:24 +0100
To: "T. Linden via RT" <bug-Git-Repository [...] rt.cpan.org>
From: "Philippe Bruhat (BooK)" <book [...] cpan.org>
On Mon, Nov 05, 2012 at 04:35:17AM -0500, T. Linden via RT wrote: Show quoted text
> > But I took a look into System::Command and I think the source is there. > It's using eval{} as well, in fact two times. Including my eval{} block > this makes 3 nested eval's. Somewhere in between (without controlling > terminal) $@ gets lost. > > I'll file an issue at System::Command.
The superfluous eval is indeed in System::Command, and the bug was alread reported: https://rt.cpan.org/Ticket/Display.html?id=80171 The master branch on github has a patch removing the extra eval {}. https://github.com/book/System-Command/commit/f19b83b5228bc6746879e0b52a7acbfe4fc82672 I'll wrap up a System-Command release soon. Please let me know if the development version on github works better for you. -- Philippe Bruhat (BooK) The more destruction we spread, the more we destroy ourselves. (Moral from Groo #12 (Image))
I applied the patch but nothing changed. Still no $@.
Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Mon, 5 Nov 2012 23:03:32 +0100
To: "T. Linden via RT" <bug-Git-Repository [...] rt.cpan.org>
From: "Philippe Bruhat (BooK)" <book [...] cpan.org>
On Mon, Nov 05, 2012 at 05:07:28AM -0500, T. Linden via RT wrote: Show quoted text
> Queue: Git-Repository > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=80610 > > > I applied the patch but nothing changed. Still no $@.
I wrote the attached test script for Git::Repository, based on the code in http://cpansearch.perl.org/src/TLINDEN/Quietly-Confident-0.06/qc It forks two child process, one that redirects STDERR, and one that does not. The results are a little different for each child: $ perl -Ilib t/24-fork.t fatal: Not a git repository (or any of the parent directories): .git ok - redirect STDERR: yes # fatal: unknown git error at t/24-fork.t line 36. ok - redirect STDERR: no # fatal: Not a git repository (or any of the parent directories): .git at t/24-fork.t line 36. 1..0 So, when STDERR is redirected, the git error ends up on the terminal, and $@ is not empty, but has the default message from Git::Repository when git died with status 128 or 129 and nothing was captured on the git subprocess' STDERR. On the other hand, when not redirecting STDERR, $@ contains the git error message as expected. I'm not sure if the issue lies with the way System::Command captures subprocess output, or with the way you redirect STDERR to a file. In the latter case, the documentation for open() has copious example of filehandle redirections. Maybe there's a better way to do it explained there? -- Philippe Bruhat (BooK) The worst curses in the world are boils, pestilence and having partners (not necessarily in that order). (Moral from Groo The Wanderer #6 (Pacific))

Message body is not shown because sender requested not to inline it.

CC: "T. Linden via RT" <bug-Git-Repository [...] rt.cpan.org>
Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Mon, 5 Nov 2012 23:15:03 +0100
From: "Philippe Bruhat (BooK)" <book [...] cpan.org>
On Mon, Nov 05, 2012 at 11:03:32PM +0100, Philippe Bruhat (BooK) wrote: Show quoted text
> On Mon, Nov 05, 2012 at 05:07:28AM -0500, T. Linden via RT wrote:
> > Queue: Git-Repository > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=80610 > > > > > I applied the patch but nothing changed. Still no $@.
> > I wrote the attached test script for Git::Repository, based on the code > in http://cpansearch.perl.org/src/TLINDEN/Quietly-Confident-0.06/qc > > It forks two child process, one that redirects STDERR, and one that > does not. The results are a little different for each child: > > $ perl -Ilib t/24-fork.t > fatal: Not a git repository (or any of the parent directories): .git > ok - redirect STDERR: yes > # fatal: unknown git error at t/24-fork.t line 36. > ok - redirect STDERR: no > # fatal: Not a git repository (or any of the parent directories): .git at t/24-fork.t line 36. > 1..0
Obviously, the test should do this instead: like( $@, qr/^fatal: Not a git repository/, "redirect STDERR: $redirect" ); Which results in: $ perl -Ilib t/24-fork.t fatal: Not a git repository (or any of the parent directories): .git # fatal: unknown git error at t/24-fork.t line 36. not ok - redirect STDERR: yes # Failed test 'redirect STDERR: yes' # at t/24-fork.t line 38. # 'fatal: unknown git error at t/24-fork.t line 36. # ' # doesn't match '(?-xism:^fatal: Not a git repository)' # fatal: Not a git repository (or any of the parent directories): .git at t/24-fork.t line 36. ok - redirect STDERR: no 1..0 More clearly exposing the problem with $@'s content. -- Philippe Bruhat (BooK) No matter how many times you explain the big problem, some people see only their small problem. (Moral to the Sage story, in Groo The Wanderer #93 (Epic))
Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Tue, 6 Nov 2012 15:46:18 +0100
To: Philippe 'BooK' Bruhat via RT <bug-Git-Repository [...] rt.cpan.org>
From: tlinden [...] cpan.org
I don't think it has anyting to do with what I do with STDERR redirecting. I tested it with the redirect disabled (thus STDERR leave as is) with the same result. I also tried to use IPC::Open3 directly for testing. I added the following code: use IPC::Open3; chdir $config{repository}; my $pid = open3(\*CHLD_IN, \*CHLD_OUT, \*CHLD_ERR, "git status"); &log("open3: pid: $pid"); &log("stderr: " . join '', <CHLD_ERR>); &log("stdout: " . join '', <CHLD_OUT>); When run in daemon mode, the logfile shows: Tue Nov 6 15:41:58 2012 [58392] open3: pid: 58393 Tue Nov 6 15:41:58 2012 [58392] stderr: fatal: Not a git repository (or any parent up to mount parent /home) Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set). Tue Nov 6 15:41:58 2012 [58392] stdout: (STDERR redirecting not enabled during that test). So, when using plain IPC::open3 directly, it seems to work. best regards, Tom -- Please note that according to the German law on data retention, information on every electronic information exchange with me is retained for a period of six months.
Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Tue, 6 Nov 2012 17:36:00 +0100
To: "T. Linden via RT" <bug-Git-Repository [...] rt.cpan.org>
From: "Philippe Bruhat (BooK)" <book [...] cpan.org>
On Tue, Nov 06, 2012 at 09:46:35AM -0500, T. Linden via RT wrote: Show quoted text
> Queue: Git-Repository > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=80610 > > > I don't think it has anyting to do with what I do with STDERR > redirecting. I tested it with the redirect disabled (thus STDERR leave > as is) with the same result. >
I'm not sure my test script is reproducing what you're seeing. Can you confirm that? If not, we should first focus on writing such a test script: being able to write a minimal test script that reproduces the issue would help immensely. If I understood correctly, the issue is defined by: - a Git::Repository call that fails, wrapped in an eval {} - $@ is empty after the eval {} - the error message is output to STDERR Is that right? -- Philippe Bruhat (BooK) Putting beauty before brains is the surest way to wind up with neither. (Moral from Groo The Wanderer #24 (Epic))
Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Fri, 9 Nov 2012 16:40:21 +0100
To: Philippe 'BooK' Bruhat via RT <bug-Git-Repository [...] rt.cpan.org>
From: Thomas Linden <tom [...] linden.at>
On Tue, Nov 06, 2012 at 11:36:18AM -0500, Philippe 'BooK' Bruhat via RT wrote: Show quoted text
> I'm not sure my test script is reproducing what you're seeing. > Can you confirm that?
Unfortunately not :) Show quoted text
> If I understood correctly, the issue is defined by: > - a Git::Repository call that fails, wrapped in an eval {} > - $@ is empty after the eval {} > - the error message is output to STDERR > > Is that right?
It is, but the signal handler is missing. Find the test script 25-fork.t attached. It reproduces the behavior. As your testscript does work, but this one not, I think the cause is the signal handler. I'm not sure, but I think it interferes with the signalhandler in System::Command. best regards, Tom -- Please note that according to the German law on data retention, information on every electronic information exchange with me is retained for a period of six months.

Message body is not shown because sender requested not to inline it.

Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Fri, 9 Nov 2012 16:42:20 +0100
To: Philippe 'BooK' Bruhat via RT <bug-Git-Repository [...] rt.cpan.org>
From: Thomas Linden <tom [...] linden.at>
PS: it doesn't make a difference wether $git is assigned inside the eval{} as in 25-fork.t or not as in 24-fork.t. I tried both versions. - Tom -- Please note that according to the German law on data retention, information on every electronic information exchange with me is retained for a period of six months.
Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Thu, 22 Nov 2012 09:29:46 +0100
To: Thomas Linden via RT <bug-Git-Repository [...] rt.cpan.org>
From: "Philippe Bruhat (BooK)" <philippe.bruhat [...] free.fr>
On Fri, Nov 09, 2012 at 10:40:34AM -0500, Thomas Linden via RT wrote: Show quoted text
> Queue: Git-Repository > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=80610 > >
> > If I understood correctly, the issue is defined by: > > - a Git::Repository call that fails, wrapped in an eval {} > > - $@ is empty after the eval {} > > - the error message is output to STDERR > > > > Is that right?
> > It is, but the signal handler is missing. Find the test script 25-fork.t > attached. It reproduces the behavior. As your testscript does work, but > this one not, I think the cause is the signal handler. I'm not sure, but > I think it interferes with the signalhandler in System::Command. >
Indeed the signal handler is causing the problem. The parent process has a signal handler, and because it is set before the call to fork(), the child process inherits it. But the child process does not need to handle the CHLD signal, since it's not going to fork and System::Command will reap the child process for the commands it launches. So, the simplest fix is simply to disable the signal handler in the child process, by running this (in the child process) right after the fork: delete $SIG{CHLD}; Note that this is also explained in the System::Command documentation: https://metacpan.org/module/System::Command#CAVEAT-EMPTOR If the subprocess started by System::Command has a short life expectancy, and no other child process is expected to die during that time, you could even disable the handler locally (use at your own risks): { local $SIG{CHLD}; my $cmd = System::Command->new(@cmd); ... } -- Philippe Bruhat (BooK) Even the worst guesser is right once in a while. (Moral from Groo The Wanderer #72 (Epic))
Subject: Re: [rt.cpan.org #80610] wierd error reporting when run without controlling terminal
Date: Mon, 26 Nov 2012 15:36:54 +0100
To: "(Philippe 'BooK' Bruhat) via RT" <bug-Git-Repository [...] rt.cpan.org>
From: tlinden [...] cpan.org
On Thu, Nov 22, 2012 at 03:29:58AM -0500, (Philippe 'BooK' Bruhat) via RT wrote: Show quoted text
> Indeed the signal handler is causing the problem. > > The parent process has a signal handler, and because it is set before > the call to fork(), the child process inherits it. But the child process > does not need to handle the CHLD signal, since it's not going to fork and > System::Command will reap the child process for the commands it launches. > > So, the simplest fix is simply to disable the signal handler in the child > process, by running this (in the child process) right after the fork: > > delete $SIG{CHLD};
Indeed! Why didn't you tell me about this earlier? just kidding.. :) Ok, Thanks a lot for the help! best regards, Tom -- Please note that according to the German law on data retention, information on every electronic information exchange with me is retained for a period of six months.