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

Fix NFE about version regression on configuration switch #9606

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

drewnoakes
Copy link
Member

@drewnoakes drewnoakes commented Nov 28, 2024

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1833278

In #8321, we optimised how we handle Roslyn projects in the language service during configuration switches, such as from Debug to Release.

Before that PR, we maintained different pipelines for each configuration. That PR re-uses the same pipeline between configurations, instead applying a delta representing the switch. This makes configuration switches a lot faster.

Project data flows through with version numbers, and each configuration tracks versions to ensure that evaluation and build data is applied with correct sequencing.

The previous PR introduced the chance for an unexpected regression in configured project version, which would lead to an exception that'd trigger a NFE and gold bar in VS.

Attempted to push a project evaluation that regressed in version.

This is one of the top NFEs in the project system.

The fix here is to consider multiple versions in each update. We combine a version that increments when the active configuration changes along with the actual configured project version. Then, when comparing versions, we first compare the active configuration version, and only if they match would we then compare the configured project version. This gives us a monotonically increasing sequence of updates across both configuration switches and project changes, which will satisfy the consuming code and prevent the NFE.

Microsoft Reviewers: Open in CodeFlow

@drewnoakes drewnoakes added the Feature-Language-Service Populating the Roslyn workspace with references, source files, analyzers, etc label Nov 28, 2024
@drewnoakes drewnoakes requested a review from a team as a code owner November 28, 2024 11:50
In dotnet#8321, we optimised how we handle Roslyn projects in the language service during configuration switches, such as from Debug to Release.

Before that PR, we maintained different pipelines for each configuration. That PR re-uses the same pipeline between configurations, instead applying a delta representing the switch. This makes configuration switches a lot faster.

Project data flows through with version numbers, and each configuration tracks versions to ensure that evaluation and build data is applied with correct sequencing.

The previous PR introduced the chance for an unexpected regression in configured project version, which would lead to an exception that'd trigger a NFE and gold bar in VS.

> Attempted to push a project evaluation that regressed in version.

This is one of the top NFEs in the project system.

The fix here is to consider multiple versions in each update. We combine a version that increments when the active configuration changes along with the actual configured project version. Then, when comparing versions, we first compare the active configuration version, and only if they match would we then compare the configured project version. This gives us a monotonically increasing sequence of updates across both configuration switches and project changes, which will satisfy the consuming code and prevent the NFE.
@drewnoakes drewnoakes force-pushed the lang-svc-version-regression branch from b3826ce to 3174149 Compare November 28, 2024 21:14
@drewnoakes drewnoakes merged commit 5983ea9 into dotnet:main Dec 3, 2024
5 checks passed
@drewnoakes drewnoakes deleted the lang-svc-version-regression branch December 3, 2024 22:25
@dotnet-policy-service dotnet-policy-service bot added this to the 17.12 milestone Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature-Language-Service Populating the Roslyn workspace with references, source files, analyzers, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants