diff --git a/ref/Microsoft.Build/net/Microsoft.Build.cs b/ref/Microsoft.Build/net/Microsoft.Build.cs index b8440d4530d..b83468d4bd9 100644 --- a/ref/Microsoft.Build/net/Microsoft.Build.cs +++ b/ref/Microsoft.Build/net/Microsoft.Build.cs @@ -1051,6 +1051,7 @@ public BuildResult() { } public Microsoft.Build.Execution.ProjectInstance ProjectStateAfterBuild { get { throw null; } set { } } public System.Collections.Generic.IDictionary ResultsByTarget { get { throw null; } } public int SubmissionId { get { throw null; } } + public System.Collections.Generic.HashSet Targets { get { throw null; } set { } } public void AddResultsForTarget(string target, Microsoft.Build.Execution.TargetResult result) { } public bool HasResultsForTarget(string target) { throw null; } public void MergeResults(Microsoft.Build.Execution.BuildResult results) { } diff --git a/ref/Microsoft.Build/netstandard/Microsoft.Build.cs b/ref/Microsoft.Build/netstandard/Microsoft.Build.cs index 5a215a8c307..7c0db60b972 100644 --- a/ref/Microsoft.Build/netstandard/Microsoft.Build.cs +++ b/ref/Microsoft.Build/netstandard/Microsoft.Build.cs @@ -1046,6 +1046,7 @@ public BuildResult() { } public Microsoft.Build.Execution.ProjectInstance ProjectStateAfterBuild { get { throw null; } set { } } public System.Collections.Generic.IDictionary ResultsByTarget { get { throw null; } } public int SubmissionId { get { throw null; } } + public System.Collections.Generic.HashSet Targets { get { throw null; } set { } } public void AddResultsForTarget(string target, Microsoft.Build.Execution.TargetResult result) { } public bool HasResultsForTarget(string target) { throw null; } public void MergeResults(Microsoft.Build.Execution.BuildResult results) { } diff --git a/scripts/Deploy-MSBuild.ps1 b/scripts/Deploy-MSBuild.ps1 index 7fb7115821e..125e9447ca5 100644 --- a/scripts/Deploy-MSBuild.ps1 +++ b/scripts/Deploy-MSBuild.ps1 @@ -8,15 +8,30 @@ Param( [string] $runtime = "Desktop" ) +Set-StrictMode -Version "Latest" +$ErrorActionPreference = "Stop" + function Copy-WithBackup ($origin) { - $destinationPath = Join-Path -Path $destination -ChildPath (Split-Path $origin -leaf) + $directoryPart = Join-Path -Path $destination $origin.IntermediaryDirectories + $destinationPath = Join-Path -Path $directoryPart (Split-Path $origin.SourceFile -leaf) if (Test-Path $destinationPath -PathType Leaf) { # Back up previous copy of the file Copy-Item $destinationPath $BackupFolder -ErrorAction Stop } - Copy-Item $origin $destinationPath -ErrorAction Stop + if (!(Test-Path $directoryPart)) { + [system.io.directory]::CreateDirectory($directoryPart) + } + + Copy-Item $origin.SourceFile $destinationPath -ErrorAction Stop + + echo "Copied $($origin.SourceFile) to $destinationPath" +} + +function FileToCopy([string] $sourceFileRelativeToRepoRoot, [string] $intermediaryDirectories) +{ + return [PSCustomObject]@{"SourceFile"=$([IO.Path]::Combine($PSScriptRoot, "..", $sourceFileRelativeToRepoRoot)); "IntermediaryDirectories"=$intermediaryDirectories} } # TODO: find destination in PATH if not specified @@ -36,58 +51,65 @@ if ($runtime -eq "Desktop") { $targetFramework = "netcoreapp2.1" } +$bootstrapBinDirectory = "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework" + $filesToCopyToBin = @( - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Build.dll" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Build.Framework.dll" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Build.Tasks.Core.dll" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Build.Utilities.Core.dll" - - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Common.CrossTargeting.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Common.CurrentVersion.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Common.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.CSharp.CrossTargeting.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.CSharp.CurrentVersion.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.CSharp.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Managed.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Managed.Before.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Managed.After.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Net.props" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.NetFramework.CurrentVersion.props" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.NetFramework.CurrentVersion.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.NetFramework.props" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.NetFramework.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.VisualBasic.CrossTargeting.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.VisualBasic.CurrentVersion.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.VisualBasic.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.Build.dll" + FileToCopy "$bootstrapBinDirectory\Microsoft.Build.Framework.dll" + FileToCopy "$bootstrapBinDirectory\Microsoft.Build.Tasks.Core.dll" + FileToCopy "$bootstrapBinDirectory\Microsoft.Build.Utilities.Core.dll" + + FileToCopy "$bootstrapBinDirectory\en\Microsoft.Build.resources.dll" "en" + FileToCopy "$bootstrapBinDirectory\en\Microsoft.Build.Tasks.Core.resources.dll" "en" + FileToCopy "$bootstrapBinDirectory\en\Microsoft.Build.Utilities.Core.resources.dll" "en" + FileToCopy "$bootstrapBinDirectory\en\MSBuild.resources.dll" "en" + + FileToCopy "$bootstrapBinDirectory\Microsoft.Common.CrossTargeting.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.Common.CurrentVersion.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.Common.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.CSharp.CrossTargeting.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.CSharp.CurrentVersion.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.CSharp.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.Managed.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.Managed.Before.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.Managed.After.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.Net.props" + FileToCopy "$bootstrapBinDirectory\Microsoft.NetFramework.CurrentVersion.props" + FileToCopy "$bootstrapBinDirectory\Microsoft.NetFramework.CurrentVersion.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.NetFramework.props" + FileToCopy "$bootstrapBinDirectory\Microsoft.NetFramework.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.VisualBasic.CrossTargeting.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.VisualBasic.CurrentVersion.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.VisualBasic.targets" ) if ($runtime -eq "Desktop") { $runtimeSpecificFiles = @( - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\MSBuild.exe" - "artifacts\bin\Microsoft.Build.Conversion\$configuration\$targetFramework\Microsoft.Build.Conversion.Core.dll" - "artifacts\bin\Microsoft.Build.Engine\$configuration\$targetFramework\Microsoft.Build.Engine.dll" - - "artifacts\bin\MSBuildTaskHost\$configuration\net35\MSBuildTaskHost.exe" - "artifacts\bin\MSBuildTaskHost\$configuration\net35\MSBuildTaskHost.pdb" - - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Data.Entity.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.ServiceModel.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.WinFx.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.WorkflowBuildExtensions.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Microsoft.Xaml.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Workflow.targets" - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\Workflow.VisualBasic.targets" + FileToCopy "$bootstrapBinDirectory\MSBuild.exe" + FileToCopy "artifacts\bin\Microsoft.Build.Conversion\$configuration\$targetFramework\Microsoft.Build.Conversion.Core.dll" + FileToCopy "artifacts\bin\Microsoft.Build.Engine\$configuration\$targetFramework\Microsoft.Build.Engine.dll" + + FileToCopy "artifacts\bin\MSBuildTaskHost\$configuration\net35\MSBuildTaskHost.exe" + FileToCopy "artifacts\bin\MSBuildTaskHost\$configuration\net35\MSBuildTaskHost.pdb" + + FileToCopy "$bootstrapBinDirectory\Microsoft.Data.Entity.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.ServiceModel.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.WinFx.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.WorkflowBuildExtensions.targets" + FileToCopy "$bootstrapBinDirectory\Microsoft.Xaml.targets" + FileToCopy "$bootstrapBinDirectory\Workflow.targets" + FileToCopy "$bootstrapBinDirectory\Workflow.VisualBasic.targets" ) } else { $runtimeSpecificFiles = @( - "artifacts\bin\MSBuild.Bootstrap\$configuration\$targetFramework\MSBuild.dll" + FileToCopy "$bootstrapBinDirectory\MSBuild.dll" ) } $filesToCopyToBin += $runtimeSpecificFiles foreach ($file in $filesToCopyToBin) { - Copy-WithBackup $([IO.Path]::Combine($PSScriptRoot, "..", $file)) + Copy-WithBackup $file } Write-Host -ForegroundColor Green "Copy succeeded" diff --git a/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs b/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs index f43d6e7718c..480753bff33 100644 --- a/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs +++ b/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs @@ -3433,7 +3433,8 @@ public void WarningsAreTreatedAsErrorsButTargetsStillSucceed() } /// - /// Helper for cache tests. Builds a project and verifies the right cache files are created. + /// Helper for memory reduction caching tests (msbuild caches configs and build results when memory usage gets high). + /// Builds a project and verifies the right cache files are created. /// private static string BuildAndCheckCache(BuildManager localBuildManager, IEnumerable exceptCacheDirectories) { @@ -3459,9 +3460,15 @@ private static string BuildAndCheckCache(BuildManager localBuildManager, IEnumer try { var services = new HostServices(); - BuildRequestData data = new BuildRequestData(fileName, new Dictionary(), MSBuildDefaultToolsVersion, new[] { "One", "Two", "Three" }, services); + var data = new BuildRequestData( + projectFullPath: fileName, + globalProperties: new Dictionary(), + toolsVersion: MSBuildDefaultToolsVersion, + targetsToBuild: new[] {"One", "Two", "Three"}, + hostServices: services); var result = localBuildManager.PendBuildRequest(data).Execute(); - Assert.Equal(BuildResultCode.Success, result.OverallResult); // "Test project failed to build correctly." + + result.OverallResult.ShouldBe(BuildResultCode.Success); } finally { @@ -3473,12 +3480,9 @@ private static string BuildAndCheckCache(BuildManager localBuildManager, IEnumer string directory = Directory.EnumerateDirectories(cacheDirectory).Except(exceptCacheDirectories).First(); // Within this directory should be a set of target results files, one for each of the targets we invoked. - var resultsFiles = Directory.EnumerateFiles(directory).Select(Path.GetFileName); + var resultsFiles = Directory.EnumerateFiles(directory).Select(Path.GetFileName).ToArray(); - Assert.Equal(3, resultsFiles.Count()); - Assert.Contains("One.cache", resultsFiles); - Assert.Contains("Two.cache", resultsFiles); - Assert.Contains("Three.cache", resultsFiles); + resultsFiles.ShouldBeSameIgnoringOrder(new []{"One.cache", "Two.cache", "Three.cache"}); // Return the cache directory created for this build. return directory; diff --git a/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs b/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs index c9e4fdd7302..93eeb2927b1 100644 --- a/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs +++ b/src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs @@ -502,7 +502,7 @@ private void TestSkipIsolationConstraints(string glob, string referencePath, boo glob = $"$([MSBuild]::Escape('{glob}'))"; - projectContents = projectContents ?? $@" + projectContents ??= $@" <{ItemTypeNames.GraphIsolationExemptReference} Include=`{glob};ShouldNotMatchAnything`/> diff --git a/src/Build.UnitTests/BackEnd/CacheAggregator_Tests.cs b/src/Build.UnitTests/BackEnd/CacheAggregator_Tests.cs index 8bd5360d434..152e157812c 100644 --- a/src/Build.UnitTests/BackEnd/CacheAggregator_Tests.cs +++ b/src/Build.UnitTests/BackEnd/CacheAggregator_Tests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using Microsoft.Build.BackEnd; +using Microsoft.Build.Collections; using Microsoft.Build.Execution; using Microsoft.Build.Framework; using Microsoft.Build.Internal; @@ -79,7 +80,7 @@ public void RejectCachesWithMoreConfigEntriesThanResultEntries() var resultsCache = new ResultsCache(); var buildResult = new BuildResult(new BuildRequest(1, 2, configurationId: 1, new List(){"a", "b"}, null, BuildEventContext.Invalid, null)); - buildResult.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); + buildResult.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult("i", "v")); resultsCache.AddResult(buildResult); aggregator.Add(configCache, resultsCache); @@ -100,12 +101,12 @@ public void RejectCachesWithMoreResultEntriesThanConfigEntries() var resultsCache = new ResultsCache(); var buildResult = new BuildResult(new BuildRequest(1, 2, configurationId: 1, new List(){"a", "b"}, null, BuildEventContext.Invalid, null)); - buildResult.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); + buildResult.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult("i", "v")); resultsCache.AddResult(buildResult); var buildResult2 = new BuildResult(new BuildRequest(1, 2, configurationId: 2, new List(){"a", "b"}, null, BuildEventContext.Invalid, null)); - buildResult2.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); + buildResult2.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult("i", "v")); resultsCache.AddResult(buildResult2); @@ -129,7 +130,7 @@ public void RejectCachesWithMismatchedIds() var resultsCache = new ResultsCache(); var buildResult = new BuildResult(new BuildRequest(1, 2, configurationId: 2, new List(){"a", "b"}, null, BuildEventContext.Invalid, null)); - buildResult.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); + buildResult.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult("i", "v")); resultsCache.AddResult(buildResult); aggregator.Add(configCache, resultsCache); @@ -143,34 +144,77 @@ public void RejectCachesWithMismatchedIds() } [Fact] - public void RejectCollidingConfigurationsFromSeparateCaches() + public void CollidingConfigurationsGetMergedViaFirstOneWinsResolution() { // collides with the config id from configCache2 + var config1 = new BuildRequestConfiguration(1, + new BuildRequestData( + projectFullPath: "path", + globalProperties: new Dictionary { ["p"] = "v" }, + toolsVersion: "13", + targetsToBuild: new[] { "foo" }, + hostServices: null), "13"); + var configCache1 = new ConfigCache(); - configCache1.AddConfiguration(new BuildRequestConfiguration(configId: 1, new BuildRequestData("path", new Dictionary(){["p"] = "v"}, "13", new []{"a", "b"}, null), "13")); + configCache1.AddConfiguration(config1); var resultsCache1 = new ResultsCache(); - var buildResult11 = new BuildResult(new BuildRequest(1, 2, configurationId: 1, new List(){"a", "b"}, null, BuildEventContext.Invalid, null)); - buildResult11.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); - resultsCache1.AddResult(buildResult11); + var buildResult1 = new BuildResult(new BuildRequest(1, 2, configurationId: 1, + new List() { "foo" }, null, BuildEventContext.Invalid, null)); + + // exists only in config1 + buildResult1.AddResultsForTarget("target1", GetNonEmptySucceedingTargetResult("i1Config1")); + // exists in both configs with different values + buildResult1.AddResultsForTarget("target3", GetNonEmptySucceedingTargetResult("i3Config1")); + // exists in both configs with the same value + buildResult1.AddResultsForTarget("target4", GetNonEmptySucceedingTargetResult("v")); + + resultsCache1.AddResult(buildResult1); + + var config2 = new BuildRequestConfiguration(1, + new BuildRequestData( + projectFullPath: "path", + globalProperties: new Dictionary { ["p"] = "v" }, + toolsVersion: "13", + targetsToBuild: new[] { "bar" }, + hostServices: null), "13"); var configCache2 = new ConfigCache(); - configCache2.AddConfiguration(new BuildRequestConfiguration(configId: 1, new BuildRequestData("path", new Dictionary(){["p"] = "v"}, "13", new []{"a", "b"}, null), "13")); + configCache2.AddConfiguration(config2); var resultsCache2 = new ResultsCache(); - var buildResult21 = new BuildResult(new BuildRequest(1, 2, configurationId: 1, new List(){"e", "f"}, null, BuildEventContext.Invalid, null)); - buildResult21.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); - resultsCache2.AddResult(buildResult21); + var buildResult2 = new BuildResult(new BuildRequest(1, 2, configurationId: 1, + new List() { "e", "f" }, null, BuildEventContext.Invalid, null)); + + // exists only in config2 + buildResult2.AddResultsForTarget("target2", GetNonEmptySucceedingTargetResult("i2Config2")); + // exists in both configs with different values + buildResult2.AddResultsForTarget("target3", GetNonEmptySucceedingTargetResult("i3Config3")); + // exists in both configs with the same value + buildResult2.AddResultsForTarget("target4", GetNonEmptySucceedingTargetResult("v")); + + + resultsCache2.AddResult(buildResult2); aggregator.Add(configCache1, resultsCache1); aggregator.Add(configCache2, resultsCache2); - using (var env = TestEnvironment.Create()) - { - env.SetEnvironmentVariable("MSBUILDDONOTLAUNCHDEBUGGER", "1"); - var e = Should.Throw(() => aggregator.Aggregate()); - e.Message.ShouldContain("Input caches should not contain entries for the same configuration"); - } + var aggregatedCache = aggregator.Aggregate(); + + aggregatedCache.ConfigCache.ShouldHaveSingleItem(); + aggregatedCache.ConfigCache.First().ProjectFullPath.ShouldEndWith("path"); + aggregatedCache.ConfigCache.First().GlobalProperties.ToDictionary().ShouldBe(new Dictionary { ["p"] = "v" }); + aggregatedCache.ConfigCache.First().ToolsVersion.ShouldBe("13"); + // first config wins + aggregatedCache.ConfigCache.First().RequestedTargetNames.ShouldBe(new []{"foo"}); + + aggregatedCache.ResultsCache.Count().ShouldBe(1); + aggregatedCache.ResultsCache.First().ResultsByTarget.Count.ShouldBe(4); + aggregatedCache.ResultsCache.First().ResultsByTarget["target1"].Items.Aggregate(string.Empty, (acc, i) => $"{acc}{i.ItemSpec}").ShouldBe("i1Config1"); + aggregatedCache.ResultsCache.First().ResultsByTarget["target2"].Items.Aggregate(string.Empty, (acc, i) => $"{acc}{i.ItemSpec}").ShouldBe("i2Config2"); + // first target result wins + aggregatedCache.ResultsCache.First().ResultsByTarget["target3"].Items.Aggregate(string.Empty, (acc, i) => $"{acc}{i.ItemSpec}").ShouldBe("i3Config1"); + aggregatedCache.ResultsCache.First().ResultsByTarget["target4"].Items.Aggregate(string.Empty, (acc, i) => $"{acc}{i.ItemSpec}").ShouldBe("v"); } [Fact] @@ -195,7 +239,7 @@ public void SingleCacheWithSingleEntry() var resultsCache = new ResultsCache(); var buildResult = new BuildResult(new BuildRequest(1, 2, configurationId: 1, new List(){"a", "b"}, null, BuildEventContext.Invalid, null)); - buildResult.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); + buildResult.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult("i", "v")); resultsCache.AddResult(buildResult); aggregator.Add(configCache, resultsCache); @@ -214,9 +258,9 @@ public void MultipleCachesMultipleEntries() var resultsCache1 = new ResultsCache(); var buildResult11 = new BuildResult(new BuildRequest(1, 2, configurationId: 1, new List(){"a", "b"}, null, BuildEventContext.Invalid, null)); - buildResult11.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); + buildResult11.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult("i", "v")); var buildResult12 = new BuildResult(new BuildRequest(1, 2, configurationId: 2, new List(){"c", "d"}, null, BuildEventContext.Invalid, null)); - buildResult12.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); + buildResult12.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult("i", "v")); resultsCache1.AddResult(buildResult11); resultsCache1.AddResult(buildResult12); @@ -226,9 +270,9 @@ public void MultipleCachesMultipleEntries() var resultsCache2 = new ResultsCache(); var buildResult21 = new BuildResult(new BuildRequest(1, 2, configurationId: 1, new List(){"e", "f"}, null, BuildEventContext.Invalid, null)); - buildResult21.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); + buildResult21.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult("i", "v")); var buildResult22 = new BuildResult(new BuildRequest(1, 2, configurationId: 2, new List(){"g", "h"}, null, BuildEventContext.Invalid, null)); - buildResult22.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult()); + buildResult22.AddResultsForTarget("a", GetNonEmptySucceedingTargetResult("i", "v")); resultsCache2.AddResult(buildResult21); resultsCache2.AddResult(buildResult22); diff --git a/src/Build.UnitTests/BuildResultUtilities.cs b/src/Build.UnitTests/BuildResultUtilities.cs index ab0d27d2f17..9991febfb43 100644 --- a/src/Build.UnitTests/BuildResultUtilities.cs +++ b/src/Build.UnitTests/BuildResultUtilities.cs @@ -2,9 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; using Microsoft.Build.BackEnd; using Microsoft.Build.Execution; using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem; @@ -25,7 +22,12 @@ public static TargetResult GetEmptySucceedingTargetResult() public static TargetResult GetNonEmptySucceedingTargetResult() { - return new TargetResult(new TaskItem[1] { new TaskItem("i", "v")}, BuildResultUtilities.GetSuccessResult()); + return GetNonEmptySucceedingTargetResult("i", "v"); + } + + public static TargetResult GetNonEmptySucceedingTargetResult(string itemInclude, string definingFile = "") + { + return new TargetResult(new TaskItem[1] { new TaskItem(itemInclude, definingFile)}, BuildResultUtilities.GetSuccessResult()); } public static WorkUnitResult GetSuccessResult() diff --git a/src/Build.UnitTests/Graph/GraphLoadedFromSolution_tests.cs b/src/Build.UnitTests/Graph/GraphLoadedFromSolution_tests.cs index 8a15d66ec99..fe438dff95e 100644 --- a/src/Build.UnitTests/Graph/GraphLoadedFromSolution_tests.cs +++ b/src/Build.UnitTests/Graph/GraphLoadedFromSolution_tests.cs @@ -687,10 +687,10 @@ private void AssertSolutionBasedGraph( if (projectConfigurations == null || graphFromSolution.ProjectNodes.All(n => n.ProjectReferences.Count == 0)) { graphFromSolution.GraphRoots.Select(GetProjectPath) - .ShouldBeEquivalentTo(graph.GraphRoots.Select(GetProjectPath)); + .ShouldBeSameIgnoringOrder(graph.GraphRoots.Select(GetProjectPath)); graphFromSolution.ProjectNodes.Select(GetProjectPath) - .ShouldBeEquivalentTo(graph.ProjectNodes.Select(GetProjectPath)); + .ShouldBeSameIgnoringOrder(graph.ProjectNodes.Select(GetProjectPath)); } var expectedCurrentConfiguration = currentSolutionConfiguration ?? solutionConfigurations.First(); diff --git a/src/Build.UnitTests/Graph/GraphTestingUtilities.cs b/src/Build.UnitTests/Graph/GraphTestingUtilities.cs index b52763372bf..f3027105152 100644 --- a/src/Build.UnitTests/Graph/GraphTestingUtilities.cs +++ b/src/Build.UnitTests/Graph/GraphTestingUtilities.cs @@ -69,7 +69,7 @@ public static void AssertNonMultitargetingNode(ProjectGraphNode node, Dictionary additionalGlobalProperties = additionalGlobalProperties ?? new Dictionary(); IsNotMultitargeting(node).ShouldBeTrue(); - node.ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(EmptyGlobalProperties.AddRange(additionalGlobalProperties)); + node.ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(EmptyGlobalProperties.AddRange(additionalGlobalProperties)); node.ProjectInstance.GetProperty(InnerBuildPropertyName).ShouldBeNull(); } @@ -81,7 +81,7 @@ public static void AssertOuterBuildEvaluation(ProjectGraphNode outerBuild, Dicti IsInnerBuild(outerBuild).ShouldBeFalse(); outerBuild.ProjectInstance.GetProperty(InnerBuildPropertyName).ShouldBeNull(); - outerBuild.ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(EmptyGlobalProperties.AddRange(additionalGlobalProperties)); + outerBuild.ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(EmptyGlobalProperties.AddRange(additionalGlobalProperties)); } public static void AssertInnerBuildEvaluation( @@ -100,7 +100,7 @@ public static void AssertInnerBuildEvaluation( if (InnerBuildPropertyIsSetViaGlobalProperty) { - innerBuild.ProjectInstance.GlobalProperties.ShouldBeEquivalentTo( + innerBuild.ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder( EmptyGlobalProperties .Add(InnerBuildPropertyName, innerBuildPropertyValue) .AddRange(additionalGlobalProperties)); diff --git a/src/Build.UnitTests/Graph/IsolateProjects_Tests.cs b/src/Build.UnitTests/Graph/IsolateProjects_Tests.cs index 7cc2060da0f..ef2af0c6b72 100644 --- a/src/Build.UnitTests/Graph/IsolateProjects_Tests.cs +++ b/src/Build.UnitTests/Graph/IsolateProjects_Tests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using Microsoft.Build.BackEnd; using Microsoft.Build.Execution; using Microsoft.Build.Framework; using Microsoft.Build.Shared; @@ -12,6 +13,7 @@ using Shouldly; using Xunit; using Xunit.Abstractions; +using static Microsoft.Build.UnitTests.Helpers; namespace Microsoft.Build.Graph.UnitTests { @@ -130,6 +132,7 @@ public IsolateProjectsTests(ITestOutputHelper testOutput) { _testOutput = testOutput; _env = TestEnvironment.Create(_testOutput); + _env.DoNotLaunchDebugger(); if (NativeMethodsShared.IsOSX) { @@ -376,6 +379,318 @@ public void UndeclaredReferenceEnforcementShouldNormalizeFilePaths(Func + + + + ".Cleanup()).Path; + + var graph = CreateProjectGraph( + _env, + dependencyEdges: new Dictionary + { + {1, new[] {3, 4}}, + {2, new[] {3, 4}}, + }, + extraContentPerProjectNumber: new Dictionary + { + { + 1, + $@" + + <{ItemTypeNames.GraphIsolationExemptReference} Include='{exemptProjectFile}' /> + + + + + + + + + " + }, + { + 2, + @" + + + " + }, + { + 3, + $@" + + <{ItemTypeNames.GraphIsolationExemptReference} Include='{exemptProjectFile}' /> + + + + + + + + + " + }, + { + 4, + $@" + + <{ItemTypeNames.GraphIsolationExemptReference} Include='{exemptProjectFile}' /> + + + + + + + + + " + } + } + ); + + var cacheFiles = new Dictionary(); + + var buildResults = ResultCacheBasedBuilds_Tests.BuildGraphUsingCacheFiles( + _env, + graph: graph, + expectedLogOutputPerNode: new Dictionary(), + outputCaches: cacheFiles, + generateCacheFiles: true, + assertBuildResults: false); + + foreach (var result in buildResults) + { + result.Value.Result.OverallResult.ShouldBe(BuildResultCode.Success); + } + + cacheFiles.Count.ShouldBe(4); + + var caches = cacheFiles.ToDictionary(kvp => kvp.Key, kvp => CacheSerialization.DeserializeCaches(kvp.Value)); + + // 1 builds the exempt project but does not contain the exempt project in its output cache because it reads the + // exempt project's results from the input caches + // 2 does not contain the exempt project in its output cache because it does not build it + var projectsWhoseOutputCacheShouldContainTheExemptProject = new[] {3, 4}; + + foreach (var cache in caches) + { + cache.Value.exception.ShouldBeNull(); + var projectNumber = ProjectNumber(cache.Key.ProjectInstance.FullPath); + + cache.Value.ConfigCache.ShouldContain(c => ProjectNumber(c.ProjectFullPath) == projectNumber); + + cache.Value.ResultsCache.ShouldContain(r => r.HasResultsForTarget("Build")); + + if (projectNumber != 2) + { + cache.Value.ResultsCache.ShouldContain(r => r.HasResultsForTarget("TargetBuildingTheExemptProject")); + } + + if (projectsWhoseOutputCacheShouldContainTheExemptProject.Contains(projectNumber)) + { + cache.Value.ConfigCache.Count().ShouldBe(2); + + var exemptConfigs = cache.Value.ConfigCache.Where(c => c.ProjectFullPath.Equals(exemptProjectFile)).ToArray(); + exemptConfigs.Length.ShouldBe(1); + + exemptConfigs.First().SkippedFromStaticGraphIsolationConstraints.ShouldBeTrue(); + + cache.Value.ResultsCache.Count().ShouldBe(2); + + var exemptResults = cache.Value.ResultsCache + .Where(r => r.ConfigurationId == exemptConfigs.First().ConfigurationId).ToArray(); + exemptResults.Length.ShouldBe(1); + + exemptResults.First().ResultsByTarget.TryGetValue("BuildExemptProject", out var targetResult); + + targetResult.ShouldNotBeNull(); + targetResult.ResultCode.ShouldBe(TargetResultCode.Success); + } + else + { + cache.Value.ConfigCache.ShouldNotContain(c => c.SkippedFromStaticGraphIsolationConstraints); + cache.Value.ConfigCache.Count().ShouldBe(1); + + cache.Value.ResultsCache.ShouldNotContain(r => r.HasResultsForTarget("BuildExemptProject")); + cache.Value.ResultsCache.Count().ShouldBe(1); + } + } + } + + [Fact] + public void ProjectExemptFromIsolationOnlyIncludesNewlyBuiltTargetsInOutputCacheFile() + { + var graph = CreateProjectGraph( + _env, + dependencyEdges: new Dictionary + { + {1, new[] {2}}, + }, + extraContentPerProjectNumber: new Dictionary + { + { + 1, + $@" + + <{ItemTypeNames.GraphIsolationExemptReference} Include='$(MSBuildThisFileDirectory)\2.proj' /> + + + + + + + + + + " + }, + { + 2, + @" + + + + + + + " + } + } + ); + + var cacheFiles = new Dictionary(); + + var buildResults = ResultCacheBasedBuilds_Tests.BuildGraphUsingCacheFiles( + _env, + graph: graph, + expectedLogOutputPerNode: new Dictionary(), + outputCaches: cacheFiles, + generateCacheFiles: true, + assertBuildResults: false); + + foreach (var result in buildResults) + { + result.Value.Result.OverallResult.ShouldBe(BuildResultCode.Success); + } + + cacheFiles.Count.ShouldBe(2); + + var caches = cacheFiles.ToDictionary(kvp => kvp.Key, kvp => CacheSerialization.DeserializeCaches(kvp.Value)); + + var cache2 = caches.FirstOrDefault(c => ProjectNumber(c.Key) == 2); + + cache2.Value.ConfigCache.ShouldHaveSingleItem(); + cache2.Value.ConfigCache.First().ProjectFullPath.ShouldBe(cache2.Key.ProjectInstance.FullPath); + + cache2.Value.ResultsCache.ShouldHaveSingleItem(); + cache2.Value.ResultsCache.First().ResultsByTarget.Keys.ShouldBeSameIgnoringOrder(new[] { "Build2" }); + + var cache1 = caches.FirstOrDefault(c => ProjectNumber(c.Key) == 1); + + cache1.Value.ConfigCache.Count().ShouldBe(2); + cache1.Value.ResultsCache.Count().ShouldBe(2); + + foreach (var config in cache1.Value.ConfigCache) + { + switch (ProjectNumber(config.ProjectFullPath)) + { + case 1: + cache1.Value.ResultsCache.GetResultsForConfiguration(config.ConfigurationId).ResultsByTarget.Keys.ShouldBeSameIgnoringOrder(new []{ "Build", "ExtraBuild"}); + break; + case 2: + cache1.Value.ResultsCache.GetResultsForConfiguration(config.ConfigurationId).ResultsByTarget.Keys.ShouldBeSameIgnoringOrder(new[] { "UncachedTarget"}); + break; + default: throw new NotImplementedException(); + } + } + } + + [Fact] + public void SelfBuildsAreExemptFromIsolationConstraints() + { + var projectContents = @" + + + + + + + + + + + + +"; + var projectFile = _env.CreateFile("build.proj", projectContents.Cleanup()).Path; + var outputCacheFileForRoot = _env.CreateFile().Path; + var outputCacheFileForReference = _env.CreateFile().Path; + + using (var buildManagerSession = new BuildManagerSession( + _env, + new BuildParameters + { + OutputResultsCacheFile = outputCacheFileForReference + })) + { + buildManagerSession.BuildProjectFile(projectFile, new[] {"SelfBuild1"}, new Dictionary + { + {"TargetFramework", "foo"} + }).OverallResult.ShouldBe(BuildResultCode.Success); + } + + using (var buildManagerSession = new BuildManagerSession( + _env, + new BuildParameters + { + InputResultsCacheFiles = new []{outputCacheFileForReference}, + OutputResultsCacheFile = outputCacheFileForRoot + })) + { + buildManagerSession.BuildProjectFile(projectFile, new[] {"Build"}).OverallResult.ShouldBe(BuildResultCode.Success); + } + + var referenceCaches = CacheSerialization.DeserializeCaches(outputCacheFileForReference); + + referenceCaches.exception.ShouldBeNull(); + + referenceCaches.ConfigCache.ShouldHaveSingleItem(); + referenceCaches.ConfigCache.First().ProjectFullPath.ShouldBe(projectFile); + referenceCaches.ConfigCache.First().GlobalProperties.ToDictionary().Keys.ShouldBe(new[] {"TargetFramework"}); + referenceCaches.ConfigCache.First().SkippedFromStaticGraphIsolationConstraints.ShouldBeFalse(); + + referenceCaches.ResultsCache.ShouldHaveSingleItem(); + referenceCaches.ResultsCache.First().ResultsByTarget.Keys.ShouldBe(new[] { "SelfBuild1" }); + + var rootCaches = CacheSerialization.DeserializeCaches(outputCacheFileForRoot); + + rootCaches.ConfigCache.Count().ShouldBe(2); + + var rootConfig = rootCaches.ConfigCache.FirstOrDefault(c => !c.GlobalProperties.Contains("TargetFramework")); + var selfBuildConfig = rootCaches.ConfigCache.FirstOrDefault(c => c.GlobalProperties.Contains("TargetFramework")); + + rootConfig.ShouldNotBeNull(); + rootConfig.SkippedFromStaticGraphIsolationConstraints.ShouldBeFalse(); + + selfBuildConfig.ShouldNotBeNull(); + // Self builds that are not resolved from the cache are exempt from isolation constraints. + selfBuildConfig.SkippedFromStaticGraphIsolationConstraints.ShouldBeTrue(); + + rootCaches.ResultsCache.Count().ShouldBe(2); + rootCaches.ResultsCache.First(r => r.ConfigurationId == rootConfig.ConfigurationId).ResultsByTarget.Keys.ShouldBe(new []{"Build"}); + rootCaches.ResultsCache.First(r => r.ConfigurationId == selfBuildConfig.ConfigurationId).ResultsByTarget.Keys.ShouldBe(new []{"SelfBuild2"}); + } + + private static int ProjectNumber(string path) => int.Parse(Path.GetFileNameWithoutExtension(path)); + private static int ProjectNumber(ProjectGraphNode node) => int.Parse(Path.GetFileNameWithoutExtension(node.ProjectInstance.FullPath)); + private void AssertBuild( string[] targets, Action assert, diff --git a/src/Build.UnitTests/Graph/ParallelWorkSet_Tests.cs b/src/Build.UnitTests/Graph/ParallelWorkSet_Tests.cs index 119edea8b05..234f6f3a47d 100644 --- a/src/Build.UnitTests/Graph/ParallelWorkSet_Tests.cs +++ b/src/Build.UnitTests/Graph/ParallelWorkSet_Tests.cs @@ -247,7 +247,7 @@ private void TestParallelWorkSet(ParallelWorkSetTestCase tt) _workSet.WaitForAllWorkAndComplete(); _workSet.IsCompleted.ShouldBeTrue(); - _workSet.CompletedWork.ShouldBeEquivalentTo((IReadOnlyCollection>) tt.ExpectedCompletedWork); + _workSet.CompletedWork.ShouldBeSameIgnoringOrder((IReadOnlyCollection>) tt.ExpectedCompletedWork); } } } diff --git a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs index 84edacaf9c7..dfe2c5334f7 100644 --- a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs +++ b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs @@ -114,13 +114,13 @@ public void UpdatingReferencesIsBidirectional() node.AddProjectReference(reference1, referenceItem1, edges); node.AddProjectReference(reference2, referenceItem2, edges); - node.ProjectReferences.ShouldBeEquivalentTo(new []{reference1, reference2}); + node.ProjectReferences.ShouldBeSameIgnoringOrder(new []{reference1, reference2}); node.ReferencingProjects.ShouldBeEmpty(); - reference1.ReferencingProjects.ShouldBeEquivalentTo(new[] {node}); + reference1.ReferencingProjects.ShouldBeSameIgnoringOrder(new[] {node}); reference1.ProjectReferences.ShouldBeEmpty(); - reference2.ReferencingProjects.ShouldBeEquivalentTo(new[] {node}); + reference2.ReferencingProjects.ShouldBeSameIgnoringOrder(new[] {node}); reference2.ProjectReferences.ShouldBeEmpty(); edges[(node, reference1)].ShouldBe(referenceItem1); @@ -336,16 +336,16 @@ public void ProjectCollectionShouldNotInfluenceGlobalProperties() var root1 = GetFirstNodeWithProjectNumber(graph, 1); var globalPropertiesFor1 = new Dictionary { ["B"] = "EntryPointB", ["C"] = "EntryPointC", ["IsGraphBuild"] = "true" }; - root1.ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(globalPropertiesFor1); - root1.ProjectReferences.First(r => GetProjectNumber(r) == 3).ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(globalPropertiesFor1); - root1.ProjectReferences.First(r => GetProjectNumber(r) == 4).ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(globalPropertiesFor1); + root1.ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(globalPropertiesFor1); + root1.ProjectReferences.First(r => GetProjectNumber(r) == 3).ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(globalPropertiesFor1); + root1.ProjectReferences.First(r => GetProjectNumber(r) == 4).ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(globalPropertiesFor1); var root2 = GetFirstNodeWithProjectNumber(graph, 2); var globalPropertiesFor2 = new Dictionary { ["IsGraphBuild"] = "true" }; - root2.ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(globalPropertiesFor2); - root2.ProjectReferences.First(r => GetProjectNumber(r) == 4).ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(globalPropertiesFor2); - root2.ProjectReferences.First(r => GetProjectNumber(r) == 5).ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(globalPropertiesFor2); + root2.ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(globalPropertiesFor2); + root2.ProjectReferences.First(r => GetProjectNumber(r) == 4).ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(globalPropertiesFor2); + root2.ProjectReferences.First(r => GetProjectNumber(r) == 5).ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(globalPropertiesFor2); } [Fact] @@ -375,7 +375,7 @@ public void ConstructWithDifferentGlobalProperties() // Projects 2 and 3 both reference project 4, but with different properties, so they should not point to the same node. GetFirstNodeWithProjectNumber(graph, 2).ProjectReferences.First().ShouldNotBe(GetFirstNodeWithProjectNumber(graph, 3).ProjectReferences.First()); GetFirstNodeWithProjectNumber(graph, 2).ProjectReferences.First().ProjectInstance.FullPath.ShouldEndWith("4.proj"); - GetFirstNodeWithProjectNumber(graph, 2).ProjectReferences.First().ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(EmptyGlobalProperties); + GetFirstNodeWithProjectNumber(graph, 2).ProjectReferences.First().ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(EmptyGlobalProperties); GetFirstNodeWithProjectNumber(graph, 3).ProjectReferences.First().ProjectInstance.FullPath.ShouldEndWith("4.proj"); GetFirstNodeWithProjectNumber(graph, 3).ProjectReferences.First().ProjectInstance.GlobalProperties.Count.ShouldBeGreaterThan(1); } @@ -1266,6 +1266,134 @@ public void GetTargetsListProjectReferenceTargetsOrDefaultComplexPropagation() } } + [Fact] + public void GetTargetsListShouldSupportTargetsMarkedASkipNonexistentTargets() + { + var graph = Helpers.CreateProjectGraph( + env: _env, + dependencyEdges: new Dictionary + { + {1, new[] {2}} + }, + extraContentPerProjectNumber: new Dictionary + { + { + 1, + @" + + + + + + + + + + + + " + }, + { + 2, + @" + + + + + + + " + } + } + ); + + var targetLists = graph.GetTargetLists(entryProjectTargets: new[] {"Build"}); + + targetLists[key: GetFirstNodeWithProjectNumber(graph: graph, projectNum: 1)].ShouldBe(expected: new []{"Build"}); + targetLists[key: GetFirstNodeWithProjectNumber(graph: graph, projectNum: 2)].ShouldBe(expected: new []{"MandatoryTarget1", "MandatoryTarget2", "OptionalTargetWhichExists"}); + + + var results = ResultCacheBasedBuilds_Tests.BuildGraphUsingCacheFiles( + env: _env, + graph: graph, + generateCacheFiles: true, + expectedLogOutputPerNode: new Dictionary(), + outputCaches: new Dictionary(), + assertBuildResults: false, + targetListsPerNode: targetLists); + + foreach (var result in results) + { + result.Value.Result.OverallResult.ShouldBe(BuildResultCode.Success); + } + } + + [Fact] + public void SkipNonexistentTargetsShouldNotHideMissedTargetResults() + { + _env.DoNotLaunchDebugger(); + + var graph = Helpers.CreateProjectGraph( + env: _env, + dependencyEdges: new Dictionary + { + {1, new[] {2}} + }, + extraContentPerProjectNumber: new Dictionary + { + { + 1, + @" + + + + + + + + + + " + }, + { + 2, + @" + + + + + + + " + } + } + ); + + var targetLists = graph.GetTargetLists(entryProjectTargets: new[] { "Build" }); + + targetLists[key: GetFirstNodeWithProjectNumber(graph: graph, projectNum: 1)].ShouldBe(expected: new[] { "Build" }); + targetLists[key: GetFirstNodeWithProjectNumber(graph: graph, projectNum: 2)].ShouldBe(expected: new[] { "MandatoryTarget1", "OptionalTargetWhichExists" }); + + + var results = ResultCacheBasedBuilds_Tests.BuildGraphUsingCacheFiles( + env: _env, + graph: graph, + generateCacheFiles: true, + expectedLogOutputPerNode: new Dictionary(), + outputCaches: new Dictionary(), + assertBuildResults: false, + targetListsPerNode: targetLists); + + results[2].Result.OverallResult.ShouldBe(BuildResultCode.Success); + results[1].Result.OverallResult.ShouldBe(BuildResultCode.Failure); + + results[1].Logger.ErrorCount.ShouldBe(1); + results[1].Logger.Errors.First().Message.ShouldContain("MSB4252"); + results[1].Logger.Errors.First().Message.ShouldContain("1.proj"); + results[1].Logger.Errors.First().Message.ShouldContain("2.proj"); + results[1].Logger.Errors.First().Message.ShouldContain(" with the (MandatoryTarget1;MandatoryTarget2;OptionalTargetWhichExists;OptionalTargetWhichDoesNotExist) target(s) but the build result for the built project is not in the engine cache"); + } + public static IEnumerable Graphs { get @@ -1726,13 +1854,13 @@ public void InnerBuildsCanHaveSeparateReferences() innerBuildWithCommonReferences.ProjectReferences.Count.ShouldBe(4); var referenceNumbersSet = innerBuildWithCommonReferences.ProjectReferences.Select(r => Path.GetFileNameWithoutExtension(r.ProjectInstance.FullPath)).ToHashSet(); - referenceNumbersSet.ShouldBeEquivalentTo(new HashSet{"2", "3"}); + referenceNumbersSet.ShouldBeSameIgnoringOrder(new HashSet{"2", "3"}); var innerBuildWithAdditionalReferences = GetNodesWithProjectNumber(graph, 1).First(n => n.ProjectInstance.GlobalProperties.TryGetValue(InnerBuildPropertyName, out string p) && p == "b"); innerBuildWithAdditionalReferences.ProjectReferences.Count.ShouldBe(8); referenceNumbersSet = innerBuildWithAdditionalReferences.ProjectReferences.Select(r => Path.GetFileNameWithoutExtension(r.ProjectInstance.FullPath)).ToHashSet(); - referenceNumbersSet.ShouldBeEquivalentTo(new HashSet{"2", "3", "4", "5"}); + referenceNumbersSet.ShouldBeSameIgnoringOrder(new HashSet{"2", "3", "4", "5"}); } [Fact] @@ -1767,7 +1895,7 @@ public void InnerBuildProducedByOuterBuildCanBeReferencedByAnotherNode() two.ProjectReferences.ShouldHaveSingleItem(); two.ProjectReferences.First().ShouldBe(referencedInnerBuild); - referencedInnerBuild.ReferencingProjects.ShouldBeEquivalentTo(new []{two, outerBuild}); + referencedInnerBuild.ReferencingProjects.ShouldBeSameIgnoringOrder(new []{two, outerBuild}); } [Fact] @@ -1852,7 +1980,7 @@ public void InnerBuildsProducedByOuterBuildsCanBeReferencedByOtherInnerBuilds() // the outer build is necessary as the referencing inner build can still call targets on it GetNodesWithProjectNumber(graph, 2).Count().ShouldBe(2); - innerBuild1WithReferenceToInnerBuild2.ProjectReferences.ShouldBeEquivalentTo(new []{outerBuild2, innerBuild2}); + innerBuild1WithReferenceToInnerBuild2.ProjectReferences.ShouldBeSameIgnoringOrder(new []{outerBuild2, innerBuild2}); } public static IEnumerable AllNodesShouldHaveGraphBuildGlobalPropertyData @@ -1953,7 +2081,7 @@ public void AllNodesShouldHaveGraphBuildGlobalProperty(Dictionary ed foreach (var node in projectGraph.ProjectNodes) { - node.ProjectInstance.GlobalProperties.ShouldBeEquivalentTo(expectedGlobalProperties); + node.ProjectInstance.GlobalProperties.ShouldBeSameIgnoringOrder(expectedGlobalProperties); } } } diff --git a/src/Build.UnitTests/Graph/ResultCacheBasedBuilds_Tests.cs b/src/Build.UnitTests/Graph/ResultCacheBasedBuilds_Tests.cs index aab69e7f57c..c58eb16a24c 100644 --- a/src/Build.UnitTests/Graph/ResultCacheBasedBuilds_Tests.cs +++ b/src/Build.UnitTests/Graph/ResultCacheBasedBuilds_Tests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.IO; using System.Linq; using System.Text; @@ -16,9 +17,7 @@ using Xunit; using Xunit.Abstractions; using static Microsoft.Build.UnitTests.Helpers; - -using ExpectedNodeBuildOutput = System.Collections.Generic.Dictionary; -using OutputCacheDictionary = System.Collections.Generic.Dictionary; +using static Microsoft.Build.Graph.UnitTests.GraphTestingUtilities; namespace Microsoft.Build.Graph.UnitTests { @@ -283,25 +282,24 @@ public static IEnumerable BuildGraphData [MemberData(nameof(BuildGraphData))] public void BuildProjectGraphUsingCaches(Dictionary edges) { - var topoSortedNodes = + var graph = CreateProjectGraph( env: _env, dependencyEdges: edges, globalProperties: null, - createProjectFile: CreateProjectFileWrapper) - .ProjectNodesTopologicallySorted.ToArray(); + createProjectFile: CreateProjectFileWrapper); - var expectedOutput = new ExpectedNodeBuildOutput(); + var expectedOutput = new Dictionary(); - var outputCaches = new OutputCacheDictionary(); + var outputCaches = new Dictionary(); // Build unchanged project files using caches. - BuildUsingCaches(topoSortedNodes, expectedOutput, outputCaches, generateCacheFiles: true); + BuildGraphUsingCacheFiles(_env, graph, expectedOutput, outputCaches, generateCacheFiles: true); // Change the project files to remove all items. var collection = _env.CreateProjectCollection().Collection; - foreach (var node in topoSortedNodes) + foreach (var node in graph.ProjectNodesTopologicallySorted) { var project = Project.FromFile( node.ProjectInstance.FullPath, @@ -315,35 +313,33 @@ public void BuildProjectGraphUsingCaches(Dictionary edges) } // Build again using the first caches. Project file changes from references should not be visible. - BuildUsingCaches( - topoSortedNodes, + BuildGraphUsingCacheFiles(_env, graph, expectedOutput, outputCaches, generateCacheFiles: false, assertBuildResults: true, // there are no items in the second build. The references are loaded from cache and have items, // but the current project is loaded from file and has no items - (node, localExpectedOutput) => localExpectedOutput[node].Skip(1).ToArray()); + expectedOutputProducer: (node, localExpectedOutput) => localExpectedOutput[node].Skip(1).ToArray()); } [Fact] public void OutputCacheShouldNotContainInformationFromInputCaches() { - var topoSortedNodes = + var graph = CreateProjectGraph( env: _env, - dependencyEdges: new Dictionary { { 1, new[] { 2, 3 } } }, + dependencyEdges: new Dictionary {{1, new[] {2, 3}}}, globalProperties: null, - createProjectFile: CreateProjectFileWrapper) - .ProjectNodesTopologicallySorted.ToArray(); + createProjectFile: CreateProjectFileWrapper); - var expectedOutput = new ExpectedNodeBuildOutput(); + var expectedOutput = new Dictionary(); - var outputCaches = new OutputCacheDictionary(); + var outputCaches = new Dictionary(); - BuildUsingCaches(topoSortedNodes, expectedOutput, outputCaches, generateCacheFiles: true); + BuildGraphUsingCacheFiles(_env, graph, expectedOutput, outputCaches, generateCacheFiles: true); - var rootNode = topoSortedNodes.First(n => Path.GetFileNameWithoutExtension(n.ProjectInstance.FullPath) == "1"); + var rootNode = graph.ProjectNodesTopologicallySorted.First(n => Path.GetFileNameWithoutExtension(n.ProjectInstance.FullPath) == "1"); var outputCache = outputCaches[rootNode]; outputCache.ShouldNotBeNull(); @@ -367,79 +363,180 @@ public void OutputCacheShouldNotContainInformationFromInputCaches() [Fact] public void MissingResultFromCacheShouldErrorDueToIsolatedBuildCacheEnforcement() { - var topoSortedNodes = + var graph = CreateProjectGraph( env: _env, dependencyEdges: new Dictionary { { 1, new[] { 2, 3 } } }, globalProperties: null, - createProjectFile: CreateProjectFileWrapper) - .ProjectNodesTopologicallySorted.ToArray(); + createProjectFile: CreateProjectFileWrapper); - var expectedOutput = new ExpectedNodeBuildOutput(); + var expectedOutput = new Dictionary(); - var outputCaches = new OutputCacheDictionary(); + var outputCaches = new Dictionary(); - BuildUsingCaches(topoSortedNodes, expectedOutput, outputCaches, generateCacheFiles: true); + BuildGraphUsingCacheFiles(_env, graph, expectedOutput, outputCaches, generateCacheFiles: true); // remove cache for project 3 to cause a cache miss - outputCaches.Remove(expectedOutput.Keys.First(n => ProjectNumber(n) == "3")); + outputCaches.Remove(expectedOutput.Keys.First(n => GetProjectNumber(n) == 3)); - var results = BuildUsingCaches(topoSortedNodes, expectedOutput, outputCaches, generateCacheFiles: false, assertBuildResults: false); + var results = BuildGraphUsingCacheFiles(_env, graph, expectedOutput, outputCaches, generateCacheFiles: false, assertBuildResults: false); - results["3"].Result.OverallResult.ShouldBe(BuildResultCode.Success); - results["2"].Result.OverallResult.ShouldBe(BuildResultCode.Success); + results[3].Result.OverallResult.ShouldBe(BuildResultCode.Success); + results[2].Result.OverallResult.ShouldBe(BuildResultCode.Success); - results["1"].Result.OverallResult.ShouldBe(BuildResultCode.Failure); - results["1"].Logger.ErrorCount.ShouldBe(1); - results["1"].Logger.Errors.First().Message.ShouldContain("MSB4252"); + results[1].Result.OverallResult.ShouldBe(BuildResultCode.Failure); + results[1].Logger.ErrorCount.ShouldBe(1); + results[1].Logger.Errors.First().Message.ShouldContain("MSB4252"); - results["1"].Logger.Errors.First().BuildEventContext.NodeId.ShouldNotBe(BuildEventContext.InvalidNodeId); - results["1"].Logger.Errors.First().BuildEventContext.ProjectInstanceId.ShouldNotBe(BuildEventContext.InvalidProjectInstanceId); - results["1"].Logger.Errors.First().BuildEventContext.ProjectContextId.ShouldNotBe(BuildEventContext.InvalidProjectContextId); - results["1"].Logger.Errors.First().BuildEventContext.TargetId.ShouldNotBe(BuildEventContext.InvalidTargetId); - results["1"].Logger.Errors.First().BuildEventContext.TaskId.ShouldNotBe(BuildEventContext.InvalidTaskId); + results[1].Logger.Errors.First().BuildEventContext.NodeId.ShouldNotBe(BuildEventContext.InvalidNodeId); + results[1].Logger.Errors.First().BuildEventContext.ProjectInstanceId.ShouldNotBe(BuildEventContext.InvalidProjectInstanceId); + results[1].Logger.Errors.First().BuildEventContext.ProjectContextId.ShouldNotBe(BuildEventContext.InvalidProjectContextId); + results[1].Logger.Errors.First().BuildEventContext.TargetId.ShouldNotBe(BuildEventContext.InvalidTargetId); + results[1].Logger.Errors.First().BuildEventContext.TaskId.ShouldNotBe(BuildEventContext.InvalidTaskId); + } + + [Fact] + public void CacheFilesShouldNotContainTransitiveContent() + { + var graph = CreateProjectGraph( + _env, + dependencyEdges: new Dictionary + { + {1, new[] {2}}, + {2, new[] {3}} + }, + extraContentPerProjectNumber: new Dictionary + { + { + 1, + @" + + + + + + + + + " + }, + { + 2, + @" + + + + + + + + + + " + }, + { + 3, + @" + + + + + + + + + " + } + } + ); + + var caches = new Dictionary(); + + var buildResults = BuildGraphUsingCacheFiles(_env, graph: graph, + expectedLogOutputPerNode: new Dictionary(), + outputCaches: caches, + generateCacheFiles: true, + assertBuildResults: false); + + foreach (var result in buildResults) + { + result.Value.Result.OverallResult.ShouldBe(BuildResultCode.Success); + } + + buildResults.Count.ShouldBe(3); + + caches.Count.ShouldBe(3); + + var rootCache = caches.FirstOrDefault(c => GetProjectNumber(c.Key) == 1); + + rootCache.ShouldNotBeNull(); + + var (configCache, resultsCache, exception) = CacheSerialization.DeserializeCaches(rootCache.Value); + + exception.ShouldBeNull(); + + configCache.ShouldHaveSingleItem(); + configCache.First().ProjectFullPath.ShouldEndWith("1.proj"); + + resultsCache.ShouldHaveSingleItem(); + + var targetResults = resultsCache.First().ResultsByTarget; + targetResults.Count.ShouldBe(3); + + var expectedTargetsInResultsCache = new[] {"Build", "BeforeBuild", "AfterBuild"}; + + foreach (var targetResult in targetResults) + { + expectedTargetsInResultsCache.ShouldContain(targetResult.Key); + } } /// /// This method runs in two modes. - /// When is true, the method will fill in the empty and , simulating a build from scratch. - /// When it is false, it uses the filled in and to simulate a fully cached build. + /// When is true, the method will fill in + /// the empty and simulating a build from scratch. + /// When it is false, it uses the filled in and to simulate a fully cached build. /// /// - /// - /// + /// + /// + /// /// /// /// /// + /// /// - private Dictionary BuildUsingCaches( - IReadOnlyCollection topoSortedNodes, - ExpectedNodeBuildOutput expectedNodeBuildOutput, - OutputCacheDictionary outputCaches, + internal static Dictionary BuildGraphUsingCacheFiles( + TestEnvironment env, + ProjectGraph graph, + Dictionary expectedLogOutputPerNode, + Dictionary outputCaches, bool generateCacheFiles, bool assertBuildResults = true, // (current node, expected output dictionary) -> actual expected output for current node - Func expectedOutputProducer = null) + Func, string[]> expectedOutputProducer = null, + IReadOnlyDictionary> targetListsPerNode = null) { - expectedOutputProducer = expectedOutputProducer ?? ((node, expectedOutputs) => expectedOutputs[node]); + expectedOutputProducer ??= ((node, expectedOutputs) => expectedOutputs[node]); - var results = new Dictionary(topoSortedNodes.Count); + var results = new Dictionary(graph.ProjectNodesTopologicallySorted.Count); if (generateCacheFiles) { outputCaches.ShouldBeEmpty(); - expectedNodeBuildOutput.ShouldBeEmpty(); + expectedLogOutputPerNode.ShouldBeEmpty(); } - foreach (var node in topoSortedNodes) + foreach (var node in graph.ProjectNodesTopologicallySorted) { if (generateCacheFiles) { - expectedNodeBuildOutput[node] = ExpectedBuildOutputForNode(node); + expectedLogOutputPerNode[node] = ExpectedBuildOutputForNode(node); } - var cacheFilesForReferences = node.ProjectReferences.Where(r => outputCaches.ContainsKey(r)).Select(r => outputCaches[r]).ToArray(); + var cacheFilesForReferences = + node.ProjectReferences.Where(r => outputCaches.ContainsKey(r)).Select(r => outputCaches[r]).ToArray(); var buildParameters = new BuildParameters { @@ -448,20 +545,21 @@ public void MissingResultFromCacheShouldErrorDueToIsolatedBuildCacheEnforcement( if (generateCacheFiles) { - outputCaches[node] = _env.DefaultTestDirectory.CreateFile($"OutputCache-{ProjectNumber(node)}").Path; + outputCaches[node] = env.DefaultTestDirectory.CreateFile($"OutputCache-{GetProjectNumber(node)}").Path; buildParameters.OutputResultsCacheFile = outputCaches[node]; } - var logger = new MockLogger(); + var logger = new MockLogger(env.Output); buildParameters.Loggers = new[] {logger}; var result = BuildProjectFileUsingBuildManager( node.ProjectInstance.FullPath, null, - buildParameters); + buildParameters, + targetListsPerNode?[node].ToArray()); - results[ProjectNumber(node)] = (result, logger); + results[GetProjectNumber(node)] = (result, logger); if (assertBuildResults) { @@ -469,7 +567,7 @@ public void MissingResultFromCacheShouldErrorDueToIsolatedBuildCacheEnforcement( var actualOutput = result.ResultsByTarget["Build"].Items.Select(i => i.ItemSpec).ToArray(); - var expectedOutputForNode = expectedOutputProducer(node, expectedNodeBuildOutput); + var expectedOutputForNode = expectedOutputProducer(node, expectedLogOutputPerNode); actualOutput.ShouldBe(expectedOutputForNode); } @@ -479,11 +577,9 @@ public void MissingResultFromCacheShouldErrorDueToIsolatedBuildCacheEnforcement( string[] ExpectedBuildOutputForNode(ProjectGraphNode node) { - var expectedOutputForNode = new List(); + var expectedOutputForNode = new List {GetProjectNumber(node).ToString()}; - expectedOutputForNode.Add(ProjectNumber(node)); - - foreach (var referenceOutput in node.ProjectReferences.SelectMany(n => expectedNodeBuildOutput[n])) + foreach (var referenceOutput in node.ProjectReferences.SelectMany(n => expectedLogOutputPerNode[n])) { if (!expectedOutputForNode.Contains(referenceOutput)) { @@ -495,8 +591,6 @@ string[] ExpectedBuildOutputForNode(ProjectGraphNode node) } } - private static string ProjectNumber(ProjectGraphNode node) => Path.GetFileNameWithoutExtension(node.ProjectInstance.FullPath); - private static TransientTestFile CreateProjectFileWrapper(TestEnvironment env, int projectNumber, int[] projectReferences, Dictionary projectReferenceTargets, string defaultTargets, string extraContent) { return CreateProjectFileWithBuildTargetAndItems(env, projectNumber, projectReferences, defaultTargets); @@ -530,7 +624,7 @@ internal static TransientTestFile CreateProjectFileWithBuildTargetAndItems( "); - return CreateProjectFile( + return Helpers.CreateProjectFile( env, projectNumber, projectReferences, diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index b2157cf4e07..cd7dc824128 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -417,6 +417,8 @@ public void BeginBuild(BuildParameters parameters) // Initialize additional build parameters. _buildParameters.BuildId = GetNextBuildId(); + // Loading caches from files turns on project isolation constraints. + // Undefined behavior for what would happen if file based caches are used without isolation. if (_buildParameters.UsesCachedResults()) { _buildParameters.IsolateProjects = true; @@ -511,22 +513,24 @@ void InitializeCaches() _configCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ConfigCache) as IConfigCache; _resultsCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ResultsCache) as IResultsCache; - if (!usesInputCaches && (_buildParameters.ResetCaches || _configCache.IsConfigCacheSizeLargerThanThreshold())) + if (usesInputCaches) + { + return; + } + + if ((_buildParameters.ResetCaches || _configCache.IsConfigCacheSizeLargerThanThreshold())) { ResetCaches(); } else { - if (!usesInputCaches) - { - List configurationsCleared = _configCache.ClearNonExplicitlyLoadedConfigurations(); + List configurationsCleared = _configCache.ClearNonExplicitlyLoadedConfigurations(); - if (configurationsCleared != null) + if (configurationsCleared != null) + { + foreach (int configurationId in configurationsCleared) { - foreach (int configurationId in configurationsCleared) - { - _resultsCache.ClearResultsForConfiguration(configurationId); - } + _resultsCache.ClearResultsForConfiguration(configurationId); } } @@ -1144,7 +1148,7 @@ internal void LoadSolutionIntoConfiguration(BuildRequestConfiguration config, Bu } ErrorUtilities.VerifyThrow(FileUtilities.IsSolutionFilename(config.ProjectFullPath), "{0} is not a solution", config.ProjectFullPath); - ProjectInstance[] instances = ProjectInstance.LoadSolutionForBuild(config.ProjectFullPath, config.GlobalProperties, config.ExplicitToolsVersionSpecified ? config.ToolsVersion : null, _buildParameters, ((IBuildComponentHost)this).LoggingService, request.BuildEventContext, false /* loaded by solution parser*/, config.TargetNames, SdkResolverService, request.SubmissionId); + ProjectInstance[] instances = ProjectInstance.LoadSolutionForBuild(config.ProjectFullPath, config.GlobalProperties, config.ExplicitToolsVersionSpecified ? config.ToolsVersion : null, _buildParameters, ((IBuildComponentHost)this).LoggingService, request.BuildEventContext, false /* loaded by solution parser*/, config.RequestedTargetNames, SdkResolverService, request.SubmissionId); // The first instance is the traversal project, which goes into this configuration config.Project = instances[0]; @@ -1816,6 +1820,11 @@ private void HandleResult(int node, BuildResult result) { configuration.ProjectInitialTargets = result.InitialTargets; } + + if (configuration.ProjectTargets == null) + { + configuration.ProjectTargets = result.Targets; + } } IEnumerable response = _scheduler.ReportResult(node, result); @@ -2484,11 +2493,15 @@ private bool ReuseOldCaches(string[] inputCacheFiles) var cacheAggregation = cacheAggregator.Aggregate(); - // using caches with override (override queried first before current cache) based on the assumption that during single project cached builds - // there's many old results, but just one single actively building project. - - _componentFactories.ReplaceFactory(BuildComponentType.ConfigCache, new ConfigCacheWithOverride(cacheAggregation.ConfigCache)); - _componentFactories.ReplaceFactory(BuildComponentType.ResultsCache, new ResultsCacheWithOverride(cacheAggregation.ResultsCache)); + // In a build session with input caches, build results from cache files are separated from new build results executed in the current build session. + // This separation is done to differentiate between the two types, as the output caches that will be written by the current build session + // should contain only the results from new builds, and should not contain old results inherited from the cache files. + // The override cache will contain the old build results, and the current cache will contain new results. + // Upon reads, both caches are interrogated (override before current), but writes should only happen in the current cache. + var configCacheWithOverride = new ConfigCacheWithOverride(cacheAggregation.ConfigCache, _buildParameters.IsolateProjects); + _componentFactories.ReplaceFactory(BuildComponentType.ConfigCache, configCacheWithOverride); + _componentFactories.ReplaceFactory(BuildComponentType.ResultsCache, + new ResultsCacheWithOverride(cacheAggregation.ResultsCache, _buildParameters.IsolateProjects, configCacheWithOverride)); return true; } diff --git a/src/Build/BackEnd/BuildManager/CacheAggregator.cs b/src/Build/BackEnd/BuildManager/CacheAggregator.cs index 7c4530234cb..a8b9cae69a8 100644 --- a/src/Build/BackEnd/BuildManager/CacheAggregator.cs +++ b/src/Build/BackEnd/BuildManager/CacheAggregator.cs @@ -63,14 +63,24 @@ private void InsertCaches(IConfigCache configCache, IResultsCache resultsCache) return; } - var seenConfigIds = new HashSet(); var configIdMapping = new Dictionary(); + // seen config id -> equivalent config id already existing in the aggregated cache (null if not existing) + var seenConfigIds = new Dictionary(); + foreach (var config in configs) { - seenConfigIds.Add(config.ConfigurationId); + var existingConfig = _aggregatedConfigCache.GetMatchingConfiguration(config); + + if (existingConfig != null) + { + // This config has been found in a previous cache file. Don't aggregate it. + // => "First config wins" conflict resolution. + seenConfigIds[config.ConfigurationId] = existingConfig.ConfigurationId; + continue; + } - ErrorUtilities.VerifyThrow(_aggregatedConfigCache.GetMatchingConfiguration(config) == null, "Input caches should not contain entries for the same configuration"); + seenConfigIds[config.ConfigurationId] = null; _lastConfigurationId = _nextConfigurationId(); configIdMapping[config.ConfigurationId] = _lastConfigurationId; @@ -83,17 +93,38 @@ private void InsertCaches(IConfigCache configCache, IResultsCache resultsCache) foreach (var result in results) { - ErrorUtilities.VerifyThrow(seenConfigIds.Contains(result.ConfigurationId), "Each result should have a corresponding configuration. Otherwise the caches are not consistent"); - - _aggregatedResultsCache.AddResult( - new BuildResult( - result, - BuildEventContext.InvalidSubmissionId, - configIdMapping[result.ConfigurationId], - BuildRequest.InvalidGlobalRequestId, - BuildRequest.InvalidGlobalRequestId, - BuildRequest.InvalidNodeRequestId + ErrorUtilities.VerifyThrow(seenConfigIds.ContainsKey(result.ConfigurationId), "Each result should have a corresponding configuration. Otherwise the caches are not consistent"); + + if (seenConfigIds[result.ConfigurationId] != null) + { + // The config is already present in the aggregated cache. Merge the new build results into the ones already present in the aggregated cache. + MergeBuildResults(result, _aggregatedResultsCache.GetResultsForConfiguration(seenConfigIds[result.ConfigurationId].Value)); + } + else + { + _aggregatedResultsCache.AddResult( + new BuildResult( + result: result, + submissionId: BuildEventContext.InvalidSubmissionId, + configurationId: configIdMapping[result.ConfigurationId], + requestId: BuildRequest.InvalidGlobalRequestId, + parentRequestId: BuildRequest.InvalidGlobalRequestId, + nodeRequestId: BuildRequest.InvalidNodeRequestId )); + } + } + } + + private void MergeBuildResults(BuildResult newResult, BuildResult existingResult) + { + foreach (var newTargetResult in newResult.ResultsByTarget) + { + // "First target result wins" conflict resolution. Seems like a reasonable heuristic, because targets in MSBuild should only run once + // for a given config, which means that a target's result should not change if the config does not changes. + if (!existingResult.HasResultsForTarget(newTargetResult.Key)) + { + existingResult.ResultsByTarget[newTargetResult.Key] = newTargetResult.Value; + } } } } diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs index 2cf6ea5f8ed..f7e96e0d262 100644 --- a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs +++ b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs @@ -7,6 +7,7 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; +using System.Linq; using System.Threading; using System.Threading.Tasks.Dataflow; using Microsoft.Build.BackEnd.Logging; @@ -784,6 +785,7 @@ private void EvaluateRequestStates() // own cache. completedEntry.Result.DefaultTargets = configuration.ProjectDefaultTargets; completedEntry.Result.InitialTargets = configuration.ProjectInitialTargets; + completedEntry.Result.Targets = configuration.ProjectTargets; } TraceEngine("ERS: Request is now {0}({1}) (nr {2}) has had its builder cleaned up.", completedEntry.Request.GlobalRequestId, completedEntry.Request.ConfigurationId, completedEntry.Request.NodeRequestId); diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs index 8446b48cae0..65bea74eaf8 100644 --- a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs +++ b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEntry.cs @@ -192,7 +192,7 @@ public void WaitForConfiguration(BuildRequestConfiguration configuration) { ErrorUtilities.VerifyThrow(configuration.WasGeneratedByNode, "Configuration has already been resolved."); - _unresolvedConfigurationsToIssue = _unresolvedConfigurationsToIssue ?? new List(); + _unresolvedConfigurationsToIssue ??= new List(); _unresolvedConfigurationsToIssue.Add(configuration); } diff --git a/src/Build/BackEnd/Components/Caching/ConfigCache.cs b/src/Build/BackEnd/Components/Caching/ConfigCache.cs index b6989894a6b..5031f924f82 100644 --- a/src/Build/BackEnd/Components/Caching/ConfigCache.cs +++ b/src/Build/BackEnd/Components/Caching/ConfigCache.cs @@ -66,6 +66,24 @@ public BuildRequestConfiguration this[int configId] #region IConfigCache Members + /// + public bool TryGetConfiguration(int configId, out BuildRequestConfiguration existingConfig) + { + lock (_lockObject) + { + if (HasConfiguration(configId)) + { + existingConfig = this[configId]; + return true; + } + else + { + existingConfig = null; + return false; + } + } + } + /// /// Adds the specified configuration to the cache. /// diff --git a/src/Build/BackEnd/Components/Caching/ConfigCacheWithOverride.cs b/src/Build/BackEnd/Components/Caching/ConfigCacheWithOverride.cs index c6c91d88f77..288c34cc030 100644 --- a/src/Build/BackEnd/Components/Caching/ConfigCacheWithOverride.cs +++ b/src/Build/BackEnd/Components/Caching/ConfigCacheWithOverride.cs @@ -9,14 +9,19 @@ namespace Microsoft.Build.Execution { + // This class composes two caches, an override cache and a current cache. + // Reads are served from both caches (override first). + // Writes should only happen in the current cache. internal class ConfigCacheWithOverride : IConfigCache { private readonly IConfigCache _override; + private readonly bool _isolateProjects; public ConfigCache CurrentCache { get; } - public ConfigCacheWithOverride(IConfigCache @override) + public ConfigCacheWithOverride(IConfigCache @override, bool isolateProjects) { _override = @override; + _isolateProjects = isolateProjects; CurrentCache = new ConfigCache(); } @@ -32,6 +37,8 @@ public void ShutdownComponent() public IEnumerator GetEnumerator() { + // Enumerators do not compose both caches to limit the influence of the override cache (reduce the number of possible states out there). + // So far all runtime examples do not need the two composed. return CurrentCache.GetEnumerator(); } @@ -49,12 +56,11 @@ public BuildRequestConfiguration this[int configId] { get { - if (_override.HasConfiguration(configId)) + if (_override.TryGetConfiguration(configId, out var overrideConfig)) { -#if DEBUG - ErrorUtilities.VerifyThrow(!CurrentCache.HasConfiguration(configId), "caches should not overlap"); -#endif - return _override[configId]; + AssertCurrentCacheDoesNotContainConfig(overrideConfig); + + return overrideConfig; } else { @@ -63,6 +69,18 @@ public BuildRequestConfiguration this[int configId] } } + public bool TryGetConfiguration(int configId, out BuildRequestConfiguration existingConfig) + { + if (_override.TryGetConfiguration(configId, out existingConfig)) + { + AssertCurrentCacheDoesNotContainConfig(existingConfig); + + return true; + } + + return CurrentCache.TryGetConfiguration(configId, out existingConfig); + } + public void AddConfiguration(BuildRequestConfiguration config) { CurrentCache.AddConfiguration(config); @@ -79,9 +97,8 @@ public BuildRequestConfiguration GetMatchingConfiguration(BuildRequestConfigurat if (overrideConfig != null) { -#if DEBUG - ErrorUtilities.VerifyThrow(CurrentCache.GetMatchingConfiguration(config) == null, "caches should not overlap"); -#endif + AssertCurrentCacheDoesNotContainConfig(overrideConfig); + return overrideConfig; } else @@ -96,9 +113,8 @@ public BuildRequestConfiguration GetMatchingConfiguration(ConfigurationMetadata if (overrideConfig != null) { -#if DEBUG - ErrorUtilities.VerifyThrow(CurrentCache.GetMatchingConfiguration(configMetadata) == null, "caches should not overlap"); -#endif + AssertCurrentCacheDoesNotContainConfig(overrideConfig); + return overrideConfig; } else @@ -109,22 +125,29 @@ public BuildRequestConfiguration GetMatchingConfiguration(ConfigurationMetadata public BuildRequestConfiguration GetMatchingConfiguration(ConfigurationMetadata configMetadata, ConfigCreateCallback callback, bool loadProject) { - return _override.GetMatchingConfiguration(configMetadata, callback, loadProject) ?? CurrentCache.GetMatchingConfiguration(configMetadata, callback, loadProject); + // Call a retrieval method without side effects to avoid creating new entries in the override cache. New entries should go into the current cache. + var overrideConfig = GetMatchingConfiguration(configMetadata); + + if (overrideConfig != null) + { + AssertCurrentCacheDoesNotContainConfig(overrideConfig); + + return overrideConfig; + } + + return CurrentCache.GetMatchingConfiguration(configMetadata, callback, loadProject); } public bool HasConfiguration(int configId) { - var overrideHasConfiguration = _override.HasConfiguration(configId); - - if (overrideHasConfiguration) + if (_override.TryGetConfiguration(configId, out var overrideConfig)) { -#if DEBUG - ErrorUtilities.VerifyThrow(!CurrentCache.HasConfiguration(configId), "caches should not overlap"); -#endif - return overrideHasConfiguration; + AssertCurrentCacheDoesNotContainConfig(overrideConfig); + + return true; } - return _override.HasConfiguration(configId) || CurrentCache.HasConfiguration(configId); + return CurrentCache.HasConfiguration(configId); } public void ClearConfigurations() @@ -146,5 +169,25 @@ public bool WriteConfigurationsToDisk() { return CurrentCache.WriteConfigurationsToDisk(); } + + private void AssertCurrentCacheDoesNotContainConfig(BuildRequestConfiguration overrideConfig) + { + ErrorUtilities.VerifyThrow(!CurrentCache.HasConfiguration(overrideConfig.ConfigurationId), "caches should not overlap"); + } + + public void BuildResultAddedForConfiguration(int configId) + { + // If a build result is added for a configuration, that configuration must exist in the CurrentCache. + // If the configuration is in the override cache, then it must be moved into the CurrentCache. + // This is because if the caches are serialized to files, both the config and the build result serialized caches must have a 1-1 mapping + // between themselves. + if (_override.TryGetConfiguration(configId, out var overrideConfig)) + { + AssertCurrentCacheDoesNotContainConfig(overrideConfig); + + _override.RemoveConfiguration(configId); + CurrentCache.AddConfiguration(overrideConfig); + } + } } } diff --git a/src/Build/BackEnd/Components/Caching/IConfigCache.cs b/src/Build/BackEnd/Components/Caching/IConfigCache.cs index 0bdc69b92a8..6f876bbc300 100644 --- a/src/Build/BackEnd/Components/Caching/IConfigCache.cs +++ b/src/Build/BackEnd/Components/Caching/IConfigCache.cs @@ -27,6 +27,14 @@ BuildRequestConfiguration this[int configId] get; } + /// + /// Check existence of entry and return value if present. + /// + /// The configuration id. + /// Corresponding configuration if configId is present. Null otherwise + /// True if the cache contains the configuration. False otherwise. + bool TryGetConfiguration(int configId, out BuildRequestConfiguration existingConfig); + /// /// Adds the configuration to the cache. /// @@ -54,7 +62,7 @@ BuildRequestConfiguration this[int configId] BuildRequestConfiguration GetMatchingConfiguration(ConfigurationMetadata configMetadata); /// - /// Gets a matching configuration. If no such configration exists, one is created and optionally loaded. + /// Gets a matching configuration. If no such configuration exists, one is created and optionally loaded. /// /// The configuration metadata to match. /// Callback to be invoked if the configuration does not exist. diff --git a/src/Build/BackEnd/Components/Caching/ResultsCacheWithOverride.cs b/src/Build/BackEnd/Components/Caching/ResultsCacheWithOverride.cs index 66b1e17ccca..4e4240c8102 100644 --- a/src/Build/BackEnd/Components/Caching/ResultsCacheWithOverride.cs +++ b/src/Build/BackEnd/Components/Caching/ResultsCacheWithOverride.cs @@ -8,15 +8,24 @@ namespace Microsoft.Build.Execution { + // This class composes two caches, an override cache and a current cache. + // Reads are served from both caches (override first) + // Writes should only happen in the current cache. internal class ResultsCacheWithOverride : IResultsCache { private readonly IResultsCache _override; + private readonly bool _isolateProjects; + private readonly ConfigCacheWithOverride _configCacheWithOverride; public ResultsCache CurrentCache { get; } - public ResultsCacheWithOverride(IResultsCache @override) + public ResultsCacheWithOverride(IResultsCache @override, bool isolateProjects, + ConfigCacheWithOverride configCacheWithOverride) { _override = @override; + _isolateProjects = isolateProjects; + _configCacheWithOverride = configCacheWithOverride; + CurrentCache = new ResultsCache(); } @@ -38,6 +47,8 @@ public void Translate(ITranslator translator) public void AddResult(BuildResult result) { CurrentCache.AddResult(result); + + _configCacheWithOverride.BuildResultAddedForConfiguration(result.ConfigurationId); } public void ClearResults() @@ -48,11 +59,10 @@ public void ClearResults() public BuildResult GetResultForRequest(BuildRequest request) { var overrideResult = _override.GetResultForRequest(request); + if (overrideResult != null) { -#if DEBUG - ErrorUtilities.VerifyThrow(CurrentCache.GetResultForRequest(request) == null, "caches should not overlap"); -#endif + AssertCurrentCacheDoesNotContainResult(overrideResult); return overrideResult; } @@ -64,9 +74,7 @@ public BuildResult GetResultsForConfiguration(int configurationId) var overrideResult = _override.GetResultsForConfiguration(configurationId); if (overrideResult != null) { -#if DEBUG - ErrorUtilities.VerifyThrow(CurrentCache.GetResultsForConfiguration(configurationId) == null, "caches should not overlap"); -#endif + AssertCurrentCacheDoesNotContainResult(overrideResult); return overrideResult; } @@ -87,16 +95,8 @@ public ResultsCacheResponse SatisfyRequest( if (overrideRequest.Type == ResultsCacheResponseType.Satisfied) { -#if DEBUG - ErrorUtilities.VerifyThrow( - CurrentCache.SatisfyRequest( - request, - configInitialTargets, - configDefaultTargets, - skippedResultsDoNotCauseCacheMiss) - .Type == ResultsCacheResponseType.NotSatisfied, - "caches should not overlap"); -#endif + AssertCurrentCacheDoesNotContainResult(_override.GetResultsForConfiguration(request.ConfigurationId)); + return overrideRequest; } @@ -119,6 +119,8 @@ public void WriteResultsToDisk() public IEnumerator GetEnumerator() { + // Enumerators do not compose both caches to limit the influence of the override cache (reduce the number of possible states out there). + // So far all runtime examples do not need the two composed. return CurrentCache.GetEnumerator(); } @@ -126,5 +128,20 @@ IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); } + + private void AssertCurrentCacheDoesNotContainResult(BuildResult overrideResult) + { + // There could be an exempt project being built for which there is already an entry in the override cache (if the exempt project is also present + // in an input cache, for example if a project both exempts a reference, and also has a ProjectReference on it). + // In this situation, the exempt project may be built with additional targets for which there are no results in the override cache. + // This will cause the newly built targets to be saved both in the override cache, and also in the current cache. + // For this particular case, skip the check that a BuildResult for a particular configuration id should be in only one of the caches, not both. + var skipCheck = _isolateProjects && _configCacheWithOverride[overrideResult.ConfigurationId].SkippedFromStaticGraphIsolationConstraints; + + if (!skipCheck) + { + ErrorUtilities.VerifyThrow(CurrentCache.GetResultsForConfiguration(overrideResult.ConfigurationId) == null, "caches should not overlap"); + } + } } } diff --git a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs index d34a2df0426..cdc37afa4ce 100644 --- a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs +++ b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs @@ -328,30 +328,30 @@ public IEnumerable ReportResult(int nodeId, BuildResult result { _schedulingData.EventTime = DateTime.UtcNow; List responses = new List(); - TraceScheduler("Reporting result from node {0} for request {1}, parent {2}.", nodeId, result.GlobalRequestId, result.ParentGlobalRequestId); + TraceScheduler(format: "Reporting result from node {0} for request {1}, parent {2}.", nodeId, result.GlobalRequestId, result.ParentGlobalRequestId); // Record these results to the cache. - _resultsCache.AddResult(result); + _resultsCache.AddResult(result: result); if (result.NodeRequestId == BuildRequest.ResultsTransferNodeRequestId) { // We are transferring results. The node to which they should be sent has already been recorded by the // HandleRequestBlockedOnResultsTransfer method in the configuration. - BuildRequestConfiguration config = _configCache[result.ConfigurationId]; - ScheduleResponse response = ScheduleResponse.CreateReportResultResponse(config.ResultsNodeId, result); - responses.Add(response); + BuildRequestConfiguration config = _configCache[configId: result.ConfigurationId]; + ScheduleResponse response = ScheduleResponse.CreateReportResultResponse(node: config.ResultsNodeId, resultToReport: result); + responses.Add(item: response); } else { // Tell the request to which this result belongs than it is done. - SchedulableRequest request = _schedulingData.GetExecutingRequest(result.GlobalRequestId); - request.Complete(result); + SchedulableRequest request = _schedulingData.GetExecutingRequest(globalRequestId: result.GlobalRequestId); + request.Complete(result: result); // Report results to our parent, or report submission complete as necessary. if (request.Parent != null) { // responses.Add(new ScheduleResponse(request.Parent.AssignedNode, new BuildRequestUnblocker(request.Parent.BuildRequest.GlobalRequestId, result))); - ErrorUtilities.VerifyThrow(result.ParentGlobalRequestId == request.Parent.BuildRequest.GlobalRequestId, "Result's parent doesn't match request's parent."); + ErrorUtilities.VerifyThrow(condition: result.ParentGlobalRequestId == request.Parent.BuildRequest.GlobalRequestId, unformattedMessage: "Result's parent doesn't match request's parent."); // When adding the result to the cache we merge the result with what ever is already in the cache this may cause // the result to have more target outputs in it than was was requested. To fix this we can ask the cache itself for the result we just added. @@ -360,37 +360,40 @@ public IEnumerable ReportResult(int nodeId, BuildResult result // Note: In this case we do not need to log that we got the results from the cache because we are only using the cache // for filtering the targets for the result instead rather than using the cache as the location where this result came from. - ScheduleResponse response = TrySatisfyRequestFromCache(request.Parent.AssignedNode, request.BuildRequest, skippedResultsDoNotCauseCacheMiss: _componentHost.BuildParameters.SkippedResultsDoNotCauseCacheMiss()); + var response = TrySatisfyRequestFromCache( + nodeForResults: request.Parent.AssignedNode, + request: request.BuildRequest, + skippedResultsDoNotCauseCacheMiss: _componentHost.BuildParameters.SkippedResultsDoNotCauseCacheMiss()); // response may be null if the result was never added to the cache. This can happen if the result has an exception in it // or the results could not be satisfied because the initial or default targets have been skipped. If that is the case // we need to report the result directly since it contains an exception if (response == null) { - response = ScheduleResponse.CreateReportResultResponse(request.Parent.AssignedNode, result.Clone()); + response = ScheduleResponse.CreateReportResultResponse(node: request.Parent.AssignedNode, resultToReport: result.Clone()); } - responses.Add(response); + responses.Add(item: response); } else { // This was root request, we can report submission complete. // responses.Add(new ScheduleResponse(result)); - responses.Add(ScheduleResponse.CreateSubmissionCompleteResponse(result)); + responses.Add(item: ScheduleResponse.CreateSubmissionCompleteResponse(rootRequestResult: result)); if (result.OverallResult != BuildResultCode.Failure) { - WriteSchedulingPlan(result.SubmissionId); + WriteSchedulingPlan(submissionId: result.SubmissionId); } } // This result may apply to a number of other unscheduled requests which are blocking active requests. Report to them as well. - List unscheduledRequests = new List(_schedulingData.UnscheduledRequests); + List unscheduledRequests = new List(collection: _schedulingData.UnscheduledRequests); foreach (SchedulableRequest unscheduledRequest in unscheduledRequests) { if (unscheduledRequest.BuildRequest.GlobalRequestId == result.GlobalRequestId) { - TraceScheduler("Request {0} (node request {1}) also satisfied by result.", unscheduledRequest.BuildRequest.GlobalRequestId, unscheduledRequest.BuildRequest.NodeRequestId); - BuildResult newResult = new BuildResult(unscheduledRequest.BuildRequest, result, null); + TraceScheduler(format: "Request {0} (node request {1}) also satisfied by result.", unscheduledRequest.BuildRequest.GlobalRequestId, unscheduledRequest.BuildRequest.NodeRequestId); + BuildResult newResult = new BuildResult(request: unscheduledRequest.BuildRequest, existingResults: result, exception: null); // Report results to the parent. int parentNode = (unscheduledRequest.Parent == null) ? InvalidNodeId : unscheduledRequest.Parent.AssignedNode; @@ -403,31 +406,34 @@ public IEnumerable ReportResult(int nodeId, BuildResult result // its configuration and set of targets are identical -- from MSBuild's perspective, it's the same. So since // we're not going to attempt to re-execute it, if there are skipped targets in the result, that's fine. We just // need to know what the target results are so that we can log them. - ScheduleResponse response = TrySatisfyRequestFromCache(parentNode, unscheduledRequest.BuildRequest, skippedResultsDoNotCauseCacheMiss: true); + var response = TrySatisfyRequestFromCache( + nodeForResults: parentNode, + request: unscheduledRequest.BuildRequest, + skippedResultsDoNotCauseCacheMiss: true); // If we have a response we need to tell the loggers that we satisified that request from the cache. if (response != null) { - LogRequestHandledFromCache(unscheduledRequest.BuildRequest, response.BuildResult); + LogRequestHandledFromCache(request: unscheduledRequest.BuildRequest, result: response.BuildResult); } else { // Response may be null if the result was never added to the cache. This can happen if the result has // an exception in it. If that is the case, we should report the result directly so that the // build manager knows that it needs to shut down logging manually. - response = GetResponseForResult(parentNode, unscheduledRequest.BuildRequest, newResult.Clone()); + response = GetResponseForResult(parentRequestNode: parentNode, requestWhichGeneratedResult: unscheduledRequest.BuildRequest, result: newResult.Clone()); } - responses.Add(response); + responses.Add(item: response); // Mark the request as complete (and the parent is no longer blocked by this request.) - unscheduledRequest.Complete(newResult); + unscheduledRequest.Complete(result: newResult); } } } // This node may now be free, so run the scheduler. - ScheduleUnassignedRequests(responses); + ScheduleUnassignedRequests(responses: responses); return responses; } @@ -1545,34 +1551,39 @@ private void HandleRequestBlockedOnResultsTransfer(SchedulableRequest parentRequ /// private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, BuildRequestBlocker blocker, List responses) { - ErrorUtilities.VerifyThrowArgumentNull(blocker, "blocker"); - ErrorUtilities.VerifyThrowArgumentNull(responses, "responses"); + ErrorUtilities.VerifyThrowArgumentNull(parameter: blocker, parameterName: "blocker"); + ErrorUtilities.VerifyThrowArgumentNull(parameter: responses, parameterName: "responses"); // The request is waiting on new requests. bool abortRequestBatch = false; - Stack requestsToAdd = new Stack(blocker.BuildRequests.Length); + Stack requestsToAdd = new Stack(capacity: blocker.BuildRequests.Length); foreach (BuildRequest request in blocker.BuildRequests) { // Assign a global request id to this request. if (request.GlobalRequestId == BuildRequest.InvalidGlobalRequestId) { - AssignGlobalRequestId(request); + AssignGlobalRequestId(request: request); } int nodeForResults = (parentRequest == null) ? InvalidNodeId : parentRequest.AssignedNode; - TraceScheduler("Received request {0} (node request {1}) with parent {2} from node {3}", request.GlobalRequestId, request.NodeRequestId, request.ParentGlobalRequestId, nodeForResults); + TraceScheduler(format: "Received request {0} (node request {1}) with parent {2} from node {3}", request.GlobalRequestId, request.NodeRequestId, request.ParentGlobalRequestId, nodeForResults); // First, determine if we have already built this request and have results for it. If we do, we prepare the responses for it // directly here. We COULD simply report these as blocking the parent request and let the scheduler pick them up later when the parent // comes back up as schedulable, but we prefer to send the results back immediately so this request can (potentially) continue uninterrupted. - ScheduleResponse response = TrySatisfyRequestFromCache(nodeForResults, request, skippedResultsDoNotCauseCacheMiss: _componentHost.BuildParameters.SkippedResultsDoNotCauseCacheMiss()); + var response = TrySatisfyRequestFromCache( + nodeForResults: nodeForResults, + request: request, + skippedResultsDoNotCauseCacheMiss: + _componentHost.BuildParameters.SkippedResultsDoNotCauseCacheMiss()); + if (null != response) { - TraceScheduler("Request {0} (node request {1}) satisfied from the cache.", request.GlobalRequestId, request.NodeRequestId); + TraceScheduler(format: "Request {0} (node request {1}) satisfied from the cache.", request.GlobalRequestId, request.NodeRequestId); // BuildResult result = (response.Action == ScheduleActionType.Unblock) ? response.Unblocker.Results[0] : response.BuildResult; - LogRequestHandledFromCache(request, response.BuildResult); - responses.Add(response); + LogRequestHandledFromCache(request: request, result: response.BuildResult); + responses.Add(item: response); // If we wish to implement an algorithm where the first failing request aborts the remaining request, check for // overall result being failure rather than just circular dependency. Sync with BasicScheduler.ReportResult and @@ -1582,40 +1593,46 @@ private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, abortRequestBatch = true; } } - else if (CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot(nodeForResults, request, responses, out var emitNonErrorLogs)) + else if (CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot( + nodeForResults, + request, + responses, + out var emitNonErrorLogs)) { - emitNonErrorLogs(_componentHost.LoggingService); + emitNonErrorLogs(obj: _componentHost.LoggingService); + + MarkConfigAsSkippedFromGraphIsolationConstraints(request: request); // Ensure there is no affinity mismatch between this request and a previous request of the same configuration. - NodeAffinity requestAffinity = GetNodeAffinityForRequest(request); + NodeAffinity requestAffinity = GetNodeAffinityForRequest(request: request); NodeAffinity existingRequestAffinity = NodeAffinity.Any; if (requestAffinity != NodeAffinity.Any) { bool affinityMismatch = false; - int assignedNodeId = _schedulingData.GetAssignedNodeForRequestConfiguration(request.ConfigurationId); + int assignedNodeId = _schedulingData.GetAssignedNodeForRequestConfiguration(configurationId: request.ConfigurationId); if (assignedNodeId != Scheduler.InvalidNodeId) { - if (!_availableNodes[assignedNodeId].CanServiceRequestWithAffinity(GetNodeAffinityForRequest(request))) + if (!_availableNodes[key: assignedNodeId].CanServiceRequestWithAffinity(nodeAffinity: GetNodeAffinityForRequest(request: request))) { // This request's configuration has already been assigned to a node which cannot service this affinity. - if (_schedulingData.GetRequestsAssignedToConfigurationCount(request.ConfigurationId) == 0) + if (_schedulingData.GetRequestsAssignedToConfigurationCount(configurationId: request.ConfigurationId) == 0) { // If there are no other requests already scheduled for that configuration, we can safely reassign. - _schedulingData.UnassignNodeForRequestConfiguration(request.ConfigurationId); + _schedulingData.UnassignNodeForRequestConfiguration(configurationId: request.ConfigurationId); } else { - existingRequestAffinity = (_availableNodes[assignedNodeId].ProviderType == NodeProviderType.InProc) ? NodeAffinity.InProc : NodeAffinity.OutOfProc; + existingRequestAffinity = (_availableNodes[key: assignedNodeId].ProviderType == NodeProviderType.InProc) ? NodeAffinity.InProc : NodeAffinity.OutOfProc; affinityMismatch = true; } } } - else if (_schedulingData.GetRequestsAssignedToConfigurationCount(request.ConfigurationId) > 0) + else if (_schedulingData.GetRequestsAssignedToConfigurationCount(configurationId: request.ConfigurationId) > 0) { // Would any other existing requests for this configuration mismatch? - foreach (SchedulableRequest existingRequest in _schedulingData.GetRequestsAssignedToConfiguration(request.ConfigurationId)) + foreach (SchedulableRequest existingRequest in _schedulingData.GetRequestsAssignedToConfiguration(configurationId: request.ConfigurationId)) { - existingRequestAffinity = GetNodeAffinityForRequest(existingRequest.BuildRequest); + existingRequestAffinity = GetNodeAffinityForRequest(request: existingRequest.BuildRequest); if (existingRequestAffinity != NodeAffinity.Any && existingRequestAffinity != requestAffinity) { // The existing request has an affinity which doesn't match this one, so this one could never be scheduled. @@ -1627,16 +1644,16 @@ private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, if (affinityMismatch) { - BuildResult result = new BuildResult(request, new InvalidOperationException(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("AffinityConflict", requestAffinity, existingRequestAffinity))); - response = GetResponseForResult(nodeForResults, request, result); - responses.Add(response); + BuildResult result = new BuildResult(request: request, exception: new InvalidOperationException(message: ResourceUtilities.FormatResourceStringStripCodeAndKeyword(resourceName: "AffinityConflict", requestAffinity, existingRequestAffinity))); + response = GetResponseForResult(parentRequestNode: nodeForResults, requestWhichGeneratedResult: request, result: result); + responses.Add(item: response); continue; } } // Now add the requests so they would naturally be picked up by the scheduler in the order they were issued, // but before any other requests in the list. This makes us prefer a depth-first traversal. - requestsToAdd.Push(request); + requestsToAdd.Push(item: request); } } @@ -1649,7 +1666,7 @@ private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, if (parentRequest != null) { // responses.Add(new ScheduleResponse(parentRequest.AssignedNode, new BuildRequestUnblocker(parentRequest.BuildRequest.GlobalRequestId))); - responses.Add(ScheduleResponse.CreateResumeExecutionResponse(parentRequest.AssignedNode, parentRequest.BuildRequest.GlobalRequestId)); + responses.Add(item: ScheduleResponse.CreateResumeExecutionResponse(node: parentRequest.AssignedNode, globalRequestIdToResume: parentRequest.BuildRequest.GlobalRequestId)); } } else @@ -1657,17 +1674,41 @@ private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, while (requestsToAdd.Count > 0) { BuildRequest requestToAdd = requestsToAdd.Pop(); - SchedulableRequest blockingRequest = _schedulingData.CreateRequest(requestToAdd, parentRequest); + SchedulableRequest blockingRequest = _schedulingData.CreateRequest(buildRequest: requestToAdd, parent: parentRequest); if (parentRequest != null) { - parentRequest.BlockByRequest(blockingRequest, blocker.TargetsInProgress); + parentRequest.BlockByRequest(blockingRequest: blockingRequest, activeTargets: blocker.TargetsInProgress); } } } } } + private void MarkConfigAsSkippedFromGraphIsolationConstraints(BuildRequest request) + { + if (!_componentHost.BuildParameters.IsolateProjects || request.IsRootRequest) + { + return; + } + + if (request.SkipStaticGraphIsolationConstraints + || + // Self builds are exempt from isolation constraints. + IsSelfBuild(request) + ) + { + _configCache[request.ConfigurationId].SkippedFromStaticGraphIsolationConstraints = true; + } + + bool IsSelfBuild(BuildRequest buildRequest) + { + return _configCache[buildRequest.ConfigurationId].ProjectFullPath.Equals( + _configCache[GetParentRequest(buildRequest).ConfigurationId].ProjectFullPath, + FileUtilities.PathComparison); + } + } + /// /// Resumes executing a request which was in the Ready state for the specified node, if any. /// @@ -1697,7 +1738,10 @@ private void ResolveRequestFromCacheAndResumeIfPossible(SchedulableRequest reque int nodeForResults = (request.Parent != null) ? request.Parent.AssignedNode : InvalidNodeId; // Do we already have results? If so, just return them. - ScheduleResponse response = TrySatisfyRequestFromCache(nodeForResults, request.BuildRequest, skippedResultsDoNotCauseCacheMiss: _componentHost.BuildParameters.SkippedResultsDoNotCauseCacheMiss()); + var response = TrySatisfyRequestFromCache( + nodeForResults, + request.BuildRequest, + skippedResultsDoNotCauseCacheMiss: _componentHost.BuildParameters.SkippedResultsDoNotCauseCacheMiss()); if (response != null) { if (response.Action == ScheduleActionType.SubmissionComplete) @@ -1732,7 +1776,11 @@ private void ResolveRequestFromCacheAndResumeIfPossible(SchedulableRequest reque } else { - CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot(nodeForResults, request.BuildRequest, responses, out _); + CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot( + nodeForResults, + request.BuildRequest, + responses, + out _); } } @@ -1778,7 +1826,7 @@ private void ResumeRequiredWork(List responses) private ScheduleResponse TrySatisfyRequestFromCache(int nodeForResults, BuildRequest request, bool skippedResultsDoNotCauseCacheMiss) { BuildRequestConfiguration config = _configCache[request.ConfigurationId]; - ResultsCacheResponse resultsResponse = _resultsCache.SatisfyRequest(request, config.ProjectInitialTargets, config.ProjectDefaultTargets, skippedResultsDoNotCauseCacheMiss); + var resultsResponse =_resultsCache.SatisfyRequest(request, config.ProjectInitialTargets, config.ProjectDefaultTargets, skippedResultsDoNotCauseCacheMiss); if (resultsResponse.Type == ResultsCacheResponseType.Satisfied) { @@ -1789,36 +1837,43 @@ private ScheduleResponse TrySatisfyRequestFromCache(int nodeForResults, BuildReq } /// True if caches misses are allowed, false otherwise - private bool CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot(int nodeForResults, BuildRequest request, List responses, out Action emitNonErrorLogs) + private bool CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot( + int nodeForResults, + BuildRequest request, + List responses, + out Action emitNonErrorLogs + ) { emitNonErrorLogs = _ => { }; var isIsolatedBuild = _componentHost.BuildParameters.IsolateProjects; - var configCache = (IConfigCache) _componentHost.GetComponent(BuildComponentType.ConfigCache); + var configs = new Lazy<(BuildRequestConfiguration RequestConfig, BuildRequestConfiguration ParentConfig)>(() => GetConfigurations(request)); // do not check root requests as nothing depends on them - if (!isIsolatedBuild || request.IsRootRequest || request.SkipStaticGraphIsolationConstraints) + if (!isIsolatedBuild || + request.IsRootRequest || + request.SkipStaticGraphIsolationConstraints || + RequestSkipsNonexistentTargetsAndAllExistingTargetsHaveResults(request)) { if (isIsolatedBuild && request.SkipStaticGraphIsolationConstraints) { // retrieving the configs is not quite free, so avoid computing them eagerly - var configs = GetConfigurations(); emitNonErrorLogs = ls => ls.LogComment( NewBuildEventContext(), MessageImportance.Normal, "SkippedConstraintsOnRequest", - configs.parentConfig.ProjectFullPath, - configs.requestConfig.ProjectFullPath); + configs.Value.ParentConfig.ProjectFullPath, + configs.Value.RequestConfig.ProjectFullPath); } return true; } - var (requestConfig, parentConfig) = GetConfigurations(); + var (requestConfig, parentConfig) = configs.Value; // allow self references (project calling the msbuild task on itself, potentially with different global properties) - if (parentConfig.ProjectFullPath.Equals(requestConfig.ProjectFullPath, StringComparison.OrdinalIgnoreCase)) + if (parentConfig.ProjectFullPath.Equals(requestConfig.ProjectFullPath, FileUtilities.PathComparison)) { return true; } @@ -1854,20 +1909,14 @@ BuildEventContext NewBuildEventContext() BuildEventContext.InvalidTaskId); } - (BuildRequestConfiguration requestConfig, BuildRequestConfiguration parentConfig) GetConfigurations() + (BuildRequestConfiguration requestConfig, BuildRequestConfiguration parentConfig) GetConfigurations(BuildRequest request) { - var buildRequestConfiguration = configCache[request.ConfigurationId]; + var configCache = (IConfigCache)_componentHost.GetComponent(BuildComponentType.ConfigCache); - // Need the parent request. It might be blocked or executing; check both. - var parentRequest = _schedulingData.BlockedRequests.FirstOrDefault(r => r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId) - ?? _schedulingData.ExecutingRequests.FirstOrDefault(r => r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId); + var buildRequestConfiguration = configCache[request.ConfigurationId]; - ErrorUtilities.VerifyThrowInternalNull(parentRequest, nameof(parentRequest)); - ErrorUtilities.VerifyThrow( - configCache.HasConfiguration(parentRequest.BuildRequest.ConfigurationId), - "All non root requests should have a parent with a loaded configuration"); + var parentConfiguration = configCache[GetParentRequest(request).ConfigurationId]; - var parentConfiguration = configCache[parentRequest.BuildRequest.ConfigurationId]; return (buildRequestConfiguration, parentConfiguration); } @@ -1875,6 +1924,61 @@ string ConcatenateGlobalProperties(BuildRequestConfiguration configuration) { return string.Join("; ", configuration.GlobalProperties.Select(p => $"{p.Name}={p.EvaluatedValue}")); } + + bool RequestSkipsNonexistentTargetsAndAllExistingTargetsHaveResults(BuildRequest buildRequest) + { + // Return early if the request does not skip nonexistent targets. + if ((buildRequest.BuildRequestDataFlags & BuildRequestDataFlags.SkipNonexistentTargets) != BuildRequestDataFlags.SkipNonexistentTargets) + { + return false; + } + + var requestResults = _resultsCache.GetResultsForConfiguration(buildRequest.ConfigurationId); + + if (requestResults == null) + { + return false; + } + + // Non-existing targets do not have results. + // We must differentiate targets that do not exist in the reference, from targets that exist but have no results. + // In isolated builds, cache misses from the former are accepted for requests that skip non-existing targets. + // Cache misses from the latter break the isolation constraint of no cache misses, even when non-existing targets get skipped. + bool any = false; + foreach (var target in request.Targets) + { + if (configs.Value.RequestConfig.ProjectTargets.Contains(target) && + !requestResults.HasResultsForTarget(target)) + { + any = true; + break; + } + } + + return !any; + } + } + + private BuildRequest GetParentRequest(BuildRequest request) + { + if (request.IsRootRequest) + { + return null; + } + else + { + var schedulerRequest = _schedulingData.BlockedRequests.FirstOrDefault(r => + r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId) + ?? _schedulingData.ExecutingRequests.FirstOrDefault(r => + r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId); + + ErrorUtilities.VerifyThrowInternalNull(schedulerRequest, nameof(schedulerRequest)); + ErrorUtilities.VerifyThrow( + _configCache.HasConfiguration(schedulerRequest.BuildRequest.ConfigurationId), + "All non root requests should have a parent with a loaded configuration"); + + return schedulerRequest.BuildRequest; + } } /// diff --git a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs index 301a9246a07..d192661d3c1 100644 --- a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs +++ b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs @@ -138,7 +138,7 @@ internal class BuildRequestConfiguration : IEquatable /// /// The target names that were requested to execute. /// - internal IReadOnlyCollection TargetNames { get; } + internal IReadOnlyCollection RequestedTargetNames { get; } /// /// Initializes a configuration from a BuildRequestData structure. Used by the BuildManager. @@ -170,14 +170,17 @@ internal BuildRequestConfiguration(int configId, BuildRequestData data, string d _explicitToolsVersionSpecified = data.ExplicitToolsVersionSpecified; _toolsVersion = ResolveToolsVersion(data, defaultToolsVersion); _globalProperties = data.GlobalPropertiesDictionary; - TargetNames = new List(data.TargetNames); + RequestedTargetNames = new List(data.TargetNames); // The following information only exists when the request is populated with an existing project. if (data.ProjectInstance != null) { _project = data.ProjectInstance; + _projectInitialTargets = data.ProjectInstance.InitialTargets; _projectDefaultTargets = data.ProjectInstance.DefaultTargets; + _projectTargets = data.ProjectInstance.Targets.Keys.ToHashSet(); + _translateEntireProjectInstanceState = data.ProjectInstance.TranslateEntireState; if (data.PropertiesToTransfer != null) @@ -214,8 +217,11 @@ internal BuildRequestConfiguration(int configId, ProjectInstance instance) _globalProperties = instance.GlobalPropertiesDictionary; _project = instance; + _projectInitialTargets = instance.InitialTargets; _projectDefaultTargets = instance.DefaultTargets; + _projectTargets = instance.Targets.Keys.ToHashSet(); + _translateEntireProjectInstanceState = instance.TranslateEntireState; IsCacheable = false; } @@ -234,13 +240,15 @@ private BuildRequestConfiguration(int configId, BuildRequestConfiguration other) _transferredProperties = other._transferredProperties; _projectDefaultTargets = other._projectDefaultTargets; _projectInitialTargets = other._projectInitialTargets; + _projectTargets = other._projectTargets; _projectFullPath = other._projectFullPath; _toolsVersion = other._toolsVersion; _explicitToolsVersionSpecified = other._explicitToolsVersionSpecified; _globalProperties = other._globalProperties; IsCacheable = other.IsCacheable; _configId = configId; - TargetNames = other.TargetNames; + RequestedTargetNames = other.RequestedTargetNames; + _skippedFromStaticGraphIsolationConstraints = other._skippedFromStaticGraphIsolationConstraints; } /// @@ -407,9 +415,12 @@ private void SetProjectBasedState(ProjectInstance project) // Clear these out so the other accessors don't complain. We don't want to generally enable resetting these fields. _projectDefaultTargets = null; _projectInitialTargets = null; + _projectTargets = null; ProjectDefaultTargets = _project.DefaultTargets; ProjectInitialTargets = _project.InitialTargets; + ProjectTargets = _project.Targets.Keys.ToHashSet(); + _translateEntireProjectInstanceState = _project.TranslateEntireState; if (IsCached) @@ -482,6 +493,16 @@ public List ProjectDefaultTargets } } + public HashSet ProjectTargets + { + get => _projectTargets; + set + { + ErrorUtilities.VerifyThrow(_projectTargets == null, "Targets cannot be reset once they have been set."); + _projectTargets = value; + } + } + /// /// Returns the node packet type /// @@ -547,6 +568,12 @@ internal int ResultsNodeId set => _resultsNodeId = value; } + public bool SkippedFromStaticGraphIsolationConstraints + { + get => _skippedFromStaticGraphIsolationConstraints; + set => _skippedFromStaticGraphIsolationConstraints = value; + } + /// /// Implementation of the equality operator. /// @@ -678,6 +705,8 @@ public List GetTargetsUsedToBuildRequest(BuildRequest request) } private Func shouldSkipStaticGraphIsolationOnReference; + private bool _skippedFromStaticGraphIsolationConstraints; + private HashSet _projectTargets; public bool ShouldSkipIsolationConstraintsForReference(string referenceFullPath) { @@ -804,6 +833,7 @@ public void Translate(ITranslator translator) translator.Translate(ref _resultsNodeId); translator.Translate(ref _savedCurrentDirectory); translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase); + translator.Translate(ref _skippedFromStaticGraphIsolationConstraints); // if the entire state is translated, then the transferred state, if exists, represents the full evaluation data if (_translateEntireProjectInstanceState && @@ -822,7 +852,9 @@ internal void TranslateForFutureUse(ITranslator translator) translator.Translate(ref _explicitToolsVersionSpecified); translator.Translate(ref _projectDefaultTargets); translator.Translate(ref _projectInitialTargets); + translator.Translate(ref _projectTargets); translator.TranslateDictionary(ref _globalProperties, ProjectPropertyInstance.FactoryForDeserialization); + translator.Translate(ref _skippedFromStaticGraphIsolationConstraints); } /// diff --git a/src/Build/BackEnd/Shared/BuildResult.cs b/src/Build/BackEnd/Shared/BuildResult.cs index 0c74fad44a3..84ac131d74c 100644 --- a/src/Build/BackEnd/Shared/BuildResult.cs +++ b/src/Build/BackEnd/Shared/BuildResult.cs @@ -115,6 +115,7 @@ public class BuildResult : INodePacket, IBuildResults private ProjectInstance _projectStateAfterBuild; private string _schedulerInducedError; + private HashSet _targets; /// /// Constructor for serialization. @@ -227,6 +228,7 @@ internal BuildResult(BuildResult result, int nodeRequestId) _circularDependency = result._circularDependency; _initialTargets = result._initialTargets; _defaultTargets = result._defaultTargets; + _targets = result._targets; _baseOverallResult = result.OverallResult == BuildResultCode.Success; } @@ -243,6 +245,7 @@ internal BuildResult(BuildResult result, int submissionId, int configurationId, _circularDependency = result._circularDependency; _initialTargets = result._initialTargets; _defaultTargets = result._defaultTargets; + _targets = result._targets; _baseOverallResult = result.OverallResult == BuildResultCode.Success; } @@ -432,6 +435,16 @@ internal List DefaultTargets { _defaultTargets = value; } } + /// + /// Returns all the targets present in the project evaluation of this result's configuration. + /// + public HashSet Targets + { + get => _targets; + set => _targets = value; + } + + /// /// Container used to transport errors from the scheduler (issued while computing a build result) /// to the TaskHost that has the proper logging context (project id, target id, task id, file location) @@ -530,6 +543,7 @@ void ITranslatable.Translate(ITranslator translator) translator.Translate(ref _nodeRequestId); translator.Translate(ref _initialTargets); translator.Translate(ref _defaultTargets); + translator.Translate(ref _targets); translator.Translate(ref _circularDependency); translator.TranslateException(ref _requestException); translator.TranslateDictionary(ref _resultsByTarget, TargetResult.FactoryForDeserialization, CreateTargetResultDictionary); diff --git a/src/Build/BackEnd/Shared/TargetResult.cs b/src/Build/BackEnd/Shared/TargetResult.cs index a22c5e63346..9f5607a5443 100644 --- a/src/Build/BackEnd/Shared/TargetResult.cs +++ b/src/Build/BackEnd/Shared/TargetResult.cs @@ -194,10 +194,15 @@ internal static TargetResult FactoryForDeserialization(ITranslator translator) /// /// Gets the name of the cache file for this configuration. /// - internal static string GetCacheFile(int configId, string targetToCache) + private static string GetCacheFile(int configId, string targetToCache) { - string filename = Path.Combine(FileUtilities.GetCacheDirectory(), String.Format(CultureInfo.InvariantCulture, Path.Combine("Results{0}", "{1}.cache"), configId, targetToCache)); - return filename; + return Path.Combine( + FileUtilities.GetCacheDirectory(), + string.Format( + CultureInfo.InvariantCulture, + Path.Combine("Results{0}", "{1}.cache"), + configId, + targetToCache)); } /// diff --git a/src/Build/Graph/ProjectInterpretation.cs b/src/Build/Graph/ProjectInterpretation.cs index 298f4ea4721..1c6ae8ca94d 100644 --- a/src/Build/Graph/ProjectInterpretation.cs +++ b/src/Build/Graph/ProjectInterpretation.cs @@ -26,6 +26,9 @@ internal sealed class ProjectInterpretation private const string ProjectReferenceTargetIsOuterBuildMetadataName = "OuterBuild"; internal const string InnerBuildReferenceItemName = "_ProjectSelfReference"; + private static readonly IImmutableSet TargetsWhichCannotBeSkippedIfNonexistent = + new[] {MSBuildConstants.DefaultTargetsMarker, MSBuildConstants.ProjectReferenceTargetsOrDefaultTargetsMarker}.ToImmutableHashSet(); + private static readonly char[] PropertySeparator = MSBuildConstants.SemicolonChar; public static ProjectInterpretation Instance = new ProjectInterpretation(); @@ -348,10 +351,10 @@ private static void RemoveFromPropertyDictionary( public readonly struct TargetsToPropagate { - private readonly ImmutableList _outerBuildTargets; - private readonly ImmutableList _allTargets; + private readonly ImmutableList _outerBuildTargets; + private readonly ImmutableList _allTargets; - private TargetsToPropagate(ImmutableList outerBuildTargets, ImmutableList nonOuterBuildTargets) + private TargetsToPropagate(ImmutableList outerBuildTargets, ImmutableList nonOuterBuildTargets) { _outerBuildTargets = outerBuildTargets; @@ -373,8 +376,8 @@ private TargetsToPropagate(ImmutableList outerBuildTargets, ImmutableLis /// public static TargetsToPropagate FromProjectAndEntryTargets(ProjectInstance project, ImmutableList entryTargets) { - var targetsForOuterBuild = ImmutableList.CreateBuilder(); - var targetsForInnerBuild = ImmutableList.CreateBuilder(); + var targetsForOuterBuild = ImmutableList.CreateBuilder(); + var targetsForInnerBuild = ImmutableList.CreateBuilder(); var projectReferenceTargets = project.GetItems(ItemTypeNames.ProjectReferenceTargets); @@ -388,7 +391,11 @@ public static TargetsToPropagate FromProjectAndEntryTargets(ProjectInstance proj var targetsAreForOuterBuild = projectReferenceTarget.GetMetadataValue(ProjectReferenceTargetIsOuterBuildMetadataName).Equals("true", StringComparison.OrdinalIgnoreCase); - var targets = ExpressionShredder.SplitSemiColonSeparatedList(targetsMetadataValue).ToArray(); + var skipNonexistentTargets = projectReferenceTarget + .GetMetadataValue("SkipNonexistentTargets") + .Equals("true", StringComparison.OrdinalIgnoreCase); + + var targets = ExpressionShredder.SplitSemiColonSeparatedList(targetsMetadataValue).Select(t => new TargetSpecification(t, skipNonexistentTargets)).ToArray(); if (targetsAreForOuterBuild) { @@ -405,19 +412,42 @@ public static TargetsToPropagate FromProjectAndEntryTargets(ProjectInstance proj return new TargetsToPropagate(targetsForOuterBuild.ToImmutable(), targetsForInnerBuild.ToImmutable()); } + private readonly struct TargetSpecification + { + public string TargetName { get; } + public bool SkipNonexistentTarget { get; } + + public TargetSpecification(string targetName, bool skipNonexistentTarget) + { + TargetName = targetName; + SkipNonexistentTarget = skipNonexistentTarget; + + ErrorUtilities.VerifyThrow(!skipNonexistentTarget || !TargetsWhichCannotBeSkippedIfNonexistent.Contains(targetName), $"some targets cannot be marked as SkipNonexistentTargets"); + } + } + public ImmutableList GetApplicableTargetsForReference(ProjectInstance reference) { switch (GetProjectType(reference)) { case ProjectType.InnerBuild: - return _allTargets; + return RemoveNonExistentTargets(_allTargets); case ProjectType.OuterBuild: - return _outerBuildTargets; + return RemoveNonExistentTargets(_outerBuildTargets); case ProjectType.NonMultitargeting: - return _allTargets; + return RemoveNonExistentTargets(_allTargets); default: throw new ArgumentOutOfRangeException(); } + + ImmutableList RemoveNonExistentTargets(ImmutableList targets) + { + return targets + .Where(t => t.SkipNonexistentTarget == false || + t.SkipNonexistentTarget && reference.Targets.ContainsKey(t.TargetName)) + .Select(t => t.TargetName) + .ToImmutableList(); + } } } } diff --git a/src/Shared/UnitTests/ObjectModelHelpers.cs b/src/Shared/UnitTests/ObjectModelHelpers.cs index 42d56c62448..62df1f5766c 100644 --- a/src/Shared/UnitTests/ObjectModelHelpers.cs +++ b/src/Shared/UnitTests/ObjectModelHelpers.cs @@ -1248,14 +1248,14 @@ internal static void AssertDictionariesEqual(IDictionary x, IDic }); } - internal static void ShouldBeEquivalentTo(this IDictionary a, IReadOnlyDictionary b) + internal static void ShouldBeSameIgnoringOrder(this IDictionary a, IReadOnlyDictionary b) { a.ShouldBeSubsetOf(b); b.ShouldBeSubsetOf(a); a.Count.ShouldBe(b.Count); } - internal static void ShouldBeEquivalentTo(this IEnumerable a, IEnumerable b) + internal static void ShouldBeSameIgnoringOrder(this IEnumerable a, IEnumerable b) { a.ShouldBeSubsetOf(b); b.ShouldBeSubsetOf(a); @@ -1343,32 +1343,35 @@ public static BuildResult BuildProjectContentUsingBuildManager(string content, M } } - public static BuildResult BuildProjectFileUsingBuildManager(string projectFile, MockLogger logger = null, BuildParameters parameters = null) + public static BuildResult BuildProjectFileUsingBuildManager( + string projectFile, + MockLogger logger = null, + BuildParameters parameters = null, + string[] targetsToBuild = null + ) { - using (var buildManager = new BuildManager()) - { - parameters = parameters ?? new BuildParameters(); + using var buildManager = new BuildManager(); + parameters ??= new BuildParameters(); - if (logger != null) - { - parameters.Loggers = parameters.Loggers == null - ? new[] {logger} - : parameters.Loggers.Concat(new[] {logger}); - } + if (logger != null) + { + parameters.Loggers = parameters.Loggers == null + ? new[] {logger} + : parameters.Loggers.Concat(new[] {logger}); + } - var request = new BuildRequestData( - projectFile, - new Dictionary(), - MSBuildConstants.CurrentToolsVersion, - new string[] {}, - null); + var request = new BuildRequestData( + projectFile, + new Dictionary(), + MSBuildConstants.CurrentToolsVersion, + targetsToBuild ?? new string[] {}, + null); - var result = buildManager.Build( - parameters, - request); + var result = buildManager.Build( + parameters, + request); - return result; - } + return result; } /// @@ -1567,6 +1570,28 @@ internal static TransientTestFile CreateProjectFile( return env.CreateFile(projectNumber + ".proj", sb.ToString()); } + internal static ProjectGraph CreateProjectGraph( + TestEnvironment env, + IDictionary dependencyEdges, + IDictionary extraContentPerProjectNumber) + { + return CreateProjectGraph( + env: env, + dependencyEdges: dependencyEdges, + globalProperties: null, + createProjectFile: (environment, projectNumber, references, projectReferenceTargets, defaultTargets, extraContent) => + { + extraContentPerProjectNumber.ShouldContainKey(projectNumber); + return CreateProjectFile( + environment, + projectNumber, + references, + projectReferenceTargets, + defaultTargets, + extraContentPerProjectNumber[projectNumber].Cleanup()); + }); + } + internal static ProjectGraph CreateProjectGraph( TestEnvironment env, // direct dependencies that the kvp.key node has on the nodes represented by kvp.value @@ -1576,7 +1601,7 @@ internal static ProjectGraph CreateProjectGraph( IEnumerable entryPoints = null, ProjectCollection projectCollection = null) { - createProjectFile = createProjectFile ?? CreateProjectFile; + createProjectFile ??= CreateProjectFile; var nodes = new Dictionary(); @@ -1893,11 +1918,11 @@ public BuildManagerSession( _buildManager.BeginBuild(actualBuildParameters, deferredMessages); } - public BuildResult BuildProjectFile(string projectFile, string[] entryTargets = null) + public BuildResult BuildProjectFile(string projectFile, string[] entryTargets = null, Dictionary globalProperties = null) { var buildResult = _buildManager.BuildRequest( new BuildRequestData(projectFile, - new Dictionary(), + globalProperties ?? new Dictionary(), MSBuildConstants.CurrentToolsVersion, entryTargets ?? new string[0], null));