Skip to content

Fix GH-13789 build failed mbstring_arginfo.h when Visual C++ on Windows #13906

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
Merged
merged 1 commit into from
Apr 7, 2024
Merged

Fix GH-13789 build failed mbstring_arginfo.h when Visual C++ on Windows #13906

merged 1 commit into from
Apr 7, 2024

Conversation

youkidearitai
Copy link
Contributor

CP932 environment can't compile. So add /utf-8 flag on CFLAGS_BD_EXT_MBSTRING.

@youkidearitai
Fix phpGH-13789 build failed mbstring_arginfo.h when Visual C++ on Wi…
22dcba9 
…ndows

Probably CP932 environment can't compile. So add /utf-8 flag.
@youkidearitai youkidearitai requested a review from alexdowad as a code owner April 7, 2024 12:48
@youkidearitai
Copy link
Contributor Author

Let's solve #13815 separately.

@alexdowad
Copy link
Contributor

I don't develop on Windows. Is there anyone else who does and who can give a 2nd opinion on this change? Maybe @cmb69?

@SakiTakamachi
Copy link
Member

@alexdowad
He is not active now.

@nielsdos If you have time, can you take a look at this?

@youkidearitai
Copy link
Contributor Author

@ranvis I tried using your idea, if you have any comment, feel free to comment.

nielsdos
nielsdos approved these changes Apr 7, 2024
View reviewed changes
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

I guess this works, it works on my Windows VM at least, and it's in line with how GCC normally uses UTF-8 too unless overridden. So this is fine for me.

@SakiTakamachi
Copy link
Member

Thank you!

@nielsdos nielsdos merged commit 0f1e979 into php:master Apr 7, 2024
@alexdowad
Copy link
Contributor

Thanks all.

As a suggestion, it would be good to give @youkidearitai commit rights to php-src.

@youkidearitai youkidearitai deleted the mb_trim_fix_compile_when_win_ja branch April 7, 2024 23:59
@youkidearitai
Copy link
Contributor Author

Thanks all.

As a suggestion, it would be good to give @youkidearitai commit rights to php-src.

I'm honored.

@hirokawa
Copy link
Contributor

hirokawa commented Apr 8, 2024

Thanks, it looks fine for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nielsdos nielsdos nielsdos approved these changes

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

Assignees
No one assigned
Labels
Category: Build System Extension: mbstring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@youkidearitai @alexdowad @SakiTakamachi @hirokawa @nielsdos