Separate Windows PSRP binary build out of Start-PSBuild #4335
Conversation
build.psm1
Outdated
| [string]$Arch = 'x64' | ||
| ) | ||
|
|
||
| if (-not $Environment.IsWindows) { return } |
There was a problem hiding this comment.
Would it be better to return error?
There was a problem hiding this comment.
Yes, I think a warning would be better.
build.psm1
Outdated
|
|
||
| # cmake is needed to build powershell.exe | ||
| if (-not (precheck 'cmake' $null)) { | ||
| throw 'cmake not found. Run "Start-PSBootstrap -Package". You can also install it from https://chocolatey.org/packages/cmake' |
There was a problem hiding this comment.
Why not introduce a new -BuildNativeWindows switch instead of reusing -Package? That way I can privately build a package w/o needing the SDK or building pwshplugin locally.
There was a problem hiding this comment.
We support building Appx package for Windows platform (New-AppxPackage) and it requires Win10 SDK, that's why I keep using -Package here. For other package formats, the windows native dependencies are not needed.
There was a problem hiding this comment.
Got it, but it doesn't appear we've ever published an appx?
There was a problem hiding this comment.
Yes, just talked to @TravisEz13. Appx is for NanoServer, and we never published one.
There was a problem hiding this comment.
I also I believe it never worked quite right.
| # Update googletest submodule for linux native cmake | ||
| if ($Environment.IsLinux -or $Environment.IsOSX) { | ||
| try { | ||
| # Update googletest submodule for linux native cmake |
There was a problem hiding this comment.
we should add a switch to bootstrap the components needed for native components so that we don't add them in systems where they are not needed.
There was a problem hiding this comment.
@TravisEz13 and I talked offline. The -package parameter is required only for creating Appx package, and before this change -package only takes effect on Unix platforms. If we don't care about Appx, for example in our release build, we can just run Start-PSBootstrap for windows.
If we can remove the script about Appx package, then we need to make the following subsequent changes:
- Add switch parameter
-BuildWindowsNativeto install dependencies for building PSRP on Windows. - Install
Wixwhen runningStart-PSPackage -Packageon Windows.
@SteveL-MSFT can you please confirm if we can remove New-AppxPackage? If so, I will take care of (1) in this PR and @TravisEz13 will take care of (2) in his packaging changes.
| if ($Environment.IsWindows -and $Package) { | ||
| $machinePath = [Environment]::GetEnvironmentVariable('Path', 'MACHINE') | ||
| $newMachineEnvironmentPath = $machinePath | ||
|
|
There was a problem hiding this comment.
for example line 1221 checks for win 10 sdk
There was a problem hiding this comment.
This is the biggest component that can reduce the docker image size.
There was a problem hiding this comment.
@joeyaiello are you ok if we remove Appx support? Seems like this is something we can bring back later.
|
I think it should be OK to remove |
|
@SteveL-MSFT could you please take another look? |
4 participants
Fix #3014
There are following major changes:
Start-PSBootstrap -BuildWindowsNativeinstalls the native dependencies required for building PSRP binary. Without-BuildWindowsNative, it only installs the dotnet-SDK on Windows platform.Start-PSBuilddoesn't build Windows PSRP binary. Instead,Start-BuildNativeWindowsBinariesis added to build it. After the build, 3 files ('pwrshplugin.dll','pwrshplugin.pdb','Install-PowerShellRemoting.ps1') will be bin-placed atsrc\powershell-win-corelike before.'psrp.windows'is added topowershell-corefeed, and we reference it inpowershell-win-core.csprojto get the Windows PSRP related files. The NuGet package is here, and the content of the package is as follows: (.dll and .pdb files are from the beta.4 release build)