From fd6b71de6a64adf947c7335fc167b241a3abb306 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Mon, 19 Sep 2022 11:53:29 -0700 Subject: [PATCH 1/5] Add better handling of AggregateExceptions in static graph-based restore --- .../MSBuildStaticGraphRestore.cs | 54 +++++++++++++++---- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs b/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs index 631f078f8b7..29c0f43bc73 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs @@ -15,6 +15,7 @@ using Microsoft.Build.Definition; using Microsoft.Build.Evaluation; using Microsoft.Build.Evaluation.Context; +using Microsoft.Build.Exceptions; using Microsoft.Build.Execution; using Microsoft.Build.Framework; using Microsoft.Build.Graph; @@ -130,7 +131,7 @@ public async Task RestoreAsync(string entryProjectFilePath, IDictionary LoadProjects(IEnumerable GetDistinctItemsOrEmpty(IMSBuildProject { return project.GetItems(itemName)?.Distinct(ProjectItemInstanceEvaluatedIncludeComparer.Instance) ?? Enumerable.Empty(); } + + /// + /// Logs an error from the specified exception. + /// + /// The with details to be logged. + private void LogErrorFromException(Exception exception) + { + switch (exception) + { + case AggregateException aggregateException: + foreach (Exception innerException in aggregateException.InnerExceptions) + { + LogErrorFromException(innerException); + } + break; + + case InvalidProjectFileException invalidProjectFileException: + // Special case the InvalidProjectFileException since it has extra information about what project file couldn't be loaded + LoggingQueue.TaskLoggingHelper.LogError( + invalidProjectFileException.ErrorSubcategory, + invalidProjectFileException.ErrorCode, + invalidProjectFileException.HelpKeyword, + invalidProjectFileException.ProjectFile, + invalidProjectFileException.LineNumber, + invalidProjectFileException.ColumnNumber, + invalidProjectFileException.EndLineNumber, + invalidProjectFileException.EndColumnNumber, + invalidProjectFileException.Message); + break; + + default: + LoggingQueue.TaskLoggingHelper.LogErrorFromException( + exception, + showStackTrace: true); + break; + } + } } } From eaac85ef2fa268a492aec932bc70a0cf67c253c5 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Mon, 19 Sep 2022 12:28:14 -0700 Subject: [PATCH 2/5] Address comments --- .../NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs b/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs index 29c0f43bc73..8cc174422ce 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs @@ -1044,7 +1044,7 @@ private void LogErrorFromException(Exception exception) switch (exception) { case AggregateException aggregateException: - foreach (Exception innerException in aggregateException.InnerExceptions) + foreach (Exception innerException in aggregateException.Flatten().InnerExceptions) { LogErrorFromException(innerException); } From 1b9a905368d67678447df5c6fda49af33d846bb2 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Mon, 19 Sep 2022 14:28:08 -0700 Subject: [PATCH 3/5] Add a functional test --- .../MsbuildRestoreTaskTests.cs | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs index df41a715d57..5a7f8f521f7 100644 --- a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs +++ b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs @@ -765,6 +765,72 @@ public Task MsbuildRestore_WithStaticGraphRestore_MessageLoggedAtDefaultVerbosit return Task.CompletedTask; } + [PlatformFact(Platform.Windows)] + public async Task MsbuildRestore_StaticGraphEvaluation_HandlesInvalidProjectFileExceptionn() + { + // Arrange + using (var pathContext = new SimpleTestPathContext()) + { + // Set up solution, project, and packages + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + + var net461 = NuGetFramework.Parse("net461"); + + var projectA = new SimpleTestProjectContext("a", ProjectStyle.PackageReference, pathContext.SolutionRoot); + + var projectB = new SimpleTestProjectContext("b", ProjectStyle.PackageReference, pathContext.SolutionRoot); + var projectC = new SimpleTestProjectContext("c", ProjectStyle.PackageReference, pathContext.SolutionRoot); + + var projectAFrameworkContext = new SimpleTestProjectFrameworkContext(net461); + + projectAFrameworkContext.ProjectReferences.Add(projectB); + projectAFrameworkContext.ProjectReferences.Add(projectC); + + var packageX = new SimpleTestPackageContext() + { + Id = "x", + Version = "1.0.0" + }; + projectA.Frameworks.Add(projectAFrameworkContext); + + packageX.Files.Clear(); + projectA.AddPackageToAllFrameworks(packageX); + packageX.AddFile("lib/net461/a.dll"); + + solution.Projects.Add(projectA); + solution.Create(pathContext.SolutionRoot); + + var configAPath = Path.Combine(Path.GetDirectoryName(projectA.ProjectPath), "NuGet.Config"); + var configText = +$@" + + + + +"; + using (var writer = new StreamWriter(configAPath)) + { + writer.Write(configText); + } + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + packageX); + + File.Delete(projectB.ProjectPath); + File.Delete(projectC.ProjectPath); + + // Restore the project with a PackageReference which generates assets + var result = _msbuildFixture.RunMsBuild(pathContext.WorkingDirectory, $"/t:restore /p:RestoreUseStaticGraphEvaluation=true {projectA.ProjectPath}", ignoreExitCode: true); + + // Assert + Assert.True(result.ExitCode == 1, result.AllOutput); + + result.AllOutput.Should().Contain($"error MSB4025: The project file could not be loaded. Could not find file '{projectB.ProjectPath}'"); + result.AllOutput.Should().Contain($"error MSB4025: The project file could not be loaded. Could not find file '{projectC.ProjectPath}'"); + } + } + [PlatformTheory(Platform.Windows)] [InlineData(true)] [InlineData(false)] From 250963f87ab415a3a26ce08fef148d57bc5fb66c Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Mon, 19 Sep 2022 14:42:01 -0700 Subject: [PATCH 4/5] Address comments --- .../MsbuildRestoreTaskTests.cs | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs index 5a7f8f521f7..b63dccff842 100644 --- a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs +++ b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs @@ -766,7 +766,7 @@ public Task MsbuildRestore_WithStaticGraphRestore_MessageLoggedAtDefaultVerbosit } [PlatformFact(Platform.Windows)] - public async Task MsbuildRestore_StaticGraphEvaluation_HandlesInvalidProjectFileExceptionn() + public void MsbuildRestore_StaticGraphEvaluation_HandlesInvalidProjectFileExceptionn() { // Arrange using (var pathContext = new SimpleTestPathContext()) @@ -793,30 +793,11 @@ public async Task MsbuildRestore_StaticGraphEvaluation_HandlesInvalidProjectFile }; projectA.Frameworks.Add(projectAFrameworkContext); - packageX.Files.Clear(); projectA.AddPackageToAllFrameworks(packageX); - packageX.AddFile("lib/net461/a.dll"); solution.Projects.Add(projectA); solution.Create(pathContext.SolutionRoot); - var configAPath = Path.Combine(Path.GetDirectoryName(projectA.ProjectPath), "NuGet.Config"); - var configText = -$@" - - - - -"; - using (var writer = new StreamWriter(configAPath)) - { - writer.Write(configText); - } - - await SimpleTestPackageUtility.CreateFolderFeedV3Async( - pathContext.PackageSource, - packageX); - File.Delete(projectB.ProjectPath); File.Delete(projectC.ProjectPath); From 77d0c08e9f90463ac40f47751f558bf982036658 Mon Sep 17 00:00:00 2001 From: Jeff Kluge Date: Tue, 20 Sep 2022 10:20:14 -0700 Subject: [PATCH 5/5] Update unit test --- .../Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs index b63dccff842..0e0d85bf28c 100644 --- a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs +++ b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs @@ -766,7 +766,7 @@ public Task MsbuildRestore_WithStaticGraphRestore_MessageLoggedAtDefaultVerbosit } [PlatformFact(Platform.Windows)] - public void MsbuildRestore_StaticGraphEvaluation_HandlesInvalidProjectFileExceptionn() + public void MsbuildRestore_StaticGraphEvaluation_HandlesInvalidProjectFileException() { // Arrange using (var pathContext = new SimpleTestPathContext()) @@ -779,12 +779,10 @@ public void MsbuildRestore_StaticGraphEvaluation_HandlesInvalidProjectFileExcept var projectA = new SimpleTestProjectContext("a", ProjectStyle.PackageReference, pathContext.SolutionRoot); var projectB = new SimpleTestProjectContext("b", ProjectStyle.PackageReference, pathContext.SolutionRoot); - var projectC = new SimpleTestProjectContext("c", ProjectStyle.PackageReference, pathContext.SolutionRoot); var projectAFrameworkContext = new SimpleTestProjectFrameworkContext(net461); projectAFrameworkContext.ProjectReferences.Add(projectB); - projectAFrameworkContext.ProjectReferences.Add(projectC); var packageX = new SimpleTestPackageContext() { @@ -799,16 +797,13 @@ public void MsbuildRestore_StaticGraphEvaluation_HandlesInvalidProjectFileExcept solution.Create(pathContext.SolutionRoot); File.Delete(projectB.ProjectPath); - File.Delete(projectC.ProjectPath); - // Restore the project with a PackageReference which generates assets var result = _msbuildFixture.RunMsBuild(pathContext.WorkingDirectory, $"/t:restore /p:RestoreUseStaticGraphEvaluation=true {projectA.ProjectPath}", ignoreExitCode: true); // Assert Assert.True(result.ExitCode == 1, result.AllOutput); result.AllOutput.Should().Contain($"error MSB4025: The project file could not be loaded. Could not find file '{projectB.ProjectPath}'"); - result.AllOutput.Should().Contain($"error MSB4025: The project file could not be loaded. Could not find file '{projectC.ProjectPath}'"); } }