Respect deps.json (#7520)

Fixes #4081 and Fixes #1887; progress towards #5037 Context MSBuild doesn't currently respect .deps.json files for plugins (tasks). This can lead to incorrect versions of assemblies being found as finding the AnyCPU version of an assembly instead of the windows-specific version. Changes Made Use AssemblyDependencyResolver as a second pass (after looking for "well-known assemblies") to automatically use the deps.json file to find the right assembly. Testing Verified that for a task assembly with a rid-specific dependency, it finds the rid-specific dependency as specified by the deps.json file. Also verified that it can find native assemblies and that the issue that inspired this (dotnet/sdk#23498) no longer reproduces after giving nuget a .deps.json file specifying the correct version.
dotnet · Apr 19, 2022 · 75795a4 · 75795a4
1 parent 67deba3
commit 75795a4
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 17 deletions.
diff --git a/src/Framework/ChangeWaves.cs b/src/Framework/ChangeWaves.cs
@@ -27,7 +27,8 @@ internal class ChangeWaves
         internal static readonly Version Wave16_10 = new Version(16, 10);
         internal static readonly Version Wave17_0 = new Version(17, 0);
         internal static readonly Version Wave17_2 = new Version(17, 2);
-        internal static readonly Version[] AllWaves = { Wave16_10, Wave17_0, Wave17_2 };
+        internal static readonly Version Wave17_4 = new Version(17, 4);
+        internal static readonly Version[] AllWaves = { Wave16_10, Wave17_0, Wave17_2, Wave17_4 };
 
         /// <summary>
         /// Special value indicating that all features behind all Change Waves should be enabled.

diff --git a/src/Shared/MSBuildLoadContext.cs b/src/Shared/MSBuildLoadContext.cs
@@ -1,7 +1,10 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using Microsoft.Build.Framework;
 using Microsoft.Build.Shared.FileSystem;
+
+using System;
 using System.Collections.Immutable;
 using System.IO;
 using System.Reflection;
@@ -15,6 +18,8 @@ namespace Microsoft.Build.Shared
     /// </summary>
     internal class MSBuildLoadContext : AssemblyLoadContext
     {
+        private AssemblyDependencyResolver? _resolver;
+
         private readonly string _directory;
 
         internal static readonly ImmutableHashSet<string> WellKnownAssemblyNames =
@@ -31,6 +36,8 @@ public MSBuildLoadContext(string assemblyPath)
             : base($"MSBuild plugin {assemblyPath}")
         {
             _directory = Directory.GetParent(assemblyPath)!.FullName;
+
+            _resolver = File.Exists(assemblyPath) ? new AssemblyDependencyResolver(assemblyPath) : null;
         }
 
         protected override Assembly? Load(AssemblyName assemblyName)
@@ -42,6 +49,19 @@ public MSBuildLoadContext(string assemblyPath)
                 return null;
             }
 
+            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
+            {
+                // respect plugin.dll.json with the AssemblyDependencyResolver
+                string? assemblyPath = _resolver?.ResolveAssemblyToPath(assemblyName);
+                if (assemblyPath != null)
+                {
+                    return LoadFromAssemblyPath(assemblyPath);
+                }
+            }
+
+            // Fall back to the older MSBuild-on-Core behavior to continue to support
+            // plugins that don't ship a .deps.json
+
             foreach (var cultureSubfolder in string.IsNullOrEmpty(assemblyName.CultureName)
                 // If no culture is specified, attempt to load directly from
                 // the known dependency paths.
@@ -73,7 +93,6 @@ public MSBuildLoadContext(string assemblyPath)
             // - the assembly from the user specified path is loaded, if it exists, into the custom ALC, or
             // - if the simple name of the assembly exists in the same folder as msbuild.exe, then that assembly gets loaded
             //   into the default ALC (so it's shared with other uses).
-
             var assemblyNameInExecutableDirectory = Path.Combine(BuildEnvironmentHelper.Instance.CurrentMSBuildToolsDirectory,
                 $"{assemblyName.Name}.dll");
 
@@ -84,5 +103,19 @@ public MSBuildLoadContext(string assemblyPath)
 
             return null;
         }
+
+        protected override IntPtr LoadUnmanagedDll(string unmanagedDllName)
+        {
+            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
+            {
+                string? libraryPath = _resolver?.ResolveUnmanagedDllToPath(unmanagedDllName);
+                if (libraryPath != null)
+                {
+                    return LoadUnmanagedDllFromPath(libraryPath);
+                }
+            }
+
+            return base.LoadUnmanagedDll(unmanagedDllName);
+        }
     }
 }
diff --git a/src/Shared/UnitTests/TypeLoader_Tests.cs b/src/Shared/UnitTests/TypeLoader_Tests.cs
@@ -6,8 +6,10 @@
 using System.IO;
 using Microsoft.Build.Shared;
 using System.Reflection;
