Summary: | [Windows] Update ORWT to know about the Windows 8 SDK and Debug Tools | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||
Component: | Tools / Tests | Assignee: | Brent Fulgham <bfulgham> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | benjamin, bfulgham, commit-queue, dbates, ddkilzer, roger_fong, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Brent Fulgham
2013-05-13 16:45:43 PDT
Created attachment 201650 [details]
Patch
Comment on attachment 201650 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201650&action=review > Tools/ChangeLog:24 > -2013-05-13 Zan Dobersek <zdobersek@igalia.com> > + Uh? > Tools/Scripts/old-run-webkit-tests:2846 > + my $ntsdPath = File::Spec->catfile(toCygwinPath($ENV{PROGRAMFILES}), "Windows Kits", "8.0", "Debuggers", "x64", "ntsd.exe"); > unless (-f $ntsdPath) { > - $ntsdPath = File::Spec->catfile(toCygwinPath($ENV{ProgramW6432}), "Debugging Tools for Windows (x64)", "ntsd.exe"); > + $ntsdPath = File::Spec->catfile(toCygwinPath($ENV{PROGRAMFILES}), "Windows Kits", "8.0", "Debuggers", "x86", "ntsd.exe"); > unless (-f $ntsdPath) { > - $ntsdPath = File::Spec->catfile(toCygwinPath($ENV{SYSTEMROOT}), "system32", "ntsd.exe"); > + $ntsdPath = File::Spec->catfile(toCygwinPath($ENV{PROGRAMFILES}), "Debugging Tools for Windows (x86)", "ntsd.exe"); > unless (-f $ntsdPath) { > - print STDERR "Can't find ntsd.exe. Crash logs will not be saved.\nSee <http://trac.webkit.org/wiki/BuildingOnWindows#GettingCrashLogs>.\n"; > - return; > + $ntsdPath = File::Spec->catfile(toCygwinPath($ENV{ProgramW6432}), "Debugging Tools for Windows (x64)", "ntsd.exe"); > + unless (-f $ntsdPath) { > + $ntsdPath = File::Spec->catfile(toCygwinPath($ENV{SYSTEMROOT}), "system32", "ntsd.exe"); > + unless (-f $ntsdPath) { > + print STDERR "Can't find ntsd.exe. Crash logs will not be saved.\nSee <http://trac.webkit.org/wiki/BuildingOnWindows#GettingCrashLogs>.\n"; > + return; > + } > + } > } > } > } Instead of this crazy nesting, I think we should have a loop going over a list of stuff we are looking for. > Tools/Scripts/webkitdirs.pm:1657 > +return; Uh? Created attachment 201729 [details]
Patch
Cleaned up the messy patch, and switched to a loop over the possible locations for ntsd.exe. Comment on attachment 201729 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201729&action=review r=me > Tools/Scripts/old-run-webkit-tests:2844 > + my $ntsdPath = shift @possiblePaths; > + > + while (not -f $ntsdPath) { > + if (!@possiblePaths) { > + print STDERR "Can't find ntsd.exe. Crash logs will not be saved.\nSee <http://trac.webkit.org/wiki/BuildingOnWindows#GettingCrashLogs>.\n"; > + return; > } > + > + $ntsdPath = shift @possiblePaths; I think this might be a little clearer written as a for() loop, but I'm also fine landing this version: my $ntsdPath; for ($ntsdPath = shift @possiblePaths; not -f $ntsdPath; $ntsdPath = shift @possiblePaths) { if (…) { …} } Comment on attachment 201729 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201729&action=review > Tools/Scripts/old-run-webkit-tests:2834 > + my @possiblePaths = (File::Spec->catfile(toCygwinPath($ENV{PROGRAMFILES}), "Windows Kits", "8.0", "Debuggers", "x64", "ntsd.exe"), > + File::Spec->catfile(toCygwinPath($ENV{PROGRAMFILES}), "Windows Kits", "8.0", "Debuggers", "x86", "ntsd.exe"), > + File::Spec->catfile(toCygwinPath($ENV{PROGRAMFILES}), "Debugging Tools for Windows (x86)", "ntsd.exe"), > + File::Spec->catfile(toCygwinPath($ENV{ProgramW6432}), "Debugging Tools for Windows (x64)", "ntsd.exe"), > + File::Spec->catfile(toCygwinPath($ENV{SYSTEMROOT}), "system32", "ntsd.exe")); May be slightly easier to read if each File::Spec->catfile() is on its own line: my @possiblePaths = ( File::Spec->catfile(…), File::Spec->catfile(…), File::Spec->catfile(…), File::Spec->catfile(…), File::Spec->catfile(…), ); But again, it's fine this way as well. Committed r150077: <http://trac.webkit.org/changeset/150077> |