From ebc4a8bd554f2ecb7a0777e954e4154ef1ebe793 Mon Sep 17 00:00:00 2001 From: Rainer Sigwald Date: Fri, 23 Apr 2021 13:27:44 -0500 Subject: [PATCH] Put fail-restore behavior under 16.10 changewave Add the behavior improvements from #6312 to the 16.10 changewave since they cause failures in cases that didn't fail before. --- documentation/wiki/ChangeWaves.md | 1 + src/MSBuild.UnitTests/XMake_Tests.cs | 46 ++++++++++++++++++++++++++++ src/MSBuild/XMake.cs | 25 ++++++++++----- 3 files changed, 64 insertions(+), 8 deletions(-) diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index 1e25829f60b..b5ec4e3736b 100644 --- a/documentation/wiki/ChangeWaves.md +++ b/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 diff --git a/src/MSBuild.UnitTests/XMake_Tests.cs b/src/MSBuild.UnitTests/XMake_Tests.cs index e014f5652e3..1513d20e3f9 100644 --- a/src/MSBuild.UnitTests/XMake_Tests.cs +++ b/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."); } + /// + /// 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. + /// + [Fact] + public void RestorePassesOnUnresolvedSdkUnderChangewave() + { + string projectContents = ObjectModelHelpers.CleanupFileContents( +$@" + + + + +"); + + using TestEnvironment env = Microsoft.Build.UnitTests.TestEnvironment.Create(); + + string logContents = ExecuteMSBuildExeExpectSuccess(projectContents, + envsToCreate: new Dictionary() { ["MSBUILDDISABLEFEATURESFROMVERSION"]=ChangeWaves.Wave16_10.ToString() }, + arguments: " /t:restore"); + + logContents.ShouldNotContain("MSB4236"); + } + + /// /// Verifies a non-existent target doesn't fail restore as long as its not considered an entry target, in this case Restore. /// @@ -2181,6 +2207,26 @@ public void RestoreFailsWhenEntryTargetIsNonExistent() logContents.ShouldContain("error MSB4057: The target \"Restore\" does not exist in the project."); } + /// + /// Verifies restore will not fail if the entry target doesn't exist, when changewave applied. + /// + [Fact] + public void RestorePassesWhenEntryTargetIsNonExistentUnderChangewave() + { + string projectContents = ObjectModelHelpers.CleanupFileContents( +@" + + + +"); + + string logContents = ExecuteMSBuildExeExpectSuccess(projectContents, + envsToCreate: new Dictionary() { ["MSBUILDDISABLEFEATURESFROMVERSION"] = ChangeWaves.Wave16_10.ToString() }, + arguments: "/t:restore"); + + logContents.ShouldNotContain("MSB4057"); + } + /// /// Verifies restore will run InitialTargets. /// diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index 0e0f82278e8..62a38783b99 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -1421,14 +1421,23 @@ 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 = - 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 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. + 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 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,