report specific error when project cannot be restored (#10720)

Co-authored-by: kbukum1 <kbukum1@github.com>
dependabot · Oct 7, 2024 · bfc7c91 · bfc7c91
1 parent 8dfab4b
commit bfc7c91
Show file tree

Hide file tree

Showing 19 changed files with 245 additions and 35 deletions.
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs
@@ -1,8 +1,10 @@
 using System.Text;
+using System.Text.Json;
 using System.Xml.Linq;
 
 using NuGetUpdater.Core;
 using NuGetUpdater.Core.Analyze;
+using NuGetUpdater.Core.Discover;
 using NuGetUpdater.Core.Test;
 using NuGetUpdater.Core.Test.Analyze;
 using NuGetUpdater.Core.Test.Update;
@@ -334,6 +336,11 @@ private static async Task RunAsync(Func<string, string[]> getArgs, string depend
                     Console.SetOut(originalOut);
                     Console.SetError(originalErr);
                 }
+
+                var resultPath = Path.Join(path, AnalyzeWorker.AnalysisDirectoryName, $"{dependencyName}.json");
+                var resultJson = await File.ReadAllTextAsync(resultPath);
+                var resultObject = JsonSerializer.Deserialize<AnalysisResult>(resultJson, DiscoveryWorker.SerializerOptions);
+                return resultObject!;
             });
 
             ValidateAnalysisResult(expectedResult, actualResult);

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs
@@ -1,4 +1,5 @@
 using System.Text;
+using System.Text.Json;
 
 using NuGetUpdater.Core;
 using NuGetUpdater.Core.Discover;
@@ -390,6 +391,11 @@ private static async Task RunAsync(
                     Console.SetOut(originalOut);
                     Console.SetError(originalErr);
                 }
+
+                var resultPath = Path.Join(path, DiscoveryWorker.DiscoveryResultFileName);
+                var resultJson = await File.ReadAllTextAsync(resultPath);
+                var resultObject = JsonSerializer.Deserialize<WorkspaceDiscoveryResult>(resultJson, DiscoveryWorker.SerializerOptions);
+                return resultObject!;
             });
 
             ValidateWorkspaceResult(expectedResult, actualResult);

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/AnalyzeWorkerTestBase.cs
@@ -35,10 +35,10 @@ protected static async Task TestAnalyzeAsync(
 
             var discoveryPath = Path.GetFullPath(DiscoveryWorker.DiscoveryResultFileName, directoryPath);
             var dependencyPath = Path.GetFullPath(relativeDependencyPath, directoryPath);
-            var analysisPath = Path.GetFullPath(AnalyzeWorker.AnalysisDirectoryName, directoryPath);
 
             var worker = new AnalyzeWorker(new Logger(verbose: true));
-            await worker.RunAsync(directoryPath, discoveryPath, dependencyPath, analysisPath);
+            var result = await worker.RunWithErrorHandlingAsync(directoryPath, discoveryPath, dependencyPath);
+            return result;
         });
 
         ValidateAnalysisResult(expectedResult, actualResult);
@@ -78,17 +78,13 @@ void ValidateDependencies(ImmutableArray<Dependency> expectedDependencies, Immut
         }
     }
 
