-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix #9244 #9251
Fix #9244 #9251
Conversation
Can you add a comment indicating that the case fall through is intentional? Otherwise, I don't see any reason not to merge this. I think you need a dup2 / SetStandardIOHandle call (whatever it's called) also so that it gets setup correctly for any C code being run (such as ios_stderr) -From my iPhone |
@vtjnash: added the dup2 and comments. |
I think you need to call both Although typical WTF moment of making fixes on windows: the result (of using the C run time from julia) is officially undefined behavior on windows and is subject to change or fail on future versions of windows. but "by coincidence" it will typically work right now. https://support.microsoft.com/kb/105305/en-us?wa=wsignin1.0). That really makes me want to start running, running faster that is. additional sources: |
@vtjnash: Maybe before modifying this patch further, we should concoct a test. Which interfaces would need to be exercised in that test? Do you think it would it be sufficient to test C stdio along with Julia's own I/O? |
Sounds wise. You would need to compile a version of ui/repl.c (aka julia.exe) for win32 (tater than console) and spawn that in the test in such a way that the stdio handles don't get correctly initialized (I think the windows shell API will do this for you if you invoke 'start', see Microsoft support link above) |
I'm going to merge this, since it only affects a case that used to fail anyway. |
I'd like to backport this as well. cc @JuliaBackports |
Backporting PR in #9282 |
Backport: Merge pull request #9251 from twadleigh/stdio-hack
The proper solution will, I'm sure, at least include proper cleanup, but this hack was sufficient to get my simple use case working for me.