Bug 116114

Summary: [Windows] Identify proper run environment for scripts
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dbates
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch ddkilzer: review+

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>