-    protected static async Task<AnalysisResult> RunAnalyzerAsync(string dependencyName, TestFile[] files, Func<string, Task> action)
+    protected static async Task<AnalysisResult> RunAnalyzerAsync(string dependencyName, TestFile[] files, Func<string, Task<AnalysisResult>> action)
     {
         // write initial files
         using var temporaryDirectory = await TemporaryDirectory.CreateWithContentsAsync(files);
 
         // run discovery
-        await action(temporaryDirectory.DirectoryPath);
-
-        // gather results
-        var resultPath = Path.Join(temporaryDirectory.DirectoryPath, AnalyzeWorker.AnalysisDirectoryName, $"{dependencyName}.json");
-        var resultJson = await File.ReadAllTextAsync(resultPath);
-        return JsonSerializer.Deserialize<AnalysisResult>(resultJson, DiscoveryWorker.SerializerOptions)!;
+        var result = await action(temporaryDirectory.DirectoryPath);
+        return result;
     }
 }
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs
@@ -25,7 +25,8 @@ protected static async Task TestDiscoveryAsync(
             await UpdateWorkerTestBase.MockNuGetPackagesInDirectory(packages, directoryPath);
 
             var worker = new DiscoveryWorker(new Logger(verbose: true));
-            await worker.RunAsync(directoryPath, workspacePath, DiscoveryWorker.DiscoveryResultFileName);
+            var result = await worker.RunWithErrorHandlingAsync(directoryPath, workspacePath);
+            return result;
         });
 
         ValidateWorkspaceResult(expectedResult, actualResult);
@@ -108,18 +109,14 @@ void ValidateDependencies(ImmutableArray<Dependency> expectedDependencies, Immut
         }
     }
 
-    protected static async Task<WorkspaceDiscoveryResult> RunDiscoveryAsync(TestFile[] files, Func<string, Task> action)
+    protected static async Task<WorkspaceDiscoveryResult> RunDiscoveryAsync(TestFile[] files, Func<string, Task<WorkspaceDiscoveryResult>> action)
     {
         // write initial files
         using var temporaryDirectory = await TemporaryDirectory.CreateWithContentsAsync(files);
 
         // run discovery
-        await action(temporaryDirectory.DirectoryPath);
-
-        // gather results
-        var resultPath = Path.Join(temporaryDirectory.DirectoryPath, DiscoveryWorker.DiscoveryResultFileName);
-        var resultJson = await File.ReadAllTextAsync(resultPath);
-        return JsonSerializer.Deserialize<WorkspaceDiscoveryResult>(resultJson, DiscoveryWorker.SerializerOptions)!;
+        var result = await action(temporaryDirectory.DirectoryPath);
+        return result;
     }
 
     internal class PropertyComparer : IEqualityComparer<Property>

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTestBase.cs
@@ -1,5 +1,3 @@
-using System.Text.Json;
-
 using NuGetUpdater.Core.Updater;
 
 using Xunit;
@@ -137,10 +135,7 @@ protected static async Task TestUpdateForProject(
             // run update
             var worker = new UpdaterWorker(new Logger(verbose: true));
             var projectPath = placeFilesInSrc ? $"src/{projectFilePath}" : projectFilePath;
-            var updateResultFile = Path.Combine(temporaryDirectory, "update-result.json");
-            await worker.RunAsync(temporaryDirectory, projectPath, dependencyName, oldVersion, newVersion, isTransitive, updateResultFile);
-            var actualResultContents = await File.ReadAllTextAsync(updateResultFile);
-            var actualResult = JsonSerializer.Deserialize<UpdateOperationResult>(actualResultContents, UpdaterWorker.SerializerOptions);
+            var actualResult = await worker.RunWithErrorHandlingAsync(temporaryDirectory, projectPath, dependencyName, oldVersion, newVersion, isTransitive);
             if (expectedResult is { })
             {
                 ValidateUpdateOperationResult(expectedResult, actualResult!);
@@ -159,7 +154,7 @@ protected static async Task TestUpdateForProject(
     protected static void ValidateUpdateOperationResult(UpdateOperationResult expectedResult, UpdateOperationResult actualResult)
     {
         Assert.Equal(expectedResult.ErrorType, actualResult.ErrorType);
-        Assert.Equal(expectedResult.ErrorDetails, actualResult.ErrorDetails);
+        Assert.Equivalent(expectedResult.ErrorDetails, actualResult.ErrorDetails);
     }
 
     protected static Task TestNoChangeforSolution(

diff --git a/...elpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs b/...elpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs
@@ -2106,7 +2106,7 @@ public async Task MissingTargetsAreReported()
             var resultContents = await File.ReadAllTextAsync(resultOutputPath);
             var result = JsonSerializer.Deserialize<UpdateOperationResult>(resultContents, UpdaterWorker.SerializerOptions)!;
             Assert.Equal(ErrorType.MissingFile, result.ErrorType);
-            Assert.Equal(Path.Combine(temporaryDirectory.DirectoryPath, "this.file.does.not.exist.targets"), result.ErrorDetails);
+            Assert.Equal(Path.Combine(temporaryDirectory.DirectoryPath, "this.file.does.not.exist.targets"), result.ErrorDetails.ToString());
         }
 
         [Fact]
@@ -2190,6 +2190,84 @@ await TestUpdateForProject("Some.Package", "1.0.0", "1.1.0",
             );
         }
 
+        [Fact]
+        public async Task MissingDependencyErrorIsReported()
+        {
+            // trying to update Some.Package from 1.0.1 to 1.0.2, but another package isn't available; update fails
+            await TestUpdateForProject("Some.Package", "1.0.1", "1.0.2",
+                packages:
+                [
+                    MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.1", "net45"),
+                    MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.2", "net45"),
+
+                    // the package `Unrelated.Package/1.0.0` is missing and will cause the update to fail
+                ],
+                // existing
+                projectContents: """
+                    <Project ToolsVersion="15.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+                      <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
+                      <PropertyGroup>
+                        <TargetFrameworkVersion>v4.5</TargetFrameworkVersion>
+                      </PropertyGroup>
+                      <ItemGroup>
+                        <None Include="packages.config" />
+                      </ItemGroup>
+                      <ItemGroup>
+                        <Reference Include="Some.Package, Version=1.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed">
+                          <HintPath>packages\Some.Package.1.0.1\lib\net45\Some.Package.dll</HintPath>
+                          <Private>True</Private>
+                        </Reference>
+                        <Reference Include="Unrelated.Package, Version=1.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed">
+                          <HintPath>packages\Unrelated.Package.1.0.0\lib\net45\Unrelated.Package.dll</HintPath>
+                          <Private>True</Private>
+                        </Reference>
+                      </ItemGroup>
+                      <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
+                    </Project>
+                    """,
+                packagesConfigContents: """
+                    <packages>
+                      <package id="Some.Package" version="1.0.1" targetFramework="net45" />
+                      <package id="Unrelated.Package" version="1.0.0" targetFramework="net45" />
+                    </packages>
+                    """,
+                // expected
+                expectedProjectContents: """
+                    <Project ToolsVersion="15.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+                      <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
+                      <PropertyGroup>
+                        <TargetFrameworkVersion>v4.5</TargetFrameworkVersion>
+                      </PropertyGroup>
+                      <ItemGroup>
+                        <None Include="packages.config" />
+                      </ItemGroup>
+                      <ItemGroup>
+                        <Reference Include="Some.Package, Version=1.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed">
+                          <HintPath>packages\Some.Package.1.0.1\lib\net45\Some.Package.dll</HintPath>
+                          <Private>True</Private>
+                        </Reference>
+                        <Reference Include="Unrelated.Package, Version=1.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed">
+                          <HintPath>packages\Unrelated.Package.1.0.0\lib\net45\Unrelated.Package.dll</HintPath>
+                          <Private>True</Private>
+                        </Reference>
+                      </ItemGroup>
+                      <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
+                    </Project>
+                    """,
+                expectedPackagesConfigContents: """
+                    <packages>
+                      <package id="Some.Package" version="1.0.1" targetFramework="net45" />
+                      <package id="Unrelated.Package" version="1.0.0" targetFramework="net45" />
+                    </packages>
+                    """,
+                expectedResult: new()
+                {
+                    ErrorType = ErrorType.UpdateNotPossible,
+                    ErrorDetails = new[] { "Unrelated.Package.1.0.0" },
+                }
+            );
+        }
+
         protected static Task TestUpdateForProject(
             string dependencyName,
             string oldVersion,

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/AnalyzeWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/AnalyzeWorker.cs
@@ -30,6 +30,13 @@ public AnalyzeWorker(Logger logger)
     }
 
     public async Task RunAsync(string repoRoot, string discoveryPath, string dependencyPath, string analysisDirectory)
+    {
+        var analysisResult = await RunWithErrorHandlingAsync(repoRoot, discoveryPath, dependencyPath);
+        var dependencyInfo = await DeserializeJsonFileAsync<DependencyInfo>(dependencyPath, nameof(DependencyInfo));
+        await WriteResultsAsync(analysisDirectory, dependencyInfo.Name, analysisResult, _logger);
+    }
+
+    internal async Task<AnalysisResult> RunWithErrorHandlingAsync(string repoRoot, string discoveryPath, string dependencyPath)
     {
         AnalysisResult analysisResult;
         var discovery = await DeserializeJsonFileAsync<WorkspaceDiscoveryResult>(discoveryPath, nameof(WorkspaceDiscoveryResult));
@@ -54,7 +61,7 @@ public async Task RunAsync(string repoRoot, string discoveryPath, string depende
             };
         }
 
-        await WriteResultsAsync(analysisDirectory, dependencyInfo.Name, analysisResult, _logger);
+        return analysisResult;
     }
 
     public async Task<AnalysisResult> RunAsync(string repoRoot, WorkspaceDiscoveryResult discovery, DependencyInfo dependencyInfo)

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs
@@ -31,6 +31,12 @@ public DiscoveryWorker(Logger logger)
     }
 
     public async Task RunAsync(string repoRootPath, string workspacePath, string outputPath)
+    {
+        var result = await RunWithErrorHandlingAsync(repoRootPath, workspacePath);
+        await WriteResultsAsync(repoRootPath, outputPath, result);
+    }
+
+    internal async Task<WorkspaceDiscoveryResult> RunWithErrorHandlingAsync(string repoRootPath, string workspacePath)
     {
         WorkspaceDiscoveryResult result;
         try
@@ -49,7 +55,7 @@ public async Task RunAsync(string repoRootPath, string workspacePath, string out
             };
         }
 
-        await WriteResultsAsync(repoRootPath, outputPath, result);
+        return result;
     }
 
     internal async Task<WorkspaceDiscoveryResult> RunAsync(string repoRootPath, string workspacePath)

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ErrorType.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/ErrorType.cs
@@ -6,4 +6,5 @@ public enum ErrorType
     None,
     AuthenticationFailure,
     MissingFile,
+    UpdateNotPossible,
 }
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NativeResult.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/NativeResult.cs
@@ -4,5 +4,5 @@ public record NativeResult
 {
     // TODO: nullable not required, `ErrorType.None` is the default anyway
     public ErrorType? ErrorType { get; init; }
-    public string? ErrorDetails { get; init; }
+    public object? ErrorDetails { get; init; }
 }
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobErrorBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/JobErrorBase.cs
@@ -7,5 +7,5 @@ public abstract record JobErrorBase
     [JsonPropertyName("error-type")]
     public abstract string Type { get; }
     [JsonPropertyName("error-details")]
-    public required string Details { get; init; }
+    public required object Details { get; init; }
 }
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UpdateNotPossible.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/UpdateNotPossible.cs
@@ -0,0 +1,6 @@
+namespace NuGetUpdater.Core.Run.ApiModel;
+
+public record UpdateNotPossible : JobErrorBase
+{
+    public override string Type => "update_not_possible";
+}
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/RunWorker.cs
@@ -88,6 +88,13 @@ private async Task<RunResult> RunWithErrorHandlingAsync(Job job, DirectoryInfo r
                 Details = ex.FilePath,
             };
         }
+        catch (UpdateNotPossibleException ex)
+        {
+            error = new UpdateNotPossible()
+            {
+                Details = ex.Dependencies,
+            };
+        }
         catch (Exception ex)
         {
             error = new UnknownError()

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/UpdateNotPossibleException.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/UpdateNotPossibleException.cs
@@ -0,0 +1,11 @@
+namespace NuGetUpdater.Core;
+
+internal class UpdateNotPossibleException : Exception
+{
+    public string[] Dependencies { get; }
+
+    public UpdateNotPossibleException(string[] dependencies)
+    {
+        Dependencies = dependencies;
+    }
+}
diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs
@@ -139,6 +139,7 @@ private static void RunNugetUpdate(List<string> updateArgs, List<string> restore
 
                     if (exitCodeAgain != 0)
                     {
+                        MSBuildHelper.ThrowOnMissingPackages(restoreOutput);
                         throw new Exception($"Unable to restore.\nOutput:\n${restoreOutput}\n");
                     }
 
@@ -147,6 +148,7 @@ private static void RunNugetUpdate(List<string> updateArgs, List<string> restore
 
                 MSBuildHelper.ThrowOnUnauthenticatedFeed(fullOutput);
                 MSBuildHelper.ThrowOnMissingFile(fullOutput);
+                MSBuildHelper.ThrowOnMissingPackages(fullOutput);
                 throw new Exception(fullOutput);
             }
         }

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/UpdaterWorker.cs
@@ -24,6 +24,16 @@ public UpdaterWorker(Logger logger)
     }
 
     public async Task RunAsync(string repoRootPath, string workspacePath, string dependencyName, string previousDependencyVersion, string newDependencyVersion, bool isTransitive, string? resultOutputPath = null)
+    {
+        var result = await RunWithErrorHandlingAsync(repoRootPath, workspacePath, dependencyName, previousDependencyVersion, newDependencyVersion, isTransitive);
+        if (resultOutputPath is { })
+        {
+            await WriteResultFile(result, resultOutputPath, _logger);
+        }
+    }
+
+    // this is a convenient method for tests
+    internal async Task<UpdateOperationResult> RunWithErrorHandlingAsync(string repoRootPath, string workspacePath, string dependencyName, string previousDependencyVersion, string newDependencyVersion, bool isTransitive)
     {
         UpdateOperationResult result = new(); // assumed to be ok until proven otherwise
         try
@@ -52,11 +62,16 @@ public async Task RunAsync(string repoRootPath, string workspacePath, string dep
                 ErrorDetails = ex.FilePath,
             };
         }
-
-        if (resultOutputPath is { })
+        catch (UpdateNotPossibleException ex)
         {
-            await WriteResultFile(result, resultOutputPath, _logger);
+            result = new()
+            {
+                ErrorType = ErrorType.UpdateNotPossible,
+                ErrorDetails = ex.Dependencies,
+            };
         }
+
+        return result;
     }
 
     public async Task<UpdateOperationResult> RunAsync(string repoRootPath, string workspacePath, string dependencyName, string previousDependencyVersion, string newDependencyVersion, bool isTransitive)

diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs
@@ -833,6 +833,17 @@ internal static void ThrowOnMissingFile(string output)
         }
     }
 
+    internal static void ThrowOnMissingPackages(string output)
+    {
+        var missingPackagesPattern = new Regex(@"Package '(?<PackageName>[^'].*)' is not found on source");
+        var matchCollection = missingPackagesPattern.Matches(output);
+        var missingPackages = matchCollection.Select(m => m.Groups["PackageName"].Value).Distinct().ToArray();
+        if (missingPackages.Length > 0)
+        {
+            throw new UpdateNotPossibleException(missingPackages);
+        }
+    }
+
     internal static bool TryGetGlobalJsonPath(string repoRootPath, string workspacePath, [NotNullWhen(returnValue: true)] out string? globalJsonPath)
     {
         globalJsonPath = PathHelper.GetFileInDirectoryOrParent(workspacePath, repoRootPath, "global.json", caseSensitive: false);