Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
232 changes: 232 additions & 0 deletions src/Build/Logging/BinaryLogger/BinaryLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.IO;
using System.IO.Compression;
using Microsoft.Build.Experimental.BuildCheck.Infrastructure.EditorConfig;
Expand Down Expand Up @@ -296,6 +297,17 @@ private static bool TryParsePathParameter(string parameter, out string filePath)

internal string FilePath { get; private set; }

/// <summary>
/// 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>
/// <remarks>
/// This property is intended for internal use by MSBuild command-line processing.
/// It should not be set by external code or logger implementations.
/// Use multiple logger instances with different Parameters instead.
/// </remarks>
public List<string> AdditionalFilePaths { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)

Copy link
Member

Choose a reason for hiding this comment

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

@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.

Copy link
Member

Choose a reason for hiding this comment

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

can we make it as a struct with path + additional params that we validate in BinaryLogger.ProcessParameters ?

Copy link
Member

Choose a reason for hiding this comment

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

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?


/// <summary> Gets or sets the verbosity level.</summary>
/// <remarks>
/// The binary logger Verbosity is always maximum (Diagnostic). It tries to capture as much
Expand Down Expand Up @@ -505,6 +517,15 @@ public void Shutdown()
}


// Log additional file paths before closing stream (so they're recorded in the binlog)
if (AdditionalFilePaths != null && AdditionalFilePaths.Count > 0 && stream != null)
{
foreach (var additionalPath in AdditionalFilePaths)
{
LogMessage("BinLogCopyDestination=" + additionalPath);
}
}

if (stream != null)
{
// It's hard to determine whether we're at the end of decoding GZipStream
Expand All @@ -514,6 +535,37 @@ public void Shutdown()
stream.Dispose();
stream = null;
}

// Copy the binlog file to additional destinations if specified
if (AdditionalFilePaths != null && AdditionalFilePaths.Count > 0)
{
foreach (var additionalPath in AdditionalFilePaths)
{
try
{
string directory = Path.GetDirectoryName(additionalPath);
if (!string.IsNullOrEmpty(directory))
{
Directory.CreateDirectory(directory);
}
File.Copy(FilePath, additionalPath, overwrite: true);
}
catch (Exception ex)
{
// Log the error but don't fail the build
// Note: We can't use LogMessage here since the stream is already closed
string message = ResourceUtilities.FormatResourceStringStripCodeAndKeyword(
out string errorCode,
out string helpKeyword,
"ErrorCopyingBinaryLog",
FilePath,
additionalPath,
ex.Message);

throw new LoggerException(message, ex, errorCode, helpKeyword);
Copy link
Member

Choose a reason for hiding this comment

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

@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?

Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

The comment states "Log the error but don't fail the build" but the code immediately throws a LoggerException. This contradicts the comment and will actually fail the build.

If the intention is to fail the build on copy errors (which seems appropriate), update the comment to reflect this. If copy failures should not fail the build, remove the throw statement and consider logging the error in a different way (e.g., writing to stderr or a separate error collection).

Suggested change
throw new LoggerException(message, ex, errorCode, helpKeyword);
Console.Error.WriteLine(message);

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

Does a single logger failing break the build? How does the loggingservice handle this?

}
}
}
}

private void RawEvents_LogDataSliceReceived(BinaryLogRecordKind recordKind, Stream stream)
Expand Down Expand Up @@ -668,5 +720,185 @@ private string GetUniqueStamp()

private static string ExpandPathParameter(string parameters)
=> $"{DateTime.UtcNow.ToString("yyyyMMdd-HHmmss")}--{EnvironmentUtilities.CurrentProcessId}--{StringUtils.GenerateRandomString(6)}";

/// <summary>
/// Extracts the file path from binary logger parameters string.
/// This is a helper method for processing multiple binlog parameters.
/// </summary>
/// <param name="parameters">The parameters string (e.g., "output.binlog" or "output.binlog;ProjectImports=None")</param>
/// <returns>The resolved file path, or "msbuild.binlog" if no path is specified</returns>
public static string ExtractFilePathFromParameters(string parameters)
{
if (string.IsNullOrEmpty(parameters))
{
return Path.GetFullPath("msbuild.binlog");
}

var paramParts = parameters.Split(MSBuildConstants.SemicolonChar, StringSplitOptions.RemoveEmptyEntries);
string filePath = null;

foreach (var parameter in paramParts)
{
if (TryInterpretPathParameterStatic(parameter, out string extractedPath))
{
filePath = extractedPath;
break;
}
}
Comment on lines +718 to +725
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot uses AI. Check for mistakes.

if (filePath == null)
{
filePath = "msbuild.binlog";
}

try
{
return Path.GetFullPath(filePath);
}
catch
{
// If path resolution fails, return the original path
return filePath;
}
}

private static bool TryInterpretPathParameterStatic(string parameter, out string filePath)
{
bool hasPathPrefix = parameter.StartsWith("LogFile=", StringComparison.OrdinalIgnoreCase);

if (hasPathPrefix)
{
parameter = parameter.Substring("LogFile=".Length);
}

parameter = parameter.Trim('"');

bool isWildcard = ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12) && parameter.Contains("{}");
bool hasProperExtension = parameter.EndsWith(".binlog", StringComparison.OrdinalIgnoreCase);
filePath = parameter;

if (!isWildcard)
{
return hasProperExtension;
}

