IPC::Open3 breaks App::Cmd::Tester when using System::Command via Git::Repository

[preaction]

Here's a demonstration and description of the problem: https://gist.github.com/preaction/5550143

Long story short: There's an interaction with App::Cmd::Tester's use of tie() on STDOUT and STDERR and IPC::Open3's explicit untie() on STDOUT and STDERR.

I'm posting this here first because I think it will be easier for System::Command to fix the issue on this end than for App::Cmd::Tester to fix the issue on that end (it would break a feature of App::Cmd::Tester, where I can find no such consequence for System::Command). Though App::Cmd::Tester is a test module, and I could also just as well write my own test module that avoided the use of tie() if this interaction cannot be mitigated by any of the involved modules.

If you agree that this is something that can be fixed on System::Command's end, I can put together a pull request (one that is better than the patch in the link above, of course). It would basically mean removing IPC::Open3 and using IPC::Run only.

[book]

Thanks for the report!

What if App::Cmd::Tester used Capture::Tiny instead of IO::TieCombine?
Its capture_merged function seems to provide the required functionality.
So with regards to "who gets the bug", I think App::Command could get one too. :-)

System::Command's use of IPC::Run is only a workaround for Windows systems.
I had no intention to use it on other systems, since IPC::Open3 worked fine. I have actually looked for several options for making System::Command run on Windows, but IPC::Run has been the first to succeed.

I might have another look at the other options again (and remove the need for both IPC::Run and IPC::Open3), but don't hold your breath.

Another option would be to add an option to System::Command that would force the use of IPC::Run. Since Git::Repository passes unrecognized options to Git::Repository::Command (a subclass of System::Command), it would simply be a matter of passing that option to Git::Repository->new(). An environment variable would be another option.

All in all, I'd rather not touch System::Command for now.

[preaction]

I'll update App::Cmd::Tester to try Capture::Tiny. A quick glance over the source of Capture::Tiny doesn't raise any red flags (except it requires v5.8.1 and App::Cmd currently seems to require v5.6.0). I'll add an issue to App::Cmd once I've done that, and add a link back here for that one other person in the entire world that will have this problem ;)

[book]

Closing this issue, given that you closed rjbs/App-Cmd#22.

[preaction]

Right, sorry I forgot to come back here. Thanks for the help!