Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Users/haiz/add verbosity for target circular dependence #5711

Merged
28 changes: 28 additions & 0 deletions src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,34 @@ public void TestCircularDependencyInCallTarget()
Assert.False(success);
}

/// <summary>
/// Tests a circular dependency target.
/// </summary>
[Fact]
public void TestCircularDependencyTarget()
{
string projectContents = @"
<Project xmlns=""http://schemas.microsoft.com/developer/msbuild/2003"">
<Target Name=""TargetA"" AfterTargets=""Build"" DependsOnTargets=""TargetB"">
<Message Text=""TargetA""></Message>
</Target>
<Target Name=""TargetB"" DependsOnTargets=""TargetC"">
<Message Text=""TargetB""></Message>
</Target>
<Target Name=""TargetC"" DependsOnTargets=""TargetA"">
<Message Text=""TargetC""></Message>
</Target>
</Project>
";
string errorMessage = @"There is a circular dependency in the target dependency graph involving target ""TargetA"". Since ""TargetC"" has ""DependsOn"" dependence on ""TargetA"", the circular is ""TargetA<-TargetC<-TargetB<-TargetA"".";

StringReader reader = new StringReader(projectContents);
Project project = new Project(new XmlTextReader(reader), null, null);
project.Build(_mockLogger).ShouldBeFalse();
_mockLogger.ErrorCount.ShouldBe(1);
_mockLogger.Errors[0].Message.ShouldBe(errorMessage);
}

/// <summary>
/// Tests that cancel with no entries after building does not fail.
/// </summary>
Expand Down
43 changes: 34 additions & 9 deletions src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -697,16 +697,9 @@ private async Task<bool> PushTargets(IList<TargetSpecification> targets, TargetE
// Does this target exist in our direct parent chain, if it is a before target (since these can cause circular dependency issues)
if (buildReason == TargetBuiltReason.BeforeTargets || buildReason == TargetBuiltReason.DependsOn || buildReason == TargetBuiltReason.None)
{
TargetEntry currentParent = parentTargetEntry;
while (currentParent != null)
if (HasCircularDependenceInTargets(parentTargetEntry, targetSpecification, out List<string> targetDependenceChain))
{
if (String.Equals(currentParent.Name, targetSpecification.TargetName, StringComparison.OrdinalIgnoreCase))
{
// We are already building this target on this request. That's a circular dependency.
ProjectErrorUtilities.ThrowInvalidProject(targetLocation, "CircularDependency", targetSpecification.TargetName);
}

currentParent = currentParent.ParentEntry;
ProjectErrorUtilities.ThrowInvalidProject(targetLocation, "CircularDependencyInTargetGraph", targetSpecification.TargetName, parentTargetEntry.Name, buildReason, targetSpecification.TargetName, string.Join("<-", targetDependenceChain));
}
}
else
Expand Down Expand Up @@ -812,5 +805,37 @@ private void ComputeAfterTargetFailures(string[] targetNames)
}
}
}

private bool HasCircularDependenceInTargets(TargetEntry parentTargetEntry, TargetSpecification targetSpecification, out List<string> circularDependenceChain)
{
TargetEntry currentParent = parentTargetEntry;
circularDependenceChain = new List<string>();
bool hasCircularDependence = false;

while (currentParent != null)
{
if (String.Equals(currentParent.Name, targetSpecification.TargetName, StringComparison.OrdinalIgnoreCase))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's relevant which StringComparison you're using here, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, correct.

{
// We are already building this target on this request. That's a circular dependency.
hasCircularDependence = true;

// Cache the circular dependence chain only when circular dependency occurs.
currentParent = parentTargetEntry;
circularDependenceChain.Add(targetSpecification.TargetName);
while (!String.Equals(currentParent.Name, targetSpecification.TargetName, StringComparison.OrdinalIgnoreCase))
{
circularDependenceChain.Add(currentParent.Name);
currentParent = currentParent.ParentEntry;
}

circularDependenceChain.Add(currentParent.Name);
break;
}

currentParent = currentParent.ParentEntry;
}

return hasCircularDependence;
}
}
}
5 changes: 5 additions & 0 deletions src/Build/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,11 @@
<comment>{StrBegin="MSB4006: "}UE: This message is shown when the build engine detects a target referenced in a circular manner -- a project cannot
request a target to build itself (perhaps via a chain of other targets).</comment>
</data>
<data name="CircularDependencyInTargetGraph" xml:space="preserve">
<value>MSB4006: There is a circular dependency in the target dependency graph involving target "{0}". Since "{1}" has "{2}" dependence on "{3}", the circular is "{4}".</value>
<comment>{StrBegin="MSB4006: "}UE: This message is shown when the build engine detects a target referenced in a circular manner -- a project cannot
request a target to build itself (perhaps via a chain of other targets).</comment>
</data>
<data name="ComparisonOnNonNumericExpression" xml:space="preserve">
<value>MSB4086: A numeric comparison was attempted on "{1}" that evaluates to "{2}" instead of a number, in condition "{0}".</value>
<comment>{StrBegin="MSB4086: "}</comment>
Expand Down
18 changes: 12 additions & 6 deletions src/Build/Resources/xlf/Strings.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 12 additions & 6 deletions src/Build/Resources/xlf/Strings.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 12 additions & 6 deletions src/Build/Resources/xlf/Strings.en.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 12 additions & 6 deletions src/Build/Resources/xlf/Strings.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading