177640 2017-09-28 20:48:58 -0700 JSC generate unified sources doesn't need to run during installhdrs. 2017-11-11 07:50:14 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=179578 InRadar P2 Normal --- 1 keith_miller keith_miller buildbot commit-queue mark.lam mitz msaboff saam webkit-bug-importer oldest_to_newest 1354431 0 keith_miller 2017-09-28 20:48:58 -0700 JSC generate unified sources doesn't need to run during installhdrs. 1354433 1 322163 keith_miller 2017-09-28 20:53:00 -0700 Created attachment 322163 Patch 1354435 2 322163 mitz 2017-09-28 20:59:07 -0700 Comment on attachment 322163 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322163&action=review > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:10307 > + INFOPLIST_FILE = "Generate Unified Sources-Info.plist"; LOL. This build setting is a path to a file in the source tree. I don’t think there’s a file with either name. But it doesn't matter, because this build setting is meaningless for a target that doesn't build a bundle or a binary with an embedded Info.plist. > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:10338 > + INSTALLHDRS_SCRIPT_PHASE = NO; This quadruplication is one of the reasons why we don’t define build settings in the .pbxproj file, but rather in .xcconfig files. You may be able to get away with simply changing the Generate Unified Sources target’s configuration file to “None” in the project editor, because the default for INSTALLHDRS_SCRIPT_PHASE is NO. If that doesn’t work, you’ll need to create a new .xcconfig file for this target and add any required settings to it. 1354443 3 322163 keith_miller 2017-09-28 21:34:24 -0700 Comment on attachment 322163 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322163&action=review >> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:10307 >> + INFOPLIST_FILE = "Generate Unified Sources-Info.plist"; > > LOL. This build setting is a path to a file in the source tree. I don’t think there’s a file with either name. But it doesn't matter, because this build setting is meaningless for a target that doesn't build a bundle or a binary with an embedded Info.plist. OHH, I didn't realize that this list of settings was just custom values. Yeah there none of the settings here are needed. TBH, it shouldn't even be different regardless of the build "configuration". >> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:10338 >> + INSTALLHDRS_SCRIPT_PHASE = NO; > > This quadruplication is one of the reasons why we don’t define build settings in the .pbxproj file, but rather in .xcconfig files. You may be able to get away with simply changing the Generate Unified Sources target’s configuration file to “None” in the project editor, because the default for INSTALLHDRS_SCRIPT_PHASE is NO. If that doesn’t work, you’ll need to create a new .xcconfig file for this target and add any required settings to it. Yeah, setting it to None seems to have worked. I also got rid of the INFOPLIST_FILE setting. For whatever reason you can't get rid of the PRODUCT_NAME setting. I don't fully understand the implications of its existence but w/e. For JSC it doesn't matter since we don't care about the FEATURE_DEFINES value in the JavaScriptCore.xcconfig but in WebCore the unified source bundler script will. It doesn't really seem worth it to redefine all the values in the xcconfig file just so that we can save 4 lines here but idk how to Xcode. 1354444 4 mitz 2017-09-28 21:38:59 -0700 (In reply to Keith Miller from comment #3) > Comment on attachment 322163 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=322163&action=review > > >> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:10307 > >> + INFOPLIST_FILE = "Generate Unified Sources-Info.plist"; > > > > LOL. This build setting is a path to a file in the source tree. I don’t think there’s a file with either name. But it doesn't matter, because this build setting is meaningless for a target that doesn't build a bundle or a binary with an embedded Info.plist. > > OHH, I didn't realize that this list of settings was just custom values. > Yeah there none of the settings here are needed. TBH, it shouldn't even be > different regardless of the build "configuration". > > >> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:10338 > >> + INSTALLHDRS_SCRIPT_PHASE = NO; > > > > This quadruplication is one of the reasons why we don’t define build settings in the .pbxproj file, but rather in .xcconfig files. You may be able to get away with simply changing the Generate Unified Sources target’s configuration file to “None” in the project editor, because the default for INSTALLHDRS_SCRIPT_PHASE is NO. If that doesn’t work, you’ll need to create a new .xcconfig file for this target and add any required settings to it. > > Yeah, setting it to None seems to have worked. I also got rid of the > INFOPLIST_FILE setting. For whatever reason you can't get rid of the > PRODUCT_NAME setting. I don't fully understand the implications of its > existence but w/e. For JSC it doesn't matter since we don't care about the > FEATURE_DEFINES value in the JavaScriptCore.xcconfig but in WebCore the > unified source bundler script will. It doesn't really seem worth it to > redefine all the values in the xcconfig file just so that we can save 4 > lines here but idk how to Xcode. You won’t need to “redefine all the values” in any case, because you can use #include and because within the same level in the build setting hierarchy, the last definition of any build setting prevails. 1354446 5 322167 keith_miller 2017-09-28 21:45:00 -0700 Created attachment 322167 Patch 1354447 6 keith_miller 2017-09-28 21:46:02 -0700 (In reply to mitz from comment #4) > > You won’t need to “redefine all the values” in any case, because you can use > #include and because within the same level in the build setting hierarchy, > the last definition of any build setting prevails. Ah, good point. 1357412 7 mitz 2017-10-05 19:32:56 -0700 Would be good to land the patch soon, as it resolves some problems in Apple’s internal build system. 1357413 8 mitz 2017-10-05 19:33:30 -0700 (In reply to mitz from comment #7) > Would be good to land the patch soon, as it resolves some problems in > Apple’s internal build system. Never mind, I overlooked the commit-queue flag. 1357419 9 322167 commit-queue 2017-10-05 20:02:00 -0700 Comment on attachment 322167 Patch Clearing flags on attachment: 322167 Committed r222954: <http://trac.webkit.org/changeset/222954> 1357420 10 commit-queue 2017-10-05 20:02:02 -0700 All reviewed patches have been landed. Closing bug. 1357422 11 webkit-bug-importer 2017-10-05 20:03:30 -0700 <rdar://problem/34849704> 322163 2017-09-28 20:53:00 -0700 2017-09-28 21:44:55 -0700 Patch bug-177640-20170928205259.patch text/plain 2680 keith_miller U3VidmVyc2lvbiBSZXZpc2lvbjogMjIyNjQ1CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAy NGY2NGE2NjhmM2FmNzgwNDM2YWZhYWRjNGI0NjBlMDQxZmFhZjJhLi40NTEwZDgyYWMzM2Y3MDAy YzI2NzI1NjZkMWNjYjg0Zjg5YjExM2JiIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNCBAQAorMjAxNy0wOS0yOCAgS2VpdGggTWlsbGVyICA8a2VpdGhfbWlsbGVyQGFwcGxl LmNvbT4KKworICAgICAgICBKU0MgZ2VuZXJhdGUgdW5pZmllZCBzb3VyY2VzIGRvZXNuJ3QgbmVl ZCB0byBydW4gZHVyaW5nIGluc3RhbGxoZHJzLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc3NjQwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgQWxzbywgZ2l2ZSB0aGUgaW5mbyBwbGlzdCBhIG1vcmUgcmVh c29uYWJsZSBuYW1lLgorCisgICAgICAgICogSmF2YVNjcmlwdENvcmUueGNvZGVwcm9qL3Byb2pl Y3QucGJ4cHJvajoKKwogMjAxNy0wOS0yOCAgTWFyayBMYW0gIDxtYXJrLmxhbUBhcHBsZS5jb20+ CiAKICAgICAgICAgdGVzdDI2MjogVW5leHBlY3RlZCBwYXNzZXMgYWZ0ZXIgcjIyMjYxNyBhbmQg cjIyMjYxOC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9KYXZhU2NyaXB0Q29y ZS54Y29kZXByb2ovcHJvamVjdC5wYnhwcm9qIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0phdmFT Y3JpcHRDb3JlLnhjb2RlcHJvai9wcm9qZWN0LnBieHByb2oKaW5kZXggZmViNzQwMTQ3NGI5NTk2 NWI0ZTZlNTlhNWU4Mzc3ODQwNGI1NGRjNS4uNmUxZjBlYjkxZGZiN2U1ZjExYjUzNTE5Zjg2ODg5 YzUzMmYxODdiOSAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL0phdmFTY3JpcHRD b3JlLnhjb2RlcHJvai9wcm9qZWN0LnBieHByb2oKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3Jl L0phdmFTY3JpcHRDb3JlLnhjb2RlcHJvai9wcm9qZWN0LnBieHByb2oKQEAgLTEwMzA0LDcgKzEw MzA0LDggQEAKIAkJCWlzYSA9IFhDQnVpbGRDb25maWd1cmF0aW9uOwogCQkJYmFzZUNvbmZpZ3Vy YXRpb25SZWZlcmVuY2UgPSAxQzkwNTE0MzBCQTlFOEE3MDA4MUU5RDAgLyogSmF2YVNjcmlwdENv cmUueGNjb25maWcgKi87CiAJCQlidWlsZFNldHRpbmdzID0gewotCQkJCUlORk9QTElTVF9GSUxF ID0gIk9mZmxpbmUgQXNzZW1ibGVyIGNvcHktSW5mby5wbGlzdCI7CisJCQkJSU5GT1BMSVNUX0ZJ TEUgPSAiR2VuZXJhdGUgVW5pZmllZCBTb3VyY2VzLUluZm8ucGxpc3QiOworCQkJCUlOU1RBTExI RFJTX1NDUklQVF9QSEFTRSA9IE5POwogCQkJCVBST0RVQ1RfTkFNRSA9ICIkKFRBUkdFVF9OQU1F KSI7CiAJCQl9OwogCQkJbmFtZSA9IERlYnVnOwpAQCAtMTAzMTMsNyArMTAzMTQsOCBAQAogCQkJ aXNhID0gWENCdWlsZENvbmZpZ3VyYXRpb247CiAJCQliYXNlQ29uZmlndXJhdGlvblJlZmVyZW5j ZSA9IDFDOTA1MTQzMEJBOUU4QTcwMDgxRTlEMCAvKiBKYXZhU2NyaXB0Q29yZS54Y2NvbmZpZyAq LzsKIAkJCWJ1aWxkU2V0dGluZ3MgPSB7Ci0JCQkJSU5GT1BMSVNUX0ZJTEUgPSAiT2ZmbGluZSBB c3NlbWJsZXIgY29weS1JbmZvLnBsaXN0IjsKKwkJCQlJTkZPUExJU1RfRklMRSA9ICJHZW5lcmF0 ZSBVbmlmaWVkIFNvdXJjZXMtSW5mby5wbGlzdCI7CisJCQkJSU5TVEFMTEhEUlNfU0NSSVBUX1BI QVNFID0gTk87CiAJCQkJUFJPRFVDVF9OQU1FID0gIiQoVEFSR0VUX05BTUUpIjsKIAkJCX07CiAJ CQluYW1lID0gUmVsZWFzZTsKQEAgLTEwMzIyLDcgKzEwMzI0LDggQEAKIAkJCWlzYSA9IFhDQnVp bGRDb25maWd1cmF0aW9uOwogCQkJYmFzZUNvbmZpZ3VyYXRpb25SZWZlcmVuY2UgPSAxQzkwNTE0 MzBCQTlFOEE3MDA4MUU5RDAgLyogSmF2YVNjcmlwdENvcmUueGNjb25maWcgKi87CiAJCQlidWls ZFNldHRpbmdzID0gewotCQkJCUlORk9QTElTVF9GSUxFID0gIk9mZmxpbmUgQXNzZW1ibGVyIGNv cHktSW5mby5wbGlzdCI7CisJCQkJSU5GT1BMSVNUX0ZJTEUgPSAiR2VuZXJhdGUgVW5pZmllZCBT b3VyY2VzLUluZm8ucGxpc3QiOworCQkJCUlOU1RBTExIRFJTX1NDUklQVF9QSEFTRSA9IE5POwog CQkJCVBST0RVQ1RfTkFNRSA9ICIkKFRBUkdFVF9OQU1FKSI7CiAJCQl9OwogCQkJbmFtZSA9IFBy b2ZpbGluZzsKQEAgLTEwMzMxLDcgKzEwMzM0LDggQEAKIAkJCWlzYSA9IFhDQnVpbGRDb25maWd1 cmF0aW9uOwogCQkJYmFzZUNvbmZpZ3VyYXRpb25SZWZlcmVuY2UgPSAxQzkwNTE0MzBCQTlFOEE3 MDA4MUU5RDAgLyogSmF2YVNjcmlwdENvcmUueGNjb25maWcgKi87CiAJCQlidWlsZFNldHRpbmdz ID0gewotCQkJCUlORk9QTElTVF9GSUxFID0gIk9mZmxpbmUgQXNzZW1ibGVyIGNvcHktSW5mby5w bGlzdCI7CisJCQkJSU5GT1BMSVNUX0ZJTEUgPSAiR2VuZXJhdGUgVW5pZmllZCBTb3VyY2VzLUlu Zm8ucGxpc3QiOworCQkJCUlOU1RBTExIRFJTX1NDUklQVF9QSEFTRSA9IE5POwogCQkJCVBST0RV Q1RfTkFNRSA9ICIkKFRBUkdFVF9OQU1FKSI7CiAJCQl9OwogCQkJbmFtZSA9IFByb2R1Y3Rpb247 Cg== 322167 2017-09-28 21:45:00 -0700 2017-10-05 20:02:00 -0700 Patch bug-177640-20170928214459.patch text/plain 2564 keith_miller U3VidmVyc2lvbiBSZXZpc2lvbjogMjIyNjQ1CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAy NGY2NGE2NjhmM2FmNzgwNDM2YWZhYWRjNGI0NjBlMDQxZmFhZjJhLi42NjM4ZjRiNjA0NGRjNDA4 MWUxNDZhNDRjNzIyOGZhYmJlNmExNDRlIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNiBAQAorMjAxNy0wOS0yOCAgS2VpdGggTWlsbGVyICA8a2VpdGhfbWlsbGVyQGFwcGxl LmNvbT4KKworICAgICAgICBKU0MgZ2VuZXJhdGUgdW5pZmllZCBzb3VyY2VzIGRvZXNuJ3QgbmVl ZCB0byBydW4gZHVyaW5nIGluc3RhbGxoZHJzLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc3NjQwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgZ2VuZXJhdGUgdW5pZmllZCBzb3VyY2VzIGRvZXNuJ3QgbmVl ZCB0byBoYXZlIGEgeGNjb25maWcgZmlsZQorICAgICAgICBzaW5jZSB3ZSBkb24ndCBoYXZlIGFu eSBmZWF0dXJlIGRlZmluZXMuIEFsc28sIHJlbW92ZSB0aGUgcGxpc3QKKyAgICAgICAgYmVjYXVz ZSB0aGVyZSdzIG5vIHBsaXN0IGZvciB0aGlzLi4uCisKKyAgICAgICAgKiBKYXZhU2NyaXB0Q29y ZS54Y29kZXByb2ovcHJvamVjdC5wYnhwcm9qOgorCiAyMDE3LTA5LTI4ICBNYXJrIExhbSAgPG1h cmsubGFtQGFwcGxlLmNvbT4KIAogICAgICAgICB0ZXN0MjYyOiBVbmV4cGVjdGVkIHBhc3NlcyBh ZnRlciByMjIyNjE3IGFuZCByMjIyNjE4LgpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRD b3JlL0phdmFTY3JpcHRDb3JlLnhjb2RlcHJvai9wcm9qZWN0LnBieHByb2ogYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvSmF2YVNjcmlwdENvcmUueGNvZGVwcm9qL3Byb2plY3QucGJ4cHJvagppbmRl eCBmZWI3NDAxNDc0Yjk1OTY1YjRlNmU1OWE1ZTgzNzc4NDA0YjU0ZGM1Li4yZTljOTQzNWY5Y2Y3 ODk3MTllZmM0YjVjMDYxY2RhMDRkZDZjODEwIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvSmF2YVNjcmlwdENvcmUueGNvZGVwcm9qL3Byb2plY3QucGJ4cHJvagorKysgYi9Tb3Vy Y2UvSmF2YVNjcmlwdENvcmUvSmF2YVNjcmlwdENvcmUueGNvZGVwcm9qL3Byb2plY3QucGJ4cHJv agpAQCAtMTAzMDIsMzYgKzEwMzAyLDI4IEBACiAJCX07CiAJCTUzQjRCRDBCMUY2OEFGODkwMEQy QkVBMyAvKiBEZWJ1ZyAqLyA9IHsKIAkJCWlzYSA9IFhDQnVpbGRDb25maWd1cmF0aW9uOwotCQkJ YmFzZUNvbmZpZ3VyYXRpb25SZWZlcmVuY2UgPSAxQzkwNTE0MzBCQTlFOEE3MDA4MUU5RDAgLyog SmF2YVNjcmlwdENvcmUueGNjb25maWcgKi87CiAJCQlidWlsZFNldHRpbmdzID0gewotCQkJCUlO Rk9QTElTVF9GSUxFID0gIk9mZmxpbmUgQXNzZW1ibGVyIGNvcHktSW5mby5wbGlzdCI7CiAJCQkJ UFJPRFVDVF9OQU1FID0gIiQoVEFSR0VUX05BTUUpIjsKIAkJCX07CiAJCQluYW1lID0gRGVidWc7 CiAJCX07CiAJCTUzQjRCRDBDMUY2OEFGODkwMEQyQkVBMyAvKiBSZWxlYXNlICovID0gewogCQkJ aXNhID0gWENCdWlsZENvbmZpZ3VyYXRpb247Ci0JCQliYXNlQ29uZmlndXJhdGlvblJlZmVyZW5j ZSA9IDFDOTA1MTQzMEJBOUU4QTcwMDgxRTlEMCAvKiBKYXZhU2NyaXB0Q29yZS54Y2NvbmZpZyAq LzsKIAkJCWJ1aWxkU2V0dGluZ3MgPSB7Ci0JCQkJSU5GT1BMSVNUX0ZJTEUgPSAiT2ZmbGluZSBB c3NlbWJsZXIgY29weS1JbmZvLnBsaXN0IjsKIAkJCQlQUk9EVUNUX05BTUUgPSAiJChUQVJHRVRf TkFNRSkiOwogCQkJfTsKIAkJCW5hbWUgPSBSZWxlYXNlOwogCQl9OwogCQk1M0I0QkQwRDFGNjhB Rjg5MDBEMkJFQTMgLyogUHJvZmlsaW5nICovID0gewogCQkJaXNhID0gWENCdWlsZENvbmZpZ3Vy YXRpb247Ci0JCQliYXNlQ29uZmlndXJhdGlvblJlZmVyZW5jZSA9IDFDOTA1MTQzMEJBOUU4QTcw MDgxRTlEMCAvKiBKYXZhU2NyaXB0Q29yZS54Y2NvbmZpZyAqLzsKIAkJCWJ1aWxkU2V0dGluZ3Mg PSB7Ci0JCQkJSU5GT1BMSVNUX0ZJTEUgPSAiT2ZmbGluZSBBc3NlbWJsZXIgY29weS1JbmZvLnBs aXN0IjsKIAkJCQlQUk9EVUNUX05BTUUgPSAiJChUQVJHRVRfTkFNRSkiOwogCQkJfTsKIAkJCW5h bWUgPSBQcm9maWxpbmc7CiAJCX07CiAJCTUzQjRCRDBFMUY2OEFGODkwMEQyQkVBMyAvKiBQcm9k dWN0aW9uICovID0gewogCQkJaXNhID0gWENCdWlsZENvbmZpZ3VyYXRpb247Ci0JCQliYXNlQ29u ZmlndXJhdGlvblJlZmVyZW5jZSA9IDFDOTA1MTQzMEJBOUU4QTcwMDgxRTlEMCAvKiBKYXZhU2Ny aXB0Q29yZS54Y2NvbmZpZyAqLzsKIAkJCWJ1aWxkU2V0dGluZ3MgPSB7Ci0JCQkJSU5GT1BMSVNU X0ZJTEUgPSAiT2ZmbGluZSBBc3NlbWJsZXIgY29weS1JbmZvLnBsaXN0IjsKIAkJCQlQUk9EVUNU X05BTUUgPSAiJChUQVJHRVRfTkFNRSkiOwogCQkJfTsKIAkJCW5hbWUgPSBQcm9kdWN0aW9uOwo=