Page MenuHome

Fix warning spam when building and still allow for Japanese locale to successfully build.
AbandonedPublic

Authored by Henrik Berglund (cyaoeu) on Jan 26 2018, 1:37 PM.

Diff Detail

Repository
rB Blender
Branch
test_fix
Build Status
Buildable 1123
Build 1123: arc lint + arc unit

Event Timeline

I wasn't done looking into this, There may be better options (like repressing the warn it now generates) or encoding the the offending non ascii characters like \0xE2\0x86\0x90 (for left arrow for instance) i send you this patch for testing, just to be sure it worked, not as a final solution.

On top of that you really cannot send other peoples work to the tracker for review, regardless if it works or not, this is not your call to make.

I wasn't done looking into this, There may be better options (like repressing the warn it now generates) or encoding the the offending non ascii characters like \0xE2\0x86\0x90 (for left arrow for instance) i send you this patch for testing, just to be sure it worked, not as a final solution.

On top of that you really cannot send other peoples work to the tracker for review, regardless if it works or not, this is not your call to make.

I'm sorry for uploading it to the tracker but you should have mentioned that you weren't done looking into it when sending the diff. It's a patch, it doesn't have to be a final solution. It does work though and should be a better alternative because it gives less warnings than the "everything is UTF-8" commit for other people but is still able to be compiled by users with Japanese locale.

Also it's not your diff straight on to the tracker, I edited it until it was getting built properly. Is it still your work then? You said it was not my call to make, does that mean I have to ask for permission before uploading patches? I would say it's my call to upload a patch and your call if you want to review it or not based on some criteria (it wasn't ready, it was an edit of your original code and so on).

Anyway, I'm using this patch locally until there's a better alternative.

That being said if there's a better solution (like fixing the thing that causes the warnings in the first place) I have no problems with that, just ignore this patch. I just want Blender to build really.

You can post your own work without asking permission, but you cannot post a derivative of someone else's work without asking their permission, even if it's based on a silly 3 line change.

What are the warnings? (could they be included, perhaps they could be silenced)

It might be possible to add add_compile_options("$<$<C_COMPILER_ID:MSVC>:/utf-8>") under the source/ directory too.

What are the warnings? (could they be included, perhaps they could be silenced)

I mentioned this in my windows support email yesterday, they are about 290 extra C4828 warnings

warning C4828: The file contains a character starting at offset 0x1ea2 that is illegal in the current source character set (codepage 65001). (compiling source file K:\BlenderGit\blender\intern\openvdb\openvdb_capi.cc)

10 originating in extern/bullet
19 in lib/boost
261 in lib/opencollada

all of the warnings are coming from comment blocks in headers. boost and opencollada have already fixed the offending characters in a newer versions,however updating boost to get rid of a silly warning seems like overkill and an unfair load on the platform teams for no good reason.

so repressing C4828 all together (or at-least until we are on newer versions of the offending libraries) has my preference.

It might be possible to add add_compile_options("$<$<C_COMPILER_ID:MSVC>:/utf-8>") under the source/ directory too.

not needed, afaict all files are now compiled with /utf-8 which is fine, our code-guideline says all code should be utf-8, so adding this switch to enforce that on all code should not be a problem (should be encouraged imho)

I did see that gaia in IRC mentioned a lot of warnings when compiling in vs2013 "<gaia> hi. when building with vs2013 i currently get about 1400 warnings all saying "cl : Command line warning D9002: ignoring unknown option '/utf-8'"", I have only built blender with 2015 myself. Is this a 2015 feature or should it work on 2013 too?

we could throw a barrier around it like this

if(MSVC_VERSION GREATER 1800)
	# Needed, otherwise system encoding causes utf-8 encoding to fail in some cases (C4819)
	add_compile_options("$<$<C_COMPILER_ID:MSVC>:/utf-8>")
	add_compile_options("$<$<CXX_COMPILER_ID:MSVC>:/utf-8>")
endif()

But honestly i'm about to terminate 2013 support anyhow so moot point really.

source/blender/makesrna/intern/CMakeLists.txt
386–391

The main reason I don't think this is a good solution - is it means we need to manually keep track of files that use certain utf-8 characters.

It's likely someone will make an edit to another file - maybe they add their name as a contributor for example - then this file needs to have the kind of check above added.

It wont be immediately obvious the addition is needed - possibly weeks or months until it's found.
So rather then adding a solution that is likely to break, I'd prefer a more global solution that means we can use utf-8 without accidental breakage on some platforms.

Campbell Barton (campbellbarton) requested changes to this revision.Jan 27 2018, 10:45 PM
This revision now requires changes to proceed.Jan 27 2018, 10:45 PM

Yes, it seems like the best approach is to fix all of the files (external libraries?) that actually give warnings with UTF-8 forced for everything. That process has started already too.