Skip to content

Simplify binary logger implementation by using BinaryLoggerParameters directly throughout codebase #50063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 5, 2025

This PR fully models the --binaryLogger (-bl) MSBuild option as a first-class ForwardedOption with complete type safety and comprehensive parameter parsing, eliminating custom handling across the CLI.

Problem

The binary logger option was inconsistently handled across CLI commands. While other MSBuild options like --property were properly modeled as ForwardedOption instances, the binary logger required custom parsing logic in dotnet watch and other commands, creating maintenance burden and inconsistency.

Solution

1. Created Comprehensive Typed Binary Logger Structure

  • BinaryLoggerParameters record with full MSBuild parameter support:
    • LogFile: File path with both explicit (LogFile=file.binlog) and shorthand (file.binlog) syntax
    • ProjectImports: Strongly-typed enum (None, Embed, ZipFile)
    • Verbosity: LoggerVerbosity enum support
    • Parameters: Generic dictionary for additional keyword=value pairs
  • Complete parsing of MSBuild's -bl[:[LogFile=]file.binlog[;ProjectImports=None,Embed,ZipFile][;keyword=value]*] syntax

2. Implemented Proper ForwardedOption Pattern

  • Uses SetForwardingFunction with ParseResult access to preserve exact token format
  • Eliminates reflection-based approach in favor of standard CLI patterns
  • Maintains case-sensitivity and token format (-bl, /bl, --binaryLogger)
  • Parses directly to BinaryLoggerParameters without intermediate wrapper types

3. Updated All Command Handlers and Utilities

  • RunCommand: Now uses typed BinaryLoggerParameters instead of LoggerUtility.DetermineBinlogger
  • MSBuildUtility: Updated both GetProjectsFromSolution and GetProjectsFromProject to use typed structures
  • dotnet-watch: Updated to use typed structures instead of custom string parsing
  • LoggerUtility: Added shared CreateBinaryLogger method for consistent handling
  • TestingPlatform: Added BinaryLoggerOption to ensure complete coverage
  • All MSBuild commands automatically include binary logger through standard forwarding

4. Enhanced BuildOptions Integration

  • Updated BuildOptions record to include BinaryLoggerParameters parameter
  • Eliminated dependency on LoggerUtility.SeparateBinLogArguments for typed commands
  • Consistent parameter extraction from ParseResult across all test scenarios

5. Preserved Backward Compatibility

The implementation maintains all existing behavior while adding comprehensive parameter support:

dotnet build -bl                                    # Creates msbuild.binlog
dotnet build /bl:test.binlog                       # Creates test.binlog with /bl token
dotnet run --binaryLogger:app.binlog               # Proper forwarding to build
dotnet test -bl:output.binlog;ProjectImports=None  # Full parameter support

Testing

  • Comprehensive parameter parsing verification for all MSBuild syntax variants
  • Manual verification confirms binary log files are created correctly with parameters
  • Token format preservation maintains existing MSBuild integration
  • dotnet watch and dotnet test now benefit from consistent option handling

This change addresses the architectural inconsistency by bringing binary logger options in line with other MSBuild forwarding patterns while adding full type safety and comprehensive parameter support that matches MSBuild's complete feature set.

Fixes #49989.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] Fully model the --binaryLogger MSBuild option in the CLI command parsers so that watch doesn't have to do custom handling Fully model the --binaryLogger MSBuild option in CLI command parsers Aug 5, 2025
@Copilot Copilot AI requested a review from baronfel August 5, 2025 21:15
Copilot finished work on behalf of baronfel August 5, 2025 21:15
@Copilot Copilot AI changed the title Fully model the --binaryLogger MSBuild option in CLI command parsers Fully model the --binaryLogger MSBuild option with typed parameters and proper ForwardedOption pattern Aug 5, 2025
@Copilot Copilot AI requested a review from baronfel August 5, 2025 23:16
Copilot finished work on behalf of baronfel August 5, 2025 23:16
.SetForwardingFunction(ForwardBinaryLoggerOptions)
.AllowSingleArgPerToken();

private static BinaryLoggerOptions? ParseBinaryLoggerOptions(ArgumentResult result)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot this is not quite what I was meaning - the output of this parsing function should be a full model of the inputs and outputs of the binarylog option in MSBuild. From the docs:

<html><body>
<!--StartFragment-->
-binaryLogger[:[LogFile=]{output.binlog}[;ProjectImports=None,Embed,ZipFile]]-bl[:[LogFile=]{output.binlog}[;ProjectImports=None,Embed,ZipFile]] | Serializes all build events to a compressed binary file. By default the file is in the current directory and named msbuild.binlog. The optional LogFile parameter accepts a string of the form {filename}.binlog, where {filename} contains valid file system characters, but also accepts a placeholder symbol {}. If {} is provided, MSBuild will fill that position with a 'unique stamp' of the form yyyyMMdd-HHmmss--<current process id>--<6-character random string>.  The binary log is a detailed description of the build process that can  later be used to reconstruct text logs and used by other analysis tools.  A binary log is usually 10-20x smaller than the most detailed text  diagnostic-level log, but it contains more information.The  binary logger by default collects the source text of project files,  including all imported projects and target files encountered during the  build. The optional ProjectImports parameter controls this behavior: -   ProjectImports=None. Don't collect the project imports. -   ProjectImports=Embed. Embed project imports in the log file (default). -   ProjectImports=ZipFile. Save project files to {output}.projectimports.zip where <output> is the same name as the binary log file name.The default setting for ProjectImports is Embed.Note: the logger doesn't collect non-MSBuild source files such as .cs, .cpp, and so on.A .binlog file can be "played back" by passing it to msbuild.exe  as an argument instead of a project/solution. Other loggers receive the  information contained in the log file as if the original build was  happening. You can read more about the binary log and its usages at MSBuild Binary Log overview. Examples: -   -bl -    -bl:output.binlog -   -bl:output.binlog;ProjectImports=None -   -bl:output.binlog;ProjectImports=ZipFile -   -bl:..\..\custom.binlog - -bl:publish-{}.binlog -   -binaryLogger
-- | --


