dotnet
Remove deprecated engine code
Backward compatibility note
Please use Visual Studio version 17.12 and lower to upgrade pre-msbuild project format.
Context
About 15 years ago a new MSBuild engine and API was created and the previous code was deprecated with very high servicing bar. Keeping the code around (and publishing the packages) is a nontrivial liability especially as we try to model and improve our security.
Business justification
Maintanance cost
- Slashing significant portion of the code that is kept in repository, translated into the production binaries and hence having some form of support/maintanance cost. The LoC proportion of production code (excluding tests) targetting to be removed: 39% (76k/272k)
- Removing the legacy binaries that we'd otherwise need to security-fix
Engineering culture
- Improving engineering efficiency - the deprecated code had duplicated and diverging code - so IDE and GH code searching often returns multitude of irrelevant results. There were cases of engineers accidentaly editing the functionality in the deprecated engine instead of in the new one.
- Community friendliness - our code base will be more lightweight and understandable
Risks
External customers breaking
The packages are published on nuget (Microsoft.Build.Engine, Microsoft.Build.Conversion.Core) with nontrivial download count.
Mitigation(s):
- Early 2022 a readme for nuget.org landing page for the 2 packages was added explicitly warning the users the package is deprecated and they should move to the new API.
- The entrypoint classes of the API have been marked as obsolete decade ago, with information about the new API. The notice is exposed on learn.microsoft page: https://learn.microsoft.com/en-us/dotnet/api/microsoft.build.buildengine.engine
- In 17.11 we added xml doc comments to all members of the public surface of the deprecated packages and the notices are being propagated to learn.microsoft page
- We'll eventually stop publishing new versions of the package (likely with 17.12 or beyond) - sending a signal that the package is not maintained anymore
- We might choose to add a vulnerability metadata (or even backfill on already published packages) to make the message even more explicit
Internal customers breaking
We have identified 7 groups of usages accross partner teams:
- 2 usages in VS legacy project upgrade wizards
- 4 usages accross the VS codebase in various components and tooling
- 1 usage in project sytstem tools VS extension
Mitigation(s):
Close cooperation with all affected partner teams:
- Had the dead code removed (3 cases)
- Helping them to onboard to a new API (2 cases)
- Agreeing and documenting breaking change (2 cases) - discontinuing support for upgrading .NET 1.0 and older (pre-MSBuild) project files
Course of action
-
[x] Update public documentation to make the fate very clear: @maridematte, sample: https://github.com/dotnet/msbuild/pull/10166. Done: https://github.com/dotnet/msbuild/pull/10198 & https://github.com/dotnet/msbuild/pull/10233
-
[x] Microsoft.Build.Conversion.Core
- [x] Untangling Microsoft.Build.Conversiion.Core from VS - PR: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/550640
- [x] Wire telemetry to see if upgrading from legacy pre-msbuild and eraly-mbuild formats is a ever used scenario in VS: https://dev.azure.com/devdiv/DevDiv/_git/a290117c-5a8a-40f7-bc2c-f14dbe3acf6d/pullrequest/552729
- [x] Discussion with Web Project team to ask for deprecation of the scenario
- [x] Investigate and rootcause 3 failing unit tests in
VC.MSBuild.Upgrade(https://dev.azure.com/devdiv/DevDiv/_git/VS/pullRequest/550640#1717097236) - [x] Discussion with the VsProject/XmlParser PM/dev owner to deprecate the sceanrio
- [x] Create VS documentation informing about the dropped scenario and workaround (probably here: https://learn.microsoft.com/en-us/visualstudio/releases/2022/port-migrate-and-upgrade-visual-studio-projects or https://learn.microsoft.com/en-us/visualstudio/ide/whats-new-visual-studio-2022) - done: https://learn.microsoft.com/en-us/visualstudio/releases/2022/port-migrate-and-upgrade-visual-studio-projects#pre-msbuild-projects
- [x] Clear the feature deprecation process: https://devdiv.visualstudio.com/DevDiv/_wiki/wikis/DevDiv.wiki/19822/Feature-Component-Deprecation-Removal - https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2152560
- [x] Create sample manual test with old format project (.NET FW 1.0) demonstrating the behavior in previous version VS and current version VS.
- [x] Facilitate the documentation of the upgrade unsupported behavior: https://github.com/MicrosoftDocs/feedback/issues/4009 - done: https://learn.microsoft.com/en-us/visualstudio/releases/2022/port-migrate-and-upgrade-visual-studio-projects#pre-msbuild-projects
- [x] Ensure the upgrade scenario error gives a clear error and instruction. Preferabely aka.ms link should be created pointing to the documentation from previous item
- [x] Remove the Microsoft.Build.Conversion package from /src/ConfigData/Packages/Microsoft.VisualStudio.MinShell/msbuild.props (VS) and automation updating the package there
- [x] Remove the package from automation pushing it to internal feed(s)
-
[x] Microsoft.Build.Engine
- [x] Untangling Microsoft.Build.Engine from VS - PR: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/552611
- Discusions with: Maui, XmlEditor, VSIP (and others?) teams to offboard from BuildEngine to the new API.
- [x] SSDT Offboarding: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2078181 https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/562571
- [x] src/vsip/Framework/Project offboarding: Confirmed a dead code that can be removed - https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/571308
- [x] Omni/Maui offboarding: https://devdiv.visualstudio.com/DevDiv/_git/VS/commit/3dbb9ba32fcc8c14534692e52a2b2d5d7dec1cdd?refName=refs/heads/main
- [x] WorkFlows offboarding: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2090889
- [x] #10321
-
[ ] Untangle Microsoft.Build.BuildEngine usages in other repos
Removal PRs:
- [x] https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/583701 - [Internal] Remove Microsoft.Build.Conversion.Core
- [ ] https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/585053 - followup leftovers removal
- [ ] https://devdiv.visualstudio.com/DevDiv/_git/WebFormsDesigner/pullrequest/585116 - removal from web designer
- [ ] https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/584809 - [Internal] Remove deprecated Microsoft.Build.Engine
- [ ] https://github.com/dotnet/msbuild/pull/10352
After actions
- [ ] Deprecating the pacakge on nuget.org: https://learn.microsoft.com/en-us/nuget/nuget-org/deprecate-packages (talk to @leecow for PoC with permissions)
- [ ] Marking vulns on packages - https://www.nuget.org/packages/NuGet.Packaging/6.8.0 (talk to John Douglas)
- [ ] Marking EOL https://github.com/NuGet/Home/pull/13598
Background - Original ticket content
This was deprecated about 15 years ago and changes have long been at a high servicing bar. Can the code be removed from main now and serviced out of older branches? (This is done in runtime repo for some packages.) This would simplify the repo, speed up build and tests, and reduce risk of inadvertent changes to it. Likely it would make it easier to switch on new analyzers and warnings in the repo as the old code wouldn't need to be excluded.
Last I checked, Visual Studio still depended on it and it had live dependencies into the current code so we couldn't just ship a frozen-in-time version. However, we should definitely look and see if VS has dropped its dependencies (wouldn't that be nice!).
live dependencies into the current code
can you clarify? meaning it has to change to adapt to changes in the current code? the M.B.F interface itself should be stable, of course.
I'm curious what VS needs it for. BTW an example of something vaguely analogous -- System.Data.SqlClient is at a high servicing bar, though of course fully supported. So we deleted it from main, and now service it out of the 6.0 branch indefinitely. I'm not sure of the long term plan, but I believe there was discussion of a "servicing" repo to gather such things in, as there are numerous other libraries like that in runtime. Not suggesting that would be the right plan here, just that it's doable to remove from main code that is still supported.
can you clarify? meaning it has to change to adapt to changes in the current code? the M.B.F interface itself should be stable, of course.
Ah, you're quite right! I had mistakenly thought that Microsoft.Build.Engine depended on Microsoft.Build, but it doesn't. Conversion does, though--and with InternalsVisibleTo, which is a bit scary. Still worth a detailed look!
(not maintained here - moved to ticket description)
Course of action:
- [ ] Update public documentation to make the fate very clear: @maridematte, sample: https://github.com/dotnet/msbuild/pull/10166
- [ ] Microsoft.Build.Conversion.Core
- [x] Untangling Microsoft.Build.Conversiion.Core from VS - PR: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/550640
- [x] Wire telemetry to see if upgrading from legacy pre-msbuild and eraly-mbuild formats is a ever used scenario in VS: https://dev.azure.com/devdiv/DevDiv/_git/a290117c-5a8a-40f7-bc2c-f14dbe3acf6d/pullrequest/552729
- [x] Discussion with Web Project team to ask for deprecation of the scenario
- [ ] Discussion with the VsProject/XmlParser PM/dev owner to deprecate the sceanrio
- [ ] Microsoft.Build.Engine
- [ ] Untangling Microsoft.Build.Conversiion.Core from VS - PR: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/550640
- [ ] Discusions with: Maui, XmlEditor, VSIP (and others?) teams to offboard from BuildEngine to the new API.
I'm a bit surprised that you're only adding docs and not ObsoleteAttribute. (It doesn't have AttributeTargets.Assembly though.)
ObsoleteAttribute on .NET Framework does not support separately-suppressible diagnostic codes, but IIRC it's possible to define an internal ObsoleteAttribute with this support and it'll be recognised by the compiler.
To deliver the message ASAP it's easier to follow the lowest risk path (so no breaking changes). We can possibly add Obsolete attributes after the docs are updated. It all depends on actual timelines of removal.