Bug 124679 – [WinCairo] Building ANGLE libraries fails.

• Status: RESOLVED
• Resolution: FIXED
• Component: ANGLE

There are some compile and linker errors, while building ANGLE libraries for WinCairo.

Created attachment 217483 [details]
Patch

[WebKit Commit Bot]

Attachment 217483 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/libEGL.vcxproj', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/libEGL.vcxproj.filters', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/libGLESv2.vcxproj', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/libGLESv2.vcxproj.filters', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_common.vcxproj', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_common.vcxproj.filters', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_glsl.vcxproj', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_glsl.vcxproj.filters', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_hlsl.vcxproj', u'Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_hlsl.vcxproj.filters', u'Source/ThirdParty/ANGLE/ChangeLog', u'Source/ThirdParty/ANGLE/src/libGLESv2/constants.h', u'Source/ThirdParty/ANGLE/src/libGLESv2/libGLESv2.def']" exit_code: 1
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:9:  #ifndef header guard has wrong style, please use: constants_h  [build/header_guard] [5]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:13:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:16:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:17:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:18:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:21:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:24:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:25:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:28:  ALIASED_LINE_WIDTH_RANGE_MIN is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:29:  ALIASED_LINE_WIDTH_RANGE_MAX is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:30:  ALIASED_POINT_SIZE_RANGE_MIN is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 11 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

(In reply to comment #2)
> Attachment 217483 [details] did not pass style-queue:
> 
> Source/ThirdParty/ANGLE/src/libGLESv2/constants.h:9:  #ifndef header guard has wrong style, please use: constants_h  [build/header_guard] [5]

Ignoring style issues since ANGLE is a Third Party library.

[Brent Fulgham]

Comment on attachment 217483 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=217483&action=review

I thought that CodeGenGLSL.cpp was replaced by CodeGen.cpp.

> Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_glsl.vcxproj:-132
> -    <ClCompile Include="..\src\compiler\CodeGenGLSL.cpp" />

Shouldn't this be "CodeGen.cpp"?

> Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_glsl.vcxproj.filters:-16
> -    </ClCompile>

Ditto.

> Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_hlsl.vcxproj:-132
> -    <ClCompile Include="..\src\compiler\CodeGenHLSL.cpp" />

Ditto.

> Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_hlsl.vcxproj.filters:-26
> -    <ClCompile Include="..\src\compiler\CodeGenHLSL.cpp">

Ditto.

(In reply to comment #4)
> (From update of attachment 217483 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=217483&action=review
> 
> I thought that CodeGenGLSL.cpp was replaced by CodeGen.cpp.
> 
> > Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_glsl.vcxproj:-132
> > -    <ClCompile Include="..\src\compiler\CodeGenGLSL.cpp" />
> 
> Shouldn't this be "CodeGen.cpp"?
> 

I might be misunderstanding here, but shouldn't we remove CodeGenGLSL.cpp from the project here, as it is no longer present? (CodeGen.cpp is added to translator_common.lib).

[Brent Fulgham]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 217483 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=217483&action=review
> > 
> > I thought that CodeGenGLSL.cpp was replaced by CodeGen.cpp.
> > 
> > > Source/ThirdParty/ANGLE/ANGLE.vcxproj/translator_glsl.vcxproj:-132
> > > -    <ClCompile Include="..\src\compiler\CodeGenGLSL.cpp" />
> > 
> > Shouldn't this be "CodeGen.cpp"?
> > 
> 
> I might be misunderstanding here, but shouldn't we remove CodeGenGLSL.cpp from the project here, as it is no longer present? (CodeGen.cpp is added to translator_common.lib).

Yes -- if CodeGen.cpp is already part of translator_common.lib, then it should all just work.

Created attachment 217569 [details]
Patch

Updated patch; In the previous patch, I added some files to translator_common which was already included in translator_glsl/hlsl. I removed these files from translator_common, and instead linked with both translator_glsl and translator_hlsl.

[Brent Fulgham]

(In reply to comment #8)
> Updated patch; In the previous patch, I added some files to translator_common which was already included in translator_glsl/hlsl. I removed these files from translator_common, and instead linked with both translator_glsl and translator_hlsl.

I thought that translator_common was supposed to include "common" stuff, and that the HLSL and GLSL libraries would share the common infrastructure?

[Brent Fulgham]

Comment on attachment 217569 [details]
Patch

r=me

(In reply to comment #9)
> (In reply to comment #8)
> > Updated patch; In the previous patch, I added some files to translator_common which was already included in translator_glsl/hlsl. I removed these files from translator_common, and instead linked with both translator_glsl and translator_hlsl.
> 
> I thought that translator_common was supposed to include "common" stuff, and that the HLSL and GLSL libraries would share the common infrastructure?

Yes, that's my understanding as well. But CodeGen.cpp was added, which depends on both hlsl and glsl, so now we need to link with both libraries.

Thanks for reviewing :)

[WebKit Commit Bot]

Comment on attachment 217569 [details]
Patch

Clearing flags on attachment: 217569

Committed r159630: <http://trac.webkit.org/changeset/159630>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.