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
Brent Fulgham
2013-05-14 12:08:35 PDT
Created attachment 201739 [details]
Patch
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. Created attachment 201749 [details]
Patch
Created attachment 201750 [details]
Patch
Created attachment 201753 [details]
Patch
Created attachment 201754 [details]
Patch
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. Committed r150092: <http://trac.webkit.org/changeset/150092> |