Skip to content

[Console] Selectively output to STDOUT or OUTPUT stream #4152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 30, 2012

Conversation

johnkary
Copy link
Contributor

Originally opened in this PR targeting master, but asked to target 2.0 instead: #4148

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1434
Todo: -

As noted in the ticket discussion and linked discussion threads, IBM i5 Series has issues with writing output to STDOUT when viewed via their QP2TERM console. The output is likely not being converted to the correct character-encoding on the system level.

This PR changes the default output stream from php://stdout to php://output for OS400 environments, which does not exhibit this issue.

I'm using php_uname('s') to check for the presence of "OS400", which is at least one of the IBM OS's exhibiting this issue. This check is only done once when executing a Console task and shouldn't see any adverse affects in speed on the 99% other platforms using Symfony.

I'm not a native to the OS400 platform so I can't really anticipate any other possible regressions that might occur from switching output streams for that platform. On my Mac, this change would strip all color output, but the PR code only changes output for OS400 environment. To my knowledge the QP2TERM console doesn't even support color, so no loss there.

I think this change is best to make the Console component at least usable out of the box for anyone else trying to build CLI applications for use on OS400.

Addresses issues with writing console output for IBM i5 Series (OS400).
The normal QP2TERM shell outputs garbage text when attempting to write
directly to STDOUT, likely because of EBCDIC character-encoding used
on IBM platforms. Writing to the OUTPUT mimics using 'echo' or 'print'
and prints properly in the console.

Fixes symfony#1434
@igorw
Copy link
Contributor

igorw commented Apr 29, 2012

#4146 might also need a fix for this.

@johnkary
Copy link
Contributor Author

@igorw Hmm. In this case for #4152 when creating a CLI application, Symfony\Component\Console\Output\ConsoleOutput is the default implementation used by Symfony\Component\Console\Application when not specifying your own OutputInterface. Our hard-coded defaults were causing problems out of the box.

I haven't looked too closely at the PRs surrounding the additions of StreamingResponse and your recent OutputStream but are we assuming anywhere that php://stdout is the default stream used when creating a streaming response? If so it MAY require a check similar to what I implemented for Console. My addition was only necessary because the output was being sent to a CLI console. If output is sent to a browser, I don't believe this would be an issue.

If you have something that needs testing on OS400 just ping me.

fabpot added a commit that referenced this pull request Apr 30, 2012
Commits
-------

5b92b9e [Console] Selectively output to STDOUT or OUTPUT stream

Discussion
----------

[Console] Selectively output to STDOUT or OUTPUT stream

Originally opened in this PR targeting master, but asked to target 2.0 instead: #4148

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1434
Todo: -

As noted in the ticket discussion and linked discussion threads, IBM i5 Series has issues with writing output to STDOUT when viewed via their QP2TERM console. The output is likely not being converted to the correct character-encoding on the system level.

This PR changes the default output stream from `php://stdout` to `php://output` for OS400 environments, which does not exhibit this issue.

I'm using `php_uname('s')` to check for the presence of "OS400", which is at least one of the IBM OS's exhibiting this issue. This check is only done once when executing a Console task and shouldn't see any adverse affects in speed on the 99% other platforms using Symfony.

I'm not a native to the OS400 platform so I can't really anticipate any other possible regressions that might occur from switching output streams for that platform. On my Mac, this change would strip all color output, but the PR code only changes output for OS400 environment. To my knowledge the QP2TERM console doesn't even support color, so no loss there.

I think this change is best to make the Console component at least usable out of the box for anyone else trying to build CLI applications for use on OS400.

---------------------------------------------------------------------------

by igorw at 2012-04-29T19:41:21Z

#4146 might also need a fix for this.

---------------------------------------------------------------------------

by johnkary at 2012-04-29T20:21:39Z

@igorw Hmm. In this case for #4152 when creating a CLI application, `Symfony\Component\Console\Output\ConsoleOutput` is the [default implementation](https://github.com/johnkary/symfony/blob/5b92b9ed432ea1d5f2c596517fc7f8aa825b1977/src/Symfony/Component/Console/Application.php#L113) used by `Symfony\Component\Console\Application` when not specifying your own `OutputInterface`. Our hard-coded defaults were causing problems out of the box.

I haven't looked too closely at the PRs surrounding the additions of `StreamingResponse` and your recent `OutputStream` but are we assuming anywhere that `php://stdout` is the default stream used when creating a streaming response? If so it MAY require a check similar to what I implemented for Console. My addition was only necessary because the output was being sent to a CLI console. If output is sent to a browser, I don't believe this would be an issue.

If you have something that needs testing on OS400 just ping me.
@fabpot fabpot merged commit 5b92b9e into symfony:2.0 Apr 30, 2012
fabpot added a commit that referenced this pull request Jun 23, 2015
…nkary)

This PR was squashed before being merged into the 2.3 branch (closes #15058).

Discussion
----------

[Console] Fix STDERR output text on IBM iSeries OS400

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | None
| License       | MIT
| Doc PR        | None

Prior to this PR a Symfony Console command would output error text as random symbols when executed via the IBM console program QSH. Affected error text output such as when a required InputArgument is missing, or when explicitly using `$output->getErrorOutput()->writeln('Some error text here')`.

![Example error output](http://i.imgur.com/PQplK1p.png)

This PR fixes error text so it properly prints to IBM console programs such as QSH and QP2SHELL.

I previously fixed STDOUT for PHP running on IBM iSeries OS400 (Zend Server) using the same approach. Since that PR was merged ConsoleOutput class began using its own output for STDERR which exhibits the same issue STDOUT did.

The following commits and previous Symfony PRs have our relevant discussion about ASCII vs EBCDIC character encoding to fix this issue:

* [Original IBM STDOUT reported in #1434](#1434)
* [My PR #4152 that fixes #1434](#4152)

Thanks!
:rocket:

Commits
-------

23c42ca [Console] Fix STDERR output text on IBM iSeries OS400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants