Skip to content

Allow build script to detect more recent Windows 10 SDK versions #7217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
PetSerAl wants to merge 1 commit into from
Closed

Allow build script to detect more recent Windows 10 SDK versions #7217

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion build.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ function Test-Win10SDK {
#
# A slightly more robust check is for the mc.exe binary within that directory.
# It is only present if the SDK is installed.
return (Test-Path "${env:ProgramFiles(x86)}\Windows Kits\10\bin\x64\mc.exe")
return (Test-Path "${env:ProgramFiles(x86)}\Windows Kits\10\bin\x64\mc.exe") -or
(Test-Path "${env:ProgramFiles(x86)}\Windows Kits\10\bin\10.0.*.0\x64\mc.exe")
Copy link
Collaborator

@iSazonov iSazonov Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see 5 folder on my computer and I don't know path default...
Why we test full path here and use env path search in line 236?
Maybe test Get-Command -Application mc.exe?

Copy link
Contributor Author

@PetSerAl PetSerAl Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we test full path here and use env path search in line 236?

Line 236? You mean this?:

PowerShell/build.psm1

Line 236 in c43ae6a

Remove-Item $HOME\source\cmakecache.txt -ErrorAction SilentlyContinue

Maybe test Get-Command -Application mc.exe?

mc.exe is not in PATH by default. It is vcvarsall.bat responsibility to locate installed SDK and add it to path:

PowerShell/build.psm1

Line 265 in c43ae6a

cmd.exe /C cd /d "$currentLocation" "&" "$vcvarsallbatPath" "$Arch" "&" mc.exe -o -d -c -U "$($_.FullName)" -h "$currentLocation\$nativeResourcesFolder" -r "$currentLocation\$nativeResourcesFolder"

Copy link
Collaborator

@iSazonov iSazonov Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry the line I meant
https://github.com/PetSerAl/PowerShell/blob/8bee005699178008ca78efa306b4d196c53c0a54/build.psm1#L266

On my computer the exe is in PATH

where.exe mc.exe
C:\Program Files (x86)\Microsoft SDKs\Windows\v7.1A\Bin\MC.Exe

Perhaps new installer doesn't add this in PATH.

Copy link
Contributor Author

@PetSerAl PetSerAl Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it is not in PATH, unless you call vcvarsall.bat in initialize developer environment:

C:\>where.exe mc.exe
INFO: Could not find files for the given pattern(s).

C:\>"\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\VC\Auxiliary\Build\vcvarsall.bat" x64
**********************************************************************
** Visual Studio 2017 Developer Command Prompt v15.0
** Copyright (c) 2017 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'

C:\>where.exe mc.exe
C:\Program Files (x86)\Windows Kits\10\bin\10.0.17134.0\x64\mc.exe

Copy link
Collaborator

@iSazonov iSazonov Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Therefore, we can not test the existence of mc.exe before launching vcvarsall.bat, is it?

Copy link
Contributor Author

@PetSerAl PetSerAl Jul 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, yes. But I am not the one who implement original check, so I can not state exact reasons, why it was implemented this way.

}

function Start-BuildNativeWindowsBinaries {
Expand Down