Bug 45651 - jscPath() is incorrect in Windows' cmd.exe shell
Summary: jscPath() is incorrect in Windows' cmd.exe shell
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Windows XP
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-13 03:56 PDT by Csaba Osztrogonác
Modified: 2010-09-13 09:18 PDT (History)
3 users (show)

See Also:


Attachments
proposed fix (1.12 KB, patch)
2010-09-13 03:59 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
proposed fix (1.12 KB, patch)
2010-09-13 08:24 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2010-09-13 03:56:58 PDT
http://trac.webkit.org/changeset/63463 broke run-javascriptore-tests,
run-sunspider, ... scripts with QtWebKit on Windows, because
$productDir/$jscName isn't exist, but $productDir/$jscName.exe.

exe suffix is neccessary when you test file existance,
but unneccessary when you execute the binarty.

http://trac.webkit.org/changeset/63463:
-    return "$productDir/$jscName";
+    return "$productDir/$jscName" if -e "$productDir/$jscName";
+    return "$productDir/JavaScriptCore.framework/Resources/$jscName";
Comment 1 Csaba Osztrogonác 2010-09-13 03:59:46 PDT
Created attachment 67394 [details]
proposed fix
Comment 2 Andreas Kling 2010-09-13 05:39:57 PDT
Comment on attachment 67394 [details]
proposed fix

> +    $jscName .= ".exe"  if (isQt() && isWindows());

This doesn't strike me as a Qt-specific issue.
Comment 3 Csaba Osztrogonác 2010-09-13 06:38:46 PDT
(In reply to comment #2)
> (From update of attachment 67394 [details])
> > +    $jscName .= ".exe"  if (isQt() && isWindows());
> 
> This doesn't strike me as a Qt-specific issue.

It shouldn't be Qt specific issue, but I'm not sure.

On Apple Windows port run-javascriptcore-tests works correctly,
it seems the file existance check works without "exe" suffix
inside cygwin. I will check it ASAP.
Comment 4 Csaba Osztrogonác 2010-09-13 08:00:31 PDT
Comment on attachment 67394 [details]
proposed fix

new patch is coming soon
Comment 5 Csaba Osztrogonác 2010-09-13 08:24:59 PDT
Created attachment 67413 [details]
proposed fix

proposed fix:
+    $jscName .= ".exe" if (isWindows() || isCygwin());

The root of the problem was that ' if -e "executableBinary" ' is false 
with ActiveState perl on Windows in cmd.exe shell, but true in CygWin 
shell if executableBinary.exe exist, but executableBinary doesn't.

It is a misleading perl feature what we should avoid, so I 
propose to add ".exe" suffix for Windows and CygWin platform too.
Comment 6 Andreas Kling 2010-09-13 09:00:40 PDT
Comment on attachment 67413 [details]
proposed fix

Nice detective work :)
r=me
Comment 7 Csaba Osztrogonác 2010-09-13 09:18:03 PDT
Comment on attachment 67413 [details]
proposed fix

Clearing flags on attachment: 67413

Committed r67391: <http://trac.webkit.org/changeset/67391>
Comment 8 Csaba Osztrogonác 2010-09-13 09:18:12 PDT
All reviewed patches have been landed.  Closing bug.