dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.22k stars 1.35k forks source link

Remove deprecated engine code #8826

Closed danmoseley closed 2 days ago

danmoseley commented 1 year ago

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

Engineering culture

Risks

External customers breaking

The packages are published on nuget (Microsoft.Build.Engine, Microsoft.Build.Conversion.Core) with nontrivial download count.

Mitigation(s):

Internal customers breaking

We have identified 7 groups of usages accross partner teams:

Mitigation(s):

Close cooperation with all affected partner teams:

Course of action

Removal PRs:

After actions

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.

KalleOlaviNiemitalo commented 1 year ago

Would solve https://github.com/dotnet/msbuild/issues/8822 as well.

rainersigwald commented 1 year ago

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!).

danmoseley commented 1 year ago

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.

rainersigwald commented 1 year ago

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!

JanKrivanek commented 5 months ago

(not maintained here - moved to ticket description)


Course of action:

KalleOlaviNiemitalo commented 5 months ago

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.

JanKrivanek commented 5 months ago

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.