Roslyn: Use Central Package Management

[jmarolf]

Problem

Back in the day when we had packages.config _all_ the nuget dependencies were specified so our scheme of having a single props file that included all versions was always correct. Now that we get implicit dependencies in package references it is possible for us to have different implicit nuget package versions for different projects. It is also possible for these implicit package references to unify to different versions on a per-project basis.

Solution

Using central package management will (based on my initial investigations) allow nuget to fail the build if implicit dependencies resolve to different versions across projects. This also has the added benefit of us adopting a nuget feature that is expected to have VS UI support vs our current approach which we do not expect to ever be supported in VS.

Implementation

I would like us to adopt and use this feature in the most idiomatic way possible. According to the nuget docs that would mean adding a Directory.Packages.props file to our repo. Briefly prototyped this approach here

Implementation Blockers

• [x] We need an SDK and MSBuild that contains this bugfix

[tmat]

It'd be good to follow up with .NET Core eng team to update Meastro to recognize Directory.Packages.props and update versions listed in it in addition to versions specified in eng\Versions.props.
/cc @markwilkie

[markwilkie]

@mmitche / @riarenas - I agree with @tmat that this makes sense to do.

@ericstj - is there any opportunity here for unification with the packaging stuff runtime does do you think? It's probably too much of stretch, but any chance to have common ways of doing things seems like goodness.

@jmarolf - how much of the implementation for Roslyn do you think is "sharable" with the rest of dotnet? If there's sufficient value there, it's be great to move to a common place for the whole product to use.

[ericstj]

@markwilkie this is about consuming packages which is orthogonal to the special production of packages done in dotnet/runtime. I see them as completely independent and think that dotnet/runtime would benefit from this work the same as any other repo and would adopt it.

[mmitche]

@tmat Can Directory.Packages.props pull properties from eng/Versions.props? If so you could avoid any Maestro changes.

[tmat]

@mmitche Yes, that would be what we would do for now. We don't want to do that long term though as it breaks the IDE experience and is not the pattern we want our customers to use.
My thinking is that in time we transition all repos to use Directory.Packages.props and deprecate the Versions.props file, if possible. Historically, we designed Versions.props file the way it is because the product lacked support for central package versioning. Now that it has this support we should migrate to it to avoid long term divergence of what we recommend our customers to do vs what we do.

[mmitche]

@tmat Versions.props also carries a variety of other functionality (e.g. package versioning). It's also highly unlikely that we could backport all this to old versions of .NET, so it's likely that Versions.props as a source of version info will likely be around forever. But we could certainly have Maestro update both, potentially making a statement that if version appears in Directory.Packages.props, it should not appear in Versions.props to avoid information duplication.

[jmarolf]

My approach is going to be to import Versions.props into Directory.Packages.props and call it a day. Two (2) of our dependencies are updated by the Maestro infrastructure and they are arcade itself and the compilers we build with. I am ok with those dependencies not being viewable/editable in the UI/Nuget tooling as they are not libraries we depend on but infrastructure.

[jmarolf]

I don't think this is something that dnceng should need to do anything special to support or, frankly, do any work at all. If other repos have situations where they need to manually update their nuget version (what roslyn does for 100% of its build dependencies) _and_ also wants these versions automatically update (?) then I guess this won't work for them. Would need to understand what that repo is doing in that case.

[mmitche]

Is there a reason the VS tooling is so restrictive about what it will understand?

[jmarolf]

Well MSBuild is a turing complete language. Supporting all patterns that are possible with it would be rather difficult in general. We need to draw the line somewhere. We could try and convince the nuget team to support Versions.props natively but I would also argue that arcade lies far outside the design of anything considered normal by .NET tooling. Supporting arcade in VS tooling implies that we think these patterns are normal and, frankly, they aren't. I do not want to hear that some external company has adopted the pattern of a using a powershell script to download a nuget package that contains a Tools.proj file that they use to download vswhere to find where msbuild is. I would like the dotnet restore and dotnet build patterns to be the norm and I would prefer that we acquiesce to the Directory.Packages.props pattern.

[tmat]

@jmarolf

My approach is going to be to import Versions.props into Directory.Packages.props and call it a day. Two (2) of our dependencies are updated by the Maestro infrastructure and they are arcade itself and the compilers we build with. I am ok with those dependencies not being viewable/editable in the UI/Nuget tooling as they are not libraries we depend on but infrastructure.

I think that's a good approach short term for Roslyn repo. I'm not saying we should block this Roslyn change and wait for Maestro to implement support for central package versioning. However, I disagree this is a good approach long term. As I already mentioned Versions.props was originally conceived to work around the lack of Project System/msbuild support for central package versioning. Since that feature is implemented now we should work towards removing the workaround.

Supporting arcade in VS tooling implies that we think these patterns are normal and, frankly, they aren't.

Exactly my argument. We need to migrate Arcade to patterns that we want our customers to use. I think you are contradicting yourself a bit - on one hand you're saying that it's ok that Meastro doesn't support for Directory.Packages.props but then you're saying that Project System shouldn't add support for Versions.props because it's not what customers should be using.

[tmat]

I do not want to hear that some external company has adopted the pattern of a using a powershell script to download a nuget package that contains a Tools.proj file that they use to download vswhere to find where msbuild is. I would like the dotnet restore and dotnet build patterns to be the norm and I would prefer that we acquiesce to the Directory.Packages.props pattern.

Definitely. All this is there to work around limitations in our products (msbuild, SDK, VSSDK, NuGet etc.). We wouldn't need a powershell script to find Desktop msbuild if we could build all repos on .NET Core. We could significantly simplify this stuff. Currently we can't use dotnet restore and dotnet build because our repos build VSIX packages and that does not work on .NET Core. The path forward here is to make VSIX packages buildable on .NET Core and then remove the Desktop msbuild logic completely.

[jmarolf]

totally agree that the approach arcade is taking is the best we can do today for a myriad of reasons.

My concern is that Version.props is about _much_ more than central package management:

• centralized version numbers for assemblies and nuget packages we ship
• automating dependency flow between repositories
• opting into sdk functionality

Whereas Directory.Packages.props is only about central package management.

We can look at productizing the other responsibilities of Version.props but we will need to design and implement those separately imho.

[tmat]

You are right, there is more in Version.props than package versions - I should have been more specific. What I meant above was only regarding package versions. Everything else would stay in Version.props. That is for Maestro to update versions in Directory.Packages.props based on Versions.xml (dependency flowing) and we would remove the {PackageName}Version variables from Version.props and from our PackageReferences.

[tmat]

We can look at productizing the other responsibilities of Version.props but we will need to design and implement those separately imho.

I agree. I don't think that's really necessary. Let's just tackle package versions for now.

[jmarolf]

I think we are aggressively agreeing here. So here is the work I think I am signing up for:

1. Update roslyn to .NET 5 RC2 (which will have the nuget fix)
2. Add Directory.Packages.props file to the repo root
3. Remove {PackageName}Version variables from Version.props
4. Submit a PR to maetro to be aware of Directory.Packages.props and update them as necessary

[am11]

We need an SDK and MSBuild that contains this bugfix

This bugfix has made it in 5.0 final and master branches of SDK and arcade repos. MSBuild, however, is still using arcade 3.x branch and 5.0 update work is tracked by https://github.com/dotnet/msbuild/issues/5515.