Added TaskAssemblyLocation to TaskStartedEventArgs2

[MichalPavlik]

Fixes #771

[KirillOsenkov]

The reader looks good! Let's also update the BuildEventArgsWriter (same changes as in the MSBuild repo)

[KirillOsenkov]

Oh, also need to update this place: https://github.com/KirillOsenkov/MSBuildStructuredLog/blob/7e1ca2ae77eaee8e273e416c56aaaf72bd311f42/src/StructuredLogger/Construction/Construction.cs#L1154

Currently we compute the Task assembly by parsing Using Task messages here: https://github.com/KirillOsenkov/MSBuildStructuredLog/blob/7e1ca2ae77eaee8e273e416c56aaaf72bd311f42/src/StructuredLogger/Construction/MessageProcessor.cs#L131-L139

We need to first take TaskStartedEventArgs2.TaskAssemblyLocation if that's not null or empty, otherwise fall back to the existing logic.

[KirillOsenkov]

Also do you have a binlog that exercises a new resource string that you added? Does that string end up in the binlog?

[MichalPavlik]

Yes, it did :) https://github.com/dotnet/msbuild/pull/9948#discussion_r1555893907

I will update the MessageProcessor. I wanted to ask where can I find the code for populating Assembly node inside task, but you was faster :)

[KirillOsenkov]

Now just the writer please ;)

[KirillOsenkov]

@MichalPavlik could you please include the changes in our copy of BuildEventArgsWriter.cs as well?

[KirillOsenkov]

@MichalPavlik also please make sure the test TestBinaryLoggerRoundtrip is passing, it's failing with these changes

[MichalPavlik]

Sorry, I had vacation. I will do it today.

[MichalPavlik]

I was debugging the test locally and I'm confused by the diff. It seems that the difference is in newline format:

I have to investigate more, but if you have any idea, please let me know. I don't think my changes would cause this kind of failure :|

[KirillOsenkov]

Perhaps @JanKrivanek can help here, he did a lot of work in making sure the format is roundtrippable.

[JanKrivanek]

The nullref exception is more concerning that the newline format - I'd suggest looking into that

[MichalPavlik]

Test result was misleading :) The root cause was in writer - test is using MSBuild to build project, but it knows nothing about TaskStartedEventArgs2, so the value was missing in binlog.

[JanKrivanek]

Lets make sure the test is catching the similar problems for the future: https://github.com/MichalPavlik/MSBuildStructuredLog/pull/1 (I do not have write access to your fork @MichalPavlik - so I created this as a PR to your branch :))

[MichalPavlik]

Great, thanks :)