[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





--- Comment #8 from Tony Gentilcore <tonyg at chromium.org>  2010-05-19 16:58:17 PST ---
(In reply to comment #6)
> (From update of attachment 56499 [details])
> 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."

Done.

> 
> > +
> > +        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:
> 

The prepare-ChangeLog script puts the description before the bug URLs. The majority of ChangeLog entries seem to follow that pattern as well. Can you explain the rationale?

> > +
> > +        * 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?

Done.

> 
> 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.
> 

Done.

> > +
> > +    # 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";
> 

Done.

> 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.