From ca12412148831a46f674fab46d2e00f205d27f01 Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Fri, 7 May 2021 14:45:12 -0700 Subject: [PATCH] Add option to fail on unresolved SDK Maximal subset of #6312 and #6372. Also removes an unnecessary test per https://github.com/dotnet/msbuild/pull/6430#discussion_r628523058 --- ref/Microsoft.Build/net/Microsoft.Build.cs | 3 +- .../netstandard/Microsoft.Build.cs | 3 +- .../BackEnd/BuildManager/BuildManager.cs | 5 ++ .../BackEnd/BuildManager/BuildRequestData.cs | 7 ++ .../Shared/BuildRequestConfiguration.cs | 4 ++ src/Build/Definition/ProjectLoadSettings.cs | 5 ++ src/Build/Evaluation/Evaluator.cs | 2 +- src/MSBuild.UnitTests/XMake_Tests.cs | 66 ++++++++++++++++++- src/MSBuild/XMake.cs | 4 +- 9 files changed, 90 insertions(+), 9 deletions(-) diff --git a/ref/Microsoft.Build/net/Microsoft.Build.cs b/ref/Microsoft.Build/net/Microsoft.Build.cs index 0c3ab22d801..d59f83ba636 100644 --- a/ref/Microsoft.Build/net/Microsoft.Build.cs +++ b/ref/Microsoft.Build/net/Microsoft.Build.cs @@ -1048,8 +1048,7 @@ public enum BuildRequestDataFlags SkipNonexistentTargets = 16, ProvideSubsetOfStateAfterBuild = 32, IgnoreMissingEmptyAndInvalidImports = 64, - SkipNonexistentNonEntryTargets = 128, - FailOnUnresolvedSdk = 256, + FailOnUnresolvedSdk = 128, } public partial class BuildResult { diff --git a/ref/Microsoft.Build/netstandard/Microsoft.Build.cs b/ref/Microsoft.Build/netstandard/Microsoft.Build.cs index 9dc716a4007..ab3acb3d087 100644 --- a/ref/Microsoft.Build/netstandard/Microsoft.Build.cs +++ b/ref/Microsoft.Build/netstandard/Microsoft.Build.cs @@ -1043,8 +1043,7 @@ public enum BuildRequestDataFlags SkipNonexistentTargets = 16, ProvideSubsetOfStateAfterBuild = 32, IgnoreMissingEmptyAndInvalidImports = 64, - SkipNonexistentNonEntryTargets = 128, - FailOnUnresolvedSdk = 256, + FailOnUnresolvedSdk = 128, } public partial class BuildResult { diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index 2e950017adb..ef032ea7791 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -1724,6 +1724,11 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission) projectLoadSettings |= ProjectLoadSettings.IgnoreMissingImports | ProjectLoadSettings.IgnoreInvalidImports | ProjectLoadSettings.IgnoreEmptyImports; } + if (submission.BuildRequestData.Flags.HasFlag(BuildRequestDataFlags.FailOnUnresolvedSdk)) + { + projectLoadSettings |= ProjectLoadSettings.FailOnUnresolvedSdk; + } + return new ProjectInstance( path, properties, diff --git a/src/Build/BackEnd/BuildManager/BuildRequestData.cs b/src/Build/BackEnd/BuildManager/BuildRequestData.cs index 4123d7e0922..6f3a29a5765 100644 --- a/src/Build/BackEnd/BuildManager/BuildRequestData.cs +++ b/src/Build/BackEnd/BuildManager/BuildRequestData.cs @@ -75,6 +75,13 @@ public enum BuildRequestDataFlags /// This is especially useful during a restore since some imports might come from packages that haven't been restored yet. /// IgnoreMissingEmptyAndInvalidImports = 1 << 6, + + /// + /// When this flag is present, an unresolved MSBuild project SDK will fail the build. This flag is used to + /// change the behavior to still fail when an SDK is missing + /// because those are more fatal. + /// + FailOnUnresolvedSdk = 1 << 7, } /// diff --git a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs index 97f9531e074..a72435698c9 100644 --- a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs +++ b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs @@ -466,6 +466,10 @@ private void SetProjectBasedState(ProjectInstance project) projectLoadSettings |= ProjectLoadSettings.IgnoreMissingImports | ProjectLoadSettings.IgnoreInvalidImports | ProjectLoadSettings.IgnoreEmptyImports; } + if (buildRequestDataFlags.HasFlag(BuildRequestDataFlags.FailOnUnresolvedSdk)) + { + projectLoadSettings |= ProjectLoadSettings.FailOnUnresolvedSdk; + } return new ProjectInstance( ProjectFullPath, globalProperties, diff --git a/src/Build/Definition/ProjectLoadSettings.cs b/src/Build/Definition/ProjectLoadSettings.cs index 7f1c9ad2d80..ef51122eca5 100644 --- a/src/Build/Definition/ProjectLoadSettings.cs +++ b/src/Build/Definition/ProjectLoadSettings.cs @@ -59,5 +59,10 @@ public enum ProjectLoadSettings /// Whether to profile the evaluation /// ProfileEvaluation = 128, + + /// + /// Used in combination with to still treat an unresolved MSBuild project SDK as an error. + /// + FailOnUnresolvedSdk = 256, } } diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index f8453160b18..6d11163c965 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -1785,7 +1785,7 @@ private List ExpandAndLoadImports(string directoryOfImportin if (!sdkResult.Success) { - if (_loadSettings.HasFlag(ProjectLoadSettings.IgnoreMissingImports)) + if (_loadSettings.HasFlag(ProjectLoadSettings.IgnoreMissingImports) && !_loadSettings.HasFlag(ProjectLoadSettings.FailOnUnresolvedSdk)) { ProjectImportedEventArgs eventArgs = new ProjectImportedEventArgs( importElement.Location.Line, diff --git a/src/MSBuild.UnitTests/XMake_Tests.cs b/src/MSBuild.UnitTests/XMake_Tests.cs index b3c65a114a6..ca9b4c90e26 100644 --- a/src/MSBuild.UnitTests/XMake_Tests.cs +++ b/src/MSBuild.UnitTests/XMake_Tests.cs @@ -2108,6 +2108,50 @@ public void RestoreIgnoresMissingImports() logContents.ShouldContain(guid2); } + /// + /// When specifying /t:restore, fail when an SDK can't be resolved. Previous behavior was to try and continue anyway but then "restore" would succeed and build workflows continue on. + /// + [Fact] + public void RestoreFailsOnUnresolvedSdk() + { + string projectContents = ObjectModelHelpers.CleanupFileContents( +$@" + + + + +"); + + string logContents = ExecuteMSBuildExeExpectFailure(projectContents, arguments: "/t:restore"); + + logContents.ShouldContain("error MSB4236: The SDK 'UnresolvedSdk' specified could not be found."); + } + + /// + /// Verifies restore will run InitialTargets. + /// + [Fact] + public void RestoreRunsInitialTargets() + { + string projectContents = ObjectModelHelpers.CleanupFileContents( + @" + + + + + + + + + +"); + + string logContents = ExecuteMSBuildExeExpectSuccess(projectContents, arguments: "/t:restore"); + + logContents.ShouldContain("InitialTarget target ran"); + logContents.ShouldContain("Restore target ran"); + } + /// /// We check if there is only one target name specified and this logic caused a regression: https://github.com/Microsoft/msbuild/issues/3317 /// @@ -2312,6 +2356,24 @@ private string CopyMSBuild() } private string ExecuteMSBuildExeExpectSuccess(string projectContents, IDictionary filesToCreate = null, IDictionary envsToCreate = null, params string[] arguments) + { + (bool result, string output) = ExecuteMSBuildExe(projectContents, filesToCreate, envsToCreate, arguments); + + result.ShouldBeTrue(() => output); + + return output; + } + + private string ExecuteMSBuildExeExpectFailure(string projectContents, IDictionary filesToCreate = null, IDictionary envsToCreate = null, params string[] arguments) + { + (bool result, string output) = ExecuteMSBuildExe(projectContents, filesToCreate, envsToCreate, arguments); + + result.ShouldBeFalse(() => output); + + return output; + } + + private (bool result, string output) ExecuteMSBuildExe(string projectContents, IDictionary filesToCreate = null, IDictionary envsToCreate = null, params string[] arguments) { using (TestEnvironment testEnvironment = UnitTests.TestEnvironment.Create()) { @@ -2337,9 +2399,7 @@ private string ExecuteMSBuildExeExpectSuccess(string projectContents, IDictionar string output = RunnerUtilities.ExecMSBuild($"\"{testProject.ProjectFile}\" {String.Join(" ", arguments)}", out success, _output); - success.ShouldBeTrue(() => output); - - return output; + return (success, output); } } } diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index 525f7df79d2..09a0f38eaf3 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -1444,13 +1444,15 @@ private static (BuildResultCode result, Exception exception) ExecuteRestore(stri // - BuildRequestDataFlags.SkipNonexistentTargets to ignore missing targets since Restore does not require that all targets exist // - BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports to ignore imports that don't exist, are empty, or are invalid because restore might // make available an import that doesn't exist yet and the might be missing a condition. + // - BuildRequestDataFlags.FailOnUnresolvedSdk to still fail in the case when an MSBuild project SDK can't be resolved since this is fatal and should + // fail the build. BuildRequestData restoreRequest = new BuildRequestData( projectFile, restoreGlobalProperties, toolsVersion, targetsToBuild: new[] { MSBuildConstants.RestoreTargetName }, hostServices: null, - flags: BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports); + flags: BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports | BuildRequestDataFlags.FailOnUnresolvedSdk); return ExecuteBuild(buildManager, restoreRequest); }