Created
May 9, 2013 19:59
-
-
Save preaction/5550143 to your computer and use it in GitHub Desktop.
Who gets the bug report? System::Command or App::Cmd?
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
use strict; | |
use warnings; | |
package MyApp; | |
use App::Cmd::Setup -app; | |
package MyApp::Command::test; | |
use App::Cmd::Setup -command; | |
use Moose; | |
sub opt_spec { | |
return ( | |
[ 'work_tree=s' => 'The full path to the git working directory' ], | |
); | |
} | |
sub execute { | |
my ( $self, $opt, $args ) = @_; | |
my $git = Git::Repository->new( work_tree => $opt->{work_tree} ); | |
# KABOOM! | |
} | |
package main; | |
use Test::Git qw( test_repository ); # Supplied by Git::Repository | |
my $repo = test_repository(); | |
use App::Cmd::Tester qw( test_app ); | |
test_app( MyApp => [ qw( test --work_tree ), $repo->work_tree ] ); | |
=cut | |
=head1 Requirements | |
This test requires App::Cmd (tested on 0.318 and 0.320) and Git::Repository (which depends on | |
System::Command 1.100); | |
=head1 Output | |
Error output is: | |
.git | |
fatal: Not a git repository: '/tmp/XXXXXXXXXX' | |
Expected output is nothing | |
=head1 The Problem | |
Git::Repository is a module for running git(1). It uses the System::Command module to | |
run a Git command and capture all the output. Git's plumbing commands are ideal for | |
scripting and automation, and Perl's text-processing utilities make it perfect for | |
parsing and interpreting Git's plumbing command output. | |
When System::Command runs a command, it uses IPC::Open3 on Linux. IPC::Open3 will do | |
some things to make sure that its environment is sane, one of which is to remove any | |
tie() on STDOUT and STDERR. | |
App::Cmd::Tester does exactly what it sounds like: It tests App::Cmd apps. In order | |
to get the most coverage, it tests from the front-end, localizing @ARGV, overriding | |
exit(), and capturing the regular STDOUT and STDERR using IO::TieCombine, which | |
again does exactly what it sounds like (uses tie() and combines multiple IO handles). | |
App::Cmd::Tester captures STDOUT and STDERR by tie(). IPC::Open3 removes tie() on | |
STDOUT and STDERR. | |
In the output, the first line (".git") is output from a command that Git::Repository | |
runs (git rev-parse --git-dir). It uses this to find what the git data directory is | |
called (in a regular working copy, it's the ".git" directory). If Git::Repository | |
can't find the git data directory, it dies with an error: "Not a git repository". | |
=head1 Possible Fixes | |
=head2 Fix 1: Do not tie STDOUT and STDERR when using App::Cmd::Tester | |
This has the problem of breaking a feature of App::Cmd::Tester: The ability | |
to get interleaved STDOUT/STDERR in chronological order. So, this is not an | |
ideal fix. | |
--- /home/doug/perl5/lib/perl5/App/Cmd/Tester.pm 2013-01-30 19:37:50.000000000 -0500 | |
+++ /home/doug/App_Cmd_Tester.pm 2013-05-09 15:33:15.708296000 -0400 | |
@@ -56,9 +56,12 @@ | |
require IO::TieCombine; | |
my $hub = IO::TieCombine->new; | |
+ use Symbol qw( gensym ); | |
- my $stdout = tie local *STDOUT, $hub, 'stdout'; | |
- my $stderr = tie local *STDERR, $hub, 'stderr'; | |
+ local *STDOUT = gensym; | |
+ local *STDERR = gensym; | |
+ open STDOUT, '>', \(my $stdout); | |
+ open STDERR, '>', \(my $stderr); | |
my $run_rv; | |
@@ -72,9 +75,9 @@ | |
my $error = $ok ? undef : $@; | |
return { | |
- stdout => $hub->slot_contents('stdout'), | |
- stderr => $hub->slot_contents('stderr'), | |
- output => $hub->combined_contents, | |
+ stdout => $stdout, | |
+ stderr => $stderr, | |
+ output => join( "\n", $stdout, $stderr ), | |
error => $error, | |
run_rv => $run_rv, | |
}; | |
=head2 Fix 2: Use IPC::Run instead of IPC::Open3 in System::Command | |
Obviously this is a hacked-up patch, but it applies and demonstrates that the | |
issue is fixed when using IPC::Run | |
--- /home/doug/perl5/lib/perl5/System/Command.pm 2013-05-09 15:34:56.000000000 -0400 | |
+++ /home/doug/System_Command.pm 2013-05-09 15:33:43.468902000 -0400 | |
@@ -18,6 +18,7 @@ | |
# MSWin32 support | |
use constant MSWin32 => $^O eq 'MSWin32'; | |
require IPC::Run if MSWin32; | |
+use IPC::Run; | |
our $VERSION = '1.100'; | |
@@ -56,17 +57,17 @@ | |
my $err = Symbol::gensym; | |
# start the command | |
- if (MSWin32) { | |
+# if (MSWin32) { | |
$pid = IPC::Run::start( | |
[@cmd], | |
'<pipe' => $in, | |
'>pipe' => $out, | |
'2>pipe' => $err, | |
); | |
- } | |
- else { | |
- $pid = eval { open3( $in, $out, $err, @cmd ); }; | |
- } | |
+# } | |
+# else { | |
+# $pid = eval { open3( $in, $out, $err, @cmd ); }; | |
+# } | |
return ( $pid, $in, $out, $err ); | |
}; | |
@@ -135,7 +136,8 @@ | |
my $self = bless { | |
cmdline => [@cmd], | |
options => $o, | |
- pid => MSWin32 ? $pid->{KIDS}[0]{PID} : $pid, | |
+# pid => MSWin32 ? $pid->{KIDS}[0]{PID} : $pid, | |
+ pid => $pid->{KIDS}[0]{PID}, | |
stdin => $in, | |
stdout => $out, | |
stderr => $err, | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment