Skip to content

Commit

Permalink
Fix condition where initial targets were skipped because they were no…
Browse files Browse the repository at this point in the history
…n entry targets
  • Loading branch information
jeffkl committed Apr 1, 2021
1 parent dd7831c commit 1936bff
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 23 deletions.
2 changes: 1 addition & 1 deletion ref/Microsoft.Build/net/Microsoft.Build.cs
Expand Up @@ -1048,7 +1048,7 @@ public enum BuildRequestDataFlags
SkipNonexistentTargets = 16,
ProvideSubsetOfStateAfterBuild = 32,
IgnoreMissingEmptyAndInvalidImports = 64,
SkipNonexistentNonTopLevelTargets = 128,
SkipNonexistentNonEntryTargets = 128,
FailOnUnresolvedSdk = 256,
}
public partial class BuildResult
Expand Down
2 changes: 1 addition & 1 deletion ref/Microsoft.Build/netstandard/Microsoft.Build.cs
Expand Up @@ -1043,7 +1043,7 @@ public enum BuildRequestDataFlags
SkipNonexistentTargets = 16,
ProvideSubsetOfStateAfterBuild = 32,
IgnoreMissingEmptyAndInvalidImports = 64,
SkipNonexistentNonTopLevelTargets = 128,
SkipNonexistentNonEntryTargets = 128,
FailOnUnresolvedSdk = 256,
}
public partial class BuildResult
Expand Down
6 changes: 3 additions & 3 deletions src/Build/BackEnd/BuildManager/BuildRequestData.cs
Expand Up @@ -77,12 +77,12 @@ public enum BuildRequestDataFlags
IgnoreMissingEmptyAndInvalidImports = 1 << 6,

/// <summary>
/// When this flag is present, non top level 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 a top lvel target does not exist.
/// 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>
SkipNonexistentNonTopLevelTargets = 1 << 7,
SkipNonexistentNonEntryTargets = 1 << 7,

/// <summary>
/// When this flag is present, an unresolved MSBuild project SDK will fail the build. This flag is used to
Expand Down
29 changes: 16 additions & 13 deletions src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs
Expand Up @@ -143,21 +143,24 @@ public async Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext
foreach (string targetName in targetNames)
{
var targetExists = _projectInstance.Targets.TryGetValue(targetName, out ProjectTargetInstance targetInstance);
// Ignore the missing target if:
// SkipNonexistentTargets is set
// -or-
// SkipNonexistentNonTopLevelTargets and the target is is not a top level target
if (!targetExists
&& entry.Request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.SkipNonexistentTargets)
|| entry.Request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.SkipNonexistentNonTopLevelTargets) && !entry.Request.Targets.Contains(targetName))

if (!targetExists)
{
_projectLoggingContext.LogComment(Framework.MessageImportance.Low,
"TargetSkippedWhenSkipNonexistentTargets", targetName);
}
else
{
targets.Add(new TargetSpecification(targetName, targetExists ? targetInstance.Location : _projectInstance.ProjectFileLocation));
// Ignore the missing target if:
// SkipNonexistentTargets is set
// -or-
// SkipNonexistentNonTopLevelTargets 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;
}
}

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
Expand Down
35 changes: 31 additions & 4 deletions src/MSBuild.UnitTests/XMake_Tests.cs
Expand Up @@ -2128,10 +2128,10 @@ public void RestoreFailsOnUnresolvedSdk()
}

/// <summary>
/// Verifies a non-existent target doesn't fail restore as long as its not considered "top-level" or a target that we're directly executing, in this case Restore.
/// 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 RestoreSkipsNonExistentNonTopLevelTargets()
public void RestoreSkipsNonExistentNonEntryTargets()
{
string restoreFirstProps = $"{Guid.NewGuid():N}.props";

Expand Down Expand Up @@ -2164,10 +2164,10 @@ public void RestoreSkipsNonExistentNonTopLevelTargets()
}

/// <summary>
/// Verifies restore will fail if the "top-level" target doesn't exist, in this case Restore.
/// Verifies restore will fail if the entry target doesn't exist, in this case Restore.
/// </summary>
[Fact]
public void RestoreFailsWhenTopLevelTargetIsNonExistent()
public void RestoreFailsWhenEntryTargetIsNonExistent()
{
string projectContents = ObjectModelHelpers.CleanupFileContents(
@"<Project DefaultTargets=""Build"">
Expand All @@ -2181,6 +2181,33 @@ public void RestoreFailsWhenTopLevelTargetIsNonExistent()
logContents.ShouldContain("error MSB4057: The target \"Restore\" does not exist in the project.");
}

/// <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
2 changes: 1 addition & 1 deletion src/MSBuild/XMake.cs
Expand Up @@ -1434,7 +1434,7 @@ private static (BuildResultCode result, Exception exception) ExecuteRestore(stri
toolsVersion,
targetsToBuild: new[] { MSBuildConstants.RestoreTargetName },
hostServices: null,
flags: BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentNonTopLevelTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports | BuildRequestDataFlags.FailOnUnresolvedSdk);
flags: BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentNonEntryTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports | BuildRequestDataFlags.FailOnUnresolvedSdk);

return ExecuteBuild(buildManager, restoreRequest);
}
Expand Down

0 comments on commit 1936bff

Please sign in to comment.