37701 2010-04-16 00:46:36 -0700 [Qt] Target(WebCore,jsc,...) must depends on static library of JavaScriptCore 2010-05-16 01:10:23 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All CLOSED FIXED Qt P2 Blocker --- 35784 36590 1 ossy webkit-qt-unassigned hausmann ismail jturcotte laszlo.gombos pvarga oldest_to_newest 213226 0 ossy 2010-04-16 00:46:36 -0700 From http://trac.webkit.org/changeset/56623 JavaScriptCore is builded as static library. But unfortunately libQtWebKit.so, jsc, QtLauncher,... don't depends on libjscore.a 213227 1 53521 ossy 2010-04-16 00:47:52 -0700 Created attachment 53521 proposed fix 213228 2 ossy 2010-04-16 00:49:35 -0700 This unpleasant bug was discovered by my co-worker Peter Varga, thx Peter. 213233 3 jturcotte 2010-04-16 01:15:57 -0700 On MSVC the library name should be "$${JAVASCRIPTCORE_TARGET}.lib" instead of "lib$${JAVASCRIPTCORE_TARGET}.a" If you tested it and it works because of some qmake behind-the-scene magic then just forget my comment. The rest looks great to me! 213237 4 ossy 2010-04-16 03:06:24 -0700 Ooops, I didn't test it with MSVC, I'll try it in an hour following your advice. Laszlo, could you check if the symbian case is correct? (In reply to comment #3) > On MSVC the library name should be "$${JAVASCRIPTCORE_TARGET}.lib" instead of > "lib$${JAVASCRIPTCORE_TARGET}.a" > > If you tested it and it works because of some qmake behind-the-scene magic then > just forget my comment. > > The rest looks great to me! 213238 5 53521 ossy 2010-04-16 03:07:14 -0700 Comment on attachment 53521 proposed fix r? removed until fix msvc case 213244 6 53527 ossy 2010-04-16 05:35:22 -0700 Created attachment 53527 proposed fix MSVC case fixed following Jocelyn's comment and tested. 213245 7 hausmann 2010-04-16 06:30:09 -0700 Ossy, can you explain _why_ this works? :) From what I can see this adds a dependency for the target rule: WebCoreLibrary: <the objects to for linking> ../path/to/JSC.a <link webcore command> So it seems that with POST_TARGETDEPS we get that additional dependency at the end. According to grep that's the case for the mingw/nmake and unix makefile generators. For these generators there's the toplevel Makefile that first calls into JavaScriptCore and then into WebCore's Makefile. If the JSC.a exists at the time WebCore's Makefile is called, then everything will be fine. But if it does _not_ exist, then I don't see how anything is fixed with the proposed patch, given that there's no _rule_ in WebCore's Makefile to actually generate the JSC.a. I must be missing something, please enlighten me :) Otherwise the patch looks fine, although I don't see POST_TARGETDEPS being used by the Symbian sbsv1/v2 generators. But then again, it'll be used by the symbian makefile based builds. Ossy, what's the actual problem you're hitting? 213248 8 ossy 2010-04-16 07:04:53 -0700 (In reply to comment #7) > Ossy, can you explain _why_ this works? :) I forgot to mention why it works. :) The answer is simple. Because in Webkit.pro JavaScriptCore subdir is in the first element of SUBDIRS, so the Makefile generated from JavaScriptCore.pro contain the build rules for libjscore.a (or jscore.lib in Symbian and MSVC case). Our problem isn't the missing libjscore.a building rules, but missing dependency I mentioned before: TARGET = libQtWebKit.so.4.6.0 ../lib/$(TARGET): $(OBJECTS) $(SUBLIBS) $(OBJCOMP) ../JavaScriptCore/libjscore.a It is useful for incremental builds. If something changed in libjscore.a, jsc binary, libQtWebKit.so _must_ be link again with the new libjscore.a. Now if you change something in a cpp of JavaScriptCore, libQtWebKit.so and jsc binary won't be linked again, but they should. > If the JSC.a exists at the time WebCore's Makefile is called, then everything > will be fine. But if it does _not_ exist, then I don't see how anything is > fixed with the proposed patch, given that there's no _rule_ in WebCore's > Makefile to actually generate the JSC.a. Absolutely correct. If the developer don't use the recommended WebKitTools/Scripts/build-webkit script, libjscore.a may not be exist before compiling WebCore. I don't think we should handle this case. :) > Otherwise the patch looks fine, although I don't see POST_TARGETDEPS being used > by the Symbian sbsv1/v2 generators. But then again, it'll be used by the > symbian makefile based builds. Unfortunately I don't know anything about Symbian build system. Is it possible that it has same problem? Does this patch help for Symbian incremental build? 213401 9 hausmann 2010-04-16 13:42:17 -0700 (In reply to comment #8) > (In reply to comment #7) > > Ossy, can you explain _why_ this works? :) > > I forgot to mention why it works. :) > > The answer is simple. Because in Webkit.pro JavaScriptCore subdir > is in the first element of SUBDIRS, so the Makefile generated from > JavaScriptCore.pro contain the build rules for libjscore.a (or > jscore.lib in Symbian and MSVC case). > > Our problem isn't the missing libjscore.a building rules, but > missing dependency I mentioned before: > > TARGET = libQtWebKit.so.4.6.0 > ../lib/$(TARGET): $(OBJECTS) $(SUBLIBS) $(OBJCOMP) > ../JavaScriptCore/libjscore.a > > It is useful for incremental builds. If something changed in libjscore.a, > jsc binary, libQtWebKit.so _must_ be link again with the new libjscore.a. > Now if you change something in a cpp of JavaScriptCore, libQtWebKit.so > and jsc binary won't be linked again, but they should. Aha! Thanks for the explanation, that makes sense. > > If the JSC.a exists at the time WebCore's Makefile is called, then everything > > will be fine. But if it does _not_ exist, then I don't see how anything is > > fixed with the proposed patch, given that there's no _rule_ in WebCore's > > Makefile to actually generate the JSC.a. > Absolutely correct. If the developer don't use the recommended > WebKitTools/Scripts/build-webkit script, libjscore.a may not be > exist before compiling WebCore. I don't think we should handle > this case. :) > > > Otherwise the patch looks fine, although I don't see POST_TARGETDEPS being used > > by the Symbian sbsv1/v2 generators. But then again, it'll be used by the > > symbian makefile based builds. > > Unfortunately I don't know anything about Symbian build system. > Is it possible that it has same problem? Does this patch help > for Symbian incremental build? It might not help Symbian builds directly, the sbs based builds have their own dependency tracking that is out of our hands anyway. 214565 10 ossy 2010-04-20 04:27:11 -0700 Landed in http://trac.webkit.org/changeset/57882 214570 11 ismail 2010-04-20 04:47:13 -0700 This breaks compilation Windows CE; NMAKE : fatal error U1073: don't know how to make '..\JavaScriptCore\release\libjscore.a' 214571 12 ossy 2010-04-20 04:50:49 -0700 (In reply to comment #11) > This breaks compilation Windows CE; > > NMAKE : fatal error U1073: don't know how to make > '..\JavaScriptCore\release\libjscore.a' Could we talk on #qtwebkit or on #webkit? What is your nick? 214572 13 ismail 2010-04-20 04:51:54 -0700 (In reply to comment #12) > (In reply to comment #11) > > This breaks compilation Windows CE; > > > > NMAKE : fatal error U1073: don't know how to make > > '..\JavaScriptCore\release\libjscore.a' > Could we talk on #qtwebkit or on #webkit? What is your nick? cartman, when I am around. Currently I can't IRC. 214574 14 ossy 2010-04-20 04:58:00 -0700 > cartman, when I am around. Currently I can't IRC. Do you build with build-webkit script? MakeFile generated from WebKit.pro should contain the rule to make '..\JavaScriptCore\release\libjscore.a' or anything similar to this. Could you tell me if there is any jscore lib in your build dir? 214576 15 ismail 2010-04-20 04:59:32 -0700 I had to revert to an earlier commit (because I need a working build atm.), I will re-report when I try trunk again. Thanks! 214594 16 ossy 2010-04-20 05:49:42 -0700 I hope I fixed it: http://trac.webkit.org/changeset/57885 Ismail, could you confirm if it works? 214630 17 hausmann 2010-04-20 07:03:41 -0700 Hmm, is this really a release blocker? AFAICS this is useful for incremental builds, in particular the bot. If you guys agree I'd like to remove the blocking dependency. 214635 18 ismail 2010-04-20 07:08:02 -0700 (In reply to comment #16) > I hope I fixed it: http://trac.webkit.org/changeset/57885 > > Ismail, could you confirm if it works? This seems to fix the build error, thank you. 214639 19 ossy 2010-04-20 07:14:12 -0700 (In reply to comment #17) > Hmm, is this really a release blocker? > > AFAICS this is useful for incremental builds, in particular the bot. If you > guys agree I'd like to remove the blocking dependency. I don't think so if it should be a release blocker. Jocelyn? 214641 20 ossy 2010-04-20 07:16:08 -0700 (In reply to comment #18) > This seems to fix the build error, thank you. Cool. So we can close this bug after you decided if it is a release blocker bug or not. 214720 21 jturcotte 2010-04-20 10:06:35 -0700 (In reply to comment #19) > (In reply to comment #17) > > Hmm, is this really a release blocker? > > > > AFAICS this is useful for incremental builds, in particular the bot. If you > > guys agree I'd like to remove the blocking dependency. > > I don't think so if it should be a release blocker. Jocelyn? People would probably wouldn't mind since they will probably use release tarballs and do a clean rebuild anyway. If we don't get that patch in the release branch, the bot that tests qtwebkit-2.0 won't test JavaScriptCore's changes until a following commit touch one of WebCore's cpp file. If you didn't see this happen yet I guess we can leave it out, else I guess its up to you. 214782 22 hausmann 2010-04-20 11:11:39 -0700 (In reply to comment #21) > If we don't get that patch in the release branch, the bot that tests > qtwebkit-2.0 won't test JavaScriptCore's changes until a following commit touch > one of WebCore's cpp file. If you didn't see this happen yet I guess we can > leave it out, else I guess its up to you. Very convincing point :). Ok, kept as blocker. 225864 23 hausmann 2010-05-14 09:06:55 -0700 P2, not a strong blocker. 225872 24 ossy 2010-05-14 09:14:02 -0700 (In reply to comment #23) > P2, not a strong blocker. (It was reopened based on comment of Ismail "cartman" Donmez.) The patch is landed a long time ago. Is there any reason why this bug is still open? I think we can close it with resolved/fixed. 226445 25 hausmann 2010-05-16 01:05:53 -0700 <cherry-pick-for-backport: r57882> 226446 26 hausmann 2010-05-16 01:06:11 -0700 <cherry-pick-for-backport: r57885> 226447 27 hausmann 2010-05-16 01:10:13 -0700 Revision r57882 cherry-picked into qtwebkit-2.0 with commit 99054d09cb5df235575f572848e109bfa8dc819d Revision r57885 cherry-picked into qtwebkit-2.0 with commit cacbdf18fc917122834042d77a9164a490aafde4 53521 2010-04-16 00:47:52 -0700 2010-04-16 05:35:22 -0700 proposed fix jsc.patch text/plain 1805 ossy ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCAxMjMyN2U0Li4zNDFlZWY0IDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTIg QEAKKzIwMTAtMDQtMTYgIENzYWJhIE9zenRyb2dvbsOhYyAgPG9zc3lAd2Via2l0Lm9yZz4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbUXRdIFRhcmdl dChXZWJDb3JlLGpzYywuLi4pIG11c3QgZGVwZW5kcyBvbiBzdGF0aWMgbGlicmFyeSBvZiBKYXZh U2NyaXB0Q29yZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9Mzc3MDEKKworICAgICAgICAqIEphdmFTY3JpcHRDb3JlLnByaTogZGVwZW5kZW5jeSBhZGRl ZC4KKwogMjAxMC0wNC0xNSAgSmVkcnplaiBOb3dhY2tpICA8amVkcnplai5ub3dhY2tpQG5va2lh LmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBLZW5uZXRoIFJvaGRlIENocmlzdGlhbnNlbi4K ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0phdmFTY3JpcHRDb3JlLnByaSBiL0phdmFTY3Jp cHRDb3JlL0phdmFTY3JpcHRDb3JlLnByaQppbmRleCBjNTVjNDhiLi41YzZmMjc0IDEwMDY0NAot LS0gYS9KYXZhU2NyaXB0Q29yZS9KYXZhU2NyaXB0Q29yZS5wcmkKKysrIGIvSmF2YVNjcmlwdENv cmUvSmF2YVNjcmlwdENvcmUucHJpCkBAIC02NywxMyArNjcsMTYgQEAgZGVmaW5lVGVzdChhZGRK YXZhU2NyaXB0Q29yZUxpYikgewogICAgIHdpbjMyLW1zdmMqIHsKICAgICAgICAgTElCUyArPSAt TCQkcGF0aFRvSmF2YVNjcmlwdENvcmVPdXRwdXQKICAgICAgICAgTElCUyArPSAtbCQkSkFWQVND UklQVENPUkVfVEFSR0VUCisgICAgICAgIFBPU1RfVEFSR0VUREVQUyArPSAkJHtwYXRoVG9KYXZh U2NyaXB0Q29yZU91dHB1dH0kJHtRTUFLRV9ESVJfU0VQfWxpYiQke0pBVkFTQ1JJUFRDT1JFX1RB UkdFVH0uYQogICAgIH0gZWxzZTpzeW1iaWFuIHsKICAgICAgICAgTElCUyArPSAtbCQke0pBVkFT Q1JJUFRDT1JFX1RBUkdFVH0ubGliCisgICAgICAgIFBPU1RfVEFSR0VUREVQUyArPSAkJHtwYXRo VG9KYXZhU2NyaXB0Q29yZU91dHB1dH0kJHtRTUFLRV9ESVJfU0VQfSQke0pBVkFTQ1JJUFRDT1JF X1RBUkdFVH0ubGliCiAgICAgfSBlbHNlIHsKICAgICAgICAgIyBNYWtlIHN1cmUganNjb3JlIHdp bGwgYmUgZWFybHkgaW4gdGhlIGxpc3Qgb2YgbGlicmFyaWVzIHRvIHdvcmthcm91bmQgYSBidWcg aW4gTWluR1cKICAgICAgICAgIyB0aGF0IGNhbid0IHJlc29sdmUgc3ltYm9scyBmcm9tIFF0Q29y ZSBpZiBsaWJqc2NvcmUgY29tZXMgYWZ0ZXIuCiAgICAgICAgIFFNQUtFX0xJQkRJUiA9ICQkcGF0 aFRvSmF2YVNjcmlwdENvcmVPdXRwdXQgJCRRTUFLRV9MSUJESVIKICAgICAgICAgTElCUyArPSAt bCQkSkFWQVNDUklQVENPUkVfVEFSR0VUCisgICAgICAgIFBPU1RfVEFSR0VUREVQUyArPSAkJHtw YXRoVG9KYXZhU2NyaXB0Q29yZU91dHB1dH0kJHtRTUFLRV9ESVJfU0VQfWxpYiQke0pBVkFTQ1JJ UFRDT1JFX1RBUkdFVH0uYQogICAgIH0KIAogICAgIHdpbjMyLSogewpAQCAtODcsNiArOTAsNyBA QCBkZWZpbmVUZXN0KGFkZEphdmFTY3JpcHRDb3JlTGliKSB7CiAKICAgICBleHBvcnQoUU1BS0Vf TElCRElSKQogICAgIGV4cG9ydChMSUJTKQorICAgIGV4cG9ydChQT1NUX1RBUkdFVERFUFMpCiAg ICAgZXhwb3J0KENPTkZJRykKIAogICAgIHJldHVybih0cnVlKQo= 53527 2010-04-16 05:35:22 -0700 2010-04-20 05:50:06 -0700 proposed fix 1.patch text/plain 1804 ossy ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCAxMjMyN2U0Li4zNDFlZWY0IDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTIg QEAKKzIwMTAtMDQtMTYgIENzYWJhIE9zenRyb2dvbsOhYyAgPG9zc3lAd2Via2l0Lm9yZz4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbUXRdIFRhcmdl dChXZWJDb3JlLGpzYywuLi4pIG11c3QgZGVwZW5kcyBvbiBzdGF0aWMgbGlicmFyeSBvZiBKYXZh U2NyaXB0Q29yZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9Mzc3MDEKKworICAgICAgICAqIEphdmFTY3JpcHRDb3JlLnByaTogZGVwZW5kZW5jeSBhZGRl ZC4KKwogMjAxMC0wNC0xNSAgSmVkcnplaiBOb3dhY2tpICA8amVkcnplai5ub3dhY2tpQG5va2lh LmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBLZW5uZXRoIFJvaGRlIENocmlzdGlhbnNlbi4K ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0phdmFTY3JpcHRDb3JlLnByaSBiL0phdmFTY3Jp cHRDb3JlL0phdmFTY3JpcHRDb3JlLnByaQppbmRleCBjNTVjNDhiLi41YzZmMjc0IDEwMDY0NAot LS0gYS9KYXZhU2NyaXB0Q29yZS9KYXZhU2NyaXB0Q29yZS5wcmkKKysrIGIvSmF2YVNjcmlwdENv cmUvSmF2YVNjcmlwdENvcmUucHJpCkBAIC02NywxMyArNjcsMTYgQEAgZGVmaW5lVGVzdChhZGRK YXZhU2NyaXB0Q29yZUxpYikgewogICAgIHdpbjMyLW1zdmMqIHsKICAgICAgICAgTElCUyArPSAt TCQkcGF0aFRvSmF2YVNjcmlwdENvcmVPdXRwdXQKICAgICAgICAgTElCUyArPSAtbCQkSkFWQVND UklQVENPUkVfVEFSR0VUCisgICAgICAgIFBPU1RfVEFSR0VUREVQUyArPSAkJHtwYXRoVG9KYXZh U2NyaXB0Q29yZU91dHB1dH0kJHtRTUFLRV9ESVJfU0VQfSQke0pBVkFTQ1JJUFRDT1JFX1RBUkdF VH0ubGliCiAgICAgfSBlbHNlOnN5bWJpYW4gewogICAgICAgICBMSUJTICs9IC1sJCR7SkFWQVND UklQVENPUkVfVEFSR0VUfS5saWIKKyAgICAgICAgUE9TVF9UQVJHRVRERVBTICs9ICQke3BhdGhU b0phdmFTY3JpcHRDb3JlT3V0cHV0fSQke1FNQUtFX0RJUl9TRVB9JCR7SkFWQVNDUklQVENPUkVf VEFSR0VUfS5saWIKICAgICB9IGVsc2UgewogICAgICAgICAjIE1ha2Ugc3VyZSBqc2NvcmUgd2ls bCBiZSBlYXJseSBpbiB0aGUgbGlzdCBvZiBsaWJyYXJpZXMgdG8gd29ya2Fyb3VuZCBhIGJ1ZyBp biBNaW5HVwogICAgICAgICAjIHRoYXQgY2FuJ3QgcmVzb2x2ZSBzeW1ib2xzIGZyb20gUXRDb3Jl IGlmIGxpYmpzY29yZSBjb21lcyBhZnRlci4KICAgICAgICAgUU1BS0VfTElCRElSID0gJCRwYXRo VG9KYXZhU2NyaXB0Q29yZU91dHB1dCAkJFFNQUtFX0xJQkRJUgogICAgICAgICBMSUJTICs9IC1s JCRKQVZBU0NSSVBUQ09SRV9UQVJHRVQKKyAgICAgICAgUE9TVF9UQVJHRVRERVBTICs9ICQke3Bh dGhUb0phdmFTY3JpcHRDb3JlT3V0cHV0fSQke1FNQUtFX0RJUl9TRVB9bGliJCR7SkFWQVNDUklQ VENPUkVfVEFSR0VUfS5hCiAgICAgfQogCiAgICAgd2luMzItKiB7CkBAIC04Nyw2ICs5MCw3IEBA IGRlZmluZVRlc3QoYWRkSmF2YVNjcmlwdENvcmVMaWIpIHsKIAogICAgIGV4cG9ydChRTUFLRV9M SUJESVIpCiAgICAgZXhwb3J0KExJQlMpCisgICAgZXhwb3J0KFBPU1RfVEFSR0VUREVQUykKICAg ICBleHBvcnQoQ09ORklHKQogCiAgICAgcmV0dXJuKHRydWUpCg==