filePath = parameter.Replace("{}", ExpandPathParameter(string.Empty), StringComparison.Ordinal);

if (!hasProperExtension)
{
filePath += ".binlog";
}
return true;
}

/// <summary>
/// Extracts the non-file-path parameters from binary logger parameters string.
/// This is used to compare configurations between multiple binlog parameters.
/// </summary>
/// <param name="parameters">The parameters string (e.g., "output.binlog;ProjectImports=None")</param>
/// <returns>A normalized string of non-path parameters, or empty string if only path parameters</returns>
public static string ExtractNonPathParameters(string parameters)
{
if (string.IsNullOrEmpty(parameters))
{
return string.Empty;
}

var paramParts = parameters.Split(MSBuildConstants.SemicolonChar, StringSplitOptions.RemoveEmptyEntries);
var nonPathParams = new List<string>();

foreach (var parameter in paramParts)
{
// Skip file path parameters
if (TryInterpretPathParameterStatic(parameter, out _))
{
continue;
}

// This is a configuration parameter (like ProjectImports=None, OmitInitialInfo, etc.)
nonPathParams.Add(parameter);
}

// Sort for consistent comparison
nonPathParams.Sort(StringComparer.OrdinalIgnoreCase);
return string.Join(";", nonPathParams);
}

/// <summary>
/// Result of processing multiple binary logger parameter sets.
/// </summary>
public class ProcessedBinaryLoggerParameters
{
/// <summary>
/// List of distinct parameter sets that need separate logger instances.
/// </summary>
public List<string> DistinctParameterSets { get; set; } = new List<string>();

/// <summary>
/// If true, all parameter sets have identical configurations (only file paths differ),
/// so a single logger can be used with file copying for additional paths.
/// </summary>
public bool AllConfigurationsIdentical { get; set; } = true;

/// <summary>
/// Additional file paths to copy the binlog to (only valid when AllConfigurationsIdentical is true).
/// </summary>
public List<string> AdditionalFilePaths { get; set; } = new List<string>();
}

/// <summary>
/// Processes multiple binary logger parameter sets and returns distinct paths and configuration info.
/// </summary>
/// <param name="binaryLoggerParameters">Array of parameter strings from command line</param>
/// <returns>Processed result with distinct parameter sets and configuration info</returns>
public static ProcessedBinaryLoggerParameters ProcessParameters(string[] binaryLoggerParameters)
{
var result = new ProcessedBinaryLoggerParameters();

if (binaryLoggerParameters == null || binaryLoggerParameters.Length == 0)
{
return result;
}

if (binaryLoggerParameters.Length == 1)
{
result.DistinctParameterSets.Add(binaryLoggerParameters[0]);
return result;
}

string primaryArguments = binaryLoggerParameters[0];
string primaryNonPathParams = ExtractNonPathParameters(primaryArguments);

var distinctFilePaths = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

// Check if all parameter sets have the same non-path configuration
for (int i = 0; i < binaryLoggerParameters.Length; i++)
{
string currentParams = binaryLoggerParameters[i];
string currentNonPathParams = ExtractNonPathParameters(currentParams);
string currentFilePath = ExtractFilePathFromParameters(currentParams);

// Check if this is a duplicate file path
if (distinctFilePaths.Add(currentFilePath))
{
if (!string.Equals(primaryNonPathParams, currentNonPathParams, StringComparison.OrdinalIgnoreCase))
{
result.AllConfigurationsIdentical = false;
}
result.DistinctParameterSets.Add(currentParams);
}
}

// If all configurations are identical, compute additional file paths for copying
if (result.AllConfigurationsIdentical && result.DistinctParameterSets.Count > 1)
{
for (int i = 1; i < result.DistinctParameterSets.Count; i++)
{
result.AdditionalFilePaths.Add(ExtractFilePathFromParameters(result.DistinctParameterSets[i]));
}
}

return result;
}
}
}
6 changes: 5 additions & 1 deletion src/Build/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2449,6 +2449,10 @@ Utilization: {0} Average Utilization: {1:###.0}</value>
<data name="SDKPathCheck_Failed" xml:space="preserve">
<value>The directory does not exist: {0}. .NET Runtime Task Host could not be instantiated. See https://aka.ms/nettaskhost for details on how to resolve this error.</value>
</data>
<data name="ErrorCopyingBinaryLog" xml:space="preserve">
<value>MSB4279: Failed to copy binary log from "{0}" to "{1}". {2}</value>
<comment>{StrBegin="MSB4279: "}UE: This is shown when the Binary Logger fails to copy the log file to one of the specified output locations.</comment>
</data>
<data name="AssemblyLoad_Warning" xml:space="preserve">
<value>The custom task '{0}' required a fallback to out-of-process execution because the UsingTask definition does not specify the correct Runtime and Architecture. This reduces build performance. Update the UsingTask element to explicitly specify Runtime and Architecture attributes (e.g., Runtime="CLR4" Architecture="x64") or use TaskFactory="TaskHostFactory".</value>
</data>
Expand All @@ -2459,7 +2463,7 @@ Utilization: {0} Average Utilization: {1:###.0}</value>
<!--
The Build message bucket is: MSB4000 - MSB4999

Next message code should be MSB4279
Next message code should be MSB4280

Don't forget to update this comment after using a new code.
-->
Expand Down
5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading