diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index b5ec4e3736b..66b119cd485 100644 --- 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 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/BuildRequestData.cs b/src/Build/BackEnd/BuildManager/BuildRequestData.cs index 673ee5f0fdf..6f3a29a5765 100644 --- a/src/Build/BackEnd/BuildManager/BuildRequestData.cs +++ b/src/Build/BackEnd/BuildManager/BuildRequestData.cs @@ -76,20 +76,12 @@ public enum BuildRequestDataFlags /// IgnoreMissingEmptyAndInvalidImports = 1 << 6, - /// - /// 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). - /// - SkipNonexistentNonEntryTargets = 1 << 7, - /// /// 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 << 8, + FailOnUnresolvedSdk = 1 << 7, } /// diff --git a/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs index 617fea73b47..21bdf35cb01 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs @@ -143,24 +143,15 @@ public async Task 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 index 659066306cc..a72435698c9 100644 --- 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 index c675c1108e8..d47f970c517 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -1785,8 +1785,7 @@ private List 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 index 1513d20e3f9..ca9b4c90e26 100644 --- 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."); } - /// - /// 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. - /// - [Fact] - public void RestoreSkipsNonExistentNonEntryTargets() - { - string restoreFirstProps = $"{Guid.NewGuid():N}.props"; - - string projectContents = ObjectModelHelpers.CleanupFileContents( -$@" - - {restoreFirstProps} - - - - - - - - - - - - - - - -"); - - string logContents = ExecuteMSBuildExeExpectSuccess(projectContents, arguments: "/restore"); - - logContents.ShouldContain("Restore target ran"); - logContents.ShouldContain("Build target ran"); - logContents.ShouldContain("Initial target ran"); - } - - /// - /// Verifies restore will fail if the entry target doesn't exist, in this case Restore. - /// - [Fact] - public void RestoreFailsWhenEntryTargetIsNonExistent() - { - string projectContents = ObjectModelHelpers.CleanupFileContents( -@" - - - -"); - - string logContents = ExecuteMSBuildExeExpectFailure(projectContents, arguments: "/t:restore"); - - 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. /// @@ -2238,11 +2138,9 @@ public void RestoreRunsInitialTargets() - - @@ -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 index b86c58acce1..09a0f38eaf3 100644 --- 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 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 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); }