diff --git a/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs b/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs index bb7f6f86505..1ffcf7c2303 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs @@ -1123,15 +1123,15 @@ private static HashSet GetAuditSuppressions(IMSBuildProject project) } /// - /// Recursively loads and evaluates MSBuild projects. + /// Recursively loads and evaluates MSBuild projects by walking ProjectReferences. /// /// An containing the entry projects to load. /// if the build is allowed to interact with the user, otherwise . /// Optional parameters to use for the MSBuild binary log. /// A factory method that creates a project adapter from an MSBuild ProjectInstance. /// A factory method that updates a project adapter with a target framework and MSBuild ProjectInstance. - /// An option delegate to finalize a project adapter once all projects have been evaluated. - /// An object containing projects and their inner nodes if they are targeting multiple frameworks. + /// An optional delegate to finalize a project adapter once all projects have been evaluated. + /// A containing projects keyed by their full path. private ConcurrentDictionary LoadProjects( IEnumerable entryProjects, bool interactive, @@ -1180,24 +1180,9 @@ private ConcurrentDictionary LoadProjects( EvaluationContext evaluationContext = EvaluationContext.Create(EvaluationContext.SharingPolicy.Shared); - // Create a ProjectGraph object and pass a factory method which creates a ProjectInstance - ProjectGraph projectGraph = new ProjectGraph(entryProjects, projectCollection, (path, properties, collection) => - { - var projectOptions = new ProjectOptions - { - EvaluationContext = evaluationContext, - GlobalProperties = properties, - Interactive = interactive, - // Ignore bad imports to maximize the chances of being able to load the project and restore - LoadSettings = ProjectLoadSettings.IgnoreEmptyImports | ProjectLoadSettings.IgnoreInvalidImports | ProjectLoadSettings.IgnoreMissingImports | ProjectLoadSettings.DoNotEvaluateElementsWithFalseCondition, - ProjectCollection = collection - }; - - return ProjectInstance.FromFile(path, projectOptions); - }); - int buildCount = 0; int failedBuildSubmissionCount = 0; + int evaluationCount = 0; var buildParameters = new BuildParameters(projectCollection) { @@ -1206,53 +1191,94 @@ private ConcurrentDictionary LoadProjects( LogTaskInputs = logTaskInputs }; + var loadSettings = ProjectLoadSettings.IgnoreEmptyImports | ProjectLoadSettings.IgnoreInvalidImports | ProjectLoadSettings.IgnoreMissingImports | ProjectLoadSettings.DoNotEvaluateElementsWithFalseCondition; + + // Initialize the set of projects to process from entry points. + // Work items are (ProjectPath, TargetFramework) tuples. A null TargetFramework + // means "outer build" — evaluate the project and discover its TFMs. + // A non-null TargetFramework means "inner build" — evaluate with that TFM. + var comparer = PathUtility.GetStringComparerBasedOnOS(); + var processedProjects = new ConcurrentDictionary(comparer); + using var workQueue = new BlockingCollection<(string ProjectPath, string TargetFramework)>(); + int pendingCount = 0; + + // Extract global properties from entry points (all share the same set) + IDictionary baseGlobalProperties = null; + foreach (var entryPoint in entryProjects) + { + string fullPath = Path.GetFullPath(entryPoint.ProjectFile); + if (processedProjects.TryAdd(fullPath, 0)) + { + Interlocked.Increment(ref pendingCount); + workQueue.Add((fullPath, (string)null)); + } + + baseGlobalProperties ??= entryPoint.GlobalProperties; + } + + // Create outer build global properties (base properties without TargetFramework) + var outerBuildGlobalProperties = baseGlobalProperties != null + ? new Dictionary(baseGlobalProperties) + : new Dictionary(); + outerBuildGlobalProperties.Remove("TargetFramework"); + + // If there are no entry projects, mark the queue as complete immediately + if (pendingCount == 0) + { + workQueue.CompleteAdding(); + } + try { // BeginBuild starts a queue which accepts build requests and applies the build parameters to all of them BuildManager.DefaultBuildManager.BeginBuild(buildParameters); - // Loop through each project and run the targets. There is no need for this to run in parallel since there is only - // one node in the process to run builds. - foreach (ProjectGraphNode projectGraphItem in projectGraph.ProjectNodes) + // Walk the project graph in parallel: each worker pulls work items from the + // queue, evaluates them, and enqueues any newly discovered work (inner builds + // for multi-targeted projects, or new outer builds for ProjectReferences). + // When a worker finishes a work item and pendingCount reaches 0, all work is done. + int workerCount = Environment.ProcessorCount; + var workers = new Task[workerCount]; + for (int i = 0; i < workerCount; i++) { - ProjectInstance projectInstance = projectGraphItem.ProjectInstance; - - if (!projectInstance.Targets.ContainsKey("_IsProjectRestoreSupported") || projectInstance.GlobalProperties == null || projectInstance.GlobalProperties.TryGetValue("TargetFramework", out string targetFramework) && string.IsNullOrWhiteSpace(targetFramework)) - { - // In rare cases, users can set an empty TargetFramework value in a project-to-project reference. Static Graph will respect that - // but NuGet does not need to do anything with that instance of the project since the actual project is still loaded correctly - // with its actual TargetFramework. - var message = MSBuildRestoreUtility.GetMessageForUnsupportedProject(projectInstance.FullPath); - MSBuildLogger.Log(message); - continue; - } - - // If the project supports restore, queue up a build of the targets needed for restore - BuildSubmission buildSubmission = BuildManager.DefaultBuildManager.PendBuildRequest( - new BuildRequestData( - projectInstance, - TargetsToBuild, - hostServices: null, - // Suppresses an error that a target does not exist because it may or may not contain the targets that we're running - BuildRequestDataFlags.SkipNonexistentTargets)); - - buildSubmission.ExecuteAsync((submission) => + workers[i] = Task.Run(() => { - BuildResult result = submission.BuildResult; - if (result.OverallResult == BuildResultCode.Failure) + foreach ((string projectPath, string targetFramework) in workQueue.GetConsumingEnumerable()) { - failedBuildSubmissionCount++; + try + { + EvaluateProject( + projectPath, + targetFramework, + evaluationContext, + outerBuildGlobalProperties, + interactive, + loadSettings, + projectCollection, + projects, + createProjectFactory, + updateProjectFactory, + processedProjects, + workQueue, + ref pendingCount, + ref evaluationCount, + ref buildCount, + ref failedBuildSubmissionCount); + } + finally + { + // Signal completion of this work item. If this was the last + // pending item, mark the queue complete so all workers exit. + if (Interlocked.Decrement(ref pendingCount) == 0) + { + workQueue.CompleteAdding(); + } + } } - - buildCount++; - - projects.AddOrUpdate( - projectInstance.FullPath, - createProjectFactory, - updateProjectFactory, - (projectInstance, targetFramework)); - }, context: null); + }); } + + Task.WaitAll(workers); } finally { @@ -1270,7 +1296,7 @@ private ConcurrentDictionary LoadProjects( } } - MSBuildLogger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.ProjectEvaluationSummary, projectGraph.ProjectNodes.Count, sw.ElapsedMilliseconds, buildCount, failedBuildSubmissionCount)); + MSBuildLogger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.ProjectEvaluationSummary, evaluationCount, sw.ElapsedMilliseconds, buildCount, failedBuildSubmissionCount)); if (failedBuildSubmissionCount != 0) { @@ -1291,6 +1317,258 @@ private ConcurrentDictionary LoadProjects( } } + /// + /// Evaluates a single work item: either an outer build (targetFramework is null) or an + /// inner build (targetFramework is set). Outer builds discover TFMs and enqueue inner + /// builds for multi-targeted projects, or run _CollectRestoreInputs for single-targeted + /// projects. Inner builds evaluate with a specific TFM, run _CollectRestoreInputs, and + /// discover new ProjectReferences. + /// This method is called from multiple threads by . + /// Newly discovered work items are added to with a corresponding + /// increment to . + /// + private void EvaluateProject( + string projectPath, + string targetFramework, + EvaluationContext evaluationContext, + Dictionary outerBuildGlobalProperties, + bool interactive, + ProjectLoadSettings loadSettings, + ProjectCollection projectCollection, + ConcurrentDictionary projects, + Func createProjectFactory, + Func updateProjectFactory, + ConcurrentDictionary processedProjects, + BlockingCollection<(string ProjectPath, string TargetFramework)> workQueue, + ref int pendingCount, + ref int evaluationCount, + ref int buildCount, + ref int failedBuildSubmissionCount) + { + if (targetFramework is null) + { + // Outer build: evaluate without TargetFramework, then either enqueue inner builds + // or run _CollectRestoreInputs directly for single-targeted projects. + EvaluateOuterBuild( + projectPath, + evaluationContext, + outerBuildGlobalProperties, + interactive, + loadSettings, + projectCollection, + projects, + createProjectFactory, + updateProjectFactory, + processedProjects, + workQueue, + ref pendingCount, + ref evaluationCount, + ref buildCount, + ref failedBuildSubmissionCount); + } + else + { + // Inner build: evaluate with a specific TargetFramework, run _CollectRestoreInputs, + // and discover ProjectReferences. + EvaluateInnerBuild( + projectPath, + targetFramework, + evaluationContext, + outerBuildGlobalProperties, + interactive, + loadSettings, + projectCollection, + projects, + updateProjectFactory, + processedProjects, + workQueue, + ref pendingCount, + ref evaluationCount, + ref buildCount, + ref failedBuildSubmissionCount); + } + } + + /// + /// Evaluates the outer build (no TargetFramework global property) for a project. + /// If the project is multi-targeted, enqueues inner build work items for each TFM. + /// If single-targeted, runs _CollectRestoreInputs and discovers ProjectReferences. + /// + private void EvaluateOuterBuild( + string projectPath, + EvaluationContext evaluationContext, + Dictionary outerBuildGlobalProperties, + bool interactive, + ProjectLoadSettings loadSettings, + ProjectCollection projectCollection, + ConcurrentDictionary projects, + Func createProjectFactory, + Func updateProjectFactory, + ConcurrentDictionary processedProjects, + BlockingCollection<(string ProjectPath, string TargetFramework)> workQueue, + ref int pendingCount, + ref int evaluationCount, + ref int buildCount, + ref int failedBuildSubmissionCount) + { + // Evaluate outer build (no TargetFramework global property) + ProjectInstance outerProjectInstance = ProjectInstance.FromFile(projectPath, new ProjectOptions + { + EvaluationContext = evaluationContext, + GlobalProperties = outerBuildGlobalProperties, + Interactive = interactive, + // Ignore bad imports to maximize the chances of being able to load the project and restore + LoadSettings = loadSettings, + ProjectCollection = projectCollection + }); + + Interlocked.Increment(ref evaluationCount); + + if (!outerProjectInstance.Targets.ContainsKey("_IsProjectRestoreSupported")) + { + var message = MSBuildRestoreUtility.GetMessageForUnsupportedProject(outerProjectInstance.FullPath); + MSBuildLogger.Log(message); + return; + } + + // Add the outer build to projects + projects.AddOrUpdate( + projectPath, + createProjectFactory, + updateProjectFactory, + (outerProjectInstance, (string)null)); + + string targetFrameworks = outerProjectInstance.GetPropertyValue("TargetFrameworks"); + + if (!string.IsNullOrWhiteSpace(targetFrameworks)) + { + // Multi-targeting: enqueue an inner build work item for each TFM + string[] tfms = targetFrameworks.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); + foreach (string tfm in tfms) + { + string trimmedTfm = tfm.Trim(); + if (!string.IsNullOrEmpty(trimmedTfm)) + { + Interlocked.Increment(ref pendingCount); + workQueue.Add((projectPath, trimmedTfm)); + } + } + } + else + { + // Single-targeting: run _CollectRestoreInputs on the outer build + BuildSubmission buildSubmission = BuildManager.DefaultBuildManager.PendBuildRequest( + new BuildRequestData( + outerProjectInstance, + TargetsToBuild, + hostServices: null, + BuildRequestDataFlags.SkipNonexistentTargets)); + + buildSubmission.Execute(); + + if (buildSubmission.BuildResult.OverallResult == BuildResultCode.Failure) + { + Interlocked.Increment(ref failedBuildSubmissionCount); + } + + Interlocked.Increment(ref buildCount); + + // Discover ProjectReferences from the outer build + DiscoverProjectReferences(projectPath, outerProjectInstance, processedProjects, workQueue, ref pendingCount); + } + } + + /// + /// Evaluates an inner build (with a specific TargetFramework global property), runs + /// _CollectRestoreInputs, and discovers new ProjectReferences. + /// + private static void EvaluateInnerBuild( + string projectPath, + string targetFramework, + EvaluationContext evaluationContext, + Dictionary outerBuildGlobalProperties, + bool interactive, + ProjectLoadSettings loadSettings, + ProjectCollection projectCollection, + ConcurrentDictionary projects, + Func updateProjectFactory, + ConcurrentDictionary processedProjects, + BlockingCollection<(string ProjectPath, string TargetFramework)> workQueue, + ref int pendingCount, + ref int evaluationCount, + ref int buildCount, + ref int failedBuildSubmissionCount) + { + var innerGlobalProperties = new Dictionary(outerBuildGlobalProperties) + { + ["TargetFramework"] = targetFramework + }; + + ProjectInstance innerProjectInstance = ProjectInstance.FromFile(projectPath, new ProjectOptions + { + EvaluationContext = evaluationContext, + GlobalProperties = innerGlobalProperties, + Interactive = interactive, + LoadSettings = loadSettings, + ProjectCollection = projectCollection + }); + + Interlocked.Increment(ref evaluationCount); + + // Run _CollectRestoreInputs on the inner build + BuildSubmission buildSubmission = BuildManager.DefaultBuildManager.PendBuildRequest( + new BuildRequestData( + innerProjectInstance, + TargetsToBuild, + hostServices: null, + // Suppresses an error that a target does not exist because it may or may not contain the targets that we're running + BuildRequestDataFlags.SkipNonexistentTargets)); + + buildSubmission.Execute(); + + if (buildSubmission.BuildResult.OverallResult == BuildResultCode.Failure) + { + Interlocked.Increment(ref failedBuildSubmissionCount); + } + + Interlocked.Increment(ref buildCount); + + projects.AddOrUpdate( + projectPath, + // The outer build should always have created the entry already, but provide a + // create factory for safety in case of unexpected ordering. + (string key, (ProjectInstance, string) args) => throw new InvalidOperationException( + $"Inner build for '{key}' TFM '{targetFramework}' arrived before the outer build created the project entry."), + updateProjectFactory, + (innerProjectInstance, targetFramework)); + + // Discover ProjectReferences from the inner build + DiscoverProjectReferences(projectPath, innerProjectInstance, processedProjects, workQueue, ref pendingCount); + } + + /// + /// Scans a built project instance for ProjectReference items and enqueues any newly + /// discovered projects as outer build work items. + /// + private static void DiscoverProjectReferences( + string projectPath, + ProjectInstance projectInstance, + ConcurrentDictionary processedProjects, + BlockingCollection<(string ProjectPath, string TargetFramework)> workQueue, + ref int pendingCount) + { + string projectDir = Path.GetDirectoryName(projectPath); + foreach (ProjectItemInstance projectRef in projectInstance.GetItems("ProjectReference")) + { + string refPath = Path.GetFullPath(Path.Combine(projectDir, projectRef.EvaluatedInclude)); + if (processedProjects.TryAdd(refPath, 0)) + { + Interlocked.Increment(ref pendingCount); + workQueue.Add((refPath, (string)null)); + } + } + } + /// /// Returns the list of distinct items with the name. /// Two items are equal if they have the same . diff --git a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs index 58cf9721beb..3bc7b63447a 100644 --- a/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs +++ b/test/NuGet.Core.FuncTests/Msbuild.Integration.Test/MsbuildRestoreTaskTests.cs @@ -2216,5 +2216,150 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } } } + + [PlatformTheory(Platform.Windows)] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, false)] + public void MsbuildRestore_NonSdkProjectReferencingSdkMultiTargetedProject_WithSetTargetFramework_Succeeds(bool useStaticGraphRestore, bool usePackageSpecFactory) + { + // Arrange - reproduces https://github.com/NuGet/Home/issues/11680 + // A non-SDK-style project references an SDK-style multi-targeted project using + // SetTargetFramework metadata on the ProjectReference. Static graph restore + // previously crashed with a NullReferenceException because the referenced project's + // OuterProject was null in MSBuildStaticGraphRestore.GetPackageSpec. + using var pathContext = new SimpleTestPathContext(); + + // SDK-style class library that multi-targets net472 and net8.0 + string sdkProjectDir = Path.Combine(pathContext.SolutionRoot, "SdkLib"); + Directory.CreateDirectory(sdkProjectDir); + string sdkProjectPath = Path.Combine(sdkProjectDir, "SdkLib.csproj"); + + File.WriteAllText(sdkProjectPath, +@" + + net472;net8.0 + +"); + + // Non-SDK-style class library targeting net472 with a ProjectReference using SetTargetFramework + string nonSdkProjectDir = Path.Combine(pathContext.SolutionRoot, "NonSdkApp"); + Directory.CreateDirectory(nonSdkProjectDir); + string nonSdkProjectPath = Path.Combine(nonSdkProjectDir, "NonSdkApp.csproj"); + + File.WriteAllText(nonSdkProjectPath, +$@" + + + + Debug + AnyCPU + Library + NonSdkApp + NonSdkApp + v4.7.2 + PackageReference + {Path.Combine(nonSdkProjectDir, "obj")}\ + + + bin\Debug\ + + + bin\Release\ + + + + TargetFramework=net472 + + + +"); + + // Create a solution containing both projects + var sdkTestProject = new SimpleTestProjectContext("SdkLib", ProjectStyle.PackageReference, pathContext.SolutionRoot); + sdkTestProject.ProjectPath = sdkProjectPath; + var nonSdkTestProject = new SimpleTestProjectContext("NonSdkApp", ProjectStyle.Unknown, pathContext.SolutionRoot); + nonSdkTestProject.ProjectPath = nonSdkProjectPath; + + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + solution.Projects.Add(sdkTestProject); + solution.Projects.Add(nonSdkTestProject); + solution.Create(); + + // Re-write both project files since solution.Create() may overwrite them + File.WriteAllText(sdkProjectPath, +@" + + net472;net8.0 + +"); + + File.WriteAllText(nonSdkProjectPath, +$@" + + + + Debug + AnyCPU + Library + NonSdkApp + NonSdkApp + v4.7.2 + PackageReference + {Path.Combine(nonSdkProjectDir, "obj")}\ + + + bin\Debug\ + + + bin\Release\ + + + + TargetFramework=net472 + + + +"); + + var environmentVariables = new Dictionary(); + environmentVariables.AddRange(_msbuildFixture.DefaultProcessEnvironmentVariables); + environmentVariables["NUGET_USE_NEW_PACKAGESPEC_FACTORY"] = usePackageSpecFactory.ToString(); + + // Act - Restore + string staticGraphArg = useStaticGraphRestore ? " /p:RestoreUseStaticGraphEvaluation=true" : string.Empty; + var restoreResult = _msbuildFixture.RunMsBuild( + pathContext.WorkingDirectory, + $"/t:restore {nonSdkProjectPath}{staticGraphArg}", + ignoreExitCode: true, + testOutputHelper: _testOutputHelper, + environmentVariables); + + // Assert - Restore succeeds + restoreResult.Success.Should().BeTrue(); + + // Assert - SDK project assets file targets + var lockFileFormat = new LockFileFormat(); + string sdkAssetsPath = Path.Combine(sdkProjectDir, "obj", "project.assets.json"); + File.Exists(sdkAssetsPath).Should().BeTrue(because: "SDK project should have an assets file after restore"); + var sdkAssetsFile = lockFileFormat.Read(sdkAssetsPath); + var sdkTargetFrameworks = sdkAssetsFile.Targets + .Where(t => string.IsNullOrEmpty(t.RuntimeIdentifier)) + .Select(t => t.TargetFramework) + .ToList(); + sdkTargetFrameworks.Should().Contain(NuGetFramework.Parse("net472")); + sdkTargetFrameworks.Should().Contain(NuGetFramework.Parse("net8.0")); + + // Assert - Non-SDK project assets file has net472 target + string nonSdkAssetsPath = Path.Combine(nonSdkProjectDir, "obj", "project.assets.json"); + File.Exists(nonSdkAssetsPath).Should().BeTrue(because: "non-SDK project should have an assets file after restore"); + var nonSdkAssetsFile = lockFileFormat.Read(nonSdkAssetsPath); + var nonSdkTargetFrameworks = nonSdkAssetsFile.Targets + .Where(t => string.IsNullOrEmpty(t.RuntimeIdentifier)) + .Select(t => t.TargetFramework) + .ToList(); + nonSdkTargetFrameworks.Should().HaveCount(1); + nonSdkTargetFrameworks.Should().Contain(NuGetFramework.Parse("net472")); + } } }