Changewave 16.10 for restore failures

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 there is no `Restore` target or an SDK is unresolveable](https://github.com/dotnet/msbuild/pull/6312)
 ### 17.0
 
 ## Change Waves No Longer In Rotation

src/Build/BackEnd/Shared/BuildRequestConfiguration.cs

@@ -466,7 +466,7 @@ internal void LoadProjectIntoConfiguration(
                     projectLoadSettings |= ProjectLoadSettings.IgnoreMissingImports | ProjectLoadSettings.IgnoreInvalidImports | ProjectLoadSettings.IgnoreEmptyImports;
                 }
 
-                if (buildRequestDataFlags.HasFlag(buildRequestDataFlags & BuildRequestDataFlags.FailOnUnresolvedSdk))
+                if (buildRequestDataFlags.HasFlag(BuildRequestDataFlags.FailOnUnresolvedSdk))
                 {
                     projectLoadSettings |= ProjectLoadSettings.FailOnUnresolvedSdk;
                 }

src/MSBuild.UnitTests/XMake_Tests.cs

@@ -19,6 +19,7 @@
 using Shouldly;
 using System.IO.Compression;
 using System.Reflection;
+using Microsoft.Build.Utilities;
 
 namespace Microsoft.Build.UnitTests
 {
@@ -2127,6 +2128,31 @@ public void RestoreFailsOnUnresolvedSdk()
             logContents.ShouldContain("error MSB4236: The SDK 'UnresolvedSdk' specified could not be found.");
         }
 
+        /// <summary>
+        /// When specifying /t:restore under an old changewave, do not 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 RestorePassesOnUnresolvedSdkUnderChangewave()
+        {
+            string projectContents = ObjectModelHelpers.CleanupFileContents(
+$@"<Project>
+  <Sdk Name=""UnresolvedSdk"" />
+  <Target Name=""Restore"">
+    <Message Text=""Restore target ran"" />
+  </Target>
+</Project>");
+
+            using TestEnvironment env = Microsoft.Build.UnitTests.TestEnvironment.Create();
+
+            string logContents = ExecuteMSBuildExeExpectSuccess(projectContents,
+                envsToCreate: new Dictionary<string, string>() { ["MSBUILDDISABLEFEATURESFROMVERSION"]=ChangeWaves.Wave16_10.ToString() },
+                arguments: " /t:restore");
+
+            logContents.ShouldNotContain("MSB4236");
+        }
+
+
         /// <summary>
         /// Verifies a non-existent target doesn't fail restore as long as its not considered an entry target, in this case Restore.
         /// </summary>
@@ -2181,6 +2207,26 @@ public void RestoreFailsWhenEntryTargetIsNonExistent()
             logContents.ShouldContain("error MSB4057: The target \"Restore\" does not exist in the project.");
         }
 
+        /// <summary>
+        /// Verifies restore will not fail if the entry target doesn't exist, when changewave applied.
+        /// </summary>
+        [Fact]
+        public void RestorePassesWhenEntryTargetIsNonExistentUnderChangewave()
+        {
+            string projectContents = ObjectModelHelpers.CleanupFileContents(
+@"<Project DefaultTargets=""Build"">
+  <Target Name=""Build"">
+    <Message Text=""Build target ran&quot;"" />
+  </Target>
+</Project>");
+
+            string logContents = ExecuteMSBuildExeExpectSuccess(projectContents,
+                envsToCreate: new Dictionary<string, string>() { ["MSBUILDDISABLEFEATURESFROMVERSION"] = ChangeWaves.Wave16_10.ToString() },
+                arguments: "/t:restore");
+
+            logContents.ShouldNotContain("MSB4057");
+        }
+
         /// <summary>
         /// Verifies restore will run InitialTargets.
         /// </summary>

src/MSBuild/XMake.cs

@@ -1421,20 +1421,31 @@ private static (BuildResultCode result, Exception exception) ExecuteRestore(stri
             restoreGlobalProperties["MSBuildRestoreSessionId"] = Guid.NewGuid().ToString("D");
 
             // Create a new request with a Restore target only and specify:
-            //  - BuildRequestDataFlags.ClearCachesAfterBuild to ensure the projects will be reloaded from disk for subsequent builds
-            //  - BuildRequestDataFlags.SkipNonexistentNonEntryTargets to ignore missing non-entry targets since Restore does not require that all targets
-            //      exist, only top-level ones like Restore itself
-            //  - 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.
+            BuildRequestDataFlags flags;
+
+            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_10))
+            {
+                flags =   BuildRequestDataFlags.ClearCachesAfterBuild                // ensure the projects will be reloaded from disk for subsequent builds
+                        | BuildRequestDataFlags.SkipNonexistentNonEntryTargets       // ignore missing non-entry targets since Restore does not require that all targets
+                                                                                     // exist, only top-level ones like Restore itself
+                        | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports  // 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;                 // still fail in the case when an MSBuild project SDK can't be resolved since this is fatal and should
+                                                                                     // fail the build.
+            }
+            else
+            {
+                // pre-16.10 flags allowed `-restore` to pass when there was no `Restore` target
+                flags = BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports;
+            }
+
             BuildRequestData restoreRequest = new BuildRequestData(
                 projectFile,
                 restoreGlobalProperties,
                 toolsVersion,
                 targetsToBuild: new[] { MSBuildConstants.RestoreTargetName },
                 hostServices: null,
-                flags: BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentNonEntryTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports | BuildRequestDataFlags.FailOnUnresolvedSdk);
+                flags);
 
             return ExecuteBuild(buildManager, restoreRequest);
         }