-using Xunit;
 using Microsoft.Build.UnitTests.Shared;
+using Xunit;
+using Xunit.Abstractions;
+using Shouldly;
 
 #nullable disable
 
@@ -19,6 +21,13 @@ public class TypeLoader_Tests
         private const string ProjectFileName = "portableTaskTest.proj";
         private const string DLLFileName = "PortableTask.dll";
 
+        private readonly ITestOutputHelper _output;
+
+        public TypeLoader_Tests(ITestOutputHelper testOutputHelper)
+        {
+            _output = testOutputHelper;
+        }
+
         [Fact]
         public void Basic()
         {
@@ -50,19 +59,18 @@ public void Regress_Mutation_ParameterOrderDoesntMatter()
         [Fact]
         public void LoadNonExistingAssembly()
         {
-            using (var dir = new FileUtilities.TempWorkingDirectory(ProjectFileFolder))
-            {
-                string projectFilePath = Path.Combine(dir.Path, ProjectFileName);
+            using var dir = new FileUtilities.TempWorkingDirectory(ProjectFileFolder);
 
-                string dllName = "NonExistent.dll";
+            string projectFilePath = Path.Combine(dir.Path, ProjectFileName);
 
-                bool successfulExit;
-                string output = RunnerUtilities.ExecMSBuild(projectFilePath + " /v:diag /p:AssemblyPath=" + dllName, out successfulExit);
-                Assert.False(successfulExit);
+            string dllName = "NonExistent.dll";
 
-                string dllPath = Path.Combine(BuildEnvironmentHelper.Instance.CurrentMSBuildToolsDirectory, dllName);
-                CheckIfCorrectAssemblyLoaded(output, dllPath, false);
-            }
+            bool successfulExit;
+            string output = RunnerUtilities.ExecMSBuild(projectFilePath + " /v:diag /p:AssemblyPath=" + dllName, out successfulExit, _output);
+            successfulExit.ShouldBeFalse();
+
+            string dllPath = Path.Combine(BuildEnvironmentHelper.Instance.CurrentMSBuildToolsDirectory, dllName);
+            CheckIfCorrectAssemblyLoaded(output, dllPath, false);
         }
 
         [Fact]
@@ -73,7 +81,7 @@ public void LoadInsideAsssembly()
                 string projectFilePath = Path.Combine(dir.Path, ProjectFileName);
 
                 bool successfulExit;
-                string output = RunnerUtilities.ExecMSBuild(projectFilePath + " /v:diag", out successfulExit);
+                string output = RunnerUtilities.ExecMSBuild(projectFilePath + " /v:diag", out successfulExit, _output);
                 Assert.True(successfulExit);
 
                 string dllPath = Path.Combine(dir.Path, DLLFileName);
@@ -95,7 +103,7 @@ public void LoadOutsideAssembly()
                 try
                 {
                     bool successfulExit;
-                    string output = RunnerUtilities.ExecMSBuild(projectFilePath + " /v:diag /p:AssemblyPath=" + movedDLLPath, out successfulExit);
+                    string output = RunnerUtilities.ExecMSBuild(projectFilePath + " /v:diag /p:AssemblyPath=" + movedDLLPath, out successfulExit, _output);
                     Assert.True(successfulExit);
 
                     CheckIfCorrectAssemblyLoaded(output, movedDLLPath);
@@ -119,7 +127,7 @@ public void LoadInsideAssemblyWhenGivenOutsideAssemblyWithSameName()
                 try
                 {
                     bool successfulExit;
-                    string output = RunnerUtilities.ExecMSBuild(projectFilePath + " /v:diag /p:AssemblyPath=" + copiedDllPath, out successfulExit);
+                    string output = RunnerUtilities.ExecMSBuild(projectFilePath + " /v:diag /p:AssemblyPath=" + copiedDllPath, out successfulExit, _output);
                     Assert.True(successfulExit);
 
                     CheckIfCorrectAssemblyLoaded(output, originalDLLPath);

diff --git a/src/Tasks/CombineTargetFrameworkInfoProperties.cs b/src/Tasks/CombineTargetFrameworkInfoProperties.cs
@@ -40,7 +40,7 @@ public override bool Execute()
             if (PropertiesAndValues != null)
             {
                 // When removing the change wave, also remove UseAttributeForTargetFrameworkInfoPropertyNames.
-                XElement root = ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_2) || UseAttributeForTargetFrameworkInfoPropertyNames ?
+                XElement root = ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4) || UseAttributeForTargetFrameworkInfoPropertyNames ?
                     new("TargetFramework", new XAttribute("Name", EscapingUtilities.Escape(RootElementName))) :
                     new(RootElementName);