Bug 32682 - [Qt] webkitdirs.pm function checkFrameworks always die()s with an error under Windows
Summary: [Qt] webkitdirs.pm function checkFrameworks always die()s with an error under...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on:
Blocks: 32683
  Show dependency treegraph
 
Reported: 2009-12-17 15:51 PST by Daniel Bates
Modified: 2009-12-25 15:59 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.36 KB, patch)
2009-12-17 16:07 PST, Daniel Bates
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2009-12-17 15:51:57 PST
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.
Comment 1 Daniel Bates 2009-12-17 16:07:32 PST
Created attachment 45108 [details]
Patch
Comment 2 WebKit Review Bot 2009-12-17 16:09:43 PST
style-queue ran check-webkit-style on attachment 45108 [details] without any errors.
Comment 3 Eric Seidel (no email) 2009-12-20 23:04:15 PST
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?
Comment 4 Daniel Bates 2009-12-23 12:14:11 PST
(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 (*).
Comment 5 Eric Seidel (no email) 2009-12-24 11:52:17 PST
Comment on attachment 45108 [details]
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.
Comment 6 Daniel Bates 2009-12-24 12:03:14 PST
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.
Comment 7 Daniel Bates 2009-12-25 15:59:31 PST
Committed in <http://trac.webkit.org/changeset/52562>.