Bug #73614 for System-Command: *COUGH* Catalyst *COUGH* hack breaks stdout catching.

Fri Dec 30 02:46:06 2011 jabbas [...] jabbas.eu - Ticket created

Subject:

*COUGH* Catalyst *COUGH* hack breaks stdout catching.

See attached test. The problem exists also with some FastCGI apps. Commenting out BEGIN block and three lines after "save standard handles" in $_spawn fixes the problem, but most probably will break usage of System::Command in Catalyst.

Subject:

syscmd.t

#!/usr/bin/perl use warnings; use strict; use System::Command; use Test::Output; sub stdout_write { print "CHECK\n"; } stdout_is(\&stdout_write, "CHECK\n", 'works'); my $cmd = System::Command->new(qw/echo/); stdout_is(\&stdout_write, "CHECK\n", 'works');

Tue Jan 03 05:11:04 2012 book [...] cpan.org - Correspondence added

On Fri Dec 30 02:46:06 2011, jabbas wrote: Show quoted text

> See attached test. > > The problem exists also with some FastCGI apps. > > Commenting out BEGIN block and three lines after "save standard

handles" Show quoted text

> in $_spawn fixes the problem, but most probably will break usage of > System::Command in Catalyst.

The problem with that catalyst patch is that it didn't come with a test. And I'm not a catalyst user, so I have no idea how to reproduce the bug. That piece of code actually comes from Git-Repository, before System- Command was split off from it: https://github.com/book/Git- Repository/commit/443a94018f5577575d548b8db4e4c313f5c70377 I don't know how to fix this. Thanks for the report. -- BooK

Tue Jan 03 05:11:05 2012 The RT System itself - Status changed from 'new' to 'open'

Wed Jan 04 16:11:53 2012 BAREFOOT [...] cpan.org - Correspondence added

This also breaks when used with Test::Command (see attached). Perhaps the original patch submitter should be contacted to see if he can explain the bug? Otherwise, at this point it seems like it's hurting more people than it's helping.

Subject:

test.t

#! /usr/bin/perl use Test::More 0.88; use Test::Command; use System::Command; stderr_like(q{ perl -le 'print STDERR "STDERR points to ", readlink "/proc/self/fd/2"' }, qr/STDERR points to/, 'should always pass'); done_testing;

Wed Jan 04 18:14:53 2012 bug-System-Command [...] rt.cpan.org - Correspondence added

CC:	bug-System-Command [...] rt.cpan.org
Subject:	[rt.cpan.org #73614] COUGH Catalyst COUGH hack breaks stdout catching.
Date:	Thu, 5 Jan 2012 00:14:43 +0100
To:	Todd Rinaldo <toddr [...] cpan.org>
From:	"Philippe Bruhat (BooK)" <philippe.bruhat [...] free.fr>

Hi, A while ago, you sent me a patch for Git::Repository, because it had some issues when used with Catalyst. The code in this patch made its way to System::Command when I move the non git-specific functionality of Git::Repository::Command into System::Command. It seems it causing problems in other situations. Can you have a look and comment? Queue: System-Command Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=73614 > Thanks in advance, -- Philippe Bruhat (BooK) The shortest distance between two points is not always the safest. (Moral from Groo The Wanderer #69 (Epic))

Thu Jan 05 14:46:38 2012 TODDR [...] cpan.org - Correspondence added

On Wed Jan 04 16:11:53 2012, BAREFOOT wrote: Show quoted text

> This also breaks when used with Test::Command (see attached). Perhaps > the original patch submitter should be contacted to see if he can > explain the bug? Otherwise, at this point it seems like it's hurting > more people than it's helping.

The short answer from Catalyst crew is that you're not supposed to run binaries from within a fastcgi app. While their purist attitudes annoyed me, especially since they refused to discuss the point, I begrudgingly decided to go with the flow. FastCGI operates via STDIN/STDOUT, etc. There is no easy way to subvert this in order to call programs within a catalyst app. It's a fairly decent question to ask: Is it safe to run external binaries from a fastcgi script? In my case, I needed to kick off long running processes. What I ended up doing was creating a separate process outside of fastcgi and apache. The process runs under Net::Daemon, validates a secret key and then runs what it's told to do. Even if you need to be interactive, this should be possible if you use this model. Good luck! Todd

Mon Jan 09 03:01:11 2012 book [...] cpan.org - Correspondence added

RT-Send-CC:

toddr [...] cpan.org

On Thu Jan 05 14:46:38 2012, TODDR wrote: Show quoted text

> On Wed Jan 04 16:11:53 2012, BAREFOOT wrote:

> > This also breaks when used with Test::Command (see attached).

> Perhaps

> > the original patch submitter should be contacted to see if he can > > explain the bug? Otherwise, at this point it seems like it's

> hurting

> > more people than it's helping.

> > The short answer from Catalyst crew is that you're not supposed to run > binaries from within a > fastcgi app. While their purist attitudes annoyed me, especially since > they refused to discuss the > point, I begrudgingly decided to go with the flow. FastCGI operates > via STDIN/STDOUT, etc. > There is no easy way to subvert this in order to call programs within > a catalyst app. It's a fairly > decent question to ask: Is it safe to run external binaries from a > fastcgi script? > > In my case, I needed to kick off long running processes. What I ended > up doing was creating a > separate process outside of fastcgi and apache. The process runs under > Net::Daemon, validates > a secret key and then runs what it's told to do. Even if you need to > be interactive, this should > be possible if you use this model. >

I decided to remove the code in v1.06, mostly because I didn't fully understand what it did, and because I have failing tests if it stays. One option I didn't explore yet, is to have an import parameter to do the filehandle swapping when asked for it. It might be worth a try if both behaviour are needed. -- BooK

Mon Jan 09 03:01:31 2012 book [...] cpan.org - Status changed from 'open' to 'resolved'

Mon Jan 09 03:01:31 2012 book [...] cpan.org - Fixed in 1.06 added

Mon Jan 09 03:01:31 2012 book [...] cpan.org - Broken in 1.02 added

Mon Jan 09 03:01:31 2012 book [...] cpan.org - Broken in 1.01 deleted