Skip to content
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

Unbuffered stdout for Windows and displaying the output of subprocess calls #24

Merged
merged 4 commits into from
Aug 14, 2017

Conversation

YannickJadoul
Copy link
Member

Cfr. side issue in #15:

  • Forcing Python on Windows to not buffer its stdout, such that the output of cibuildwheel and that of its subprocess calls is nicely interleaved.
  • Changing subprocess.check_output to check_call such that the stdout of these calls is not swallowed into a never-printed string by cibuildwheel.

@YannickJadoul
Copy link
Member Author

@joerick There are several other workarounds, if you don't like the sys.stdout = os.fdopen(...) trick.

@YannickJadoul
Copy link
Member Author

Seems to be working, if you ask me. Two more little things:

  • The lines of the run_tests.py (https://ci.appveyor.com/project/joerick/cibuildwheel/build/1.0.91#L1643) are still buffered and only output at the very end, because this solution is only applied to cibuildwheel itself. What to do with that? In principle, that is not part of the cibuildwheel library, so we can just as well change appveyor.yml to call python -u or set PYTHONUNBUFFERED=1. But is there then still a point to this PR?

  • I've applied the solution only for the windows.py file (since OS X and Linux don't seem to have any problems), and applied a single flush() at the end of the preamble. The other option is either applying the fix when Windows is detected, or just applying it for all OSes.

@joerick
Copy link
Contributor

joerick commented Aug 14, 2017

Excellent PR @YannickJadoul! Thank you for the detailed write-up 👍

The lines of the run_tests.py (https://ci.appveyor.com/project/joerick/cibuildwheel/build/1.0.91#L1643) are still buffered and only output at the very end, because this solution is only applied to cibuildwheel itself. What to do with that? In principle, that is not part of the cibuildwheel library, so we can just as well change appveyor.yml to call python -u or set PYTHONUNBUFFERED=1.

Lets use python -u in appveyor.yml for that.

But is there then still a point to this PR?

Yes! for all the users don't have to worry about that themselves :)

@joerick
Copy link
Contributor

joerick commented Aug 14, 2017

Cool, I've made those changes. I'm good to merge and do a release once the lights go green.

@YannickJadoul
Copy link
Member Author

Cool, seems to be working! And as far as I can see, the output is 1) there and 2) in the right order :-)

Thanks for the quick reaction and solution!

@joerick joerick merged commit adf9844 into pypa:master Aug 14, 2017
@joerick
Copy link
Contributor

joerick commented Aug 14, 2017

Released as 0.4.1 🎉

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.

2 participants