Skip to content
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.

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

Skip executing analyzers by default on local builds for solutions in the repo #44047

Merged
merged 4 commits into from
May 8, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 0 additions & 1 deletion azure-pipelines-official.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ stages:
-officialSourceBranchName $(SourceBranchName)
-officialIbcSourceBranchName $(IbcSourceBranchName)
-officialIbcDropId $(IbcDropId)
-skipAnalyzers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Analyzer execution has been switched from opt-out mode to opt-in mode in build scripts.

skipAnalyzers flag has been replaced with runAnalyzers flag, which defaults to false. It is only explicitly set to true for build-correctness test leg.

/p:RepositoryName=$(Build.Repository.Name)
/p:VisualStudioDropAccessToken=$(System.AccessToken)
/p:VisualStudioDropName=$(VisualStudio.DropName)
Expand Down
12 changes: 6 additions & 6 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions eng/build-utils.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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) {
Expand Down Expand Up @@ -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

Expand Down
11 changes: 5 additions & 6 deletions eng/build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ param (
[switch]$buildServerLog,
[switch]$ci,
[switch]$procdump,
[switch]$skipAnalyzers,
[switch]$runAnalyzers,
mavasani marked this conversation as resolved.
Show resolved Hide resolved
[switch][Alias('d')]$deployExtensions,
[switch]$prepareMachine,
[switch]$useGlobalNuGetCache = $true,
Expand Down Expand Up @@ -97,7 +97,7 @@ function Print-Usage() {
Write-Host " -bootstrapConfiguration Build configuration for bootstrap compiler: 'Debug' or 'Release'"
Write-Host " -msbuildEngine <value> 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"
Expand All @@ -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() {
Expand Down Expand Up @@ -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
mavasani marked this conversation as resolved.
Show resolved Hide resolved
$script:bootstrap = $false
}

Expand Down Expand Up @@ -215,7 +215,6 @@ function BuildSolution() {
}

$projects = Join-Path $RepoRoot $solution
$enableAnalyzers = !$skipAnalyzers
$toolsetBuildProj = InitializeToolset

$testTargetFrameworks = if ($testCoreClr) { "netcoreapp3.1" } else { "" }
Expand Down Expand Up @@ -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 `
Expand Down
2 changes: 1 addition & 1 deletion eng/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 12 additions & 0 deletions eng/targets/Imports.targets
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,18 @@
<RoslynCheckCodeStyle Condition="'$(RoslynCheckCodeStyle)' == '' AND ('$(ContinuousIntegrationBuild)' != 'true' OR '$(RoslynEnforceCodeStyle)' == 'true')">true</RoslynCheckCodeStyle>
</PropertyGroup>

<!--
PERF: Set default value for 'UseRoslynAnalyzers' to determine if analyzers should be executed.
Default to 'false' in all non-CI builds to improve local build time (i.e. csc/vbc invocations both inside Visual Studio and from command line prompt), except if:
1. We are enforcing code style, i.e. '$(RoslynEnforceCodeStyle)' == 'true' OR
2. We are explicitly running code analysis via "Run Code Analysis" command, i.e. '$(RunCodeAnalysis)' == 'true'.
Otherwise, default to 'true'.
-->
<PropertyGroup Condition="'$(UseRoslynAnalyzers)' == ''">
<UseRoslynAnalyzers Condition="'$(DesignTimeBuild)' != 'true' AND '$(ContinuousIntegrationBuild)' != 'true' AND !('$(RoslynEnforceCodeStyle)' == 'true' OR '$(RunCodeAnalysis)' == 'true')">false</UseRoslynAnalyzers>
Copy link
Member

Choose a reason for hiding this comment

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

When does $(RunCodeAnalysis) get set to true? Not familiar with the workflows that enable that property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This property is set to true when you execute "Run Code Analysis" context command in Visual Studio, either from top level "Analyze" menu OR from solution explorer context menu for project/solution: https://docs.microsoft.com/en-us/visualstudio/code-quality/how-to-run-code-analysis-manually-for-managed-code?view=vs-2019. Starting VS2019 16.5, this command force completes all the applicable analyzers on the selected project/solution.

<UseRoslynAnalyzers Condition="'$(UseRoslynAnalyzers)' == ''">true</UseRoslynAnalyzers>
</PropertyGroup>

<PropertyGroup Condition="'$(DisableNullableWarnings)' == 'true'">
<NoWarn>$(NoWarn);Nullable</NoWarn>
</PropertyGroup>
Expand Down
1 change: 0 additions & 1 deletion eng/targets/Settings.props
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
<ProduceReferenceAssembly>true</ProduceReferenceAssembly>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<GenerateFullPaths>true</GenerateFullPaths>
<UseRoslynAnalyzers Condition="'$(UseRoslynAnalyzers)' == ''">true</UseRoslynAnalyzers>

<!-- Set to non-existent file to prevent common targets from importing Microsoft.CodeAnalysis.targets -->
<CodeAnalysisTargets>NON_EXISTENT_FILE</CodeAnalysisTargets>
Expand Down
2 changes: 1 addition & 1 deletion eng/test-build-correctness.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion eng/test-determinism.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ try {
Create-Directory $errorDirLeft
Create-Directory $errorDirRight

$skipAnalyzers = $true
$runAnalyzers = $false
$binaryLog = $true
$officialBuildId = ""
$ci = $true
Expand Down