229337 2021-08-20 08:22:36 -0700 GLContextGLX.cpp may sometimes fail build because of X11 include files 2022-01-07 08:23:56 -0800 1 1 1 Unclassified WebKit WebCore Misc. WebKit Local Build Unspecified Unspecified NEW InRadar P2 Normal --- 1 nina webkit-unassigned fujii mcatanzaro pnormand webkit-bug-importer oldest_to_newest 1785832 0 nina 2021-08-20 08:22:36 -0700 The file should probably be marked @no-unify. 1785833 1 435989 nina 2021-08-20 08:23:34 -0700 Created attachment 435989 patch 1788025 2 webkit-bug-importer 2021-08-27 08:23:21 -0700 <rdar://problem/82439830> 1808852 3 442489 nina 2021-10-26 08:15:23 -0700 Created attachment 442489 patch updated patch 1809139 4 fujii 2021-10-26 18:35:45 -0700 WebKit has a lot of #undef in WTF and WebCore layer for this issue, for example "#undef True" and "#undef None". Does this change make it possible to remove it? For example, https://github.com/WebKit/WebKit/blob/bb27304754d0bb686fc3ddfccc42fb8ec1f14d62/Source/WTF/wtf/Threading.h#L64,L68 1809141 5 fujii 2021-10-26 18:39:23 -0700 Maybe, No. Because it can't compile if both X11 header and undef-ing headers are included. I think we should take #undef-ing approach. 1809145 6 fujii 2021-10-26 18:57:37 -0700 WebKit has ANGLEHeaders.h to include ANGLE headers. https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/graphics/angle/ANGLEHeaders.h I think we should add X11Headers.h to include X11 headers. WDYT? 1809224 7 nina 2021-10-27 04:48:15 -0700 possibly, I marked the file no-unify since in practice other OpenGL files do the same thing (there's a bunch of others in the same file) and I think it's the cleanest way to get around it (it should work unconditionally regardless of what the X11 headers do) but I'm fine either way 1809389 8 fujii 2021-10-27 12:50:53 -0700 In my understanding, @no-unify approach can't solve the issue completely because #include order is ruled by WebKit coding style. If a header defining None or True are included after X11 header, it can't compile. Is my understanding right? 1809627 9 nina 2021-10-28 03:49:15 -0700 if the build for the file is non-unified, it solves the issue because the file is compiled on its own - and therefore we get control over *which* headers are included and can make sure they are non-conflicting 1809793 10 fujii 2021-10-28 13:04:43 -0700 Then, we no longer need the #undef. could you remove all #undef for the issue in the patch? 1810046 11 nina 2021-10-29 05:46:06 -0700 you mean for the ANGLE bits? 1810225 12 fujii 2021-10-29 13:53:17 -0700 WebKit has a lot of #undef in WTF and WebCore layer for this issue For example, https://github.com/WebKit/WebKit/blob/bb27304754d0bb686fc3ddfccc42fb8ec1f14d62/Source/WTF/wtf/Threading.h#L64,L68 If you can solve the issue completely by @no-unify approach, we no longer need the #undef. I think @no-unify approach can't solve the issue completely because #include order is ruled by WebKit coding style. If a header defining None or True are included after X11 header, it can't compile. We need #undef. @no-unify approach isn't a good idea. I think we should add X11Headers.h to include all X11 headers we need. 1810536 13 nina 2021-10-31 18:54:20 -0700 (In reply to Fujii Hironori from comment #12) > WebKit has a lot of #undef in WTF and WebCore layer for this issue > For example, > https://github.com/WebKit/WebKit/blob/ > bb27304754d0bb686fc3ddfccc42fb8ec1f14d62/Source/WTF/wtf/Threading.h#L64,L68 > If you can solve the issue completely by @no-unify approach, we no longer > need the #undef. > > I think @no-unify approach can't solve the issue completely because > #include order is ruled by WebKit coding style. > If a header defining None or True are included after X11 header, it can't > compile. We need #undef. @no-unify approach isn't a good idea. > I think we should add X11Headers.h to include all X11 headers we need. this is never really a problem for the problematic file in this case, because the X11 headers are included last, and it's a source file - which was my point all along, if you eliminate it being unified with another source file, there should be no problem anymore (your concern would only be a problem if the file itself was a header, or if we didn't control the include order of headers) 1810537 14 fujii 2021-10-31 19:24:23 -0700 Sonny, but I don't understand. Can we remove all #undef? For example, https://github.com/WebKit/WebKit/blob/bb27304754d0bb686fc3ddfccc42fb8ec1f14d62/Source/WTF/wtf/Threading.h#L64,L68 If so, please do it. If not, please give up the @no-unify approach, that don't solve the issue. 1810649 15 nina 2021-11-01 09:25:51 -0700 (In reply to Fujii Hironori from comment #14) > Sonny, but I don't understand. > Can we remove all #undef? > For example, > https://github.com/WebKit/WebKit/blob/ > bb27304754d0bb686fc3ddfccc42fb8ec1f14d62/Source/WTF/wtf/Threading.h#L64,L68 > If so, please do it. > If not, please give up the @no-unify approach, that don't solve the issue. it does not fix that because that's a header; this patch is specifically for the one failing file 1828907 16 mcatanzaro 2022-01-07 08:23:56 -0800 I agree with Fujii that #undef is preferable to @no-unify. Don't hesitate to use @no-unify if you really need to, but I don't see why #undef couldn't do the job here. 435989 2021-08-20 08:23:34 -0700 2021-10-26 08:15:23 -0700 patch gl-context-glx-no-unify.patch text/plain 1281 nina ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBhMGY4M2E3ODI5NmEuLjEzMzkzNmZmNjVlNiAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAt MSwzICsxLDE4IEBACisyMDIxLTA4LTIwICBEYW5pZWwgS29sZXNhICA8ZGtvbGVzYUBpZ2FsaWEu Y29tPgorCisgICAgICAgIE1hcmsgR0xDb250ZXh0R0xYLmNwcCBhcyBuby11bmlmeQorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjI5MzM3CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSGF2aW5nIHRoaXMgYSBw YXJ0IG9mIHVuaWZpZWQgYnVpbGQgY2FuIGJyZWFrIGJ1aWxkcyBzaW5jZSBYMTEKKyAgICAgICAg aW5jbHVkZSBmaWxlcyBhcmUgaW5jbHVkZWQsIHdoaWNoIGhhdmUgbWFjcm9zIHRoYXQgY29uZmxp Y3QKKyAgICAgICAgd2l0aCBXVEYncyBlbnVtcy4KKworICAgICAgICBObyBuZXcgdGVzdHMgKE9P UFMhKS4KKworICAgICAgICAqIFNvdXJjZXNHVEsudHh0OgorCiAyMDIxLTA4LTIwICBBbGFuIEJ1 anRhcyAgPHphbGFuQGFwcGxlLmNvbT4KIAogICAgICAgICBbSUZDXSBVbmRvIGluY29ycmVjdCBJ bmxpbmVUZXh0Qm94IC0+IExlZ2FjeUlubGluZVRleHRCb3ggcmVuYW1pbmcKZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJDb3JlL1NvdXJjZXNHVEsudHh0IGIvU291cmNlL1dlYkNvcmUvU291cmNlc0dU Sy50eHQKaW5kZXggZTIyMmY2NmIxNTZhLi4yYzYwM2Y0NTEyYTMgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL1NvdXJjZXNHVEsudHh0CisrKyBiL1NvdXJjZS9XZWJDb3JlL1NvdXJjZXNHVEsu dHh0CkBAIC04NCw3ICs4NCw3IEBAIHBsYXRmb3JtL2dyYXBoaWNzL2VnbC9HTENvbnRleHRFR0xM aWJXUEUuY3BwIEBuby11bmlmeQogcGxhdGZvcm0vZ3JhcGhpY3MvZWdsL0dMQ29udGV4dEVHTFdh eWxhbmQuY3BwIEBuby11bmlmeQogcGxhdGZvcm0vZ3JhcGhpY3MvZWdsL0dMQ29udGV4dEVHTFgx MS5jcHAgQG5vLXVuaWZ5CiAKLXBsYXRmb3JtL2dyYXBoaWNzL2dseC9HTENvbnRleHRHTFguY3Bw CitwbGF0Zm9ybS9ncmFwaGljcy9nbHgvR0xDb250ZXh0R0xYLmNwcCBAbm8tdW5pZnkKIAogcGxh dGZvcm0vZ3JhcGhpY3MvZ3RrL0NvbG9yR3RrLmNwcAogcGxhdGZvcm0vZ3JhcGhpY3MvZ3RrL0Rp c3BsYXlSZWZyZXNoTW9uaXRvckd0ay5jcHAK 442489 2021-10-26 08:15:23 -0700 2021-10-26 08:15:23 -0700 patch glx-no-unify.patch text/plain 1206 nina ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAyMWM1YmQxNzdjMzUuLjQxOGRkMGM2MDExMCAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAt MSwzICsxLDE2IEBACisyMDIxLTEwLTI2ICBEYW5pZWwgS29sZXNhICA8ZGtvbGVzYUBpZ2FsaWEu Y29tPgorCisgICAgICAgIE1hcmsgR0xDb250ZXh0R0xYLmNwcCBhcyBuby11bmlmeQorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjI5MzM3CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSGF2aW5nIHRoaXMgYSBw YXJ0IG9mIHVuaWZpZWQgYnVpbGQgY2FuIGJyZWFrIGJ1aWxkcyBzaW5jZSBYMTEKKyAgICAgICAg aW5jbHVkZSBmaWxlcyBhcmUgaW5jbHVkZWQsIHdoaWNoIGhhdmUgbWFjcm9zIHRoYXQgY29uZmxp Y3QKKyAgICAgICAgd2l0aCBXVEYncyBlbnVtcy4KKworICAgICAgICAqIFNvdXJjZXNHVEsudHh0 OgorCiAyMDIxLTEwLTI2ICBBbnR0aSBLb2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KIAogICAg ICAgICBGaXggOjpwYXJ0KGZvbyk6aG92ZXIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL1Nv dXJjZXNHVEsudHh0IGIvU291cmNlL1dlYkNvcmUvU291cmNlc0dUSy50eHQKaW5kZXggNmZhMWMx YWY4MzIzLi5lZGM4OWU5YmM2ZWUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL1NvdXJjZXNH VEsudHh0CisrKyBiL1NvdXJjZS9XZWJDb3JlL1NvdXJjZXNHVEsudHh0CkBAIC05NCw3ICs5NCw3 IEBAIHBsYXRmb3JtL2dyYXBoaWNzL2VnbC9HTENvbnRleHRFR0xMaWJXUEUuY3BwIEBuby11bmlm eQogcGxhdGZvcm0vZ3JhcGhpY3MvZWdsL0dMQ29udGV4dEVHTFdheWxhbmQuY3BwIEBuby11bmlm eQogcGxhdGZvcm0vZ3JhcGhpY3MvZWdsL0dMQ29udGV4dEVHTFgxMS5jcHAgQG5vLXVuaWZ5CiAK LXBsYXRmb3JtL2dyYXBoaWNzL2dseC9HTENvbnRleHRHTFguY3BwCitwbGF0Zm9ybS9ncmFwaGlj cy9nbHgvR0xDb250ZXh0R0xYLmNwcCBAbm8tdW5pZnkKIAogcGxhdGZvcm0vZ3JhcGhpY3MvZ3Rr L0NvbG9yR3RrLmNwcAogcGxhdGZvcm0vZ3JhcGhpY3MvZ3RrL0Rpc3BsYXlSZWZyZXNoTW9uaXRv ckd0ay5jcHAK