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

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

Merged
merged 3 commits into from
Jun 18, 2021

Conversation

mavasani
Copy link
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
Member

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.

@mavasani
Copy link
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
Member

@chsienki, @RikkiGibson for second review

Copy link
Contributor

@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