handle _all_ instances of global.json when initializing MSBuild (#9511

)
dependabot · Apr 16, 2024 · 2e9903a · 2e9903a
1 parent 94df76f
commit 2e9903a
Show file tree

Hide file tree

Showing 15 changed files with 72 additions and 87 deletions.
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs
@@ -293,22 +293,30 @@ public async Task WithDirsProjAndDirectoryBuildPropsThatIsOutOfDirectoryButStill
             );
         }
 
-        [Fact]
-        public async Task UpdaterDoesNotUseRepoGlobalJsonForMSBuildTasks()
+        [Theory]
+        [InlineData(null)]
+        [InlineData("src")]
+        public async Task UpdaterDoesNotUseRepoGlobalJsonForMSBuildTasks(string? workingDirectoryPath)
         {
             // This is a _very_ specific scenario where the `NuGetUpdater.Cli` tool might pick up a `global.json` from
             // the root of the repo under test and use it's `sdk` property when trying to locate MSBuild.  To properly
             // test this, it must be tested in a new process where MSBuild has not been loaded yet and the runner tool
             // must be started with its working directory at the test repo's root.
             using var tempDir = new TemporaryDirectory();
-            await File.WriteAllTextAsync(Path.Join(tempDir.DirectoryPath, "global.json"), """
+            var globalJsonPath = Path.Join(tempDir.DirectoryPath, "global.json");
+            var srcGlobalJsonPath = Path.Join(tempDir.DirectoryPath, "src", "global.json");
+            string globalJsonContent = """
                 {
                   "sdk": {
                     "version": "99.99.99"
                   }
                 }
-                """);
-            await File.WriteAllTextAsync(Path.Join(tempDir.DirectoryPath, "project.csproj"), """
+                """;
+            await File.WriteAllTextAsync(globalJsonPath, globalJsonContent);
+            Directory.CreateDirectory(Path.Join(tempDir.DirectoryPath, "src"));
+            await File.WriteAllTextAsync(srcGlobalJsonPath, globalJsonContent);
+            var projectPath = Path.Join(tempDir.DirectoryPath, "src", "project.csproj");
+            await File.WriteAllTextAsync(projectPath, """
                 <Project Sdk="Microsoft.NET.Sdk">
                   <PropertyGroup>
                     <TargetFramework>net8.0</TargetFramework>
@@ -325,7 +333,7 @@ public async Task UpdaterDoesNotUseRepoGlobalJsonForMSBuildTasks()
                 "--repo-root",
                 tempDir.DirectoryPath,
                 "--solution-or-project",
-                Path.Join(tempDir.DirectoryPath, "project.csproj"),
+                projectPath,
                 "--dependency",
                 "Newtonsoft.Json",
                 "--new-version",
@@ -336,15 +344,25 @@ public async Task UpdaterDoesNotUseRepoGlobalJsonForMSBuildTasks()
             ]);
 
             // verify base run
-            var (exitCode, output, error) = await ProcessEx.RunAsync(executableName, executableArgs, workingDirectory: tempDir.DirectoryPath);
+            var workingDirectory = tempDir.DirectoryPath;
+            if (workingDirectoryPath is not null)
+            {
+                workingDirectory = Path.Join(workingDirectory, workingDirectoryPath);
+            }
+
+            var (exitCode, output, error) = await ProcessEx.RunAsync(executableName, executableArgs, workingDirectory: workingDirectory);
             Assert.True(exitCode == 0, $"Error running update on unsupported SDK.\nSTDOUT:\n{output}\nSTDERR:\n{error}");
 
             // verify project update
-            var updatedProjectContents = await File.ReadAllTextAsync(Path.Join(tempDir.DirectoryPath, "project.csproj"));
+            var updatedProjectContents = await File.ReadAllTextAsync(projectPath);
             Assert.Contains("13.0.1", updatedProjectContents);
 
             // verify `global.json` untouched
-            var updatedGlobalJsonContents = await File.ReadAllTextAsync(Path.Join(tempDir.DirectoryPath, "global.json"));
+            var updatedGlobalJsonContents = await File.ReadAllTextAsync(globalJsonPath);
+            Assert.Contains("99.99.99", updatedGlobalJsonContents);
+
+            // verify `src/global.json` untouched
+            var updatedSrcGlobalJsonContents = await File.ReadAllTextAsync(srcGlobalJsonPath);
             Assert.Contains("99.99.99", updatedGlobalJsonContents);
         }
 

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/TestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/TestBase.cs
@@ -0,0 +1,10 @@
+namespace NuGetUpdater.Core.Test
+{
+    public abstract class TestBase
+    {
+        protected TestBase()
+        {
+            MSBuildHelper.RegisterMSBuild(Environment.CurrentDirectory, Environment.CurrentDirectory);
+        }
+    }
+}
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs
@@ -1,16 +1,9 @@
-using System.Collections.Generic;
-
 using Xunit;
 
 namespace NuGetUpdater.Core.Test.Update;
 
-public class PackagesConfigUpdaterTests
+public class PackagesConfigUpdaterTests : TestBase
 {
-    public PackagesConfigUpdaterTests()
-    {
-        MSBuildHelper.RegisterMSBuild();
-    }
-
     [Theory]
     [MemberData(nameof(PackagesDirectoryPathTestData))]
     public void PathToPackagesDirectoryCanBeDetermined(string projectContents, string dependencyName, string dependencyVersion, string expectedPackagesDirectoryPath)

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs
@@ -10,7 +10,7 @@ namespace NuGetUpdater.Core.Test.Update;
 using TestFile = (string Path, string Content);
 using TestProject = (string Path, string Content, Guid ProjectId);
 
-public abstract class UpdateWorkerTestBase
+public abstract class UpdateWorkerTestBase : TestBase
 {
     protected static Task TestNoChange(
         string dependencyName,

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DirsProj.cs
@@ -10,11 +10,6 @@ public partial class UpdateWorkerTests
 {
     public class DirsProj : UpdateWorkerTestBase
     {
-        public DirsProj()
-        {
-            MSBuildHelper.RegisterMSBuild();
-        }
-
         [Fact]
         public async Task UpdateSingleDependencyInDirsProj()
         {

diff --git a/...t/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DotNetTools.cs b/...t/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.DotNetTools.cs
@@ -8,11 +8,6 @@ public partial class UpdateWorkerTests
 {
     public class DotNetTools : UpdateWorkerTestBase
     {
-        public DotNetTools()
-        {
-            MSBuildHelper.RegisterMSBuild();
-        }
-
         [Fact]
         public async Task NoChangeWhenDotNetToolsJsonNotFound()
         {

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.GlobalJson.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.GlobalJson.cs
@@ -8,11 +8,6 @@ public partial class UpdateWorkerTests
 {
     public class GlobalJson : UpdateWorkerTestBase
     {
-        public GlobalJson()
-        {
-            MSBuildHelper.RegisterMSBuild();
-        }
-
         [Fact]
         public async Task NoChangeWhenGlobalJsonNotFound()
         {

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.Mixed.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.Mixed.cs
@@ -8,11 +8,6 @@ public partial class UpdateWorkerTests
 {
     public class Mixed : UpdateWorkerTestBase
     {
-        public Mixed()
-        {
-            MSBuildHelper.RegisterMSBuild();
-        }
-
         [Fact]
         public async Task ForPackagesProject_UpdatePackageReference_InBuildProps()
         {

diff --git a/...elpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs b/...elpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs
@@ -9,11 +9,6 @@ public partial class UpdateWorkerTests
 {
     public class PackagesConfig : UpdateWorkerTestBase
     {
-        public PackagesConfig()
-        {
-            MSBuildHelper.RegisterMSBuild();
-        }
-
         [Fact]
         public async Task UpdateSingleDependencyInPackagesConfig()
         {

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.Sdk.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.Sdk.cs
@@ -8,11 +8,6 @@ public partial class UpdateWorkerTests
 {
     public class Sdk : UpdateWorkerTestBase
     {
-        public Sdk()
-        {
-            MSBuildHelper.RegisterMSBuild();
-        }
-
         [Theory]
         [InlineData("net472")]
         [InlineData("netstandard2.0")]

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/MSBuildHelperTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/MSBuildHelperTests.cs
@@ -6,13 +6,8 @@ namespace NuGetUpdater.Core.Test.Utilities;
 
 using TestFile = (string Path, string Content);
 
-public class MSBuildHelperTests
+public class MSBuildHelperTests : TestBase
 {
-    public MSBuildHelperTests()
-    {
-        MSBuildHelper.RegisterMSBuild();
-    }
-
     [Fact]
     public void GetRootedValue_FindsValue()
     {

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs
@@ -26,7 +26,7 @@ public DiscoveryWorker(Logger logger)
 
     public async Task RunAsync(string repoRootPath, string workspacePath, string outputPath)
     {
-        MSBuildHelper.RegisterMSBuild();
+        MSBuildHelper.RegisterMSBuild(Environment.CurrentDirectory, repoRootPath);
 
         // When running under unit tests, the workspace path may not be rooted.
         if (!Path.IsPathRooted(workspacePath) || !Directory.Exists(workspacePath))

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs
@@ -12,7 +12,7 @@ public UpdaterWorker(Logger logger)
 
     public async Task RunAsync(string repoRootPath, string workspacePath, string dependencyName, string previousDependencyVersion, string newDependencyVersion, bool isTransitive)
     {
-        MSBuildHelper.RegisterMSBuild();
+        MSBuildHelper.RegisterMSBuild(Environment.CurrentDirectory, repoRootPath);
 
         if (!Path.IsPathRooted(workspacePath) || !File.Exists(workspacePath))
         {

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs
@@ -24,37 +24,31 @@ internal static partial class MSBuildHelper
 
     public static bool IsMSBuildRegistered => MSBuildPath.Length > 0;
 
-    static MSBuildHelper()
-    {
-        RegisterMSBuild();
-    }
-
-    public static void RegisterMSBuild()
+    public static void RegisterMSBuild(string currentDirectory, string rootDirectory)
     {
         // Ensure MSBuild types are registered before calling a method that loads the types
         if (!IsMSBuildRegistered)
         {
-            var globalJsonPath = "global.json";
-            var tempGlobalJsonPath = globalJsonPath + Guid.NewGuid().ToString();
-            var globalJsonExists = File.Exists(globalJsonPath);
-            try
+            var candidateDirectories = PathHelper.GetAllDirectoriesToRoot(currentDirectory, rootDirectory);
+            var globalJsonPaths = candidateDirectories.Select(d => Path.Combine(d, "global.json")).Where(File.Exists).Select(p => (p, p + Guid.NewGuid().ToString())).ToArray();
+            foreach (var (globalJsonPath, tempGlobalJsonPath) in globalJsonPaths)
             {
-                if (globalJsonExists)
-                {
-                    Console.WriteLine("Temporarily removing `global.json` for MSBuild detection.");
-                    File.Move(globalJsonPath, tempGlobalJsonPath);
-                }
+                Console.WriteLine($"Temporarily removing `global.json` from `{Path.GetDirectoryName(globalJsonPath)}` for MSBuild detection.");
+                File.Move(globalJsonPath, tempGlobalJsonPath);
+            }
 
+            try
+            {
                 var defaultInstance = MSBuildLocator.QueryVisualStudioInstances().First();
                 MSBuildPath = defaultInstance.MSBuildPath;
                 MSBuildLocator.RegisterInstance(defaultInstance);
             }
             finally
             {
-                if (globalJsonExists)
+                foreach (var (globalJsonpath, tempGlobalJsonPath) in globalJsonPaths)
                 {
-                    Console.WriteLine("Restoring `global.json` after MSBuild detection.");
-                    File.Move(tempGlobalJsonPath, globalJsonPath);
+                    Console.WriteLine($"Restoring `global.json` to `{Path.GetDirectoryName(globalJsonpath)}` after MSBuild discovery.");
+                    File.Move(tempGlobalJsonPath, globalJsonpath);
                 }
             }
         }

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/PathHelper.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/PathHelper.cs
@@ -34,20 +34,11 @@ public static string JoinPath(string? path1, string path2)
     public static string GetFullPathFromRelative(string rootPath, string relativePath)
         => Path.GetFullPath(JoinPath(rootPath, relativePath.NormalizePathToUnix()));
 
-    /// <summary>
-    /// Check in every directory from <paramref name="initialPath"/> up to <paramref name="rootPath"/> for the file specified in <paramref name="fileName"/>.
-    /// </summary>
-    /// <returns>The path of the found file or null.</returns>
-    public static string? GetFileInDirectoryOrParent(string initialPath, string rootPath, string fileName, bool caseSensitive = true)
+    public static string[] GetAllDirectoriesToRoot(string initialDirectoryPath, string rootDirectoryPath)
     {
-        if (File.Exists(initialPath))
-        {
-            initialPath = Path.GetDirectoryName(initialPath)!;
-        }
-
         var candidatePaths = new List<string>();
-        var rootDirectory = new DirectoryInfo(rootPath);
-        var candidateDirectory = new DirectoryInfo(initialPath);
+        var rootDirectory = new DirectoryInfo(rootDirectoryPath);
+        var candidateDirectory = new DirectoryInfo(initialDirectoryPath);
         while (candidateDirectory.FullName != rootDirectory.FullName)
         {
             candidatePaths.Add(candidateDirectory.FullName);
@@ -58,8 +49,22 @@ public static string GetFullPathFromRelative(string rootPath, string relativePat
             }
         }
 
-        candidatePaths.Add(rootPath);
+        candidatePaths.Add(rootDirectoryPath);
+        return candidatePaths.ToArray();
+    }
+
+    /// <summary>
+    /// Check in every directory from <paramref name="initialPath"/> up to <paramref name="rootPath"/> for the file specified in <paramref name="fileName"/>.
+    /// </summary>
+    /// <returns>The path of the found file or null.</returns>
+    public static string? GetFileInDirectoryOrParent(string initialPath, string rootPath, string fileName, bool caseSensitive = true)
+    {
+        if (File.Exists(initialPath))
+        {
+            initialPath = Path.GetDirectoryName(initialPath)!;
+        }
 
+        var candidatePaths = GetAllDirectoriesToRoot(initialPath, rootPath);
         foreach (var candidatePath in candidatePaths)
         {
             try