Bug 116114 - [Windows] Identify proper run environment for scripts
Summary: [Windows] Identify proper run environment for scripts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-14 12:08 PDT by Brent Fulgham
Modified: 2013-05-14 17:15 PDT (History)
2 users (show)

See Also:


Attachments
Patch (3.74 KB, patch)
2013-05-14 12:11 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (4.01 KB, patch)
2013-05-14 13:19 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (4.00 KB, patch)
2013-05-14 13:21 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (4.30 KB, patch)
2013-05-14 13:31 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (4.31 KB, patch)
2013-05-14 13:32 PDT, Brent Fulgham
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2013-05-14 12:08:35 PDT
With the VS2010 (and beyond) builds we are targeting x86 and x64 build targets. Consequently, we need a way for our scripts to know if they should run using the 'bin' directory (for 64-bit stuff), or the new 'bin32' directory (for 32-bit stuff).

This patch updated webkitdirs.pm with new logic to identify the specific build being used, and which run path to use based on the Visual Studio revision.

We do not currently build the 64-bit version, so we hard-code the 32-bit path for now.  A future update will identify and run the 64-bit variation.
Comment 1 Brent Fulgham 2013-05-14 12:11:51 PDT
Created attachment 201739 [details]
Patch
Comment 2 David Kilzer (:ddkilzer) 2013-05-14 13:10:29 PDT
Comment on attachment 201739 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=201739&action=review

r=me, but consider extracting the duplicate code into a method.

> Tools/Scripts/webkitdirs.pm:1627
>      my $programFilesPath = $ENV{'PROGRAMFILES(X86)'} || $ENV{'PROGRAMFILES'} || "C:\\Program Files";

Since this code is now duplicated in determineVisualStudioInstallDir(), should we put this in a function?  I'd feel a little better if we did.
Comment 3 Brent Fulgham 2013-05-14 13:19:54 PDT
Created attachment 201749 [details]
Patch
Comment 4 Brent Fulgham 2013-05-14 13:21:17 PDT
Created attachment 201750 [details]
Patch
Comment 5 Brent Fulgham 2013-05-14 13:31:16 PDT
Created attachment 201753 [details]
Patch
Comment 6 Brent Fulgham 2013-05-14 13:32:41 PDT
Created attachment 201754 [details]
Patch
Comment 7 David Kilzer (:ddkilzer) 2013-05-14 16:03:24 PDT
Comment on attachment 201754 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=201754&action=review

r=me with added return statements.  Thanks!

> Tools/Scripts/webkitdirs.pm:421
> +    $programFilesPath = $ENV{'PROGRAMFILES(X86)'} || $ENV{'PROGRAMFILES'} || "C:\\Program Files";

We should probably have a 'return $programFilesPath;' after this line.

I know Perl will do the right thing here, but it's a little too non-obvious for people who aren't Perl experts.

> Tools/Scripts/webkitdirs.pm:434
> +    chomp($vsInstallDir = `cygpath "$vsInstallDir"`) if isCygwin();

Ditto for return statement here.  (Actually, will chomp() do the right thing here?)

> Tools/Scripts/webkitdirs.pm:443
> +    $vsVersion = ($installDir =~ /Microsoft Visual Studio ([0-9]+\.[0-9]*)/) ? $1 : "8";

Ditto for return statement here.
Comment 8 Brent Fulgham 2013-05-14 17:15:05 PDT
Committed r150092: <http://trac.webkit.org/changeset/150092>