Skip to content

Add functionality to enable skipping analyzers for implicitly trigger…#54143

Merged
mavasani merged 3 commits intodotnet:mainfrom
mavasani:SkipAnalyzersOnIndirectBuild
Jun 18, 2021
Merged

Add functionality to enable skipping analyzers for implicitly trigger…#54143
mavasani merged 3 commits intodotnet:mainfrom
mavasani:SkipAnalyzersOnIndirectBuild

Conversation

@mavasani
Copy link
Copy Markdown
Contributor

@mavasani mavasani commented Jun 16, 2021

…ed builds inside Visual Studio

Work towards AB#1337109

Overview

For builds which are implicitly triggered inside Visual Studio from commands such as 'Run Tests' or 'Start Debugging', we implicitly skip analyzers to speed up these builds.

Such builds will set a special property IsImplicitlyTriggeredBuild to true, which guards the implicit skip analyzers logic. We display a special message to inform the users about us implicitly skipping analyzers to speed up the build. Note this message was provided to us by the UX team.

To enable MSBuild's incremental build logic and project system's fast-upto-date check logic to work correctly, we create/touch a special semaphore file to indicate the time stamp for last build with skipAnalyzers flag. This semaphore file is passed as a custom additional file input to builds without skipAnalyzers flag to ensure correct incremental builds. Additionally, we pass this file as an 'UpToDateCheckInput' item for project system's fast-upto-date check.

Testing

AB#1337109 requires few more WIP PRs to go through to work end-to-end:

  1. Changes on Unit testing repo (Run tests and Debug tests commands) and Start Debugging command to set IsImplicitlyTriggeredBuild to true for builds indirectly triggered from these commands. I am working on these changes.
  2. Change to project-system repo to update the fast-upto-date check logic to work correctly. @drewnoakes is driving that work.

I was able to locally build bits with both the above changes + changes from this PR and verify end-to-end functioning of this feature for Run Tests/Debug Tests command. Demo provided below.

Demo

ImplicitlySkipAnalyzersForIndirectBuilds

…ed builds inside Visual Studio

Work towards [AB#1337109](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1337109)

For builds which are indirectly triggered inside Visual Studio from commands such as 'Run Tests' or 'Start Debugging', we implicitly skip analyzers to speed up these builds.

Such builds will set a special property `IsIndirectlyTriggeredBuildInsideVisualStudio` to `true`, which guards the implicit skip analyzers logic. We display a special message to inform the users about us implicitly skipping analyzers to speed up the build.

To enable MSBuild's incremental build logic and project system's fast-upto-date check logic to work correctly, we create/touch a special semaphore file to indicate the time stamp for last build with skipAnalyzers flag. This semaphore file is passed as a custom additional file input to builds without skipAnalyzers flag to ensure correct incremental builds. Additionally, we pass this file as an 'UpToDateCheckInput' item for project system's fast-upto-date check.
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

IsIndirectlyTriggeredBuildInsideVisualStudi

How about:

IsImplicitlyTriggeredBuild? I'm not sure why we'd need to say it was visual studio, and I imagine this being useful for other hosts.

@jaredpar jaredpar requested a review from RikkiGibson June 16, 2021 20:01
@mavasani
Copy link
Copy Markdown
Contributor Author

IsImplicitlyTriggeredBuild? I'm not sure why we'd need to say it was visual studio, and I imagine this being useful for other hosts.

That sounds reasonable, I'll update the naming.

@mavasani mavasani changed the title Add functionality to enable skipping analyzers for indirectly trigger… Add functionality to enable skipping analyzers for implicitly trigger… Jun 17, 2021
@jaredpar
Copy link
Copy Markdown
Member

@chsienki, @RikkiGibson for second review

Copy link
Copy Markdown
Member

@chsienki chsienki left a comment

Choose a reason for hiding this comment

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

Same as @jared said: as long as we're explicitly aware that a user can get potentially different compilation outcomes between test + build, LGTM.

@mavasani mavasani enabled auto-merge June 18, 2021 02:27
@mavasani mavasani merged commit 296cdd2 into dotnet:main Jun 18, 2021
@ghost ghost modified the milestones: 17.0.P2, Next Jun 18, 2021
@mavasani mavasani deleted the SkipAnalyzersOnIndirectBuild branch June 18, 2021 03:51
mavasani added a commit to mavasani/roslyn that referenced this pull request Jun 21, 2021
Restrict the functionality added in dotnet#54143 to only kick in for SDK style projects. This feature needs work in the project system for upto-date check to work correctly, and we are currently only commited to doing this work for CPS/SDK style projects.
@RikkiGibson RikkiGibson modified the milestones: Next, 17.0.P2 Jun 29, 2021
mavasani added a commit to mavasani/roslyn that referenced this pull request Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants