Summary: | Windows build break due to warning C4819 | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kwang Yul Seo <skyul> | ||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, darin, eric, jshin, tyoshino | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | Windows XP | ||||||
Bug Depends on: | 26375 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Kwang Yul Seo
2009-07-18 11:08:08 PDT
Created attachment 33029 [details]
Disable C4819 to fix build for non-English Windows
Now this bug depends on https://bugs.webkit.org/show_bug.cgi?id=26375, but I see no erros with WebKit/win/WebCoreLocalizedStrings.cpp in Windows-949 codepage (Korean). I marked the bugs as "dependent" to simply point out to anyone looking at either that they're both solving similar issues. We should solve both issues in the same way. (In reply to comment #3) > I marked the bugs as "dependent" to simply point out to anyone looking at > either that they're both solving similar issues. We should solve both issues > in the same way. 1. All bad characters in MixedMode.h, ConditionalMacros.h are in comments and they seem to just get garbled. 2. The only bad character in AssertMacros.h and TargetConditionals.h is the copyright symbol (\xA9. In other files "(C)" is used. It is in ISO/IEC 8859-1 but not in ASCII) in copyright notice head comment. 3. The only bad character in Multiprocessing.h is also the copyright symbol \xA9. Only this character is non-comment. It's contained in a string literal macro MPCopyrightNotice. So, I think Kwang's proposal will work for 1 and 2. I'm not sure if 3 is problematic or not. As I explained in that bug entry (26375) as follows, suppressing warning cannot be applied to 26375 case. > Save mode: Unicode (UTF-8 without signature) - Codepage 65001 > The source was encoded by UTF-8 w/o BOM > Build: Failure. We need to remove /Wx option (take warnings as errors) > Build w/o /Wx: Successful > Result: localized string not found Vice versa, it doesn't make sense to replace copyright marks in comments by \xA9. Maybe, we should use "(C)". Is it acceptable, Apple guys? Thank you. A long time ago, I filed a bug at http://bugreport.apple.com about QuickTime SDK headers having multiple non-ASCII bytes in the comment. I have yet to hear back. (bug id is 6469108 ) Either we have to do what the patch here does or add a simple script to clean up Qt SDK headers because they're not a part of webkit. For WebKitTools/DumpRenderTree/win/TestNetscapePlugin/main.cpp, we should just remove a non-ascii character (I made a one-line patch a long time ago, but haven't submitted, yet). BTW, this does not happen on Windows with the default codepage set to single-byte code pages because most of slots in 0x80 - 0xFF are filled up in those code pages. In CJK codepages, most of stand-alone bytes with MSB=1 are invalid and lead to C4819. (In reply to comment #5) > A long time ago, I filed a bug at http://bugreport.apple.com about QuickTime > SDK headers having multiple non-ASCII bytes in the comment. I have yet to hear > back. > (bug id is 6469108 ) > FYI, For code page 932 (Japanese), WebKitSupportLibrary/win/include/ColorSync/ColorSyncDeprecated.h also causes build problem. It contains \xC4\xB6 in some comment line. Since this file is contained in WebKitSupportLibrary.zip that is automatically checked MD5 and extracted into WebKitSupportLibrary directory by build-webkit script, when we fix these file manually by ourselves, we also have to modify build-webkit script to skip calling update-webkit-support-libs script (and extract WebKitSupportLibrary.zip by ourselves). > Either we have to do what the patch here does or add a simple script to clean > up Qt SDK headers because they're not a part of webkit. > > > For WebKitTools/DumpRenderTree/win/TestNetscapePlugin/main.cpp, we should just > remove a non-ascii character (I made a one-line patch a long time ago, but > haven't submitted, yet). > > > BTW, this does not happen on Windows with the default codepage set to > single-byte code pages because most of slots in 0x80 - 0xFF are filled up in > those code pages. In CJK codepages, most of stand-alone bytes with MSB=1 are > invalid and lead to C4819. I've gave up manually fixing these characters one month ago, and changed system locale of development machine to English. So, I have no problem now. But still it would be nice to have WebKit build work with these locales.. Comment on attachment 33029 [details]
Disable C4819 to fix build for non-English Windows
r=me
Is there some way to put this disabling in fewer places and still affect all the configurations?
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/WebCore.vcproj/QTMovieWin.vcproj M WebCore/WebCore.vcproj/WebCore.vcproj M WebKit/win/ChangeLog M WebKit/win/WebKit.vcproj/WebKit.vcproj M WebKitTools/ChangeLog M WebKitTools/DumpRenderTree/win/TestNetscapePlugin/TestNetscapePlugin.vcproj Committed r46398 M WebKit/win/ChangeLog M WebKit/win/WebKit.vcproj/WebKit.vcproj M WebCore/ChangeLog M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/WebCore.vcproj/QTMovieWin.vcproj M WebKitTools/DumpRenderTree/win/TestNetscapePlugin/TestNetscapePlugin.vcproj M WebKitTools/ChangeLog r46398 = bb31450c05ec594fa6398b23a9d46d36b003371c (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46398 |