Embed source file from CodeTaskFactory and RoslynCodeTaskFactory in binlog

[surayya-MS]

Fixes #9686

Context

See more details in the issue description.

In this PR I made changes to embed the generated file that is used for compilation from CodeTaskFactory and RoslynCodeTaskFactory. It could be the source file or an actual generated file (like from Fragment).
Because there is a race between embedding the generated file and deleting it, instead I save its contents and use the existing method AddFileFromMemory to embed it.
The file is embedded in binlog as {projectDirectory}/{guid}-{taskName}-compilation-file.tmp

Changes Made

Introduced new event args for embedding generated file in binlog GeneratedFileUsedEventArgs similar to ResponseFileUsedEventArgs

namespace Microsoft.Build.Framework
{
    [Serializable]
    public class GeneratedFileUsedEventArgs : BuildMessageEventArgs
    {
        public GeneratedFileUsedEventArgs();

        public GeneratedFileUsedEventArgs(string filePath, string content);

        public string? FilePath { set; get; }

        public string? Content { set; get; }
    }
}

Introduced new method for the logger that is used for tasks:

TaskLoggingHelper.LogIncludeGeneratedFile(string filePath, string content)

Testing

Added unit tests and tested manually.

Notes

[rainersigwald]

Question: this is doing what the bug asked for, which is logging the .cs file if it was separate. Looking at it, I wonder if we should try to do one better and embed the generated source file too, regardless of where the C# came from--that is, log the file we tried to compile instead of the source.

[surayya-MS]

Question: this is doing what the bug asked for, which is logging the .cs file if it was separate. Looking at it, I wonder if we should try to do one better and embed the generated source file too, regardless of where the C# came from--that is, log the file we tried to compile instead of the source.

@rainersigwald , I agree that logging the generated file would improve debuggability. However, on the second thought wouldn't that be redundant because the code is already in the .csproj file? And what about Type "Fragment" and "Method"?
Example:

<Task>
	<Code Type="Class" Language="cs">
		using Microsoft.Build.Framework;
		using Microsoft.Build.Utilities;

		public class MyTask : Microsoft.Build.Utilities.Task
		{
		public override bool Execute()
		{
		Log.LogMessage(MessageImportance.High, "MyTask has been executed. The value of StringComparison is: " + StringComparison);
		return true;
		}
		}
	</Code>
</Task>

[rainersigwald]

I think logging the generated file for Fragment and Method types would be the most helpful thing, since the interaction of the code we generate and the code from the project file could be going wrong and seeing it all in one place could help explain why.

Agreed that if you have 100% of the code in the project there's no need to log it redundantly.

[surayya-MS]

There are some strange fails on Windows Full and Windows Core (Now it is only Windows Full and I didn't change the code). The error message is:

Child node "1" exited prematurely. Shutting down. Diagnostic information may be found in files in "C:\Users\VssAdministrator\AppData\Local\Temp\MSBuildTemp\" and will be named MSBuild_*.failure.txt. This location can be changed by setting the MSBUILDDEBUGPATH environment variable to a different directory.Microsoft.Build.Exceptions.InternalLoggerException: The build stopped unexpectedly because of an unexpected logger failure. ---> System.ArgumentNullException: Parameter "BuildEventContext" cannot be null.
   at Microsoft.Build.Shared.ErrorUtilities.ThrowArgumentNull(String parameterName, String resourceName) in /_/src/Shared/ErrorUtilities.cs:line 575
   at Microsoft.Build.Shared.ErrorUtilities.VerifyThrowArgumentNull(Object parameter, String parameterName, String resourceName) in /_/src/Shared/ErrorUtilities.cs:line 568
   at Microsoft.Build.Shared.ErrorUtilities.VerifyThrowArgumentNull(Object parameter, String parameterName) in /_/src/Shared/ErrorUtilities.cs:line 557
   at Microsoft.Build.BackEnd.Logging.ParallelConsoleLogger.MessageHandler(Object sender, BuildMessageEventArgs e) in /_/src/Build/Logging/ParallelLogger/ParallelConsoleLogger.cs:line 1132
   at Microsoft.Build.BackEnd.Logging.EventSourceSink.RaiseMessageEvent(Object sender, BuildMessageEventArgs buildEvent) in /_/src/Build/BackEnd/Components/Logging/EventSourceSink.cs:line 323
   --- End of inner exception stack trace ---
   at Microsoft.Build.Exceptions.InternalLoggerException.Throw(Exception innerException, BuildEventArgs e, String messageResourceName, Boolean initializationException, String[] messageArgs) in /_/src/Build/Errors/InternalLoggerException.cs:line 253
   at Microsoft.Build.BackEnd.Logging.EventSourceSink.RaiseMessageEvent(Object sender, BuildMessageEventArgs buildEvent) in /_/src/Build/BackEnd/Components/Logging/EventSourceSink.cs:line 344
   at Microsoft.Build.BackEnd.Logging.EventSourceSink.Consume(BuildEventArgs buildEvent) in /_/src/Build/BackEnd/Components/Logging/EventSourceSink.cs:line 223
   at Microsoft.Build.BackEnd.Logging.EventSourceSink.Consume(BuildEventArgs buildEvent, Int32 sinkId) in /_/src/Build/BackEnd/Components/Logging/EventSourceSink.cs:line 211
   at Microsoft.Build.BackEnd.Logging.LoggingService.RouteBuildEvent(KeyValuePair`2 nodeEvent) in /_/src/Build/BackEnd/Components/Logging/LoggingService.cs:line 1630
   at Microsoft.Build.BackEnd.Logging.LoggingService.RouteBuildEvent(Object loggingEvent) in /_/src/Build/BackEnd/Components/Logging/LoggingService.cs:line 1614
   at Microsoft.Build.BackEnd.Logging.LoggingService.LoggingEventProcessor(Object loggingEvent) in /_/src/Build/BackEnd/Components/Logging/LoggingService.cs:line 1454

Given the error message, my guess was that the unexpected error could be thrown while the execution of the new tests. So, I removed the tests to see if Windows Full and Windows Core will fail again, and they failed again. When reverting the changes back now Windows Core passes and Windows Full not.

@rainersigwald , @ladipro what do you think might be the problem?

[ladipro]

I've left a few comments inline. Overall, I'm wondering if the complexity added with IHasProjectFullPath is really needed. If the paths persisted in binlog were relative or just file names, presumably viewing the binlog wouldn't be hampered. Worst case if somebody needed full paths, they could be trivially reconstructed at binlog read time.

[surayya-MS]

Thanks @ladipro! I investigated if it is possible to get the full path of the file relative to the current project inside of BinnaryLogger and it turns out it is. Removed the interface.

[JanKrivanek]

Overall looks good.

Before signing off I'd want to see a sanity test of NodePackets translations (just a simple addition to existing test) and possibly moving the serialization logic to the owning type (which will allow getting rid of the Serialization attribute)

[JanKrivanek]

Solid feature with solid tests - ship it! :-)