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.

Sign up for GitHub

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

Jump to bottom

fix issue 18801 - std.stdio.File doesn't work with MSVCRT's UTF-8 mode #6477

Merged
merged 1 commit into from
Sep 17, 2020
Merged

fix issue 18801 - std.stdio.File doesn't work with MSVCRT's UTF-8 mode #6477

merged 1 commit into from
Sep 17, 2020

Conversation

aG0aep6G
Copy link
Contributor

No description provided.

aG0aep6G
aG0aep6G commented Apr 26, 2018
View reviewed changes
std/stdio.d
// "wide-oriented" for us.
immutable int mode = _setmode(f.fileno, _O_TEXT);
// Set some arbitrary mode to obtain the previous one.
_setmode(f.fileno, mode); // Restore previous mode.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit awkward, but I couldn't find a function that just gets the mode.

@aG0aep6G aG0aep6G changed the title fix issue 18801 - std.stdio.File doesn't work with MSVCRT's UTF-8 mode [WIP] fix issue 18801 - std.stdio.File doesn't work with MSVCRT's UTF-8 mode Apr 26, 2018
@dlang-bot dlang-bot added the Review:WIP Work In Progress - not ready for review or pulling label Apr 26, 2018
@dlang-bot
Copy link
Contributor

dlang-bot commented Apr 26, 2018 •
edited
Loading

Thanks for your pull request and interest in making D better, @aG0aep6G! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
✓ 18801 normal std.stdio.File doesn't work with MSVCRT's UTF-8 mode

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#6477"

@aG0aep6G aG0aep6G force-pushed the 18801 branch 4 times, most recently from f2dc502 to d230f7c Compare April 29, 2018 09:02
@aG0aep6G
Copy link
Contributor Author

I can't make sense of this.

The Win64 tester apparently sees the mode as _O_WTEXT when it should be _O_U8TEXT. But then it writes a UTF-8 BOM anyway. BUT THEN it encodes "foo" in UTF-16. And all that happens without D's File or LockingTextWriter being involved at all. It's just fopen and fputwc.

I'm open to suggestions.

@wilzbach
Copy link
Member

wilzbach commented May 9, 2019

(rebased)

@aG0aep6G
Copy link
Contributor Author

Rebased properly (hopefully). I fully expect this to still fail in the same nonsensical way, though.

@aG0aep6G aG0aep6G force-pushed the 18801 branch 2 times, most recently from 7709a27 to 6854f0c Compare September 13, 2020 18:44
@aG0aep6G aG0aep6G changed the title [WIP] fix issue 18801 - std.stdio.File doesn't work with MSVCRT's UTF-8 mode fix issue 18801 - std.stdio.File doesn't work with MSVCRT's UTF-8 mode Sep 13, 2020
@aG0aep6G
Copy link
Contributor Author

Looks like the new Windows testing setup turned this green. With the nonsensical failures gone, this might be good to go.

There is a failure on buildkite, though: Apparently, CyberShadow/ae segfaults. I'm not sure how to address that. Is it caused by this PR? Do I need to investigate it? @CyberShadow?

@Geod24
Copy link
Member

Geod24 commented Sep 13, 2020

CyberShadow/ae#46

@aG0aep6G
Copy link
Contributor Author

Sweet. All green.

@dlang-bot dlang-bot merged commit 3754e92 into dlang:master Sep 17, 2020
@aG0aep6G aG0aep6G deleted the 18801 branch September 17, 2020 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thewilsonator thewilsonator thewilsonator approved these changes

@CyberShadow CyberShadow Awaiting requested review from CyberShadow CyberShadow is a code owner

@schveiguy schveiguy Awaiting requested review from schveiguy schveiguy is a code owner

Assignees
No one assigned
Labels
Merge:auto-merge Review:WIP Work In Progress - not ready for review or pulling Severity:Bug Fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aG0aep6G @dlang-bot @wilzbach @Geod24 @thewilsonator