131277 2014-04-06 05:08:58 -0700 [CMake] Include the ICU directories after everything else 2016-01-03 17:27:38 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 1 rakuco rakuco bunhere cgarcia commit-queue gyuyoung.kim mrobinson sergio zan oldest_to_newest 997994 0 rakuco 2014-04-06 05:08:58 -0700 [CMake] Include the ICU directories after everything else 997995 1 228698 rakuco 2014-04-06 05:14:43 -0700 Created attachment 228698 Patch 998000 2 228698 mrobinson 2014-04-06 09:15:44 -0700 Comment on attachment 228698 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? 998001 3 rakuco 2014-04-06 09:24:20 -0700 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. 998005 4 mrobinson 2014-04-06 09:42:45 -0700 Sounds almost like Source/ThirdParty/ANGLE/include/GLES2/ shouldn't be on the include path. 998006 5 rakuco 2014-04-06 09:48:33 -0700 (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? 998007 6 mrobinson 2014-04-06 10:08:18 -0700 (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. 998010 7 rakuco 2014-04-06 10:26:20 -0700 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? 998011 8 mrobinson 2014-04-06 10:28:01 -0700 (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. 998013 9 mrobinson 2014-04-06 10:29:23 -0700 (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. 998100 10 rakuco 2014-04-07 01:17:26 -0700 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. 1152199 11 228698 mcatanzaro 2016-01-03 17:27:38 -0800 Comment on attachment 228698 Patch Moving ${ICU_INCLUDE_DIRS} and ${ICU_LIBRARIES} is a strange hack; I don't want to do this. 228698 2014-04-06 05:14:43 -0700 2016-01-03 17:27:38 -0800 Patch bug-131277-20140406151421.patch text/plain 2288 rakuco SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2Njg0OSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDE0LTA0LTA2ICBSYXBoYWVs IEt1Ym8gZGEgQ29zdGEgIDxyYXBoYWVsLmt1Ym8uZGEuY29zdGFAaW50ZWwuY29tPgorCisgICAg ICAgIFtDTWFrZV0gSW5jbHVkZSB0aGUgSUNVIGRpcmVjdG9yaWVzIGFmdGVyIGV2ZXJ5dGhpbmcg ZWxzZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMx Mjc3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBD TWFrZUxpc3RzLnR4dDogQWRkIElDVV9JTkNMVURFX0RJUlMgYWZ0ZXIgYWxsIHRoZSBkaXJlY3Rv cmllcyBwYXJ0IG9mIFdlYktpdCBpdHNlbGYsIGFzCisgICAgICAgIHdlIGFsd2F5cyB3YW50IHRv IHByZWZlciBsb2NhbCBoZWFkZXJzIGluc3RlYWQgb2Ygc3lzdGVtIG9uZXMuIFByb2JsZW1zIGFy ZSBwcm9uZSB0byBoYXBwZW4KKyAgICAgICAgd2hlbiBlLmcuIC91c3IvbG9jYWwvaW5jbHVkZSBp cyBwYXNzZWQgYmVmb3JlIHRoZSBBTkdMRSBwYXRocyBhbmQgd2UgdGh1cyBlbmQgdXAgdXNpbmcg dGhlCisgICAgICAgIHN5c3RlbSBHTCBoZWFkZXJzIGluc3RlYWQuIFdoaWxlIGhlcmUsIG1vdmUg SUNVX0xJQlJBUklFUyB0byBXZWJDb3JlX0xJQlJBUklFUyBub3cgdGhhdCBpdAorICAgICAgICBp cyB1bmNvbmRpdGlvbmFsbHkgZGVwZW5kZWQgdXBvbi4KKwogMjAxNC0wNC0wNSAgQnJpYW4gSi4g QnVyZyAgPGJ1cmdAY3Mud2FzaGluZ3Rvbi5lZHU+CiAKICAgICAgICAgV2ViIEluc3BlY3Rvcjog cmVtb3ZlIHVudXNlZCB0ZXN0IGhvb2sgc2V0SW5zcGVjdG9yUmVzb3VyY2VzRGF0YVNpemVMaW1p dHMKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NNYWtlTGlzdHMudHh0Cj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJDb3JlL0NNYWtlTGlzdHMudHh0CShyZXZpc2lvbiAxNjY4NDEpCisrKyBTb3VyY2Uv V2ViQ29yZS9DTWFrZUxpc3RzLnR4dAkod29ya2luZyBjb3B5KQpAQCAtMjU3Myw2ICsyNTczLDcg QEAgc2V0KFdlYkNvcmVfUExVR19JTlNfUkVTT1VSQ0VTCiBzZXQoV2ViQ29yZV9MSUJSQVJJRVMK ICAgICBKYXZhU2NyaXB0Q29yZQogICAgIFdURgorICAgICR7SUNVX0xJQlJBUklFU30KICkKIAog aWYgKEVOQUJMRV9FTkNSWVBURURfTUVESUEgT1IgRU5BQkxFX0VOQ1JZUFRFRF9NRURJQV9WMikK QEAgLTI3ODYsMTIgKzI3ODcsNiBAQCBsaXN0KEFQUEVORCBXZWJDb3JlX1NPVVJDRVMKICAgICBw bGF0Zm9ybS90ZXh0L1RleHRDb2RlY0lDVS5jcHAKICAgICBwbGF0Zm9ybS90ZXh0L1RleHRFbmNv ZGluZ0RldGVjdG9ySUNVLmNwcAogKQotbGlzdChBUFBFTkQgV2ViQ29yZV9JTkNMVURFX0RJUkVD VE9SSUVTCi0gICAgJHtJQ1VfSU5DTFVERV9ESVJTfQotKQotbGlzdChBUFBFTkQgV2ViQ29yZV9M SUJSQVJJRVMKLSAgICAke0lDVV9MSUJSQVJJRVN9Ci0pCiAKIGlmIChXVEZfVVNFX0xFVkVMREIp CiAgICAgbGlzdChBUFBFTkQgV2ViQ29yZV9JTkNMVURFX0RJUkVDVE9SSUVTCkBAIC0yOTU4LDYg KzI5NTMsMTQgQEAgaWYgKFdURl9VU0VfM0RfR1JBUEhJQ1MpCiAgICAgKQogZW5kaWYgKCkKIAor IyBUaGlzIGlzIGEgbm9uLW9wdGlvbmFsIGRlcGVuZGVuY3ksIGJ1dCB3ZSBsZWF2ZSBpdCBhdCB0 aGUgZW5kIHRvIGF2b2lkCisjIHBhc3NpbmcgcmVndWxhciBpbmNsdWRlIHBhdGhzIChpZS4gL3Vz ci9sb2NhbC9pbmNsdWRlKSBiZWZvcmUgYWxsIHRoZSBvbmVzIGluCisjIFdlYktpdCwgc2luY2Ug ZS5nLiBhIEdMIGhlYWRlciBtYXkgYmUgcHJlc2VudCBpbiBBTkdMRSBhbmQgdGhlIHN5c3RlbSdz IEdMCisjIGltcGxlbWVudGF0aW9uLCBhbmQgd2UgbmVlZCB0byBwaWNrIHRoZSBmb3JtZXIuCits aXN0KEFQUEVORCBXZWJDb3JlX0lOQ0xVREVfRElSRUNUT1JJRVMKKyAgICAke0lDVV9JTkNMVURF X0RJUlN9CispCisKIHNldChXZWJDb3JlVGVzdFN1cHBvcnRfSU5DTFVERV9ESVJFQ1RPUklFUwog ICAgICIke1dFQkNPUkVfRElSfS9wbGF0Zm9ybS9tb2NrIgogICAgICIke1dFQkNPUkVfRElSfS90 ZXN0aW5nIgo=