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.21k stars 1.35k forks source link

[Bug]: No way to load NuGet.Frameworks into Default ALC #10228

Closed hknielsen closed 1 day ago

hknielsen commented 3 months ago

Issue Description

While using in-proc BuildManager, theres no way to properly handle NuGet.Frameworks loading. https://github.com/dotnet/msbuild/blob/main/src/Build/Utilities/NuGetFrameworkWrapper.cs#L56

assemblyName are never set while running in none msbuild.exe or Visual Studio, and as far as I can tell, its loaded into a separate temp ALC, and not the default as the rest of the Microsoft Build assemblies. Either it should be possible to setup that one want it to be loaded into the Default ALC, or the CurrentReflectionContext ALC.

With this current behavior im having a hard time not having to include the NuGet.Frameworks.dll published, where it needs to be the same version as the NuGet.Frameworks.dll in the dotnet SDK.

Steps to Reproduce

Have ie. ProjectCreator nuget package or other assemblies that depends on Nuget.Frameworks dll. Use BuildManager In-Proc to restore projects. If published version dont match as the one in the SDK, theres file load failures.

Have the ability to load the Nuget.Frameworks dll in the Default ALC or CurrentReflectionContext should help

Expected Behavior

NuGet.Frameworks.dll is loaded correctly, either as part as the MSBuild Locator, or in a more controlled ALC setup.

Actual Behavior

NuGet.Frameworks.dll needs to be published, and need to match exact dll version as the dotnet sdk, making it hard on dev machines as dotnet sdk is tied to the local installed version.

Analysis

No response

Versions & Configurations

No response

YuliiaKovalova commented 3 months ago

Hi @hknielsen,

Thank you for reporting this issue.

It appears to have the same symptoms as described in issue #10213. We plan to address it as part of the MSBuildLocator API v2 updates (see microsoft/MSBuildLocator#282). I am currently compiling a list of breaking changes, and your contributions to this list are welcome.

Please stay tuned - this issue is on our roadmap.

hknielsen commented 3 months ago

Hi @YuliiaKovalova, Thanks, and it would be great if the MSBuildLocator could handle Nuget.Framework loading. That said, MSBuild also need to have some changes because of the code I linked :) So we have control of where and what is loaded into ALC's

hknielsen commented 2 weeks ago

@YuliiaKovalova as this is going to be a breaking change, is there any chance this will be in MSBuild that will follow the net9 timeline? (since this needs msbuild changes as well).

YuliiaKovalova commented 2 weeks ago

Hi @hknielsen,

We will triage this ticket and I will let you know the priority of it. Stay tuned!

YuliiaKovalova commented 4 days ago

Hi @hknielsen,

I've thoroughly investigated this ticket and identified that the root cause is related to the runtime's assembly loading mechanism. To illustrate the issue, I've prepared a sample project which I'll attach to this ticket. Here's a breakdown of the situation:

  1. The main project targets .NET 9.0 and loads NuGet.Frameworks.dll from the SDK folder.

  2. ProjectCreator has a dependency on an older version of NuGet.Frameworks.dll than the one already loaded in the Assembly Load Context (ALC); {C6A7784F-13A2-48B2-9C6E-2166C46335CD}

  3. The runtime detects a NuGet.Frameworks.dll assembly in the bin folder adjacent to the executable. Consequently, it attempts to load this version instead of the one from the SDK folder ON PROJECT CREATION.

This conflict in assembly versions leads to the observed issue. As a workaround, it's possible to catch the assembly resolution attempt and return it:

        AppDomain.CurrentDomain.AssemblyResolve += (sender, eventArgs) =>
        {
            var assemblyName = new AssemblyName(eventArgs.Name);

            var loadedAssembly = AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(a => a.GetName().Name == assemblyName.Name);

            if (loadedAssembly != null)
            {
                return loadedAssembly;
            }

            return null;
        };

repro_sample.zip

Please let us know if this workaround helps. We're documenting restrictions for such libraries as part of https://github.com/dotnet/msbuild/issues/10213.

hknielsen commented 3 days ago

@YuliiaKovalova it does not work, at least not for our test projects. I think it might be related that its tests and the test system's load in all the assemblies thats in the build folder? I tried both NUnit and XUnit, with their adapters, and in both cases I cant get it to work. Let me know if theres other things I can try :)

hknielsen commented 3 days ago

Yeah confirmed its loaded before the test runs, and even setting up a ModuleInitializer to setup the AssemblyResolve its been loaded.

hknielsen commented 3 days ago

@YuliiaKovalova This repro gives the exact error that I run into; TestProject1.zip

I didnt add the AssemblyResolve, but adding it does not help. As you can see we use net8, but that should not make any difference I would think, in this case.

YuliiaKovalova commented 3 days ago

I see your problem.

In the meantime, I have checked that reference to Nuget.Frameworks was removed from MSBuild.ProjectCreation starting from version 11.0.0 https://github.com/jeffkl/MSBuildProjectCreator/compare/v10.0.0...v11.0.0

Could you upgrade the version of this package ? For me MSBuild.ProjectCreation v13.0.0 works fine.

hknielsen commented 3 days ago

Upgrading that, then I need to also update the Microsoft.Build - tried that, but then have the same issue as Microsoft.NET.Test.Sdk package also have a dependency on NuGet.Frameworks

hknielsen commented 3 days ago

Oh wait, upgrading that as that also has removed that dependency, let me try on our real project

hknielsen commented 1 day ago

@YuliiaKovalova that worked! Tests on various IDE's and computers, with various dotnet sdk's. Thanks for the help, this so nice to get solved :)

YuliiaKovalova commented 1 day ago

@hknielsen thank you for updates! I am sure this discussion will be useful for other customers too.

Have a nice day!