Bug 36048 - Detect if the Platform SDK is missing when building with Visual C++ Express Edition
: Detect if the Platform SDK is missing when building with Visual C++ Express E...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Tools / Tests
: 528+ (Nightly build)
: PC Windows XP
: P2 Normal
Assigned To: Daniel Bates
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-11 21:02 PST by Daniel Bates
Modified: 2010-03-23 14:47 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.87 KB, patch)
2010-03-12 07:29 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (2.62 KB, patch)
2010-03-20 10:16 PDT, Daniel Bates
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2010-03-11 21:02:39 PST
As mentioned on the Improving Life on Windows wiki page <http://trac.webkit.org/wiki/ImprovingLifeOnWindows>, we should detect when the Windows Platform SDK is missing and inform the user. This will also fix the first error listed in <http://trac.webkit.org/wiki/BuildingOnWindows#VisualCExpressEdition>.
Comment 1 Daniel Bates 2010-03-12 07:29:37 PST
Created attachment 50597 [details]
Patch
Comment 2 Eric Seidel 2010-03-15 15:22:44 PDT
Comment on attachment 50597 [details]
Patch

Yay!
Comment 3 WebKit Commit Bot 2010-03-16 00:08:12 PDT
Comment on attachment 50597 [details]
Patch

Clearing flags on attachment: 50597

Committed r56044: <http://trac.webkit.org/changeset/56044>
Comment 4 WebKit Commit Bot 2010-03-16 00:08:17 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Adam Roben (:aroben) 2010-03-16 08:37:22 PDT
Sweet!
Comment 6 Adam Barth 2010-03-16 23:50:22 PDT
Reverted r56044 for reason:

This patch broke Windows Debug (Tests)

Committed r56099: <http://trac.webkit.org/changeset/56099>
Comment 7 Daniel Bates 2010-03-17 07:52:45 PDT
Thanks for rolling this out. Will look into.

(In reply to comment #6)
> Reverted r56044 for reason:
> 
> This patch broke Windows Debug (Tests)
> 
> Committed r56099: <http://trac.webkit.org/changeset/56099>
Comment 8 Daniel Bates 2010-03-20 10:16:10 PDT
Created attachment 51221 [details]
Patch

Moved check for Platform SDK into subroutine buildVisualStudioProject from subroutine setupCygwinEnv so that we only check for its existence when we are are going to build. This resolves the issue observed on the Windows Debug (Test) bot where run-webkit-tests is passed the --root argument and thus does not build DRT (hence, there is no need to check for the Platform SDK).

Also, updated the criterion for determining whether the Platform SDK is installed by checking for the existence of its registry entry instead of a fixed install location.
Comment 9 Adam Roben (:aroben) 2010-03-23 14:37:00 PDT
Comment on attachment 51221 [details]
Patch

> +my $isBuildingWithVCExpress = 0;
>  
>  # Defined in VCSUtils.
>  sub exitStatus($);
> @@ -1026,6 +1027,7 @@ sub setupCygwinEnv()
>              print "*************************************************************\n";
>              die;
>          }
> +        $isBuildingWithVCExpress = 1;
>      }

The name of this variable makes it sound like it will only be set to 1 when a build is actually in progress. Maybe $willUseVCExpressWhenBuilding or something like that would be better?

> +sub dieIfWindowsPlatformSDKNotInstalled
> +{
> +    my $windowsPlatformSDKRegistryEntry = "/proc/registry/HKEY_LOCAL_MACHINE/SOFTWARE/Microsoft/MicrosoftSDK/InstalledSDKs/D2FF9F89-8AA2-4373-8A31-C838BF4DBBE1";
> +
> +    if (-e $windowsPlatformSDKRegistryEntry) {
> +        return;
> +    }

This can be written as:

return if -e $windowsPlatformSDKRegistryEntry;

r=me
Comment 10 Daniel Bates 2010-03-23 14:39:20 PDT
(In reply to comment #9)
> (From update of attachment 51221 [details])
> > +my $isBuildingWithVCExpress = 0;
> >  
> >  # Defined in VCSUtils.
> >  sub exitStatus($);
> > @@ -1026,6 +1027,7 @@ sub setupCygwinEnv()
> >              print "*************************************************************\n";
> >              die;
> >          }
> > +        $isBuildingWithVCExpress = 1;
> >      }
> 
> The name of this variable makes it sound like it will only be set to 1 when a
> build is actually in progress. Maybe $willUseVCExpressWhenBuilding or something
> like that would be better?

Will change before I land.

> 
> > +sub dieIfWindowsPlatformSDKNotInstalled
> > +{
> > +    my $windowsPlatformSDKRegistryEntry = "/proc/registry/HKEY_LOCAL_MACHINE/SOFTWARE/Microsoft/MicrosoftSDK/InstalledSDKs/D2FF9F89-8AA2-4373-8A31-C838BF4DBBE1";
> > +
> > +    if (-e $windowsPlatformSDKRegistryEntry) {
> > +        return;
> > +    }
> 
> This can be written as:
> 
> return if -e $windowsPlatformSDKRegistryEntry;

Will change before I land.
Comment 11 Daniel Bates 2010-03-23 14:47:49 PDT
Committed r56419: <http://trac.webkit.org/changeset/56419>