Bug 32682 – [Qt] webkitdirs.pm function checkFrameworks always die()s with an error under Windows

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Daniel Bates]

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.

[Daniel Bates]

Created attachment 45108 [details]
Patch

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45108 [details] without any errors.

[Eric Seidel (no email)]

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?

[Daniel Bates]

(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 (*).

[Eric Seidel (no email)]

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.

[Daniel Bates]

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.

[Daniel Bates]

Committed in <http://trac.webkit.org/changeset/52562>.