MSBuild assembly loading should use greater than or equal rather than exact assembly version comparisons

[dsplaisted]

The MSBuild assembly loading code has checks that the resolved assembly version exactly matches the requested version:

msbuild/src/Shared/MSBuildLoadContext.cs

Line 66 in 808b2ae

if (candidateAssemblyName.Version != assemblyName.Version)

msbuild/src/Shared/CoreCLRAssemblyLoader.cs

Line 167 in 808b2ae

if (candidateAssemblyName.Version != assemblyName.Version)

These should probably be changed to instead check that the resolved version is greater than or equal to the requested version, rather than checking for an exact match.

This caused a failure in creating the 6.0.100 build, in this commit: https://github.com/dotnet/sdk/tree/ff7b275720f88e6d5773c748eb6d320bc1365d63

When building with this SDK, the following exception would occur in the RestoreTask:

System.MissingMethodException: Method not found: 'Void NuGet.RuntimeModel.JsonObjectWriter..ctor(Newtonsoft.Json.JsonWriter)'.
at NuGet.ProjectModel.LockFileFormat.WriteLockFile(LockFile lockFile)
at NuGet.ProjectModel.LockFileFormat.Write(TextWriter textWriter, LockFile lockFile)
at NuGet.ProjectModel.LockFileFormat.Write(Stream stream, LockFile lockFile)
at NuGet.ProjectModel.LockFileFormat.Write(String filePath, LockFile lockFile)
at NuGet.Commands.RestoreResult.<>c__DisplayClass48_0.b__2(String outputPath)
at NuGet.Common.FileUtility.Replace(Action1 writeSourceFile, String destFilePath) at NuGet.Commands.RestoreResult.CommitAssetsFileAsync(LockFileFormat lockFileFormat, IRestoreResult result, ILogger log, Boolean toolCommit, CancellationToken token) at NuGet.Commands.RestoreResult.CommitAsync(ILogger log, CancellationToken token) at NuGet.Commands.RestoreRunner.CommitAsync(RestoreResultPair restoreResult, CancellationToken token) at NuGet.Commands.RestoreRunner.ExecuteAndCommitAsync(RestoreSummaryRequest summaryRequest, CancellationToken token) at NuGet.Commands.RestoreRunner.CompleteTaskAsync(List1 restoreTasks)
at NuGet.Commands.RestoreRunner.RunAsync(IEnumerable`1 restoreRequests, RestoreArgs restoreContext, CancellationToken token)
at NuGet.Commands.RestoreRunner.RunAsync(RestoreArgs restoreContext, CancellationToken token)
at NuGet.Build.Tasks.BuildTasksUtility.RestoreAsync(DependencyGraphSpec dependencyGraphSpec, Boolean interactive, Boolean recursive, Boolean noCache, Boolean ignoreFailedSources, Boolean disableParallel, Boolean force, Boolean forceEvaluate, Boolean hideWarningsAndErrors, Boolean restorePC, Boolean cleanupAssetsForUnsupportedProjects, ILogger log, CancellationToken cancellationToken)
at NuGet.Build.Tasks.RestoreTask.ExecuteAsync(ILogger log)

This happened because this SDK had different versions of some NuGet DLLs. (The reason for this was that dotnet/templating needed to update to build with the final 6.0.0 NuGet packages. That transitively updated the NuGet packages that templating depended on to 6.0.0 in the SDK repo. However, there were other NuGet packages that the SDK depended on which did not get updated, and were still using version 6.0.0-rc.262).

Concretely, the failure happened like this (or close to it, I believe):

The NuGet RestoreTask depended on NuGet.ProjectModel, and NuGet.ProjectModel depended on Newtonsoft.Json. For both of these references, the exact version of the DLL that was referenced was found, so per the code in MSBuildLoadContext, these DLLs would be loaded into the MSBuildLoadContext AssemblyLoadContext.

NuGet.ProjectModel also depended on NuGet.Packaging. However, NuGet.ProjectModel was from the 6.0.0-rc.262 package, while the SDK was using the later 6.0.0 package for NuGet.Packaging. So the resolved assembly version of NuGet.Packaging did not exactly match the requested version. This caused the code in MSBuildLoadContext to not load NuGet.Packaging in that ALC. Rather, it returned null, which caused the load to fall back to the default load context. So NuGet.Packaging was loaded in the default assembly load context, and its dependency on Newtonsoft.Json was also loaded in the default ALC.

So when there was a call from NuGet.ProjectModel to NuGet.Packaging with a method parameter type that came from Newtonsoft.Json, the two assemblies did not agree on the assembly that type came from: NuGet.ProjectModel was using the version of Newtonsoft.Json from the MSBuildLoadContext ALC, and NuGet.Packaging was using the version of Newtonsoft.Json from the default ALC. Hence the MissingMethodException.

I think the way to fix this is to update the check to verify that the candidate assembly's version is greater than or equal to the requested version:

if (candidateAssemblyName.Version != assemblyName.Version)

That way the dependencies of a task will be loaded in the task's ALC, whether or not the version matches exactly.

There is similar code in CoreCLRAssemblyLoader. I don't think we hit that code path for this failure, but it should probably also be changed.

[Forgind]

Couple questions:
When I was looking at the code, I noticed that dotnet/sdk@87aaa50 removed most of the -rtm.version parts but did not do so for Microsoft.TemplateEngine.Cli. Is that an intentional version mismatch for some reason? Why?
The change sounds very simple: replace != with >= in two (more?) places and possibly modify the default behavior if one is null. I thought the repro was to go to the commit you mentioned and try build.cmd /restore, but that succeeded on my computer. Do I need to have some specific nuget packages installed and others not? Do I need to use the newly built SDK to build something else?

[dsplaisted]

I think you need to build this commit: https://github.com/dotnet/sdk/tree/ff7b275720f88e6d5773c748eb6d320bc1365d63

You may also need to clear your NuGet global packages folder, as there were multiple different versions of the 6.0.0 NuGet packages produced.

Also for the SDK repo it restores by default, so it's possible that specifying /restore would mean it would only restore and not build.

[rainersigwald]

Reactivating since this was reverted in #7415.