Bug 124679

Summary: [WinCairo] Building ANGLE libraries fails.
Product: WebKit Reporter: peavo
Component: ANGLEAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, dino, kondapallykalyan, roger_fong
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description peavo 2013-11-20 14:41:13 PST
There are some compile and linker errors, while building ANGLE libraries for WinCairo.
Comment 1 peavo 2013-11-20 14:47:44 PST
Created attachment 217483 [details]
Patch
Comment 2 WebKit Commit Bot 2013-11-20 14:49:21 PST
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.
Comment 3 Brent Fulgham 2013-11-20 14:53:21 PST
(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.
Comment 4 Brent Fulgham 2013-11-20 14:55:18 PST
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.
Comment 5 peavo 2013-11-20 15:04:07 PST
(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).
Comment 6 Brent Fulgham 2013-11-20 15:42:23 PST
(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.
Comment 7 peavo 2013-11-21 08:03:21 PST
Created attachment 217569 [details]
Patch
Comment 8 peavo 2013-11-21 08:07:07 PST
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.
Comment 9 Brent Fulgham 2013-11-21 09:07:00 PST
(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?
Comment 10 Brent Fulgham 2013-11-21 09:08:19 PST
Comment on attachment 217569 [details]
Patch

r=me
Comment 11 peavo 2013-11-21 09:25:53 PST
(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 :)
Comment 12 WebKit Commit Bot 2013-11-21 09:34:10 PST
Comment on attachment 217569 [details]
Patch

Clearing flags on attachment: 217569

Committed r159630: <http://trac.webkit.org/changeset/159630>
Comment 13 WebKit Commit Bot 2013-11-21 09:34:13 PST
All reviewed patches have been landed.  Closing bug.