From 76ee5f5ab890deda8746e49b9c0a0cad731af8c2 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 7 May 2020 05:56:53 -0700 Subject: [PATCH 1/4] Skip executing analyzers by default on build This change skips executing analyzers by default on explicit builds (both inside VS and on command line prompt). This should speed up the overall build time of Roslyn.sln when building locally. The only time a developer would want analyzers to run locally outside the typing scenario is to compute the exact set of warnings for a specific project/solution before pushing your local changes to CI. They can now perform the following steps to achieve this: 1. On command line: Build with `/p:UseRoslynAnalyzers=true` or `/p:RunCodeAnalysis=true` 2. Inside VS: 1. Start VS with `%RoslynEnforceCodeStyle% = true`, all builds in VS should execute analyzers 2. Use `Run Code Analysis` command from `Analyze` menu on selected project/solution --- azure-pipelines-official.yml | 1 - azure-pipelines.yml | 12 ++++++------ eng/build-utils.ps1 | 8 ++++---- eng/build.ps1 | 11 +++++------ eng/build.sh | 2 +- eng/targets/Imports.targets | 12 ++++++++++++ eng/targets/Settings.props | 1 - eng/test-build-correctness.ps1 | 2 +- eng/test-determinism.ps1 | 2 +- 9 files changed, 30 insertions(+), 21 deletions(-) diff --git a/azure-pipelines-official.yml b/azure-pipelines-official.yml index 57a8fddcf18fa..5d2f16d1b13d8 100644 --- a/azure-pipelines-official.yml +++ b/azure-pipelines-official.yml @@ -103,7 +103,6 @@ stages: -officialSourceBranchName $(SourceBranchName) -officialIbcSourceBranchName $(IbcSourceBranchName) -officialIbcDropId $(IbcDropId) - -skipAnalyzers /p:RepositoryName=$(Build.Repository.Name) /p:VisualStudioDropAccessToken=$(System.AccessToken) /p:VisualStudioDropName=$(VisualStudio.DropName) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index debbd3fcc5df1..ee60afa3a978d 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -37,7 +37,7 @@ jobs: timeoutInMinutes: 90 steps: - - script: eng/cibuild.cmd -configuration $(_configuration) -prepareMachine -testDesktop -$(_testKind) -procdump -skipAnalyzers + - script: eng/cibuild.cmd -configuration $(_configuration) -prepareMachine -testDesktop -$(_testKind) -procdump displayName: Build and Test - task: PublishTestResults@2 @@ -65,7 +65,7 @@ jobs: timeoutInMinutes: 90 steps: - - script: eng/cibuild.cmd -configuration Debug -prepareMachine -testDesktop -skipAnalyzers + - script: eng/cibuild.cmd -configuration Debug -prepareMachine -testDesktop displayName: Build and Test - task: PublishTestResults@2 @@ -99,7 +99,7 @@ jobs: timeoutInMinutes: 90 steps: - - script: eng/cibuild.cmd -configuration $(_configuration) -prepareMachine -msbuildEngine:dotnet -testCoreClr -skipAnalyzers + - script: eng/cibuild.cmd -configuration $(_configuration) -prepareMachine -msbuildEngine:dotnet -testCoreClr displayName: Build and Test - task: PublishTestResults@2 @@ -190,7 +190,7 @@ jobs: # _configuration: Debug timeoutInMinutes: 90 steps: - - script: ./eng/cibuild.sh --configuration $(_configuration) --prepareMachine --skipAnalyzers $(_args) + - script: ./eng/cibuild.sh --configuration $(_configuration) --prepareMachine $(_args) displayName: Build and Test - task: PublishTestResults@2 displayName: Publish xUnit Test Results @@ -216,7 +216,7 @@ jobs: queue: BuildPool.Ubuntu.1604.amd64.Open timeoutInMinutes: 90 steps: - - script: ./eng/cibuild.sh --configuration Debug --prepareMachine --docker --sourceBuild --skipAnalyzers + - script: ./eng/cibuild.sh --configuration Debug --prepareMachine --docker --sourceBuild displayName: Build - task: PublishBuildArtifacts@1 displayName: Publish Logs @@ -233,7 +233,7 @@ jobs: timeoutInMinutes: 90 steps: - - script: ./eng/cibuild.sh --configuration Debug --prepareMachine --testCoreClr --skipAnalyzers + - script: ./eng/cibuild.sh --configuration Debug --prepareMachine --testCoreClr displayName: Build and Test - task: PublishTestResults@2 diff --git a/eng/build-utils.ps1 b/eng/build-utils.ps1 index e76f65fed6d9e..0b75e2de04274 100644 --- a/eng/build-utils.ps1 +++ b/eng/build-utils.ps1 @@ -247,7 +247,7 @@ function Get-PackageDir([string]$name, [string]$version = "") { return $p } -function Run-MSBuild([string]$projectFilePath, [string]$buildArgs = "", [string]$logFileName = "", [switch]$parallel = $true, [switch]$summary = $true, [switch]$warnAsError = $true, [string]$configuration = $script:configuration, [switch]$skipAnalyzers = $false) { +function Run-MSBuild([string]$projectFilePath, [string]$buildArgs = "", [string]$logFileName = "", [switch]$parallel = $true, [switch]$summary = $true, [switch]$warnAsError = $true, [string]$configuration = $script:configuration, [switch]$runAnalyzers = $false) { # Because we override the C#/VB toolset to build against our LKG package, it is important # that we do not reuse MSBuild nodes from other jobs/builds on the machine. Otherwise, # we'll run into issues such as https://github.com/dotnet/roslyn/issues/6211. @@ -268,8 +268,8 @@ function Run-MSBuild([string]$projectFilePath, [string]$buildArgs = "", [string] $args += " /m" } - if ($skipAnalyzers) { - $args += " /p:UseRoslynAnalyzers=false" + if ($runAnalyzers) { + $args += " /p:UseRoslynAnalyzers=true" } if ($binaryLog) { @@ -317,7 +317,7 @@ function Make-BootstrapBuild([switch]$force32 = $false) { $projectPath = "src\NuGet\$packageName\$packageName.Package.csproj" $force32Flag = if ($force32) { " /p:BOOTSTRAP32=true" } else { "" } - Run-MSBuild $projectPath "/restore /t:Pack /p:RoslynEnforceCodeStyle=false /p:UseRoslynAnalyzers=false /p:DotNetUseShippingVersions=true /p:InitialDefineConstants=BOOTSTRAP /p:PackageOutputPath=`"$dir`" /p:EnableNgenOptimization=false /p:PublishWindowsPdb=false $force32Flag" -logFileName "Bootstrap" -configuration $bootstrapConfiguration -skipAnalyzers + Run-MSBuild $projectPath "/restore /t:Pack /p:RoslynEnforceCodeStyle=false /p:UseRoslynAnalyzers=false /p:DotNetUseShippingVersions=true /p:InitialDefineConstants=BOOTSTRAP /p:PackageOutputPath=`"$dir`" /p:EnableNgenOptimization=false /p:PublishWindowsPdb=false $force32Flag" -logFileName "Bootstrap" -configuration $bootstrapConfiguration -runAnalyzers $packageFile = Get-ChildItem -Path $dir -Filter "$packageName.*.nupkg" Unzip "$dir\$packageFile" $dir diff --git a/eng/build.ps1 b/eng/build.ps1 index 61b814b2e598c..cf24f002759fb 100644 --- a/eng/build.ps1 +++ b/eng/build.ps1 @@ -37,7 +37,7 @@ param ( [switch]$buildServerLog, [switch]$ci, [switch]$procdump, - [switch]$skipAnalyzers, + [switch]$runAnalyzers, [switch][Alias('d')]$deployExtensions, [switch]$prepareMachine, [switch]$useGlobalNuGetCache = $true, @@ -97,7 +97,7 @@ function Print-Usage() { Write-Host " -bootstrapConfiguration Build configuration for bootstrap compiler: 'Debug' or 'Release'" Write-Host " -msbuildEngine Msbuild engine to use to run build ('dotnet', 'vs', or unspecified)." Write-Host " -procdump Monitor test runs with procdump" - Write-Host " -skipAnalyzers Do not run analyzers during build operations" + Write-Host " -runAnalyzers Run analyzers during build operations" Write-Host " -prepareMachine Prepare machine for CI run, clean up processes after build" Write-Host " -useGlobalNuGetCache Use global NuGet cache." Write-Host " -warnAsError Treat all warnings as errors" @@ -120,7 +120,7 @@ function Print-Usage() { # specified. # # In this function it's okay to use two arguments to extend the effect of another. For -# example it's okay to look at $testVsi and infer $skipAnalyzers. It's not okay though to infer +# example it's okay to look at $testVsi and infer $runAnalyzers. It's not okay though to infer # $build based on say $testDesktop. It's possible the developer wanted only for testing # to execute, not any build. function Process-Arguments() { @@ -178,7 +178,7 @@ function Process-Arguments() { if ($testVsi) { # Avoid spending time in analyzers when requested, and also in the slowest integration test builds - $script:skipAnalyzers = $true + $script:runAnalyzers = $false $script:bootstrap = $false } @@ -215,7 +215,6 @@ function BuildSolution() { } $projects = Join-Path $RepoRoot $solution - $enableAnalyzers = !$skipAnalyzers $toolsetBuildProj = InitializeToolset $testTargetFrameworks = if ($testCoreClr) { "netcoreapp3.1" } else { "" } @@ -258,7 +257,7 @@ function BuildSolution() { /p:Publish=$publish ` /p:ContinuousIntegrationBuild=$ci ` /p:OfficialBuildId=$officialBuildId ` - /p:UseRoslynAnalyzers=$enableAnalyzers ` + /p:UseRoslynAnalyzers=$runAnalyzers ` /p:BootstrapBuildPath=$bootstrapDir ` /p:TestTargetFrameworks=$testTargetFrameworks ` /p:TreatWarningsAsErrors=true ` diff --git a/eng/build.sh b/eng/build.sh index be6f3426f4da3..c8dbf5ea290d6 100755 --- a/eng/build.sh +++ b/eng/build.sh @@ -31,7 +31,7 @@ usage() echo " --ci Building in CI" echo " --docker Run in a docker container if applicable" echo " --bootstrap Build using a bootstrap compilers" - echo " --skipAnalyzers Do not run analyzers during build operations" + echo " --runAnalyzers Run analyzers during build operations" echo " --prepareMachine Prepare machine for CI run, clean up processes after build" echo " --warnAsError Treat all warnings as errors" echo " --sourceBuild Simulate building for source-build" diff --git a/eng/targets/Imports.targets b/eng/targets/Imports.targets index 8b8d2b0762091..43364a7f6afd8 100644 --- a/eng/targets/Imports.targets +++ b/eng/targets/Imports.targets @@ -34,6 +34,18 @@ true + + + false + true + + $(NoWarn);Nullable diff --git a/eng/targets/Settings.props b/eng/targets/Settings.props index a0ec480d03d58..61d1dd9ba2b3a 100644 --- a/eng/targets/Settings.props +++ b/eng/targets/Settings.props @@ -23,7 +23,6 @@ true true true - true NON_EXISTENT_FILE diff --git a/eng/test-build-correctness.ps1 b/eng/test-build-correctness.ps1 index bc0ce119ce2af..b59154f40d312 100644 --- a/eng/test-build-correctness.ps1 +++ b/eng/test-build-correctness.ps1 @@ -34,7 +34,7 @@ try { Push-Location $RepoRoot Write-Host "Building Roslyn" - Exec-Block { & (Join-Path $PSScriptRoot "build.ps1") -restore -build -ci:$ci -configuration:$configuration -pack -binaryLog -useGlobalNuGetCache:$false -warnAsError:$true -properties "/p:RoslynEnforceCodeStyle=true"} + Exec-Block { & (Join-Path $PSScriptRoot "build.ps1") -restore -build -ci:$ci -runAnalyzers:$true -configuration:$configuration -pack -binaryLog -useGlobalNuGetCache:$false -warnAsError:$true -properties "/p:RoslynEnforceCodeStyle=true"} # Verify the state of our various build artifacts Write-Host "Running BuildBoss" diff --git a/eng/test-determinism.ps1 b/eng/test-determinism.ps1 index 835c0e42d587c..9ecac2ecd91e6 100644 --- a/eng/test-determinism.ps1 +++ b/eng/test-determinism.ps1 @@ -249,7 +249,7 @@ try { Create-Directory $errorDirLeft Create-Directory $errorDirRight - $skipAnalyzers = $true + $runAnalyzers = $false $binaryLog = $true $officialBuildId = "" $ci = $true From 71716fd556552c83a8a948324f7c7180020c29ff Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 7 May 2020 06:57:51 -0700 Subject: [PATCH 2/4] Dummy commit violating build enforced code style rule to test CI failure --- src/Workspaces/Core/Portable/CodeActions/CodeAction.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 77aa9283fc823..b3afcde46c4e0 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -5,6 +5,7 @@ #nullable enable using System; +using System.IO; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; From 4f7053245cb8b7d898c2d4af4f2e28f7a44f1962 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 7 May 2020 11:43:31 -0700 Subject: [PATCH 3/4] Revert "Dummy commit violating build enforced code style rule to test CI failure" This reverts commit 71716fd556552c83a8a948324f7c7180020c29ff. --- src/Workspaces/Core/Portable/CodeActions/CodeAction.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index b3afcde46c4e0..77aa9283fc823 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -5,7 +5,6 @@ #nullable enable using System; -using System.IO; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; From 59f745b81f040169c560c7e708b4df34a8bee2bf Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 7 May 2020 16:23:30 -0700 Subject: [PATCH 4/4] Address feedback --- eng/build.ps1 | 4 ++-- eng/build.sh | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/eng/build.ps1 b/eng/build.ps1 index cf24f002759fb..03864e6c1ddd5 100644 --- a/eng/build.ps1 +++ b/eng/build.ps1 @@ -37,7 +37,7 @@ param ( [switch]$buildServerLog, [switch]$ci, [switch]$procdump, - [switch]$runAnalyzers, + [switch][Alias('a')]$runAnalyzers, [switch][Alias('d')]$deployExtensions, [switch]$prepareMachine, [switch]$useGlobalNuGetCache = $true, @@ -97,7 +97,7 @@ function Print-Usage() { Write-Host " -bootstrapConfiguration Build configuration for bootstrap compiler: 'Debug' or 'Release'" Write-Host " -msbuildEngine Msbuild engine to use to run build ('dotnet', 'vs', or unspecified)." Write-Host " -procdump Monitor test runs with procdump" - Write-Host " -runAnalyzers Run analyzers during build operations" + Write-Host " -runAnalyzers Run analyzers during build operations (short: -a)" Write-Host " -prepareMachine Prepare machine for CI run, clean up processes after build" Write-Host " -useGlobalNuGetCache Use global NuGet cache." Write-Host " -warnAsError Treat all warnings as errors" diff --git a/eng/build.sh b/eng/build.sh index c8dbf5ea290d6..4369538d5c03e 100755 --- a/eng/build.sh +++ b/eng/build.sh @@ -64,7 +64,7 @@ verbosity='minimal' binary_log=false ci=false bootstrap=false -skip_analyzers=false +run_analyzers=false prepare_machine=false warn_as_error=false properties="" @@ -129,8 +129,8 @@ while [[ $# > 0 ]]; do # Bootstrap requires restore restore=true ;; - --skipanalyzers) - skip_analyzers=true + --runAnalyzers) + run_analyzers=true ;; --preparemachine) prepare_machine=true @@ -224,10 +224,9 @@ function BuildSolution { local projects="$repo_root/$solution" # https://github.com/dotnet/roslyn/issues/23736 - local enable_analyzers=!$skip_analyzers UNAME="$(uname)" if [[ "$UNAME" == "Darwin" ]]; then - enable_analyzers=false + run_analyzers=false fi # NuGet often exceeds the limit of open files on Mac and Linux @@ -273,7 +272,7 @@ function BuildSolution { /p:Test=$test \ /p:Pack=$pack \ /p:Publish=$publish \ - /p:UseRoslynAnalyzers=$enable_analyzers \ + /p:UseRoslynAnalyzers=$run_analyzers \ /p:BootstrapBuildPath="$bootstrap_dir" \ /p:ContinuousIntegrationBuild=$ci \ /p:TreatWarningsAsErrors=true \