TargetBuiltReason should have a field for InitialTargets

[KirillOsenkov]

Currently we liberally use TargetBuiltReason.None
https://source.dot.net/#Microsoft.Build.Framework/TargetBuiltReason.cs,e71ba02076cc1555,references

Specifically we use None for InitialTargets, so it's hard to tell why a target was built:

Need to be careful about this check though:

msbuild/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs

Line 710 in f7f0b19

if (buildReason == TargetBuiltReason.BeforeTargets || buildReason == TargetBuiltReason.DependsOn || buildReason == TargetBuiltReason.None)

[KirillOsenkov]

I do notice that the message for the TargetStartedEventArgs includes the (entry point) string

So there's some indication already.

[JanKrivanek]

@maridematte pls check if this is caused by same problem or unrelated: KirillOsenkov/MSBuildStructuredLog#762

[maridematte]

Investigation results:

We currently assign the TargetBuiltReason when pushing targets to the stack

msbuild/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs

Line 162 in cd64b7b

await PushTargets(targets, null, baseLookup, false, false, TargetBuiltReason.None);

During this process we define if they're being pushed because of another target and assign either BeforeTargets, DependsOn or AfterTargets. However at that step we only have access to a list of targets to be built, but not which ones are the initial targets or the entry points for the build.
The list of targets to build passed is a mix of initial targets, and the rest of the targets all joined together

msbuild/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs

Line 758 in cd64b7b

var allTargets = new List<string>(initialTargets.Count + nonInitialTargets.Count);

To make the change requested here we would have to partially rewrite how targets are ordered to build, or do some pretty hacky stuff on the code and change all the tests to match, and we do not have the capacity for such a big change at the moment. Moving this to backlog for now.

[KirillOsenkov]

yes this is definitely very low priority. I understand why it would be hard to implement given the current implementation.

Perhaps instead we should just log which targets are initial at the beginning.

[JanKrivanek]

Or could we reconstruct the initial targets from the BuildRequestEntry passed to that method? (entry.RequestConfiguration.ProjectInitialTargets)

That should not require any contract change.

[JanKrivanek]

Btw. is the AfterTargets BuildReason populated as well?

I'm wondering whether the KirillOsenkov/MSBuildStructuredLog#762 is an MSBuild issue or Viewer issue.

[rainersigwald]

It looks like we populate it

msbuild/src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs

Line 432 in cd64b7b

bool didPushTargets = await PushTargets(afterTargets, currentTargetEntry.ParentEntry, currentTargetEntry.Lookup, currentTargetEntry.ErrorTarget, currentTargetEntry.StopProcessingOnCompletion, TargetBuiltReason.AfterTargets);

but I wouldn't guarantee it is correct on the MSBuild side.

[KirillOsenkov]

I took a look and MSBuild is not setting the ParentTarget for AfterTargets for that particular case:
KirillOsenkov/MSBuildStructuredLog#762 (comment)

[KirillOsenkov]

Would be interesting to get to the bottom of this. We can use this bug or file a new one.