<!--EndFragment-->
</body>
</html>-binaryLogger[:[LogFile=]{output.binlog}
[;ProjectImports=None,Embed,ZipFile]]

-bl[:[LogFile=]{output.binlog}
[;ProjectImports=None,Embed,ZipFile]] 	Serializes all build events to a compressed binary file. By default the file is in the current directory and named msbuild.binlog. The optional LogFile parameter accepts a string of the form {filename}.binlog, where {filename} contains valid file system characters, but also accepts a placeholder symbol {}. If {} is provided, MSBuild will fill that position with a 'unique stamp' of the form yyyyMMdd-HHmmss--<current process id>--<6-character random string>. The binary log is a detailed description of the build process that can later be used to reconstruct text logs and used by other analysis tools. A binary log is usually 10-20x smaller than the most detailed text diagnostic-level log, but it contains more information.

The binary logger by default collects the source text of project files, including all imported projects and target files encountered during the build. The optional ProjectImports parameter controls this behavior:

- ProjectImports=None. Don't collect the project imports.
- ProjectImports=Embed. Embed project imports in the log file (default).
- ProjectImports=ZipFile. Save project files to {output}.projectimports.zip where <output> is the same name as the binary log file name.

The default setting for ProjectImports is Embed.
Note: the logger doesn't collect non-MSBuild source files such as .cs, .cpp, and so on.
A .binlog file can be "played back" by passing it to msbuild.exe as an argument instead of a project/solution. Other loggers receive the information contained in the log file as if the original build was happening. You can read more about the binary log and its usages at [MSBuild Binary Log overview](https://github.com/dotnet/msbuild/blob/main/documentation/wiki/Binary-Log.md).

Examples:
- -bl
- -bl:output.binlog
- -bl:output.binlog;ProjectImports=None
- -bl:output.binlog;ProjectImports=ZipFile
- -bl:..\..\custom.binlog
- -bl:publish-{}.binlog
- -binaryLogger

We need to parse, model, support all of these.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enhanced the binary logger parsing to fully support all MSBuild binary logger parameters. The implementation now properly models:

  • LogFile parameter (both explicit LogFile=file.binlog and shorthand file.binlog forms)
  • ProjectImports parameter with enum values: None, Embed, ZipFile
  • Verbosity parameter with LoggerVerbosity enum values
  • Additional parameters through generic Parameters dictionary

The parser handles the complete MSBuild syntax: -bl[:[LogFile=]file.binlog[;ProjectImports=None,Embed,ZipFile][;keyword=value]*]

All examples from the docs work correctly:

  • -bl (flag form)
  • -bl:output.binlog
  • -bl:output.binlog;ProjectImports=None
  • -bl:LogFile=custom.binlog;ProjectImports=ZipFile

Updated in commit b1a8f42.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot you don't need both 'binarloggeroptions' and 'binaryloggerparameters' - parse directly to the binaryloggerparameters and use that type everywhere else in the codebase where you currently use binaryloggeroptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eliminated the BinaryLoggerOptions record and now parse directly to BinaryLoggerParameters throughout the codebase. Updated all usage sites including:

  • CommonOptions.cs: Parse directly to BinaryLoggerParameters
  • LoggerUtility.CreateBinaryLogger: Takes BinaryLoggerParameters parameter
  • Test commands: Updated BuildOptions record and all usages
  • RunCommand: Updated property type and parameter names
  • dotnet-watch: Updated CommandLineOptions to use BinaryLoggerParameters

The implementation maintains all functionality while using a single, consistent type throughout. Manual testing confirms binary logging works correctly with all parameter variants.

Fixed in commit 72326b3.

@Copilot Copilot AI changed the title Fully model the --binaryLogger MSBuild option with typed parameters and proper ForwardedOption pattern Fully model the --binaryLogger MSBuild option with comprehensive parameter support and proper ForwardedOption pattern Aug 6, 2025
@Copilot Copilot AI requested a review from baronfel August 6, 2025 15:06
Copilot finished work on behalf of baronfel August 6, 2025 15:06
@Copilot Copilot AI changed the title Fully model the --binaryLogger MSBuild option with comprehensive parameter support and proper ForwardedOption pattern Simplify binary logger implementation by using BinaryLoggerParameters directly throughout codebase Aug 6, 2025
Copilot finished work on behalf of baronfel August 6, 2025 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fully model the --binaryLogger MSBuild option in the CLI command parsers so that watch doesn't have to do custom handling
2 participants