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

Fix output stream failures (Windows) #4375



Copy link

@memsharded memsharded commented Jan 24, 2019

Changelog: Fix: Implemented retrial of output to stdout stream when the OS (Windows) is holding it and producing IOError for output
Docs: Omit

Close #4277

@ghost ghost assigned memsharded Jan 24, 2019
@ghost ghost added the stage: review label Jan 24, 2019
@memsharded memsharded added this to the 1.12 milestone Jan 24, 2019
@lasote lasote changed the title Feature/unzip conan progress bar Fix output stream failures (Windows) Jan 24, 2019
@lasote lasote requested a review from jgsogo Jan 24, 2019
Copy link

@jgsogo jgsogo left a comment

Add some disclaimer around the weird for that fixes the issue

if newline:
data = "%s\n" % data

for _ in range(3):
Copy link

@jgsogo jgsogo Jan 25, 2019

Ok, this might work and fix the issue, but at least we need some kind of disclaimer for this block of code, maybe a link to the issue, an explanation, (I suppose that a test is impossible)... otherwise anyone having a look at this hack may delete it.

Copy link

@SSE4 SSE4 Jan 25, 2019

@jgsogo may be it's possible to mock self._stream.write in test, so it throws an error 3 times?

Copy link
Member Author

@memsharded memsharded Jan 25, 2019

Added comments and a test patching the stream.write.

conans/util/ Show resolved Hide resolved
jgsogo approved these changes Jan 25, 2019
@memsharded memsharded merged commit 0b7b3ca into conan-io:develop Jan 25, 2019
2 checks passed
@ghost ghost removed the stage: review label Jan 25, 2019
@memsharded memsharded deleted the feature/unzip_conan_progress_bar branch Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants