Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Windows build failed with Ninja #419

Closed
Jason239 opened this issue Jun 26, 2018 · 14 comments
Closed

Windows build failed with Ninja #419

Jason239 opened this issue Jun 26, 2018 · 14 comments
Assignees
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@Jason239
Copy link

I don't know if it's right to ask a question here.
I have a question about building on Windows
I was succeeded to build binary immediately on Ubuntu.
but on Windows, failed.
After entering the command 'ninja -C out \ Release_x64', an error occurs during the build with 'The warn is treated as an error and no generated 'object' file exists.' message.
especially, most of this errors were files in 'boringssl'.
I tried to edit .gyp scripts, but it didn't work.

what should I do?

@kqyang
Copy link
Contributor

kqyang commented Jun 26, 2018

@leesanggoo Thanks for filing the issue. Can you provide more information? For example, (1) Windows version. (2) Packager source code revision. (3) The error messages.

@kqyang kqyang added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Jun 26, 2018
@Jason239
Copy link
Author

@kqyang Thanks for your quick reply.
This is the execution environment.

(1) Windows version : Windows 10 Pro, x64
(2) Packager source code revision : Latest version(6b4a75b)
(3) Visual Studio version : 2015 Professional v14.0.25431.01 Update 3
(4) The error messages :
failed_log.txt

Please tell me if you need any more information.

@shaka-bot shaka-bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Jun 27, 2018
@kqyang
Copy link
Contributor

kqyang commented Jun 27, 2018

@leesanggoo Thanks for the info!

Here is what I saw from the log:

d:\works\shaka_packager\src\packager\third_party\boringssl\src\crypto\internal.h: error C2220: °æ°í°¡ ¿À·ù·Î ó¸®µÇ¾î »ý
¼ºµÈ 'object' ÆÄÀÏÀÌ ¾ø½À´Ï´Ù.
d:\works\shaka_packager\src\packager\third_party\boringssl\src\crypto\internal.h: warning C4819: ÇöÀç ÄÚµå ÆäÀÌÁö(949)¿¡
¼­ Ç¥½ÃÇÒ ¼ö ¾ø´Â ¹®ÀÚ°¡ ÆÄÀÏ¿¡ µé¾î ÀÖ½À´Ï´Ù. µ¥ÀÌÅÍ°¡ ¼Õ½ÇµÇÁö ¾Ê°Ô ÇÏ·Á¸é ÇØ´ç ÆÄÀÏÀ» À¯´ÏÄÚµå Çü½ÄÀ¸·Î ÀúÀåÇϽʽÿÀ.
...
d:\works\shaka_packager\src\packager\third_party\boringssl\src\crypto\base64\base64.c: error C2220: °æ°í°¡ ¿À·ù·Î 󸮵Ç
¾î »ý¼ºµÈ 'object' ÆÄÀÏÀÌ ¾ø½À´Ï´Ù.
d:\works\shaka_packager\src\packager\third_party\boringssl\src\crypto\base64\base64.c: warning C4819: ÇöÀç ÄÚµå ÆäÀÌÁö(9
49)¿¡¼­ Ç¥½ÃÇÒ ¼ö ¾ø´Â ¹®ÀÚ°¡ ÆÄÀÏ¿¡ µé¾î ÀÖ½À´Ï´Ù. µ¥ÀÌÅÍ°¡ ¼Õ½ÇµÇÁö ¾Ê°Ô ÇÏ·Á¸é ÇØ´ç ÆÄÀÏÀ» À¯´ÏÄÚµå Çü½ÄÀ¸·Î ÀúÀåÇϽÊ
½Ã¿À.
...
d:\works\shaka_packager\src\packager\third_party\boringssl\src\include\openssl\aead.h: error C2220: °æ°í°¡ ¿À·ù·Î 󸮵Ç
¾î »ý¼ºµÈ 'object' ÆÄÀÏÀÌ ¾ø½À´Ï´Ù.
d:\works\shaka_packager\src\packager\third_party\boringssl\src\include\openssl\aead.h: warning C4819: ÇöÀç ÄÚµå ÆäÀÌÁö(9
49)¿¡¼­ Ç¥½ÃÇÒ ¼ö ¾ø´Â ¹®ÀÚ°¡ ÆÄÀÏ¿¡ µé¾î ÀÖ½À´Ï´Ù. µ¥ÀÌÅÍ°¡ ¼Õ½ÇµÇÁö ¾Ê°Ô ÇÏ·Á¸é ÇØ´ç ÆÄÀÏÀ» À¯´ÏÄÚµå Çü½ÄÀ¸·Î ÀúÀåÇϽÊ
½Ã¿À.

According to https://msdn.microsoft.com/en-us/library/ms173715.aspx

The file contains a character that cannot be represented in the current code page (number). Save the file in Unicode format to prevent data loss.

C4819 occurs when an ANSI source file is compiled on a system with a codepage that cannot represent all characters in the file.

I checked these files and there are indeed non-ASCII characters in the comment section of these files. It is safe to just suppress the warning during compilation. We'll prepare a fix soon.

In the mean while, if you want to try, you can edit the source code yourself to suppress the warning:

--- c/packager/third_party/boringssl/boringssl.gyp
+++ w/packager/third_party/boringssl/boringssl.gyp
@@ -34,7 +34,7 @@
       },
       # TODO(davidben): Fix size_t truncations in BoringSSL.
       # https://crbug.com/429039
-      'msvs_disabled_warnings': [ 4267, ],
+      'msvs_disabled_warnings': [ 4267, 4819, ],
       'conditions': [
         ['component == "shared_library"', {
           'defines': [
@@ -73,7 +73,7 @@
       'dependencies': [ 'boringssl_asm' ],
       # TODO(davidben): Fix size_t truncations in BoringSSL.
       # https://crbug.com/429039
-      'msvs_disabled_warnings': [ 4267, ],
+      'msvs_disabled_warnings': [ 4267, 4819, ],
       'conditions': [
         ['component == "shared_library"', {
           'defines': [

@kqyang kqyang added type: bug Something isn't working correctly and removed needs triage labels Jun 27, 2018
@kqyang kqyang self-assigned this Jun 27, 2018
@kqyang kqyang added this to the v2.2 milestone Jun 27, 2018
@Jason239
Copy link
Author

Jason239 commented Jun 28, 2018

@kqyang Thank you!

But I've run Ninja after edit it(boringssl.gyp), but the same warning occurred. Am I right..?
And, is there any solution about C2220 error..? Do I need to install something on my local machine?

@kqyang
Copy link
Contributor

kqyang commented Jun 28, 2018

Ah, you need to run gclient runhooks before running ninja. This updates the build files, which is necessary after changing gyp files.

C2220 is the result of C4819, so it will be gone after fixing C4819.

There could be C4819 in other third_party modules though, but we can address them one by one.

@Jason239
Copy link
Author

oh, I see. Thank you for your detailed explanation.

So I fixed some modules, and It but I've been in trouble again at 'gtest'.
I already added 'msvs_disabled_warnings': [ 4819, ]' in gtest.gyp,
but the same error continues to occur. (error C2220, warning C4819)

I edited it like this,

'includes': [
   'gtest.gypi',
 ],
 'targets': [
   {
     'target_name': 'gtest',
     'toolsets': ['host', 'target'],
     'type': 'static_library',
     'msvs_disabled_warnings': [ 4267, 4819, ],        # here
     'sources': [
       '<@(gtest_sources)',
     ],
    ...

I haven't used gyp, so I don't know well.. Is it wrong?

@kqyang
Copy link
Contributor

kqyang commented Jun 29, 2018

Thanks for trying. That looks correct.

Can you post the whole build log?

You may also send a pull request so we can review your change.

@Jason239
Copy link
Author

Jason239 commented Jul 3, 2018

Here is it.
gtest_error_log.txt

I edited only that part.

@kqyang
Copy link
Contributor

kqyang commented Jul 3, 2018

@leesanggoo Thanks for the log. It is because the header file is indirectly included in another module, i.e. in a dependent module, so you'll need to modify the dependent settings in line 196 of gtest.gyp: https://github.com/google/shaka-packager/blob/master/packager/testing/gtest.gyp#L196. Let us know if it works if you add 4819 there too.

@Jason239
Copy link
Author

Jason239 commented Jul 4, 2018

Thank you for your active support, the build finally succeeded!
I've added 4819 to almost modules.

But.. there is one more question to ask.
This is the default x86 build, but my system is 64-bit.
It doesn't matter if I use it, but I tried to x64 build as follows.

SET GYP_DEFINES='target_arch=x64'
gclient runhooks
ninja.exe -C .\out\Release_x64\

but the 'fatal error LNK1112' is occurred.
Here is the log.
x64_log.txt

Could you explain me what I did wrong?

@kqyang
Copy link
Contributor

kqyang commented Jul 4, 2018

@leesanggoo That is great! I am a bit surprised that you have to update almost all modules though. Can you try if you can get it work by just having the below change?

--- c/packager/common.gypi
+++ w/packager/common.gypi
@@ -61,6 +61,10 @@
             '-Wno-unguarded-availability',
           ],
         },
+        'msvs_disabled_warnings': [ 4819, ],
+        'direct_dependent_settings': {
+          'msvs_disabled_warnings': [ 4819, ],
+        },
         'conditions': [
           ['clang==0', {
             'cflags': [

As for the link error with 64-bit build, is your system 32-bit or 64-bit? You won't be able to build for 64-bit if your operation system is 32-bit. You can build for both 32-bit and 64-bit if your system is 64-bit.

@Jason239
Copy link
Author

Jason239 commented Jul 4, 2018

@kqyang I got the source again and tried to build it again by modifying only that part, but I get 4819 from modules as 'base', 'media'.

And.. my system is x64, but I'll use x86 build. So It's okay. Thank you very much!

@Jason239
Copy link
Author

Jason239 commented Jul 6, 2018

I do not know if I should send a pull request with msvs_disabled_warnings added for all modules, but I will close the issue.
Thank you so much for your support.

@Jason239 Jason239 closed this as completed Jul 6, 2018
@kqyang
Copy link
Contributor

kqyang commented Jul 6, 2018

@leesanggoo Thanks for putting your effort in resolving the problem yourself. If you can send a pull request, it will be really appreciated.

As for the x86/x64 build, I am not very familiar with Windows command shell, I am afraid that the setting for GYP_DEFINES may not be carried forward, so another thing you can try is to set GYP_DEFINES and gclient runhooks in a single command:

GYP_DEFINES='target_arch=x64' gclient runhooks
ninja.exe -C .\out\Release_x64\

@kqyang kqyang reopened this Jul 10, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Sep 10, 2018
@shaka-project shaka-project locked and limited conversation to collaborators Sep 10, 2018
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

3 participants