diff --git a/azure-pipelines.yml b/azure-pipelines.yml index d375f17bce720..08aa4304783ab 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -385,33 +385,22 @@ stages: steps: - template: eng/pipelines/checkout-windows-task.yml - - task: PowerShell@2 + - powershell: eng/build.ps1 -configuration Release -prepareMachine -ci -restore -binaryLogName Restore.binlog displayName: Restore - inputs: - pwsh: true - filePath: eng/build.ps1 - arguments: -configuration Release -prepareMachine -ci -restore -binaryLogName Restore.binlog - - task: PowerShell@2 + - powershell: eng/build.ps1 -configuration Release -prepareMachine -ci -build -pack -publish -sign -binaryLogName Build.binlog displayName: Build - inputs: - pwsh: true - filePath: eng/build.ps1 - arguments: -configuration Release -prepareMachine -ci -build -pack -publish -sign -binaryLogName Build.binlog - script: $(Build.SourcesDirectory)\artifacts\bin\BuildBoss\Release\net472\BuildBoss.exe -r "$(Build.SourcesDirectory)/" -c Release -p Roslyn.sln displayName: Validate Build Artifacts - - pwsh: | - ./eng/validate-rules-missing-documentation.ps1 -ci + - powershell: eng/validate-rules-missing-documentation.ps1 -ci displayName: Validate rules missing documentation - - pwsh: | - ./eng/generate-compiler-code.ps1 -test -configuration Release + - powershell: eng/generate-compiler-code.ps1 -test -configuration Release displayName: Generate Syntax Files - - pwsh: | - ./eng/validate-code-formatting.ps1 -ci -rootDirectory $(Build.SourcesDirectory)\src -includeDirectories Compilers\CSharp\Portable\Generated\, Compilers\VisualBasic\Portable\Generated\, ExpressionEvaluator\VisualBasic\Source\ResultProvider\Generated\ + - powershell: eng/validate-code-formatting.ps1 -ci -rootDirectory $(Build.SourcesDirectory)\src -includeDirectories Compilers\CSharp\Portable\Generated\, Compilers\VisualBasic\Portable\Generated\, ExpressionEvaluator\VisualBasic\Source\ResultProvider\Generated\ displayName: Validate Generated Syntax Files condition: or(ne(variables['Build.Reason'], 'PullRequest'), eq(variables['compilerChange'], 'true')) @@ -434,10 +423,10 @@ stages: steps: - template: eng/pipelines/checkout-windows-task.yml - - pwsh: eng/make-bootstrap.ps1 -output $(bootstrapDir) + - powershell: eng/make-bootstrap.ps1 -output $(bootstrapDir) displayName: Build Bootstrap Compiler - - pwsh: eng/test-determinism.ps1 -configuration Debug -bootstrapDir $(bootstrapDir) + - powershell: eng/test-determinism.ps1 -configuration Debug -bootstrapDir $(bootstrapDir) -ci displayName: Build - Validate determinism - template: eng/pipelines/publish-logs.yml @@ -479,7 +468,7 @@ stages: steps: - template: eng/pipelines/checkout-unix-task.yml - - pwsh: eng/todo-check.ps1 + - powershell: eng/todo-check.ps1 displayName: Validate TODO/PROTOTYPE comments are not present - job: Correctness_Rebuild diff --git a/docs/contributing/Powershell Guidelines.md b/docs/contributing/Powershell Guidelines.md index db528cdad125d..9a1852178a291 100644 --- a/docs/contributing/Powershell Guidelines.md +++ b/docs/contributing/Powershell Guidelines.md @@ -1,123 +1,95 @@ -Powershell Guidelines -============== +# Powershell Guidelines -Powershell is primarily used in this repo as the backbone of our infrastructure. As such -Powershell scripts need to be portable, reliable and take advantage of stop on error +Powershell is primarily used in this repo as the backbone of our infrastructure. As such +Powershell scripts need to be portable, reliable and take advantage of stop on error approaches. The guidelines below are meant to push scripts to this mindset of execution. -# Coding Style +## Coding Style 1. Opening braces always go at the end of an expression / statement. 1. Closing braces always occur on an otherwise empty line. -1. We use four space indentation. +1. We use two space indentation. 1. Use Pascal casing for functions and where possible follow the Verb-Name convention. 1. Use Camel casing for all other identifier. -1. Use full command names instead of aliass. For example `Get-Content` vs. `gc`. Aliases can be -overridden by the environment and hence are not considered portable. +1. Use full command names instead of aliases. For example `Get-Content` vs. `gc`. Aliases can be +overridden by the environment and hence are not considered portable. -# General Guidelines +## General Guidelines -## Body +### Script Template -All scripts shall include the following two statements after parameter declarations: +All Powershell scripts that execute in CI need to conform to the following template. + +```powershell +[CmdletBinding(PositionalBinding=$false)] +param ( + [switch]$ci = $false, + [string]$configuration = "Debug") -``` powershell Set-StrictMode -version 2.0 $ErrorActionPreference="Stop" -``` -This both forces Powershell into a more strict mode of interepretation and swaps out the -"On Error Resume Next" approach for an "On Error Stop" model. Both of these help with our -goals of reliability as it makes errors hard stops (usless specifically stated otherwise) +try { + . (Join-Path $PSScriptRoot "build-utils.ps1") + $prepareMachine = $ci -The body of a Powershell script shall be wrapped inside the following try / catch template: + # Body of Powershell script goes here -``` powershell -try { - # Body of Powershell script goes here + ExitWithExitCode 0 } catch { - Write-Host $_ - Write-Host $_.Exception - exit 1 + Write-Host $_ + Write-Host $_.Exception + Write-Host $_.ScriptStackTrace + ExitWithExitCode 1 } ``` -This will force scripts to exit with an error code when an unhandled exception occurs. - -## Parameters - -The parameter block shall occur at the top of the script. Authors should consider disabling -positional binding: - -``` powershell -[CmdletBinding(PositionalBinding=$false)] -param ( - [switch]$test64 = $false, - [switch]$testDeterminism = $false) -``` - -This helps alert callers to casual typos, which would otherwise go unnoticed, by making it an -error. +The rationale for these parts are: -If the script is complicated enough that it contains a usage / help display, then the following -pattern can be used: +- `Set-StrictMode`: forces Powershell into a stricter mode of interpretation and +swaps out the "On Error Resume Next" approach for an "On Error Stop" model. Both of these +help with our goals of reliability as it makes errors hard stops (unless specifically stated otherwise) +- `$prepareMachine = $ci`: is necessary to ensure `ExitWithExitCode` properly exits all +build processes in CI environments (conforming to arcade guidelines). +- `ExitWithExitCode`: arcade standard for exiting a script that deals with process management +- `PositionalBinding=$false`: alerts callers when they have incorrect parameters invoking a +script -``` powershell -[CmdletBinding(PositionalBinding=$false)] -param ( - [switch]$build = $false, - [string]$msbuildDir = "", - [parameter(ValueFromRemainingArguments=$true)] $badArgs) - -try { - if ($badArgs -ne $null) { - Print-Usage - exit 1 - } -} +### Coding Guidelines -``` - -## Executing windows commands +Use `Exec-*` functions to execute programs or `dotnet` commands. This adds automatic +error detection on invocation failure, incorrect parameters, etc ... -Invoking windows commands should be done via the Exec function. This adds automatic error detection -to the invocation and removes the need for error prone if checking after every command. - -``` powershell +```powershell # DO NOT & msbuild /v:m /m Roslyn.sln -# DO -Exec-Block { & msbuild /v:m /m Roslyn.sln } -``` +& dotnet build Roslyn.sln -Note this will not work for the rare Windows commands which use 0 as an exit code on failure. For -example robocopy and corflags. +# DO +Exec-Command "msbuild" "/v:m /m Roslyn.sln" +Exec-DotNet "build Roslyn.sln" +``` -In some cases windows commands need to have their argument list built up dynamically. When that -happens do not use Invoke-Expression to execute the command, instead use Exec-Command. The former -does not fail when the windows command fails, can invoke powershell argument parsing and doesn't -have a mechanism for echoing output to console. The Exec-Command uses Process directly and can support -the major functionality needed. +Scripts that have many executions of `dotnet` commands can store the `dotnet` command in a variable +and use `Exec-Command` instead. +Call `Test-LastExitCode` after invoking a powershell script to make sure failure is not ignored. -``` powershell -$command = "C:\Program Files (x86)\Microsoft Visual Studio\Preview\Dogfood\MSBuild\15.0\Bin\MSBuild.exe" -$args = "/v:m Roslyn.sln" -if (...) { - $args += " /fl /flp:v=diag" -} +```powershell # DO NOT -Invoke-Expression "& $command $args" +& eng/make-bootstrap.ps1 +Write-Host "Done with Bootstrap" + # DO -Exec-Command $command $args +& eng/make-bootstrap.ps1 +Test-LastExitCode +Write-Host "Done with Bootstrap" ``` -## Comarisons with null Whenever comparing with `$null` always make sure to put `$null` on the left hand side of the operator. For non-collection types this doesn't really affect behavior. For collection types though -having a collection on the left hand side changes the meaning of `-ne` and `-eq`. Instead of checking -for `$null` it will instead compare collection contents. +having a collection on the left hand side changes the meaning of `-ne` and `-eq`. Instead of checking for `$null` it will instead compare collection contents. ``` powershell # DO NOT @@ -126,3 +98,11 @@ if ($e -ne $null) { ... } if ($null -ne $e) { ... } ``` +### Powershell vs. pwsh + +The Roslyn infrastructure should use `powershell` for execution not `pwsh`. The general .NET infra +still uses `Powershell` and calls into our scripts. Moving to `pwsh` in our scripts creates errors +in source and unified build. Until that moves to `pwsh` our scripts need to stay on `Powershell`. + +The exception is that our VS Code helper scripts should use `pwsh`. That is not a part of our +infrastructure and needs to run cross platform hence `pwsh` is appropriate. diff --git a/eng/build-utils.ps1 b/eng/build-utils.ps1 index 642ceadf7a8bc..64dd3a3d343f4 100644 --- a/eng/build-utils.ps1 +++ b/eng/build-utils.ps1 @@ -67,26 +67,6 @@ function GetReleasePublishData([string]$releaseName) { } } -# Handy function for executing a command in powershell and throwing if it -# fails. -# -# Use this when the full command is known at script authoring time and -# doesn't require any dynamic argument build up. Example: -# -# Exec-Block { & $msbuild Test.proj } -# -# Original sample came from: http://jameskovacs.com/2010/02/25/the-exec-problem/ -function Exec-Block([scriptblock]$cmd) { - & $cmd - - # Need to check both of these cases for errors as they represent different items - # - $?: did the powershell script block throw an error - # - $lastexitcode: did a windows command executed by the script block end in error - if ((-not $?) -or ($lastexitcode -ne 0)) { - throw "Command failed to execute: $cmd" - } -} - function Exec-CommandCore([string]$command, [string]$commandArgs, [switch]$useConsole = $true, [switch]$echoCommand = $true) { if ($echoCommand) { Write-Host "$command $commandArgs" @@ -162,19 +142,6 @@ function Exec-Command([string]$command, [string]$commandArgs, [switch]$useConsol Exec-CommandCore -command $command -commandArgs $commandArgs -useConsole:$useConsole -echoCommand:$echoCommand } -# Handy function for executing a powershell script in a clean environment with -# arguments. Prefer this over & sourcing a script as it will both use a clean -# environment and do proper error checking -# -# The -useConsole argument controls if the process should re-use the current -# console for output or return output as a string -function Exec-Script([string]$script, [string]$scriptArgs = "", [switch]$useConsole = $true, [switch]$echoCommand = $true) { - if ($args -ne "") { - throw "Extra arguments passed to Exec-Script: $args" - } - Exec-CommandCore -command "pwsh" -commandArgs "-noprofile -executionPolicy RemoteSigned -file `"$script`" $scriptArgs" -useConsole:$useConsole -echoCommand:$echoCommand -} - # Handy function for executing a dotnet command without having to track down the # proper dotnet executable or ensure it's on the path. function Exec-DotNet([string]$commandArgs = "", [switch]$useConsole = $true, [switch]$echoCommand = $true) { @@ -202,6 +169,12 @@ function Ensure-DotnetSdk() { throw "Could not find dotnet executable in $dotnetInstallDir" } +function Test-LastExitCode() { + if ($LASTEXITCODE -ne 0) { + throw "Last command failed with exit code $LASTEXITCODE" + } +} + # Walks up the source tree, starting at the given file's directory, and returns a FileInfo object for the first .csproj file it finds, if any. function Get-ProjectFile([object]$fileInfo) { Push-Location diff --git a/eng/build.ps1 b/eng/build.ps1 index f0da543aed6d8..d49c55719bcd5 100644 --- a/eng/build.ps1 +++ b/eng/build.ps1 @@ -752,7 +752,8 @@ try { if ($bootstrap -and $bootstrapDir -eq "") { Write-Host "Building bootstrap Compiler" $bootstrapDir = Join-Path (Join-Path $ArtifactsDir "bootstrap") "build" - Exec-Script (Join-Path $PSScriptRoot "make-bootstrap.ps1") "-output $bootstrapDir -force -ci:$ci" + & eng/make-bootstrap.ps1 -output $bootstrapDir -force -ci:$ci + Test-LastExitCode } if ($restore -or $build -or $rebuild -or $pack -or $sign -or $publish) { diff --git a/eng/generate-compiler-code.cmd b/eng/generate-compiler-code.cmd index 98108945d1e07..808137ef77cb7 100644 --- a/eng/generate-compiler-code.cmd +++ b/eng/generate-compiler-code.cmd @@ -1,3 +1,3 @@ @echo off -pwsh -noprofile -executionPolicy RemoteSigned -file "%~dp0\generate-compiler-code.ps1" %* +powershell -noprofile -executionPolicy RemoteSigned -file "%~dp0\generate-compiler-code.ps1" %* diff --git a/eng/make-bootstrap.cmd b/eng/make-bootstrap.cmd index d081540d85808..fe6cae0b6e9f9 100644 --- a/eng/make-bootstrap.cmd +++ b/eng/make-bootstrap.cmd @@ -1,2 +1,2 @@ @echo off -pwsh -noprofile -file "%~dp0\make-bootstrap.ps1" %* \ No newline at end of file +powershell -noprofile -file "%~dp0\make-bootstrap.ps1" %* \ No newline at end of file diff --git a/eng/pipelines/build-bootstrap.yml b/eng/pipelines/build-bootstrap.yml index 7b68ac40bbded..8a35f2ea446be 100644 --- a/eng/pipelines/build-bootstrap.yml +++ b/eng/pipelines/build-bootstrap.yml @@ -7,14 +7,11 @@ parameters: steps: - template: checkout-windows-task.yml - - pwsh: | - ./eng/make-bootstrap.ps1 -ci -toolset ${{parameters.toolset}} -output '$(Build.SourcesDirectory)/artifacts/bootstrap/ci-bootstrap' - + - powershell: eng/make-bootstrap.ps1 -ci -toolset ${{parameters.toolset}} -output '$(Build.SourcesDirectory)/artifacts/bootstrap/ci-bootstrap' displayName: Build Bootstrap Compiler - - pwsh: | - ./eng/test-build-correctness.ps1 -ci -configuration Release -enableDumps -bootstrapDir '$(Build.SourcesDirectory)/artifacts/bootstrap/ci-bootstrap' - displayName: Build - Validate correctness + - powershell: eng/test-build-correctness.ps1 -ci -configuration Release -enableDumps -bootstrapDir '$(Build.SourcesDirectory)/artifacts/bootstrap/ci-bootstrap' + displayName: Build - Validate Correctness - template: publish-logs.yml parameters: diff --git a/eng/test-build-correctness.cmd b/eng/test-build-correctness.cmd index 1a26bbd2d41ed..1c063890db1fe 100644 --- a/eng/test-build-correctness.cmd +++ b/eng/test-build-correctness.cmd @@ -1,2 +1,2 @@ @echo off -pwsh -noprofile -file "%~dp0\test-build-correctness.ps1" %* +powershell -noprofile -file "%~dp0\test-build-correctness.ps1" %* diff --git a/eng/test-build-correctness.ps1 b/eng/test-build-correctness.ps1 index ca0bd28d8c718..9c131c1d189c4 100644 --- a/eng/test-build-correctness.ps1 +++ b/eng/test-build-correctness.ps1 @@ -45,12 +45,14 @@ try { if ($bootstrapDir -eq "") { Write-Host "Building bootstrap compiler" - $bootstrapDir = Join-Path $ArtifactsDir "bootstrap" "correctness" - Exec-Script (Join-Path $PSScriptRoot "make-bootstrap.ps1") "-output $bootstrapDir -ci:$ci" + $bootstrapDir = Join-Path $ArtifactsDir (Join-Path "bootstrap" "correctness") + & eng/make-bootstrap.ps1 -output $bootstrapDir -ci:$ci + Test-LastExitCode } Write-Host "Building Roslyn" - Exec-Script (Join-Path $PSScriptRoot "build.ps1") "-restore -build -bootstrapDir:$bootstrapDir -ci:$true -prepareMachine:$true -runAnalyzers:$true -configuration:$configuration -pack -binaryLog -useGlobalNuGetCache:$false -warnAsError:$true -properties `"/p:RoslynEnforceCodeStyle=true`"" + & eng/build.ps1 -restore -build -bootstrapDir:$bootstrapDir -ci:$ci -prepareMachine:$prepareMachine -runAnalyzers:$true -configuration:$configuration -pack -binaryLog -useGlobalNuGetCache:$false -warnAsError:$true -properties:"/p:RoslynEnforceCodeStyle=true" + Test-LastExitCode Subst-TempDir @@ -62,7 +64,8 @@ try { # Verify the state of our generated syntax files Write-Host "Checking generated compiler files" - Exec-Script (Join-Path $PSScriptRoot "generate-compiler-code.ps1") "-test -configuration:$configuration" + & eng/generate-compiler-code.ps1 -test -configuration:$configuration + Test-LastExitCode Exec-DotNet "tool run dotnet-format whitespace . --folder --include-generated --include src/Compilers/CSharp/Portable/Generated/ src/Compilers/VisualBasic/Portable/Generated/ src/ExpressionEvaluator/VisualBasic/Source/ResultProvider/Generated/ --verify-no-changes" Write-Host "" diff --git a/eng/test-determinism.cmd b/eng/test-determinism.cmd index e090fc5a3edb6..4f3fccf35b03f 100644 --- a/eng/test-determinism.cmd +++ b/eng/test-determinism.cmd @@ -1,2 +1,2 @@ @echo off -pwsh -noprofile -file "%~dp0\test-determinism.ps1" %* +powershell -noprofile -file "%~dp0\test-determinism.ps1" %* diff --git a/eng/test-determinism.ps1 b/eng/test-determinism.ps1 index da25ef2586abf..57c965bfe5831 100644 --- a/eng/test-determinism.ps1 +++ b/eng/test-determinism.ps1 @@ -266,6 +266,8 @@ function Run-Test() { try { . (Join-Path $PSScriptRoot "build-utils.ps1") + Push-Location $RepoRoot + $prepareMachine = $ci # Create all of the logging directories $errorDir = Join-Path $LogDir "DeterminismFailures" @@ -276,7 +278,6 @@ try { Create-Directory $errorDirLeft Create-Directory $errorDirRight - $ci = $true $runAnalyzers = $false $binaryLog = $true $officialBuildId = "" @@ -286,21 +287,20 @@ try { if ($bootstrapDir -eq "") { Write-Host "Building bootstrap compiler" $bootstrapDir = Join-Path $ArtifactsDir "bootstrap" "determinism" - Exec-Script (Join-Path $PSScriptRoot "make-bootstrap.ps1") "-output $bootstrapDir -ci:$ci" + & eng/make-bootstrap.ps1 -output $bootstrapDir -ci:$ci + Test-LastExitCode } Run-Test - exit 0 + ExitWithExitCode 0 } catch { Write-Host $_ Write-Host $_.Exception Write-Host $_.ScriptStackTrace - exit 1 + ExitWithExitCode 1 } finally { - Write-Host "Stopping VBCSCompiler" - Get-Process VBCSCompiler -ErrorAction SilentlyContinue | Stop-Process - Write-Host "Stopped VBCSCompiler" + Pop-Location } diff --git a/eng/test-rebuild.cmd b/eng/test-rebuild.cmd index a1c7084cccd91..ae4bc34fcc827 100644 --- a/eng/test-rebuild.cmd +++ b/eng/test-rebuild.cmd @@ -1,2 +1,2 @@ @echo off -pwsh -noprofile -file "%~dp0\test-rebuild.ps1" %* +powershell -noprofile -file "%~dp0\test-rebuild.ps1" %* diff --git a/eng/test-rebuild.ps1 b/eng/test-rebuild.ps1 index 8e45c47cd943d..b6c52e0c349ec 100644 --- a/eng/test-rebuild.ps1 +++ b/eng/test-rebuild.ps1 @@ -34,7 +34,8 @@ try { if ($bootstrap) { Write-Host "Building Roslyn" - Exec-Script (Join-Path $PSScriptRoot "build.ps1") "-restore -build -bootstrap -prepareMachine:$prepareMachine -ci:$ci -useGlobalNuGetCache:$useGlobalNuGetCache -configuration:$configuration -pack -binaryLog" + & eng/build.ps1 -restore -build -bootstrap -prepareMachine:$prepareMachine -ci:$ci -useGlobalNuGetCache:$useGlobalNuGetCache -configuration:$configuration -pack -binaryLog + Test-LastExitCode } Subst-TempDir diff --git a/eng/validate-rules-missing-documentation.cmd b/eng/validate-rules-missing-documentation.cmd index f90d3a1a973e7..c67f4927e835f 100644 --- a/eng/validate-rules-missing-documentation.cmd +++ b/eng/validate-rules-missing-documentation.cmd @@ -1,2 +1,2 @@ @echo off -pwsh -noprofile -executionPolicy RemoteSigned -file "%~dp0\validate-rules-missing-documentation.ps1" %* +powershell -noprofile -executionPolicy RemoteSigned -file "%~dp0\validate-rules-missing-documentation.ps1" %*