Expose LogTaskInputs to tasks #6803
Conversation
src/Framework/EngineServices.cs
Outdated
| /// <summary> | ||
| /// This version added the IsTaskInputLoggingEnabled property. | ||
| /// </summary> | ||
| public const int Version2 = 2; |
There was a problem hiding this comment.
Why add a new variable here instead of just incrementing Version? are you trying to make it extra tough to accidentally have to changes that update the version to the same thing?
There was a problem hiding this comment.
I use named constants to basically keep a change log and have something formal to attach the comments to so they make it into the public documentation. It's just a convenience thing, trying to make it nicer for callers to use the class.
| /// Returns <see langword="true"/> if the build is configured to log all task inputs. | ||
| /// </summary> | ||
| public bool IsTaskInputLoggingEnabled => | ||
| BuildEngine is IBuildEngine10 buildEngine10 && buildEngine10.EngineServices.IsTaskInputLoggingEnabled; |
There was a problem hiding this comment.
|| Traits.Instance.EscapeHatches.LogTaskInputs? Maybe || (Traits.Instance.EscapeHatches.LogTaskInputs && BuildEngine is not IBuildEngine 10)?
There was a problem hiding this comment.
I don’t think escape hatch is needed here anymore. It doesn’t work in other nodes anyway.
There was a problem hiding this comment.
This is basically saying that we're fine without the optimization in scenarios where EngineServices is not supported, which is primarily the .NET Framework 3.5 task host. I agree with Kirill that using the escape hatch here wouldn't work reliably. And it's unlikely that 3.5 tasks would want to take advantage of this new optimization, anyway.
Forgind
added
the
merge-when-branch-open
When binary logger is present, LogTaskInputs is set, so MSBuild automatically logs all inputs and outputs, there's no need to manually do so. Starting with MSBuild 17.5 taskLogging.IsTaskInputLoggingEnabled is available, but we're still referencing Microsoft.Build.Utilities.v4.0.dll for net472 so it's not available. Instead of using reflection, just read the environment. This results in significant savings to binlog size and potentially faster restore as well (and less memory allocations). Fixes NuGet/Home#10696 See related: * dotnet/msbuild#6305 * dotnet/msbuild#6803
When binary logger is present, LogTaskInputs is set, so MSBuild automatically logs all inputs and outputs, there's no need to manually do so. Starting with MSBuild 17.5 taskLogging.IsTaskInputLoggingEnabled is available, but we're still referencing Microsoft.Build.Utilities.v4.0.dll for net472 so it's not available. Instead of using reflection, just read the environment. This results in significant savings to binlog size and potentially faster restore as well (and less memory allocations). Fixes NuGet/Home#10696 See related: * dotnet/msbuild#6305 * dotnet/msbuild#6803
Fixes #6305
Context
The
LogTaskInputsflag passed to MSBuild inBuildParameterscan be used by tasks to optimize logging. When it's true, a task can omit its own duplicated logging of inputs.Changes Made
LogTaskInputsis plumbed through and exposed asIsTaskInputLoggingEnabledonEngineServicesand also onTaskLoggingHelperfor convenience. The RAR task is updated to use the new property.Testing
/v:diagand inspecting the log.