Skip to content

move ProjectCache out of experimental - #12051

Merged
YuliiaKovalova merged 14 commits into
dotnet:mainfrom
JanProvaznik:dev/janpro/projectcache-attempt-4
Jul 4, 2025
Merged

YuliiaKovalova merged 14 commits into
dotnet:mainfrom
JanProvaznik:dev/janpro/projectcache-attempt-4

Conversation

@JanProvaznik

@JanProvaznik JanProvaznik commented Jun 20, 2025 •

Copy link
Copy Markdown
Member

Fixes #11575

Context

ProjectCache should not be in experimental due to being used in production
but we need to maintain backward compatibility with plugins compiled against the previous namespace
internals use the new namespace

Changes Made

adapt ProjectCacheService to work with both namespaces to maintain backward compatibility, loading both kinds of plugins and converting results to the new namespace
copy existing classes to maintain binary compatibility and mark them obsolete

Testing

Tests run on 2 scenarios, an old namespace plugin loaded from assembly file. And in memory plugin with the new namespace. This tests both current behavior and binary compatibility.

Notes

Needs adjustment on VS insertion.

VS side changes: https://devdiv.visualstudio.com/DevDiv/_git/VS/commit/e3f1029a4c04344c0a18ec4b1a6674dfd96e003a
final exp insertion https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/646925

@JanProvaznik

JanProvaznik commented Jun 20, 2025 •

Copy link
Copy Markdown
Member Author

@rainersigwald ptal if the strategy of modifying ProjectCacheService to load both kinds of plugins and duplicating the logic with both types looks acceptable.

Locally it seems to work backwards compatibly with the sample plugin, though I'll have to adjust the test class done

@JanProvaznik
JanProvaznik marked this pull request as ready for review June 23, 2025 09:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the ProjectCache API out of the experimental namespace into the main Microsoft.Build.ProjectCache namespace while preserving backward compatibility with plugins compiled against the old experimental API.

  • Renamed namespace imports and type references from Microsoft.Build.Experimental.ProjectCache to Microsoft.Build.ProjectCache.
  • Updated ProjectCacheService to detect and interoperate with both new and experimental plugin instances via wrapper adapters.
  • Introduced obsolete wrapper classes under the Experimental folder and updated the project file and serialization contracts to include them.

Reviewed Changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/MSBuild/XMake.cs Updated catch block to handle new and experimental exceptions
src/Build/Microsoft.Build.csproj Added compilation of experimental compatibility wrappers
src/Build/CompatibilitySuppressions.xml Added suppressions for moved BuildParameters.ProjectCacheDescriptor
src/Build/BackEnd/Components/Communications/SerializationContractInitializer.cs Registered both new and experimental ProjectCacheException
src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs Extended service to load and invoke both plugin types
src/Build/BackEnd/Components/ProjectCache/* Renamed namespaces and added obsolete compatibility types
src/Build/BackEnd/BuildManager/BuildParameters.cs Updated remarks and namespace for ProjectCacheDescriptor
src/Build/BackEnd/BuildManager/BuildManager.cs Replaced experimental calls in shutdown logic
src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs Switched test imports to new namespace
src/Build.UnitTests/BackEnd/TranslationHelpers.cs Handled known moved exception types
src/Build.UnitTests/BackEnd/Scheduler_Tests.cs Updated test imports for scheduler
Comments suppressed due to low confidence (3)

src/Build/CompatibilitySuppressions.xml:4

  • [nitpick] Consider consolidating repeated <Suppression> entries for the same member across frameworks, or add a comment explaining why each is necessary to improve maintainability.
  <Suppression>

src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs:15

  • Tests have been updated to reference the new namespace, but there are no tests covering the backward‐compatibility path for experimental plugins. Consider adding unit tests to exercise the experimental–to–new compatibility adapters.
using Microsoft.Build.ProjectCache;

src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs:958

  • The catch clause references Experimental.ProjectCache.ProjectCacheException but there is no Experimental namespace alias or using directive. Use the full namespace (Microsoft.Build.Experimental.ProjectCache.ProjectCacheException) or add a using Experimental = Microsoft.Build.Experimental.ProjectCache; alias.
                    catch (Exception e) when (e is not ProjectCacheException && e is not Experimental.ProjectCache.ProjectCacheException)

Comment thread src/MSBuild/XMake.cs Outdated
Comment thread src/Build/Microsoft.Build.csproj Outdated
Comment thread src/Build/BackEnd/BuildManager/BuildManager.cs Outdated
@YuliiaKovalova
YuliiaKovalova self-requested a review June 23, 2025 15:47
@JanProvaznik

Copy link
Copy Markdown
Member Author

there are 7 places in VS where this namespace is used and presumably will have to be adjusted

@YuliiaKovalova

Copy link
Copy Markdown
Member

there are 7 places in VS where this namespace is used and presumably will have to be adjusted

@JanProvaznik

Copy link
Copy Markdown
Member Author

there are 7 places in VS where this namespace is used and presumably will have to be adjusted

If I understand correctly that's a specific cache implementation which yes should migrate eventually, but if it's already compiled it needs to remain compatible. VS is different because there is extra logic around the caches as a host.

Comment thread src/Build/BackEnd/Components/ProjectCache/ProjectCacheDescriptor.cs
Comment thread src/Build/BackEnd/BuildManager/BuildParameters.cs
Comment thread src/Build/BackEnd/Components/ProjectCache/CacheResult.cs
Comment thread src/Build/BackEnd/Components/ProjectCache/Experimental/ProjectCacheDescriptor.cs Outdated

@rainersigwald rainersigwald left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, so the VS-side changes are because there's a cache plugin there that dynamically loads another one, that will also need to handle the transition? I can live with that!

LGTM but let's double check with @dfederm -- any additional testing you'd like to see?

Comment thread src/Build.UnitTests/BackEnd/TranslationHelpers.cs Outdated
Comment thread src/Build/BackEnd/BuildManager/BuildManager.cs

@dfederm dfederm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. The listed testing looks sufficient to me. I imagine with this sort of change things would either completely work or completely break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move ProjectCache to non-Experimental

6 participants