dotnet · rainersigwald · May 11, 2021 · May 7, 2021 · May 7, 2021 · May 7, 2021
diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md
@@ -27,7 +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)
+- [Fail restore operations when an SDK is unresolveable](https://github.com/dotnet/msbuild/pull/6430)
 ### 17.0
 
 ## Change Waves No Longer In Rotation
diff --git 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
@@ -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/BuildRequestData.cs b/src/Build/BackEnd/BuildManager/BuildRequestData.cs
@@ -76,20 +76,12 @@ public enum BuildRequestDataFlags
         /// </summary>
         IgnoreMissingEmptyAndInvalidImports = 1 << 6,
 
-        /// <summary>
-        /// When this flag is present, non entry target(s) in the build request will be skipped if those targets
-        /// are not defined in the Project to build. The build will still fail if an entry target does not exist.
-        /// This only applies to this build request (if another target calls the "missing target" at any other point
-        /// this will still result in an error).
-        /// </summary>
-        SkipNonexistentNonEntryTargets = 1 << 7,
-
         /// <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 << 8,
+        FailOnUnresolvedSdk = 1 << 7,
     }
 
     /// <summary>

diff --git a/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs
@@ -143,24 +143,15 @@ public async Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext
             foreach (string targetName in targetNames)
             {
                 var targetExists = _projectInstance.Targets.TryGetValue(targetName, out ProjectTargetInstance targetInstance);
-
-                if (!targetExists)
+                if (!targetExists && entry.Request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.SkipNonexistentTargets))
                 {
-                    // Ignore the missing target if:
-                    //  SkipNonexistentTargets is set
-                    //  -or-
-                    //  SkipNonexistentNonEntryTargets and the target is is not a top level target
-                    if (entry.Request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.SkipNonexistentTargets)
-                        || entry.Request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.SkipNonexistentNonEntryTargets) && !entry.Request.Targets.Contains(targetName))
-                    {
-                        _projectLoggingContext.LogComment(Framework.MessageImportance.Low,
-                            "TargetSkippedWhenSkipNonexistentTargets", targetName);
-
-                        continue;
-                    }
+                    _projectLoggingContext.LogComment(Framework.MessageImportance.Low,
+                        "TargetSkippedWhenSkipNonexistentTargets", targetName);
+                }
+                else
+                {
+                    targets.Add(new TargetSpecification(targetName, targetExists ? targetInstance.Location : _projectInstance.ProjectFileLocation));
                 }
-
-                targets.Add(new TargetSpecification(targetName, targetExists ? targetInstance.Location : _projectInstance.ProjectFileLocation));
             }
 
             // Push targets onto the stack.  This method will reverse their push order so that they

diff --git a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs
@@ -470,7 +470,6 @@ private void SetProjectBasedState(ProjectInstance project)
                 {
                     projectLoadSettings |= ProjectLoadSettings.FailOnUnresolvedSdk;
                 }
-
                 return new ProjectInstance(
                     ProjectFullPath,
                     globalProperties,

diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs
@@ -1785,8 +1785,7 @@ private List<ProjectRootElement> ExpandAndLoadImports(string directoryOfImportin
 
                 if (!sdkResult.Success)
                 {
-                    // Ignore the missing import if IgnoreMissingImports is set unless FailOnUnresolvedSdk is also set
-                    if (_loadSettings.HasFlag(ProjectLoadSettings.IgnoreMissingImports) && !_loadSettings.HasFlag(ProjectLoadSettings.FailOnUnresolvedSdk))
+                    if (_loadSettings.HasFlag(ProjectLoadSettings.IgnoreMissingImports) && (!ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_10) || !_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
@@ -19,7 +19,6 @@
 using Shouldly;
 using System.IO.Compression;
 using System.Reflection;
-using Microsoft.Build.Utilities;
 
 namespace Microsoft.Build.UnitTests
 {
@@ -2128,105 +2127,6 @@ 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>
-        [Fact]
-        public void RestoreSkipsNonExistentNonEntryTargets()
-        {
-            string restoreFirstProps = $"{Guid.NewGuid():N}.props";
-
-            string projectContents = ObjectModelHelpers.CleanupFileContents(
-$@"<Project DefaultTargets=""Build"" InitialTargets=""TargetThatComesFromRestore"">
-  <PropertyGroup>
-    <RestoreFirstProps>{restoreFirstProps}</RestoreFirstProps>
-  </PropertyGroup>
-
-  <Import Project=""$(RestoreFirstProps)"" />
-  <Target Name=""Restore"">
-    <Message Text=""Restore target ran"" />
-    <ItemGroup>
-      <Lines Include=""&lt;Project&gt;&lt;Target Name=&quot;TargetThatComesFromRestore&quot;&gt;&lt;Message Text=&quot;Initial target ran&quot; /&gt;&lt;/Target&gt;&lt;/Project&gt;"" />
-    </ItemGroup>
-
-    <WriteLinesToFile File=""$(RestoreFirstProps)"" Lines=""@(Lines)"" Overwrite=""true"" />
-  </Target>
-
-  <Target Name=""Build"">
-    <Message Text=""Build target ran&quot;"" />
-  </Target>
-</Project>");
-
-            string logContents = ExecuteMSBuildExeExpectSuccess(projectContents, arguments: "/restore");
-
-            logContents.ShouldContain("Restore target ran");
-            logContents.ShouldContain("Build target ran");
-            logContents.ShouldContain("Initial target ran");
-        }
-
-        /// <summary>
-        /// Verifies restore will fail if the entry target doesn't exist, in this case Restore.
-        /// </summary>
-        [Fact]
-        public void RestoreFailsWhenEntryTargetIsNonExistent()
-        {
-            string projectContents = ObjectModelHelpers.CleanupFileContents(
-@"<Project DefaultTargets=""Build"">
-  <Target Name=""Build"">
-    <Message Text=""Build target ran&quot;"" />
-  </Target>
-</Project>");
-
-            string logContents = ExecuteMSBuildExeExpectFailure(projectContents, arguments: "/t:restore");
-
-            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>
@@ -2238,11 +2138,9 @@ public void RestoreRunsInitialTargets()
   <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>
@@ -2500,7 +2398,7 @@ private string ExecuteMSBuildExeExpectFailure(string projectContents, IDictionar
                 bool success;
 
                 string output = RunnerUtilities.ExecMSBuild($"\"{testProject.ProjectFile}\" {String.Join(" ", arguments)}", out success, _output);
-                
+
                 return (success, output);
             }
         }

diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs
@@ -1440,31 +1440,19 @@ 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 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;
-            }
-
+            //  - BuildRequestDataFlags.ClearCachesAfterBuild to ensure the projects will be reloaded from disk for subsequent builds
+            //  - 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);
+                flags: BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports | BuildRequestDataFlags.FailOnUnresolvedSdk);
 
             return ExecuteBuild(buildManager, restoreRequest);
         }