32680 2009-12-17 15:30:06 -0800 [Qt] webkitdirs.pm function builtDylibPathForName returns wrong path to library 2009-12-22 23:55:24 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC Windows XP RESOLVED FIXED P2 Normal --- 32683 1 dbates dbates aroben bweinstein cjerdonek eric hausmann kenneth ossy webkit.review.bot zoltan oldest_to_newest 172970 0 dbates 2009-12-17 15:30:06 -0800 The function builtDylibPathForName returns a path with respect to the file named QtWebKit.dll. But this files does not exist. Instead, the file is named QtWebKit4.dll. 172978 1 45107 dbates 2009-12-17 16:06:33 -0800 Created attachment 45107 Patch 172980 2 webkit.review.bot 2009-12-17 16:09:25 -0800 style-queue ran check-webkit-style on attachment 45107 without any errors. 173067 3 zoltan 2009-12-18 00:27:01 -0800 I think we should put either a new if isQt() inside isWindows() or elsif with isQt() && isWindows() for better readability. Btw, Simon why do we use other lib name on Windows? 173086 4 hausmann 2009-12-18 02:49:28 -0800 (In reply to comment #3) > I think we should put either a new if isQt() inside isWindows() or elsif with > isQt() && isWindows() for better readability. > > Btw, Simon why do we use other lib name on Windows? I agree, this needs an isQt() somewhere :) It is a convention in Qt to add the major number to the name of the DLL, similar to how the major version on Linux for example is part of the name and respected by the dynamic linker. An identical major version number indicates binary compatibility towards newer versions. 173150 5 dbates 2009-12-18 07:43:15 -0800 Oops. Yeah, I forgot to add an if isQt(). Will update patch. (In reply to comment #4) > (In reply to comment #3) > > I think we should put either a new if isQt() inside isWindows() or elsif with > > isQt() && isWindows() for better readability. > > > > Btw, Simon why do we use other lib name on Windows? > > I agree, this needs an isQt() somewhere :) > > It is a convention in Qt to add the major number to the name of the DLL, > similar to how the major version on Linux for example is part of the name and > respected by the dynamic linker. An identical major version number indicates > binary compatibility towards newer versions. 173152 6 dbates 2009-12-18 07:54:39 -0800 Never mind, there is already an outer "if (isQt())", <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitdirs.pm?rev=51932#L528>. Hence, we add the QT_MAJOR_VERSION to the DLL name if we are using Qt and are on the Windows or Cygwin platform. So, Simon, Zoltan, are you asking to fold in this outer "if (isQt())" into the inner if, elsif, else block? (In reply to comment #5) > Oops. Yeah, I forgot to add an if isQt(). Will update patch. 173184 7 zoltan 2009-12-18 09:28:24 -0800 (In reply to comment #6) > Never mind, there is already an outer "if (isQt())", > <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitdirs.pm?rev=51932#L528>. > Hence, we add the QT_MAJOR_VERSION to the DLL name if we are using Qt and are > on the Windows or Cygwin platform. > > So, Simon, Zoltan, are you asking to fold in this outer "if (isQt())" into the > inner if, elsif, else block? You're right. Why there is a isCygwin(), normally you compile Qt with MinGW... Please remove isCygwin() and it will be okay in that block. 173195 8 45158 dbates 2009-12-18 09:37:22 -0800 Created attachment 45158 Patch Updated patch based on Zoltan's comment. 173198 9 webkit.review.bot 2009-12-18 09:39:46 -0800 style-queue ran check-webkit-style on attachment 45158 without any errors. 173578 10 45158 eric 2009-12-19 12:26:42 -0800 Comment on attachment 45158 Patch I'm confused. Why wouldn't this break other windows ports? 173579 11 dbates 2009-12-19 12:33:25 -0800 We only do this for the Qt Windows port. Notice, the outer "if isQt()" on line 528 <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitdirs.pm?rev=51932#L528>. Hence, this will not interfere with the other Windows ports. (In reply to comment #10) > (From update of attachment 45158 [details]) > I'm confused. Why wouldn't this break other windows ports? 173884 12 45158 eric 2009-12-20 23:06:52 -0800 Comment on attachment 45158 Patch I'm going to trust you that this is sane. I know next to nothing about the Qt build, but I expect that you do given your recent job. If this is wrong for Qt we can always roll it out. rs=me 174540 13 dbates 2009-12-22 23:55:24 -0800 Committed in <http://trac.webkit.org/changeset/52522>. 45107 2009-12-17 16:06:33 -0800 2009-12-18 09:37:22 -0800 Patch Bug32680.patch text/plain 1807 dbates SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDUyMjkyKQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMDktMTItMTcgIERhbmllbCBCYXRlcyAgPGRi YXRlc0B3ZWJraXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMjY4MAor CisgICAgICAgIEZpeGVzIGFuIGlzc3VlIGluIGJ1aWx0RHlsaWJQYXRoRm9yTmFtZSBzbyB0aGF0 IGl0IHJldHVybnMgdGhlIGNvcnJlY3QKKyAgICAgICAgcGF0aCB0byB0aGUgYnVpbHQgUXRXZWJL aXQgbGlicmFyeSBvbiB0aGUgUXQgV2luZG93cyBwb3J0LiBDdXJyZW50bHksCisgICAgICAgIGl0 IHJldHVybnMgdGhlIHBhdGggdG8gdGhlIGZpbGUgbmFtZWQgUXRXZWJLaXQuZGxsIGJ1dCB0aGlz IGZpbGUgZG9lcworICAgICAgICBub3QgZXhpc3Qgb24gdGhlIFF0IFdpbmRvd3MgYnVpbGQuIElu c3RlYWQsIHRoZSBmaWxlIGlzIG5hbWVkCisgICAgICAgIFF0V2ViS2l0NC5kbGwuCisKKyAgICAg ICAgKiBTY3JpcHRzL3dlYmtpdGRpcnMucG06CisKIDIwMDktMTItMTcgIEFsZWphbmRybyBHLiBD YXN0cm8gIDxhbGV4QGlnYWxpYS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRXJpYyBTZWlk ZWwuCkluZGV4OiBXZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdGRpcnMucG0KPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRkaXJzLnBtCShyZXZpc2lvbiA1MjI1NCkKKysr IFdlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0ZGlycy5wbQkod29ya2luZyBjb3B5KQpAQCAtNTMw LDcgKzUzMCwxMiBAQCBzdWIgYnVpbHREeWxpYlBhdGhGb3JOYW1lCiAgICAgICAgIGlmIChpc0Rh cndpbigpIGFuZCAtZCAiJGNvbmZpZ3VyYXRpb25Qcm9kdWN0RGlyL2xpYi8kbGlicmFyeU5hbWUu ZnJhbWV3b3JrIikgewogICAgICAgICAgICAgcmV0dXJuICIkY29uZmlndXJhdGlvblByb2R1Y3RE aXIvbGliLyRsaWJyYXJ5TmFtZS5mcmFtZXdvcmsvJGxpYnJhcnlOYW1lIjsKICAgICAgICAgfSBl bHNpZiAoaXNXaW5kb3dzKCkgb3IgaXNDeWd3aW4oKSkgewotICAgICAgICAgICAgcmV0dXJuICIk Y29uZmlndXJhdGlvblByb2R1Y3REaXIvbGliLyRsaWJyYXJ5TmFtZS5kbGwiOworICAgICAgICAg ICAgY2hvbXAobXkgJG1rc3BlYyA9IGBxbWFrZSAtcXVlcnkgUU1BS0VfTUtTUEVDU2ApOworICAg ICAgICAgICAgbXkgJHF0TWFqb3JWZXJzaW9uID0gcmV0cmlldmVRTWFrZXNwZWNWYXIoIiRta3Nw ZWMvcWNvbmZpZy5wcmkiLCAiUVRfTUFKT1JfVkVSU0lPTiIpOworICAgICAgICAgICAgaWYgKCRx dE1ham9yVmVyc2lvbiBlcSAidW5rbm93biIpIHsKKyAgICAgICAgICAgICAgICAkcXRNYWpvclZl cnNpb24gPSAiIjsKKyAgICAgICAgICAgIH0KKyAgICAgICAgICAgIHJldHVybiAiJGNvbmZpZ3Vy YXRpb25Qcm9kdWN0RGlyL2xpYi8kbGlicmFyeU5hbWUkcXRNYWpvclZlcnNpb24uZGxsIjsKICAg ICAgICAgfSBlbHNlIHsKICAgICAgICAgICAgIHJldHVybiAiJGNvbmZpZ3VyYXRpb25Qcm9kdWN0 RGlyL2xpYi9saWIkbGlicmFyeU5hbWUuc28iOwogICAgICAgICB9Cg== 45158 2009-12-18 09:37:22 -0800 2009-12-20 23:06:51 -0800 Patch Bug32680_r2.patch text/plain 1873 dbates SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDUyMjkzKQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMDktMTItMTggIERhbmllbCBCYXRlcyAgPGRi YXRlc0ByaW0uY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMjY4MAorCisg ICAgICAgIEZpeGVzIGFuIGlzc3VlIGluIGJ1aWx0RHlsaWJQYXRoRm9yTmFtZSBzbyB0aGF0IGl0 IHJldHVybnMgdGhlIGNvcnJlY3QKKyAgICAgICAgcGF0aCB0byB0aGUgYnVpbHQgUXRXZWJLaXQg bGlicmFyeSBvbiB0aGUgUXQgV2luZG93cyBwb3J0LiBDdXJyZW50bHksCisgICAgICAgIGl0IHJl dHVybnMgdGhlIHBhdGggdG8gdGhlIGZpbGUgbmFtZWQgUXRXZWJLaXQuZGxsIGJ1dCB0aGlzIGZp bGUgZG9lcworICAgICAgICBub3QgZXhpc3Qgb24gdGhlIFF0IFdpbmRvd3MgYnVpbGQuIEluc3Rl YWQsIHRoZSBmaWxlIGlzIG5hbWVkCisgICAgICAgIFF0V2ViS2l0NC5kbGwuCisKKyAgICAgICAg KiBTY3JpcHRzL3dlYmtpdGRpcnMucG06CisKIDIwMDktMTItMTcgIEFsZWphbmRybyBHLiBDYXN0 cm8gIDxhbGV4QGlnYWxpYS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRXJpYyBTZWlkZWwu CkluZGV4OiBXZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdGRpcnMucG0KPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g V2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRkaXJzLnBtCShyZXZpc2lvbiA1MjI1NCkKKysrIFdl YktpdFRvb2xzL1NjcmlwdHMvd2Via2l0ZGlycy5wbQkod29ya2luZyBjb3B5KQpAQCAtNTI5LDgg KzUyOSwxMyBAQCBzdWIgYnVpbHREeWxpYlBhdGhGb3JOYW1lCiAgICAgICAgICRsaWJyYXJ5TmFt ZSA9ICJRdFdlYktpdCI7CiAgICAgICAgIGlmIChpc0RhcndpbigpIGFuZCAtZCAiJGNvbmZpZ3Vy YXRpb25Qcm9kdWN0RGlyL2xpYi8kbGlicmFyeU5hbWUuZnJhbWV3b3JrIikgewogICAgICAgICAg ICAgcmV0dXJuICIkY29uZmlndXJhdGlvblByb2R1Y3REaXIvbGliLyRsaWJyYXJ5TmFtZS5mcmFt ZXdvcmsvJGxpYnJhcnlOYW1lIjsKLSAgICAgICAgfSBlbHNpZiAoaXNXaW5kb3dzKCkgb3IgaXND eWd3aW4oKSkgewotICAgICAgICAgICAgcmV0dXJuICIkY29uZmlndXJhdGlvblByb2R1Y3REaXIv bGliLyRsaWJyYXJ5TmFtZS5kbGwiOworICAgICAgICB9IGVsc2lmIChpc1dpbmRvd3MoKSkgewor ICAgICAgICAgICAgY2hvbXAobXkgJG1rc3BlYyA9IGBxbWFrZSAtcXVlcnkgUU1BS0VfTUtTUEVD U2ApOworICAgICAgICAgICAgbXkgJHF0TWFqb3JWZXJzaW9uID0gcmV0cmlldmVRTWFrZXNwZWNW YXIoIiRta3NwZWMvcWNvbmZpZy5wcmkiLCAiUVRfTUFKT1JfVkVSU0lPTiIpOworICAgICAgICAg ICAgaWYgKCRxdE1ham9yVmVyc2lvbiBlcSAidW5rbm93biIpIHsKKyAgICAgICAgICAgICAgICAk cXRNYWpvclZlcnNpb24gPSAiIjsKKyAgICAgICAgICAgIH0KKyAgICAgICAgICAgIHJldHVybiAi JGNvbmZpZ3VyYXRpb25Qcm9kdWN0RGlyL2xpYi8kbGlicmFyeU5hbWUkcXRNYWpvclZlcnNpb24u ZGxsIjsKICAgICAgICAgfSBlbHNlIHsKICAgICAgICAgICAgIHJldHVybiAiJGNvbmZpZ3VyYXRp b25Qcm9kdWN0RGlyL2xpYi9saWIkbGlicmFyeU5hbWUuc28iOwogICAgICAgICB9Cg==