Skip to content

Commit

Permalink
Add option to fail on unresolved SDK
Browse files Browse the repository at this point in the history
Maximal subset of dotnet#6312 and dotnet#6372.

Also removes an unnecessary test per dotnet#6430 (comment)
  • Loading branch information
Forgind committed May 7, 2021
1 parent 2d8014e commit ca12412
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 9 deletions.
3 changes: 1 addition & 2 deletions ref/Microsoft.Build/net/Microsoft.Build.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,8 +1048,7 @@ public enum BuildRequestDataFlags
SkipNonexistentTargets = 16,
ProvideSubsetOfStateAfterBuild = 32,
IgnoreMissingEmptyAndInvalidImports = 64,
SkipNonexistentNonEntryTargets = 128,
FailOnUnresolvedSdk = 256,
FailOnUnresolvedSdk = 128,
}
public partial class BuildResult
{
Expand Down
3 changes: 1 addition & 2 deletions ref/Microsoft.Build/netstandard/Microsoft.Build.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1043,8 +1043,7 @@ public enum BuildRequestDataFlags
SkipNonexistentTargets = 16,
ProvideSubsetOfStateAfterBuild = 32,
IgnoreMissingEmptyAndInvalidImports = 64,
SkipNonexistentNonEntryTargets = 128,
FailOnUnresolvedSdk = 256,
FailOnUnresolvedSdk = 128,
}
public partial class BuildResult
{
Expand Down
5 changes: 5 additions & 0 deletions src/Build/BackEnd/BuildManager/BuildManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions src/Build/BackEnd/BuildManager/BuildRequestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 4 additions & 0 deletions src/Build/BackEnd/Shared/BuildRequestConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,10 @@ internal void LoadProjectIntoConfiguration(
projectLoadSettings |= ProjectLoadSettings.IgnoreMissingImports | ProjectLoadSettings.IgnoreInvalidImports | ProjectLoadSettings.IgnoreEmptyImports;
}

if (buildRequestDataFlags.HasFlag(BuildRequestDataFlags.FailOnUnresolvedSdk))
{
projectLoadSettings |= ProjectLoadSettings.FailOnUnresolvedSdk;
}
return new ProjectInstance(
ProjectFullPath,
globalProperties,
Expand Down
5 changes: 5 additions & 0 deletions src/Build/Definition/ProjectLoadSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
2 changes: 1 addition & 1 deletion src/Build/Evaluation/Evaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1785,7 +1785,7 @@ static string EvaluateProperty(string value, IElementLocation location,

if (!sdkResult.Success)
{
if (_loadSettings.HasFlag(ProjectLoadSettings.IgnoreMissingImports))
if (_loadSettings.HasFlag(ProjectLoadSettings.IgnoreMissingImports) && !_loadSettings.HasFlag(ProjectLoadSettings.FailOnUnresolvedSdk))
{
ProjectImportedEventArgs eventArgs = new ProjectImportedEventArgs(
importElement.Location.Line,
Expand Down
66 changes: 63 additions & 3 deletions src/MSBuild.UnitTests/XMake_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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())
{
Expand All @@ -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);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/MSBuild/XMake.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit ca12412

Please sign in to comment.