Bug 131277 – [CMake] Include the ICU directories after everything else

• Status: NEW
• Component: New Bugs

[Raphael Kubo da Costa (:rakuco)]

[CMake] Include the ICU directories after everything else

[Raphael Kubo da Costa (:rakuco)]

Created attachment 228698 [details]
Patch

[Martin Robinson]

Comment on attachment 228698 [details]
Patch

I'm not sure I understand the danger. If we are using the system version of libicu, don't we want t compile against the system version's headers?

[Raphael Kubo da Costa (:rakuco)]

This isn't really about ICU, but rather that we want to include all WebKit paths before system ones. This isn't normally a problem on Linux where most headers are in /usr/include, which isn't passed explicitly to the compiler.

However, if I try to build WebKitGTK on another platform such as FreeBSD, where most headers are in /usr/local/include (including ICU's), I end up in a situation where the compiler gets passed something like this:

  g++ -I/path/to/WebKit/Source/WebCore -I/path/to/WebKit/Source/WebCore/html
      -I/path/to/WebKit/Source/WebCore/css [...]
      -I/usr/local/include -I/path/to/WebKit/Source/ThirdParty/ANGLE/include

This causes my system's GL headers to be picked up instead of ANGLE's, which can cause build failures when they differ because of constant names and other things.

[Martin Robinson]

Sounds almost like Source/ThirdParty/ANGLE/include/GLES2/ shouldn't be on the include path.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #4)
> Sounds almost like Source/ThirdParty/ANGLE/include/GLES2/ shouldn't be on the include path.

Isn't it needed to build ANGLE and doesn't the code in platform/graphics/opengl assume ANGLE is used?

[Martin Robinson]

(In reply to comment #5)
> (In reply to comment #4)
> > Sounds almost like Source/ThirdParty/ANGLE/include/GLES2/ shouldn't be on the include path.
> 
> Isn't it needed to build ANGLE and doesn't the code in platform/graphics/opengl assume ANGLE is used?

At least for WebKitGTK+, the headers and GL library from the system should be used. ANGLE is only used now to convert GLES shaders to desktop shaders.

[Raphael Kubo da Costa (:rakuco)]

At the moment, if I try to use my system (Mesa) GL headers and libraries, the build fails because Extensions3DOpenGLES.h references PFNGLFRAMEBUFFERTEXTURE2DMULTISAMPLEIMG and PFNGLRENDERBUFFERSTORAGEMULTISAMPLEIMG, which have had different names for years.

Should this code be made to work with the system GL libraries? How about platforms that have to use ANGLE?

[Martin Robinson]

(In reply to comment #7)
> At the moment, if I try to use my system (Mesa) GL headers and libraries, the build fails because Extensions3DOpenGLES.h references PFNGLFRAMEBUFFERTEXTURE2DMULTISAMPLEIMG and PFNGLRENDERBUFFERSTORAGEMULTISAMPLEIMG, which have had different names for years.

Yes, I think we should correct the names, if possible.

> Should this code be made to work with the system GL libraries? How about platforms that have to use ANGLE?

I don't know of any platforms that use ANGLE for the GLES2 implementation.

[Martin Robinson]

(In reply to comment #8)

> I don't know of any platforms that use ANGLE for the GLES2 implementation.

Actually, it is possible that ANGLE is used by the Windows ports.

[Raphael Kubo da Costa (:rakuco)]

I'm trying to create a patch that removes the ANGLE directories from the list of include paths, but the fact that GLSLANG/ is also in Source/ThirdParty/ANGLE/include along GLES2/ and friends makes things difficult.

[Michael Catanzaro]

Comment on attachment 228698 [details]
Patch

Moving ${ICU_INCLUDE_DIRS} and ${ICU_LIBRARIES} is a strange hack; I don't want to do this.