[Webkit-unassigned] [Bug 39296] build-webkit doesn't recognize Platform SDK on win64

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=39296


Daniel Bates <dbates at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #56499|review?                     |review+
               Flag|                            |




--- Comment #6 from Daniel Bates <dbates at webkit.org>  2010-05-19 15:08:15 PST ---
(From update of attachment 56499)
Thanks for going back and creating a new patch. I have some minor nits:

> diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
> index 4f06670e4bcfa8824b6e9034642e578ba69ceb0a..f7afb31e53adb55d7c10e2d9549493a78f2c84a4 100644
> --- a/WebKitTools/ChangeLog
> +++ b/WebKitTools/ChangeLog
> @@ -1,3 +1,17 @@
> +2010-05-18  Tony Gentilcore  <tonyg at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Look in registry64 for Platform SDK.

I would change this to read "Look in /proc/registry64 for the Platform SDK on 64-bit Windows."

> +
> +        The build-webkit script failed for me on Vista 64. A web search turned
> +        up this blog post with a patch that worked for me:
> +        http://www.nicholaswilson.me.uk/2010/04/hacking-webkit-fail/
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=39296

I would move the bug URL line so that it appears after the "Reviewed by NOBODY (OOPS!)." line:

> +
> +        * Scripts/webkitdirs.pm:
> +
>  2010-05-08  Robert Hogan  <robert at roberthogan.net>
>  
>          Reviewed by Simon Hausmann.
> diff --git a/WebKitTools/Scripts/webkitdirs.pm b/WebKitTools/Scripts/webkitdirs.pm
> index c512009ecdecd2f9b6c8fc9e82665a2144b0614e..648bce19bdd8233007d97a5abc24e1d82440d03e 100644
> --- a/WebKitTools/Scripts/webkitdirs.pm
> +++ b/WebKitTools/Scripts/webkitdirs.pm
> @@ -1051,9 +1051,14 @@ sub setupCygwinEnv()
>  
>  sub dieIfWindowsPlatformSDKNotInstalled
>  {
> -    my $windowsPlatformSDKRegistryEntry = "/proc/registry/HKEY_LOCAL_MACHINE/SOFTWARE/Microsoft/MicrosoftSDK/InstalledSDKs/D2FF9F89-8AA2-4373-8A31-C838BF4DBBE1";
> -
> -    return if -e $windowsPlatformSDKRegistryEntry;
> +    my $registry32 = "/proc/registry";
> +    my $registry64 = "/proc/registry64";

The names of these variables seem a bit vague, maybe rename them to $registry32Path, $registry64Path?

I would append "/" to the end of both of these values then remove the "/" at the beginning of the value for $windowsPlatformSDKRegistryEntry so that $windowsPlatformSDKRegistryEntry better resembles a Windows registry path when we print it in the error message "Cannot find ...."

> +    my $windowsPlatformSDKRegistryEntry = "/HKEY_LOCAL_MACHINE/SOFTWARE/Microsoft/MicrosoftSDK/InstalledSDKs/D2FF9F89-8AA2-4373-8A31-C838BF4DBBE1";

As aforementioned, I would remove the leading "/" in the value for $windowsPlatformSDKRegistryEntry.

> +
> +    # FIXME It would be better to detect if this is a 64-bit Windows build
> +    # and only check the appropriate entry. But for now we just blindly check
> +    # both.
> +    return if (-e $registry32 . $windowsPlatformSDKRegistryEntry) || (-e $registry64 . $windowsPlatformSDKRegistryEntry);
>  
>      print "*************************************************************\n";
>      print "Cannot find '$windowsPlatformSDKRegistryEntry'.\n";

Since we are no longer using a fixed file system path, I would suggest changing the error message line to read:

print "Cannot find registry entry '$windowsPlatformSDKRegistryEntry'.\n";

r=me.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.