Add option to fail on unresolved SDK

Maximal subset of dotnet#6312 and dotnet#6372.

Also removes an unnecessary test per dotnet#6430 (comment)

documentation/wiki/ChangeWaves.md

@@ -27,6 +27,7 @@ The opt-out comes in the form of setting the environment variable `MSBuildDisabl
 - [Error when a property expansion in a condition has whitespace](https://github.com/dotnet/msbuild/pull/5672)
 - [Allow Custom CopyToOutputDirectory Location With TargetPath](https://github.com/dotnet/msbuild/pull/6237)
 - [Allow users that have certain special characters in their username to build successfully when using exec](https://github.com/dotnet/msbuild/pull/6223)
+- [Fail restore operations when an SDK is unresolveable](https://github.com/dotnet/msbuild/pull/6430)
 ### 17.0
 
 ## Change Waves No Longer In Rotation

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
     {

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
     {

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,

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.
         /// </summary>
         IgnoreMissingEmptyAndInvalidImports = 1 << 6,
+
+        /// <summary>
+        /// When this flag is present, an unresolved MSBuild project SDK will fail the build.  This flag is used to
+        /// change the <see cref="IgnoreMissingEmptyAndInvalidImports" /> behavior to still fail when an SDK is missing
+        /// because those are more fatal.
+        /// </summary>
+        FailOnUnresolvedSdk = 1 << 7,
     }
 
     /// <summary>

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,

src/Build/Definition/ProjectLoadSettings.cs

@@ -59,5 +59,10 @@ public enum ProjectLoadSettings
         /// Whether to profile the evaluation
         /// </summary>
         ProfileEvaluation = 128,
+
+        /// <summary>
+        /// Used in combination with <see cref="IgnoreMissingImports" /> to still treat an unresolved MSBuild project SDK as an error.
+        /// </summary>
+        FailOnUnresolvedSdk = 256,
     }
 }

src/Build/Evaluation/Evaluator.cs

@@ -1785,7 +1785,7 @@ private List<ProjectRootElement> 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,

src/MSBuild.UnitTests/XMake_Tests.cs

@@ -2108,6 +2108,50 @@ public void RestoreIgnoresMissingImports()
             logContents.ShouldContain(guid2);
         }
 
+        /// <summary>
+        /// 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.
+        /// </summary>
+        [Fact]
+        public void RestoreFailsOnUnresolvedSdk()
+        {
+            string projectContents = ObjectModelHelpers.CleanupFileContents(
+$@"<Project>
+  <Sdk Name=""UnresolvedSdk"" />
+  <Target Name=""Restore"">
+    <Message Text=""Restore target ran"" />
+  </Target>
+</Project>");
+
+            string logContents = ExecuteMSBuildExeExpectFailure(projectContents, arguments: "/t:restore");
+
+            logContents.ShouldContain("error MSB4236: The SDK 'UnresolvedSdk' specified could not be found.");
+        }
+
+        /// <summary>
+        /// Verifies restore will run InitialTargets.
+        /// </summary>
+        [Fact]
+        public void RestoreRunsInitialTargets()
+        {
+            string projectContents = ObjectModelHelpers.CleanupFileContents(
+                @"<Project DefaultTargets=""Build"" InitialTargets=""InitialTarget"">
+  <Target Name=""InitialTarget"">
+    <Message Text=""InitialTarget target ran&quot;"" />
+  </Target>
+  <Target Name=""Restore"">
+    <Message Text=""Restore target ran&quot;"" />
+  </Target>
+  <Target Name=""Build"">
+    <Message Text=""Build target ran&quot;"" />
+  </Target>
+</Project>");
+
+            string logContents = ExecuteMSBuildExeExpectSuccess(projectContents, arguments: "/t:restore");
+
+            logContents.ShouldContain("InitialTarget target ran");
+            logContents.ShouldContain("Restore target ran");
+        }
+
         /// <summary>
         /// We check if there is only one target name specified and this logic caused a regression: https://github.com/Microsoft/msbuild/issues/3317
         /// </summary>
@@ -2312,6 +2356,24 @@ private string CopyMSBuild()
         }
 
         private string ExecuteMSBuildExeExpectSuccess(string projectContents, IDictionary<string, string> filesToCreate = null,  IDictionary<string, string> 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<string, string> filesToCreate = null, IDictionary<string, string> 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<string, string> filesToCreate = null, IDictionary<string, string> 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);
             }
         }
     }

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 <Import /> 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);
         }