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

[Static Graph] ProjectGraph.GetTargetLists throws on invalid target names #4599

Merged
merged 4 commits into from
Sep 17, 2019

Conversation

cdmihai
Copy link
Contributor

@cdmihai cdmihai commented Aug 9, 2019

Just cleanup: clarifying some documentation, validating some inputs, avoiding needless iterations.

}

[Fact]
public void GetTargetListsReturnsEmptyTargetsForAllNodesWhenDefaultTargetsAreRequestedAndThereAreNoDefaultTargets()
Copy link
Member

Choose a reason for hiding this comment

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

Under what circumstance are there no default targets? Doesn't MSBuild always have a default target (if none is explicitly listed, it's the first one defined in the logical project)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The project file has no targets. Added a comment on it.

src/Build/Graph/ProjectGraph.cs Outdated Show resolved Hide resolved

if (targetNames.Any(targetName => string.IsNullOrWhiteSpace(targetName)))
{
throw new ArgumentException(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("OM_TargetNameNullOrEmpty", nameof(GetTargetLists)));
Copy link
Member

Choose a reason for hiding this comment

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

Is this enough context? I was thinking more along the lines of "what project caused this error?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's a bit harder to answer. There is no actual project here. These targets are the graph wide targets the whole graph needs to be built with. The closest thing for identification would be the set of graph roots

@rainersigwald rainersigwald added this to the MSBuild 16.4 milestone Aug 13, 2019
@rainersigwald rainersigwald merged commit 2bf1ee7 into dotnet:master Sep 17, 2019
@cdmihai cdmihai deleted the clarifyDocumentation branch December 30, 2020 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants