-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[Issue #218] StdOut was not being redirected properly and was causing… #220
Conversation
build is failing because .Net 4+ features were used and are incompatible with .Net 2.0. |
build should be fixed now. if so, please merge. |
IIRC the plan in #218 was to actually make it working. OTOH it can be applicable as a hotfix. I will try to test and merge something this week. Was traveling, so missed the PR. Please also revert the permission change in the file. There is no reason for these files to be executable |
Yes, the idea was to do a hot fix on the main problem, and then create a new task for the remaining work. I tried to explain that in the PR description, sorry if that wasn't clear. I've been busy this week but will address the file perms soon. |
ps.RedirectStandardInput = true; // this creates a pipe for stdin to the new process, instead of having it inherit our stdin. | ||
ps.RedirectStandardOutput = true; | ||
ps.RedirectStandardError = true; | ||
ps.RedirectStandardInput = false; // this creates a pipe for stdin to the new process, instead of having it inherit our stdin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd guess this comment is also outdated now
… the child process to hang.
file perms and comment issues have been fixed. |
I would like to work on some more items but would like to see this PR closed or merged before that happens. |
Will get it integrated tonight. Sorry, was traveling
…On Jun 5, 2017 3:04 PM, "Paul Nikonowicz" ***@***.***> wrote:
I would like to work on some more items but would like to see this PR
closed or merged before that happens.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#220 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AC3IoAe0G9bf6tPAMLzh4tdmJcNS4Q-Pks5sA_z3gaJpZM4NefuI>
.
|
…ld redirect STDOUT/STDERR when LogHandler is defined It restores logging of executables, which has been broken in winsw#220. Not a regression, because the change has not been released yet
… the child process to hang.
When this is merged, we should create a new task to include logging of stdout properly using the Log4Net feature that was recently completed.
Edited by @NextTurn: Fixes #218