32682 2009-12-17 15:51:57 -0800 [Qt] webkitdirs.pm function checkFrameworks always die()s with an error under Windows 2009-12-25 15:59:31 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC Windows XP RESOLVED FIXED P2 Normal --- 32683 1 dbates dbates eric kenneth mrowe webkit.review.bot zoltan oldest_to_newest 172974 0 dbates 2009-12-17 15:51:57 -0800 The function checkFrameworks always die()s with an error under Windows since the clause "unless (-x $path)" is always satisfied because files under Windows do not have a explict executable bit. Moreover, the clause and current error message do not match. That is, the framework may exist but may not have its executable bit set. Hence, we should die with an error that explains that the framework was found but its executable bit was not set. 172979 1 45108 dbates 2009-12-17 16:07:32 -0800 Created attachment 45108 Patch 172981 2 webkit.review.bot 2009-12-17 16:09:43 -0800 style-queue ran check-webkit-style on attachment 45108 without any errors. 173882 3 eric 2009-12-20 23:04:15 -0800 I'm confused. How are scripts which use this function working today? Or is it only when using WindowsPerl instead of CygWinPerl that this fails? 174625 4 dbates 2009-12-23 12:14:11 -0800 (In reply to comment #3) > I'm confused. How are scripts which use this function working today? Or is it > only when using WindowsPerl instead of CygWinPerl that this fails? This issue arises when you are not using Cygwin, say you are using the Qt Command Prompt. (*)Testing with -x in Perl under Windows always returns false because Windows does not share the same concept of the executable bit as Unix/Linux. Moreover, we never reach line 573 of webkitdirs.pm <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitdirs.pm?rev=52522#L573> (whose unless condition would always fail and cause us to die() by (*)) when we are using Cygwin because of line 568 <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitdirs.pm?rev=52522#L568>, which causes us to return immediately. However, when you are not using Cygwin (say you're using the Qt Command Prompt), then we will reach line 573 and die() because of (*). 174797 5 45108 eric 2009-12-24 11:52:17 -0800 Comment on attachment 45108 Patch I think the code should have a comment about why we have this. At some level the separate -x check isn't even really necessary. The -e check is most important. If somehow someone has their permissions wrong, I'm not sure we need to catch that here. I'd just remove the -x check. Or if you decide to keep it, please add a comment about why it's separate from the -e check. 174801 6 dbates 2009-12-24 12:03:14 -0800 I'll remove the -x check when I land this. (In reply to comment #5) > (From update of attachment 45108 [details]) > I think the code should have a comment about why we have this. At some level > the separate -x check isn't even really necessary. The -e check is most > important. If somehow someone has their permissions wrong, I'm not sure we > need to catch that here. > > I'd just remove the -x check. Or if you decide to keep it, please add a > comment about why it's separate from the -e check. 174907 7 dbates 2009-12-25 15:59:31 -0800 Committed in <http://trac.webkit.org/changeset/52562>. 45108 2009-12-17 16:07:32 -0800 2009-12-24 11:52:17 -0800 Patch Bug32682.patch text/plain 1394 dbates SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDUyMjkzKQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMDktMTItMTcgIERhbmllbCBCYXRlcyAgPGRi YXRlc0B3ZWJraXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMjY4Mgor CisgICAgICAgIEZpeGVzIGFuIGlzc3VlIHdoZXJlIGNoZWNrRnJhbWV3b3JrcyBhbHdheXMgZGll cyB3aXRoIGFuIGVycm9yIHVuZGVyIFdpbmRvd3MKKyAgICAgICAgc2luY2UgdGhlIGNsYXVzZSAi dW5sZXNzICgteCAkcGF0aCkiIGlzIGFsd2F5cyBzYXRpc2ZpZWQgYmVjYXVzZSBmaWxlcyB1bmRl cgorICAgICAgICBXaW5kb3dzIGRvIG5vdCBoYXZlIGFuIGV4cGxpY3QgZXhlY3V0YWJsZSBiaXQu CisKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtpdGRpcnMucG06CisKIDIwMDktMTItMTcgIEFsZWph bmRybyBHLiBDYXN0cm8gIDxhbGV4QGlnYWxpYS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkg RXJpYyBTZWlkZWwuCkluZGV4OiBXZWJLaXRUb29scy9TY3JpcHRzL3dlYmtpdGRpcnMucG0KPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gV2ViS2l0VG9vbHMvU2NyaXB0cy93ZWJraXRkaXJzLnBtCShyZXZpc2lvbiA1 MjI1NCkKKysrIFdlYktpdFRvb2xzL1NjcmlwdHMvd2Via2l0ZGlycy5wbQkod29ya2luZyBjb3B5 KQpAQCAtNTYzLDcgKzU2Myw4IEBAIHN1YiBjaGVja0ZyYW1ld29ya3MKICAgICBwdXNoKEBmcmFt ZXdvcmtzLCAiV2ViS2l0IikgaWYgaXNBcHBsZU1hY1dlYktpdCgpOwogICAgIGZvciBteSAkZnJh bWV3b3JrIChAZnJhbWV3b3JrcykgewogICAgICAgICBteSAkcGF0aCA9IGJ1aWx0RHlsaWJQYXRo Rm9yTmFtZSgkZnJhbWV3b3JrKTsKLSAgICAgICAgZGllICJDYW4ndCBmaW5kIGJ1aWx0IGZyYW1l d29yayBhdCBcIiRwYXRoXCIuXG4iIHVubGVzcyAteCAkcGF0aDsKKyAgICAgICAgZGllICJDYW4n dCBmaW5kIGJ1aWx0IGZyYW1ld29yayBhdCBcIiRwYXRoXCIuXG4iIHVubGVzcyAtZSAkcGF0aDsK KyAgICAgICAgZGllICJUaGUgZnJhbWV3b3JrIFwiJHBhdGhcIiBpcyBub3QgZXhlY3V0YWJsZS5c bllvdSBtdXN0IHNldCBpdHMgZXhlY3V0YWJsZSBiaXQuXG4iIHVubGVzcyAoaXNXaW5kb3dzKCkg fHwgLXggJHBhdGgpOwogICAgIH0KIH0KIAo=