Skip to content

Conversation

@brianrob
Copy link
Member

@brianrob brianrob commented Mar 25, 2021

Fixes #6289

Context

#6126 changes the way that NuGet.Frameworks.dll is loaded to be used by NuGetFrameworkWrapper from a call to LoadFile to a call to LoadFrom. The path passed in to the Load*` call is the same in both cases, but the behavior is different when running on .NET Framework.

On .NET Framework, calling LoadFile results in IJW loading rules being followed, and a different copy of NuGet.Frameworks.dll
than the one requested being loaded. It also appears to match the one loaded into the LOAD context. Calling LoadFrom results in the specified assembly being loaded, but it doesn't match the copy of the assembly loaded into the LOAD context. Thus, it remains in the LOADFROM context instead of being promoted to the LOAD context.

Later on, there is a collision between code from the two instances of NuGet.Frameworks.dll that are loaded into the LOAD context and the LOADFROM context, and this is where the MissingMethodException is thrown.

Note, this does not happen on the .NET Core bootstrap build because the loader behavior is significantly simpler.

Changes Made

Choose the Load* API based on the target framework at build time. On .NET Core, use LoadFrom and on .NET Framework, use LoadFile. This type of precedent already exists in MSBuild where there is different load behavior for .NET Framework and .NET Core.

Testing

Tested local bootstrap builds that were built on .NET Framework and .NET Core using the repro in #6289.

Copy link
Member

@KirillOsenkov KirillOsenkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fantastic, thanks @brianrob!

Also thanks to @davkean for looking.

@KirillOsenkov
Copy link
Member

I've filed #6302 to find a more robust solution long term.

Copy link
Member

@benvillalobos benvillalobos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a very interesting conversation to follow on teams 🙂

@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Mar 26, 2021
@Forgind Forgind merged commit 62f3a5e into dotnet:main Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to build using bootstrap: Method not found: 'NuGet.ProjectModel.LockFile.GetTarget'

4 participants