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:

MSBuildStructuredLog/src/StructuredLogger/Construction/Construction.cs

Line 1154 in 7e1ca2a

string assembly = GetTaskAssembly(taskName);

Currently we compute the Task assembly by parsing Using Task messages here:

MSBuildStructuredLog/src/StructuredLogger/Construction/MessageProcessor.cs

Lines 131 to 139 in 7e1ca2a

	string rawMessage = Reflector.GetMessage(args);
	var match = Strings.UsingTask(message, rawMessage);
	if (match.Success)
	{
	construction.SetTaskAssembly(
	Intern(match.Groups["task"].Value),
	Intern(match.Groups["assembly"].Value));
	return;
	}

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 :)
dotnet/msbuild#9948 (comment)

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: MichalPavlik#1 (I do not have write access to your fork @MichalPavlik - so I created this as a PR to your branch :))

[MichalPavlik]

Great, thanks :)

[KirillOsenkov]

Thank you!