Support multiple binary logs from command line arguments #12706
Conversation
- Added AdditionalFilePaths property to BinaryLogger to track multiple output destinations - Modified ProcessBinaryLogger to collect all distinct binlog paths from multiple -bl flags - Updated BinaryLogger.Shutdown to copy the binlog to all additional paths - Added ExtractFilePathFromParameters helper method to parse file paths from parameters - Added error message resource for copy failures Co-authored-by: baronfel <[email protected]>
- Added MultipleBinaryLogsCreatesMultipleFiles test to verify all specified binlog files are created - Added MultipleBinaryLogsWithDuplicatesCreateDistinctFiles test to verify duplicates are handled correctly - Both tests verify files are identical Co-authored-by: baronfel <[email protected]>
- Enhanced ProcessBinaryLogger to detect configuration differences - If all binlog options only differ in file paths, use optimized copy approach - If configurations differ (e.g., ProjectImports settings), create separate logger instances - Added ExtractNonPathParameters method to compare configurations - Added test for different configurations creating separate loggers - This approach respects all configuration options while optimizing when possible Co-authored-by: baronfel <[email protected]>
| /// Gets or sets additional output file paths. When set, the binlog will be copied to all these paths | ||
| /// after the build completes. The primary FilePath will be used as the temporary write location. | ||
| /// </summary> | ||
| public List<string> AdditionalFilePaths { get; set; } |
There was a problem hiding this comment.
@rainersigwald is this the binlog API that end-users end up using? If so we probably want to think about this exposed shape a bit more and force this stuff into 'normal 'logger parameter parsing instead.
There was a problem hiding this comment.
Added documentation clarifying this is for internal MSBuild use only. External code should create multiple logger instances instead. The property needs to remain public since it's accessed from the MSBuild assembly, but the remarks make it clear this isn't intended for general API use. (commit 0403ab7)
There was a problem hiding this comment.
@rainersigwald / @YuliiaKovalova this is the API-design open question that I have about this whole effort - we should figure out what the desired way for API-based consumers to set multiple paths would be.
There was a problem hiding this comment.
can we make it as a struct with path + additional params that we validate in BinaryLogger.ProcessParameters ?
There was a problem hiding this comment.
That's part of the 'design' aspect of my question - if it's something that shouldn't be modified after Initialize/ProcessParameters then maybe init or 'required' is the appropriate access modifier level here?
|
@copilot it's possible for the engine itself to create a binlog (see AppendDebuggingLoggers for where this can happen) - as part of this PR we should take this into account when we compute the optimal number of BinaryLogger class instances to create - if there is already a 'normal' binlog requested for the build when MSBUILDDEBUGENGINE is set, we should just be able to add the engine-generated binlog path to the set of paths to copy the final 'normal' binlog to. |
- Added remarks to AdditionalFilePaths clarifying it's for MSBuild command-line processing - Documented that external code should use multiple logger instances instead - Addressed feedback that this shouldn't be part of the public logger API surface Co-authored-by: baronfel <[email protected]>
The current implementation handles MSBUILDDEBUGENGINE correctly - when enabled, Optimizing this to add the debug path to an existing binlog's copy list would require:
Since MSBUILDDEBUGENGINE is a special debugging feature (not commonly used in production) and the current behavior (two independent loggers) is functionally correct, I've kept the implementation simple. If this optimization becomes important, we can revisit it as a follow-up enhancement. |
- Use LogMessage to log copy destinations before stream close - Throw LoggerException for copy errors (can't use LogMessage after stream close) - Move distinct paths processing logic to BinaryLogger.ProcessParameters() - Add ProcessedBinaryLoggerParameters class to encapsulate results - Simplify XMake.ProcessBinaryLogger() to use the new method Co-authored-by: YuliiaKovalova <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| additionalPath, | ||
| ex.Message); | ||
|
|
||
| throw new LoggerException(message, ex, errorCode, helpKeyword); |
There was a problem hiding this comment.
@rainersigwald I am not sure how to handle it better - I some places I see Console.WriteLine usage, but it's not clear when we can use it?
- Replace hardcoded ".binlog" with BinlogFileExtension constant - Replace hardcoded "LogFile=" with LogFileParameterPrefix constant - Add XML documentation to TryInterpretPathParameterStatic method - Convert ProcessedBinaryLoggerParameters from class to readonly struct - Change AdditionalFilePaths and DistinctParameterSets to IReadOnlyList<string> Co-authored-by: YuliiaKovalova <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
When users specify the same binary log path multiple times, they will now see a message indicating which duplicate paths were ignored. This helps users identify potential mistakes in their command-line arguments. Co-authored-by: YuliiaKovalova <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This pull request implements support for multiple binary log files when specified via command-line arguments (e.g., -bl:1.binlog -bl:2.binlog), addressing the issue where previously only the last-specified binlog was created. The implementation intelligently handles two scenarios: when all binlog configurations are identical (optimized with a single logger and file copying), and when configurations differ (creating separate logger instances).
Key Changes
- Smart parameter processing: New
BinaryLogger.ProcessParameters()method analyzes multiple-blflags, deduplicates paths, and determines whether configurations are identical - Optimized file copying: When configurations match, uses a single logger with post-build file copying to additional destinations for consistency and performance
- Duplicate path handling: Automatically filters duplicate paths and informs users via console output
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Build/Logging/BinaryLogger/BinaryLogger.cs | Core implementation: new ProcessParameters static method, AdditionalFilePaths property, file extraction/comparison methods, ProcessedBinaryLoggerParameters struct, and Shutdown method updates for file copying |
| src/MSBuild/XMake.cs | Command-line processing updates to call BinaryLogger.ProcessParameters() and create appropriate logger instances based on configuration analysis |
| src/MSBuild.UnitTests/XMake_Tests.cs | Three new test methods covering multiple binlogs with identical configs, duplicate path handling, and different configurations |
| src/Build/Resources/Strings.resx | New error message resource ErrorCopyingBinaryLog (MSB4279) for file copy failures |
| src/MSBuild/Resources/Strings.resx | New informational message DuplicateBinaryLoggerPathsIgnored for duplicate path warnings |
| src/Build/Resources/xlf/*.xlf | Localization files updated with new ErrorCopyingBinaryLog resource (marked as "new" state for translation) |
| src/MSBuild/Resources/xlf/*.xlf | Localization files updated with new DuplicateBinaryLoggerPathsIgnored resource (marked as "new" state for translation) |
| [Fact] | ||
| public void MultipleBinaryLogsCreatesMultipleFiles() | ||
| { | ||
| var testProject = _env.CreateFile("TestProject.proj", @" | ||
| <Project> | ||
| <Target Name=""Build""> | ||
| <Message Text=""Hello World!"" /> | ||
| </Target> | ||
| </Project> | ||
| "); | ||
|
|
||
| string binLogLocation = _env.DefaultTestDirectory.Path; | ||
| string binLog1 = Path.Combine(binLogLocation, "1.binlog"); | ||
| string binLog2 = Path.Combine(binLogLocation, "2.binlog"); | ||
| string binLog3 = Path.Combine(binLogLocation, "3.binlog"); | ||
|
|
||
| string output = RunnerUtilities.ExecMSBuild($"\"{testProject.Path}\" \"/bl:{binLog1}\" \"/bl:{binLog2}\" \"/bl:{binLog3}\"", out var success, _output); | ||
|
|
||
| success.ShouldBeTrue(output); | ||
|
|
||
| // Verify all three binlog files exist | ||
| File.Exists(binLog1).ShouldBeTrue("First binlog file should exist"); | ||
| File.Exists(binLog2).ShouldBeTrue("Second binlog file should exist"); | ||
| File.Exists(binLog3).ShouldBeTrue("Third binlog file should exist"); | ||
|
|
||
| // Verify all files have content (are not empty) | ||
| new FileInfo(binLog1).Length.ShouldBeGreaterThan(0, "First binlog should not be empty"); | ||
| new FileInfo(binLog2).Length.ShouldBeGreaterThan(0, "Second binlog should not be empty"); | ||
| new FileInfo(binLog3).Length.ShouldBeGreaterThan(0, "Third binlog should not be empty"); | ||
|
|
||
| // Verify all files are identical (have the same content) | ||
| byte[] file1Bytes = File.ReadAllBytes(binLog1); | ||
| byte[] file2Bytes = File.ReadAllBytes(binLog2); | ||
| byte[] file3Bytes = File.ReadAllBytes(binLog3); | ||
|
|
||
| file1Bytes.SequenceEqual(file2Bytes).ShouldBeTrue("First and second binlog should be identical"); | ||
| file1Bytes.SequenceEqual(file3Bytes).ShouldBeTrue("First and third binlog should be identical"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void MultipleBinaryLogsWithDuplicatesCreateDistinctFiles() | ||
| { | ||
| var testProject = _env.CreateFile("TestProject.proj", @" | ||
| <Project> | ||
| <Target Name=""Build""> | ||
| <Message Text=""Hello World!"" /> | ||
| </Target> | ||
| </Project> | ||
| "); | ||
|
|
||
| string binLogLocation = _env.DefaultTestDirectory.Path; | ||
| string binLog1 = Path.Combine(binLogLocation, "1.binlog"); | ||
| string binLog2 = Path.Combine(binLogLocation, "2.binlog"); | ||
|
|
||
| // Specify binLog1 twice - should only create two distinct files | ||
| string output = RunnerUtilities.ExecMSBuild($"\"{testProject.Path}\" \"/bl:{binLog1}\" \"/bl:{binLog2}\" \"/bl:{binLog1}\"", out var success, _output); | ||
|
|
||
| success.ShouldBeTrue(output); | ||
|
|
||
| // Verify both binlog files exist | ||
| File.Exists(binLog1).ShouldBeTrue("First binlog file should exist"); | ||
| File.Exists(binLog2).ShouldBeTrue("Second binlog file should exist"); | ||
|
|
||
| // Verify both files are identical | ||
| byte[] file1Bytes = File.ReadAllBytes(binLog1); | ||
| byte[] file2Bytes = File.ReadAllBytes(binLog2); | ||
|
|
||
| file1Bytes.SequenceEqual(file2Bytes).ShouldBeTrue("Binlog files should be identical"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void MultipleBinaryLogsWithDifferentConfigurationsCreatesSeparateLoggers() | ||
| { | ||
| var testProject = _env.CreateFile("TestProject.proj", @" | ||
| <Project> | ||
| <Import Project=""Imported.proj"" /> | ||
| <Target Name=""Build""> | ||
| <Message Text=""Hello World!"" /> | ||
| </Target> | ||
| </Project> | ||
| "); | ||
|
|
||
| _env.CreateFile("Imported.proj", @" | ||
| <Project> | ||
| <PropertyGroup> | ||
| <TestProp>Value</TestProp> | ||
| </PropertyGroup> | ||
| </Project> | ||
| "); | ||
|
|
||
| string binLogLocation = _env.DefaultTestDirectory.Path; | ||
| string binLog1 = Path.Combine(binLogLocation, "with-imports.binlog"); | ||
| string binLog2 = Path.Combine(binLogLocation, "no-imports.binlog"); | ||
|
|
||
| // One with default imports, one with ProjectImports=None | ||
| string output = RunnerUtilities.ExecMSBuild($"\"{testProject.Path}\" \"/bl:{binLog1}\" \"/bl:{binLog2};ProjectImports=None\"", out var success, _output); | ||
|
|
||
| success.ShouldBeTrue(output); | ||
|
|
||
| // Verify both binlog files exist | ||
| File.Exists(binLog1).ShouldBeTrue("First binlog file should exist"); | ||
| File.Exists(binLog2).ShouldBeTrue("Second binlog file should exist"); | ||
|
|
||
| // Verify files are different sizes (one has imports embedded, one doesn't) | ||
| long size1 = new FileInfo(binLog1).Length; | ||
| long size2 = new FileInfo(binLog2).Length; | ||
|
|
||
| size1.ShouldBeGreaterThan(size2, "Binlog with imports should be larger than one without"); | ||
| } |
There was a problem hiding this comment.
Missing test coverage for the file copy error scenario. The Shutdown method can throw a LoggerException when file copying fails (lines 539-568), but none of the tests verify this error handling behavior.
Consider adding a test that:
- Sets up a scenario where copying would fail (e.g., invalid destination path, insufficient permissions, or locked file)
- Verifies that an appropriate exception is thrown with the correct error message
- Ensures the primary binlog file is still created successfully
Added dedicated unit tests for the new public static methods: - ExtractFilePathFromParameters: Tests for null/empty input, various parameter formats, LogFile= prefix, and full path resolution - ExtractNonPathParameters: Tests for extracting and sorting configuration parameters - ProcessParameters: Tests for null/empty arrays, single parameter, multiple identical configs with optimization, different configs, duplicate path filtering (case-insensitive), and mixed scenarios with duplicates Co-authored-by: YuliiaKovalova <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
- Extract common path interpretation logic into TryInterpretPathParameterCore to eliminate duplication between TryInterpretPathParameter and TryInterpretPathParameterStatic - Store extracted file paths during first loop in ProcessParameters to avoid redundant ExtractFilePathFromParameters calls when computing additional paths - Both instance and static methods now call the shared core method with appropriate wildcard expander functions Co-authored-by: YuliiaKovalova <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| additionalPath, | ||
| ex.Message); | ||
|
|
||
| throw new LoggerException(message, ex, errorCode, helpKeyword); |
There was a problem hiding this comment.
Does a single logger failing break the build? How does the loggingservice handle this?
|
sorry I didn't read it, but quick question: should we just have a single logger and then when the file is written at the end, just copy it to the other specified paths? |
|
@KirillOsenkov yes, that's the implementation here. The only caveat is that there are LoggerParameters that change the data included in the binlog in fun and interesting ways, so we group the file paths by the set of parameters and create 1 logger for each unique set of parameters, and each of those loggers does the write-then-copy for the set of files each one owns. |
Fixes #12705
Context
Today it's possible to provide the
-blflag multiple times, but the behavior of the engine is that the last-provided flag wins. This is confusing, and in extreme cases can mean that tools that need binlogs and provide them via things like response files can be overridden by a user's invocation. Tools like CodeQL have a harder time adopting binlog usage for C# code analysis because they can't control where the binlog is generated.Changes Made
Implemented support for writing multiple binlogs when supplied via command line arguments. The implementation intelligently handles two scenarios:
Same Configuration (Optimized): When all
-blflags have the same configuration (only file paths differ), a single BinaryLogger writes to one location and copies to additional destinations at build completion for consistency and performance.Different Configurations: When
-blflags have different configurations (e.g.,ProjectImports=NonevsProjectImports=Embed), separate BinaryLogger instances are created to respect each configuration.Key changes:
AdditionalFilePathsproperty to BinaryLogger (documented as internal-use only)BinaryLogger.ProcessParameters()static method to process multiple parameter sets and return distinct paths with configuration infoProcessedBinaryLoggerParametersreadonly struct to encapsulate the processing resultsShutdown()method to copy binlog to additional paths after stream closeLogMessageto log copy destinations before stream closeLoggerExceptionfor copy errors (stream is closed, can't use LogMessage)XMake.ProcessBinaryLogger()to use the new BinaryLogger.ProcessParameters() methodDuplicateFilePathsproperty to track and report duplicate pathsTryInterpretPathParametermethods to eliminate code duplication via shared core methodProcessParametersto avoid redundant path extraction callsTesting
ExtractFilePathFromParameters,ExtractNonPathParameters, andProcessParametersNotes
AdditionalFilePathsproperty is documented as internal-use only - external code should create multiple logger instances instead-blflags are automatically deduplicated - only the first occurrence is keptOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.