Home: Breaking change introduced by merging NuGet.Packaging.Core.Types into NuGet.Packaging.Core

[sbomer]

Details about Problem

I have a tasks package with a dependency on NuGet.ProjectModel, which pulls in the dependencies NuGet.Packaging.Core.Types.dll and NuGet.Packaging.Core.dll. When I run this on recent SDKs, a newer NuGet.Packaging.Core.dll gets loaded, and I see the following:

error MSB4018: System.MissingMethodException: Method not found: 'System.Collections.Generic.IList`1<NuGet.Packaging.Core.PackageDependency> NuGet.ProjectModel.LockFileTargetLibrary.get_Dependencies() cil managed'.

Looking into this, I found that the problem is that the version of NuGet.Packaging.Core.dll I compiled with expects the type PackageDependency to exist in NuGet.Packaging.Core.Types.dll:

.method public hidebysig specialname instance class [System.Runtime]System.Collections.Generic.IList`1<class [NuGet.Packaging.Core.Types]NuGet.Packaging.Core.PackageDependency>
get_Dependencies() cil managed

but the version of NuGet.Packaging.Core.dll loaded at runtime expects it to be in NuGet.Packaging.Core.dll:

.method public hidebysig specialname instance class [System.Runtime]System.Collections.Generic.IList`1<class [NuGet.Packaging.Core]NuGet.Packaging.Core.PackageDependency>
get_Dependencies() cil managed

It looks like the type PackageDependency was moved from NuGet.Packaging.Core.Types to NuGet.Packaging.Core in https://github.com/NuGet/NuGet.Client/commit/e4fa62f7924b55f275a5da116ca31b30ffe271ff (thanks @ericstj!).

I can work around this in my case, but wanted to report the breaking change along with the example of how this impacted my package, because it could cause problems for other NuGet consumers who may not be able to control their dependency versions or recompile their assemblies.

[emgarten]

Thanks for the report @sbomer, 4.3.0 contains some long planned refactoring to reduce the number of nuget assemblies.

Do you have any general advice for how scenarios like this could be improved with packages? If NuGet.Packaging.Core contained a type forwarding assembly for NuGet.Packaging.Core.Types would it solve this?

[ericstj]

@emgarten typically that's what we'd do if we moved types in .NET. In this case you moved types up, so if you fix it, make sure you test the case where someone as an old reference and new in the same graph. IIRC NuGet doesn't like it when packages invert their dependencies between versions and may flag it as a cycle.

[mconnew]

FYI, this has broken OmniSharp usage in vscode when using the latest .Net Core SDK. OmniSharp calls into the assembly Microsoft.NET.Build.Tasks.dll from the SDK which compiled against the older nuget assemblies (which are included with the SDK). OmniSharp also has copies of the nuget assemblies as it directly references types in them and was compiled against the newer assemblies. Microsoft.NET.Build.Tasks is picking up the versions in the OmniSharp folder and failing to find the type PackageDependency. The currently is no way to fix this without either recompiling the SDK or recompiling OmniSharp.

[dagood]

This is also currently breaking build infrastructure in the .NET Core CLI repository.

The "Microsoft.DotNet.VersionTools" package uses PackageReaderBase.GetIdentity() to extract package IDs from a nupkg file, and it's built using an old version of NuGet. The CLI repository does need to upgrade to new NuGet versions regularly. Currently the fix would be to upgrade and recompile VersionTools, but there are other users this break would affect without any benefit. It would be much better if the break were fixed in the NuGet packages so CLI can upgrade again and the break goes away.

The specific break we hit is that the return type NuGet.Packaging.Core.PackageIdentity was moved.

/cc @weshaggard @eerhardt @livarcocc

[emgarten]

@dagood where is the conflicting version coming from, is this conflicting simply with the CLI or MSBuild having a copy of NuGet?

[eerhardt]

The conflicting version comes from:

1. The CLI is using the new NuGet version, and that version of NuGet sits next to MSBuild. So it always wins.
2. The CLI's build is executing a Task that depends on an external package called Microsoft.DotNet.VersionTools which has a dependency on the old version of NuGet. See https://dotnet.myget.org/feed/dotnet-buildtools/package/nuget/Microsoft.DotNet.VersionTools NuGet.Packaging (>= 4.0.0-rc-2129).

So when the task calls into VersionTools to execute its logic, and it tries to call into NuGet it fails because the new version is loaded with the ABI breaking change.

[emgarten]

and that version of NuGet sits next to MSBuild. So it always wins

I think that is what needs to be fixed. Tracked here: https://github.com/dotnet/cli/issues/4214

NuGet is going to have breaking API changes at times, and more and more tools are going to use NuGet.* libraries. Either we need to fix how NuGet is loaded in the CLI, or a merged version of the assemblies needs to be used either by the CLI or tools so that this won't conflict.

[weshaggard]

NuGet is going to have breaking API changes at times

Why do we think it is OK for nuget to have breaking API changes? For better or worse it is a library that used by lots of folks so we should go out of our way to not have API breaking changes.

[emgarten]

While I agree NuGet could do better on avoiding breaking API changes, it hasn't been treated as a framework like this in the past. In the case of Omnisharp the problem occurs between preview builds of NuGet, which means that every build of NuGet needs to be compatible with every other build, not just RTM releases.

Also, even if there were zero breaking API changes, older versions of NuGet would still be loaded from the CLI when a user is trying to use a higher version, which would lead to missing methods.

[ericstj]

Build-to-build breaking changes don't matter. It's release-to-release that can't break.

Missing methods won't occur if you version your assembly when you add API.

[dasMulli]

It's release-to-release that can't break.

This is what broke OmniSharp https://github.com/OmniSharp/omnisharp-vscode/issues/1495 (looks like a new mono version with new SxS loading behaviour is going to help resolving this)

At the moment, I can get this to work for .NET Core SDK 1.0.0->2.0.0-preview1, or I can get it to work for .NET Core SDK 2.0.0-preview2.

[emgarten]

Missing methods won't occur if you version your assembly when you add API.

NuGet has started changing the version number for preview releases to solve that.
Preview2: 4.3.0.2
Preview3: 4.3.0.3

[emgarten]

@dasMulli does the version change help for mono, or there are still more issues?

[dasMulli]

@emgarten This work is done by @DustinCampbell, I'm just following along (and had similar problems with a custom tool and https://github.com/dotnet/cli/issues/4214)

[DustinCampbell]

Note: a new version of Mono (5.2) is required which just came out today on the alpha channel. It adds a new flag (--assembly-loader=strict) that you can set to get SxS assembly loading behavior that is similar to desktop CLR.

I really hope this refactoring was worth the pain it caused. 😞