Skip to content

Commit 5a41589

Browse files
authored
[watch] Fix /bl option parsing, use it for build logging (#50002)
1 parent 83b21d6 commit 5a41589

19 files changed

+345
-67
lines changed

src/BuiltInTools/dotnet-watch/Build/BuildReporter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@
77

88
namespace Microsoft.DotNet.Watch;
99

10-
internal sealed class BuildReporter(IReporter reporter, EnvironmentOptions environmentOptions)
10+
internal sealed class BuildReporter(IReporter reporter, GlobalOptions options, EnvironmentOptions environmentOptions)
1111
{
1212
public IReporter Reporter => reporter;
1313
public EnvironmentOptions EnvironmentOptions => environmentOptions;
1414

1515
public Loggers GetLoggers(string projectPath, string operationName)
16-
=> new(reporter, environmentOptions.GetTestBinLogPath(projectPath, operationName));
16+
=> new(reporter, environmentOptions.GetBinLogPath(projectPath, operationName, options));
1717

1818
public void ReportWatchedFiles(Dictionary<string, FileItem> fileItems)
1919
{

src/BuiltInTools/dotnet-watch/Build/EvaluationResult.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,12 @@ public void WatchFiles(FileWatcher fileWatcher)
3838
string rootProjectPath,
3939
IEnumerable<string> buildArguments,
4040
IReporter reporter,
41+
GlobalOptions options,
4142
EnvironmentOptions environmentOptions,
4243
bool restore,
4344
CancellationToken cancellationToken)
4445
{
45-
var buildReporter = new BuildReporter(reporter, environmentOptions);
46+
var buildReporter = new BuildReporter(reporter, options, environmentOptions);
4647

4748
// See https://github.com/dotnet/project-system/blob/main/docs/well-known-project-properties.md
4849

src/BuiltInTools/dotnet-watch/CommandLine/CommandLineOptions.cs

Lines changed: 80 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
using System.CommandLine;
66
using System.CommandLine.Parsing;
77
using System.Data;
8+
using System.Diagnostics;
9+
using Microsoft.Build.Logging;
810
using Microsoft.DotNet.Cli;
911
using Microsoft.DotNet.Cli.Commands.Run;
1012
using Microsoft.DotNet.Cli.Extensions;
@@ -15,19 +17,28 @@ internal sealed class CommandLineOptions
1517
{
1618
public const string DefaultCommand = "run";
1719

20+
private static readonly ImmutableArray<string> s_binaryLogOptionNames = ["-bl", "/bl", "-binaryLogger", "--binaryLogger", "/binaryLogger"];
21+
1822
public bool List { get; init; }
19-
required public GlobalOptions GlobalOptions { get; init; }
23+
public required GlobalOptions GlobalOptions { get; init; }
2024

2125
public string? ProjectPath { get; init; }
2226
public string? TargetFramework { get; init; }
2327
public bool NoLaunchProfile { get; init; }
2428
public string? LaunchProfileName { get; init; }
2529

26-
public string? ExplicitCommand { get; init; }
27-
30+
/// <summary>
31+
/// Arguments passed to <see cref="Command"/>.
32+
/// </summary>
2833
public required IReadOnlyList<string> CommandArguments { get; init; }
34+
35+
/// <summary>
36+
/// Arguments passed to `dotnet build` and to design-time build evaluation.
37+
/// </summary>
2938
public required IReadOnlyList<string> BuildArguments { get; init; }
3039

40+
public string? ExplicitCommand { get; init; }
41+
3142
public string Command => ExplicitCommand ?? DefaultCommand;
3243

3344
// this option is referenced from inner logic and so needs to be reference-able
@@ -128,6 +139,7 @@ internal sealed class CommandLineOptions
128139
Output = output,
129140
Error = output
130141
});
142+
131143
if (!rootCommandInvoked)
132144
{
133145
// help displayed:
@@ -145,10 +157,16 @@ internal sealed class CommandLineOptions
145157
}
146158
}
147159

148-
var commandArguments = GetCommandArguments(parseResult, watchOptions, explicitCommand);
160+
var commandArguments = GetCommandArguments(parseResult, watchOptions, explicitCommand, out var binLogToken, out var binLogPath);
149161

150162
// We assume that forwarded options, if any, are intended for dotnet build.
151-
var buildArguments = buildOptions.Select(option => ((IForwardedOption)option).GetForwardingFunction()(parseResult)).SelectMany(args => args).ToArray();
163+
var buildArguments = buildOptions.Select(option => ((IForwardedOption)option).GetForwardingFunction()(parseResult)).SelectMany(args => args).ToList();
164+
165+
if (binLogToken != null)
166+
{
167+
buildArguments.Add(binLogToken);
168+
}
169+
152170
var targetFrameworkOption = (Option<string>?)buildOptions.SingleOrDefault(option => option.Name == "--framework");
153171

154172
return new()
@@ -160,6 +178,7 @@ internal sealed class CommandLineOptions
160178
NoHotReload = parseResult.GetValue(noHotReloadOption),
161179
NonInteractive = parseResult.GetValue(NonInteractiveOption),
162180
Verbose = parseResult.GetValue(verboseOption),
181+
BinaryLogPath = ParseBinaryLogFilePath(binLogPath),
163182
},
164183

165184
CommandArguments = commandArguments,
@@ -173,12 +192,36 @@ internal sealed class CommandLineOptions
173192
};
174193
}
175194

195+
/// <summary>
196+
/// Parses the value of msbuild option `-binaryLogger[:[LogFile=]output.binlog[;ProjectImports={None,Embed,ZipFile}]]`.
197+
/// Emulates https://github.com/dotnet/msbuild/blob/7f69ea906c29f2478cc05423484ad185de66e124/src/Build/Logging/BinaryLogger/BinaryLogger.cs#L481.
198+
/// See https://github.com/dotnet/msbuild/issues/12256
199+
/// </summary>
200+
internal static string? ParseBinaryLogFilePath(string? value)
201+
=> value switch
202+
{
203+
null => null,
204+
_ => (from parameter in value.Split(';', StringSplitOptions.RemoveEmptyEntries)
205+
where !string.Equals(parameter, "ProjectImports=None", StringComparison.OrdinalIgnoreCase) &&
206+
!string.Equals(parameter, "ProjectImports=Embed", StringComparison.OrdinalIgnoreCase) &&
207+
!string.Equals(parameter, "ProjectImports=ZipFile", StringComparison.OrdinalIgnoreCase) &&
208+
!string.Equals(parameter, "OmitInitialInfo", StringComparison.OrdinalIgnoreCase)
209+
let path = (parameter.StartsWith("LogFile=", StringComparison.OrdinalIgnoreCase) ? parameter["LogFile=".Length..] : parameter).Trim('"')
210+
let pathWithExtension = path.EndsWith(".binlog", StringComparison.OrdinalIgnoreCase) ? path : $"{path}.binlog"
211+
select pathWithExtension)
212+
.LastOrDefault("msbuild.binlog")
213+
};
214+
176215
private static IReadOnlyList<string> GetCommandArguments(
177216
ParseResult parseResult,
178217
IReadOnlyList<Option> watchOptions,
179-
Command? explicitCommand)
218+
Command? explicitCommand,
219+
out string? binLogToken,
220+
out string? binLogPath)
180221
{
181222
var arguments = new List<string>();
223+
binLogToken = null;
224+
binLogPath = null;
182225

183226
foreach (var child in parseResult.CommandResult.Children)
184227
{
@@ -199,23 +242,11 @@ private static IReadOnlyList<string> GetCommandArguments(
199242
continue;
200243
}
201244

202-
// Some options _may_ be computed or have defaults, so not all may have an IdentifierToken.
203-
// For those that do not, use the Option's Name instead.
204-
var optionNameToForward = optionResult.IdentifierToken?.Value ?? optionResult.Option.Name;
245+
var optionNameToForward = GetOptionNameToForward(optionResult);
205246
if (optionResult.Tokens.Count == 0 && !optionResult.Implicit)
206247
{
207248
arguments.Add(optionNameToForward);
208249
}
209-
else if (optionResult.Option.Name == "--property")
210-
{
211-
foreach (var token in optionResult.Tokens)
212-
{
213-
// While dotnet-build allows "/p Name=Value", dotnet-msbuild does not.
214-
// Any command that forwards args to dotnet-msbuild will fail if we don't use colon.
215-
// See https://github.com/dotnet/sdk/issues/44655.
216-
arguments.Add($"{optionNameToForward}:{token.Value}");
217-
}
218-
}
219250
else
220251
{
221252
foreach (var token in optionResult.Tokens)
@@ -227,8 +258,6 @@ private static IReadOnlyList<string> GetCommandArguments(
227258
}
228259
}
229260

230-
var tokens = parseResult.UnmatchedTokens.ToArray();
231-
232261
// Assuming that all tokens after "--" are unmatched:
233262
var dashDashIndex = IndexOf(parseResult.Tokens, t => t.Value == "--");
234263
var unmatchedTokensBeforeDashDash = parseResult.UnmatchedTokens.Count - (dashDashIndex >= 0 ? parseResult.Tokens.Count - dashDashIndex - 1 : 0);
@@ -240,10 +269,32 @@ private static IReadOnlyList<string> GetCommandArguments(
240269
{
241270
var token = parseResult.UnmatchedTokens[i];
242271

243-
if (i < unmatchedTokensBeforeDashDash && !seenCommand && token == explicitCommand?.Name)
272+
if (i < unmatchedTokensBeforeDashDash)
244273
{
245-
seenCommand = true;
246-
continue;
274+
if (!seenCommand && token == explicitCommand?.Name)
275+
{
276+
seenCommand = true;
277+
continue;
278+
}
279+
280+
// Workaround: commands do not have forwarding option for -bl
281+
// https://github.com/dotnet/sdk/issues/49989
282+
foreach (var name in s_binaryLogOptionNames)
283+
{
284+
if (token.StartsWith(name, StringComparison.OrdinalIgnoreCase))
285+
{
286+
if (token.Length == name.Length)
287+
{
288+
binLogToken = token;
289+
binLogPath = "";
290+
}
291+
else if (token.Length > name.Length + 1 && token[name.Length] == ':')
292+
{
293+
binLogToken = token;
294+
binLogPath = token[(name.Length + 1)..];
295+
}
296+
}
297+
}
247298
}
248299

249300
if (!dashDashInserted && i >= unmatchedTokensBeforeDashDash)
@@ -258,6 +309,11 @@ private static IReadOnlyList<string> GetCommandArguments(
258309
return arguments;
259310
}
260311

312+
private static string GetOptionNameToForward(OptionResult optionResult)
313+
// Some options _may_ be computed or have defaults, so not all may have an IdentifierToken.
314+
// For those that do not, use the Option's Name instead.
315+
=> optionResult.IdentifierToken?.Value ?? optionResult.Option.Name;
316+
261317
private static Command? TryGetSubcommand(ParseResult parseResult)
262318
{
263319
// Assuming that all tokens after "--" are unmatched:

src/BuiltInTools/dotnet-watch/CommandLine/EnvironmentOptions.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ internal sealed record EnvironmentOptions(
5252
TestOutput: EnvironmentVariables.TestOutputDir
5353
);
5454

55-
private static int s_uniqueLogId;
55+
private int _uniqueLogId;
5656

5757
public bool RunningAsTest { get => (TestFlags & TestFlags.RunningAsTest) != TestFlags.None; }
5858

@@ -64,9 +64,9 @@ private static string GetMuxerPathFromEnvironment()
6464
return muxerPath;
6565
}
6666

67-
public string? GetTestBinLogPath(string projectPath, string operationName)
68-
=> TestFlags.HasFlag(TestFlags.RunningAsTest)
69-
? Path.Combine(TestOutput, $"Watch.{operationName}.{Path.GetFileName(projectPath)}.{Interlocked.Increment(ref s_uniqueLogId)}.binlog")
70-
: null;
67+
public string? GetBinLogPath(string projectPath, string operationName, GlobalOptions options)
68+
=> options.BinaryLogPath != null
69+
? $"{Path.Combine(WorkingDirectory, options.BinaryLogPath)[..^".binlog".Length]}-dotnet-watch.{operationName}.{Path.GetFileName(projectPath)}.{Interlocked.Increment(ref _uniqueLogId)}.binlog"
70+
: null;
7171
}
7272
}

src/BuiltInTools/dotnet-watch/CommandLine/GlobalOptions.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,10 @@ internal sealed class GlobalOptions
99
public bool Verbose { get; init; }
1010
public bool NoHotReload { get; init; }
1111
public bool NonInteractive { get; init; }
12+
13+
/// <summary>
14+
/// Path to binlog file (absolute or relative to working directory, includes .binlog extension),
15+
/// or null to not generate binlog files.
16+
/// </summary>
17+
public string? BinaryLogPath { get; init; }
1218
}

src/BuiltInTools/dotnet-watch/HotReload/HotReloadDotNetWatcher.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public async Task WatchAsync(CancellationToken shutdownCancellationToken)
111111

112112
var projectMap = new ProjectNodeMap(evaluationResult.ProjectGraph, _context.Reporter);
113113
compilationHandler = new CompilationHandler(_context.Reporter, _context.ProcessRunner);
114-
var scopedCssFileHandler = new ScopedCssFileHandler(_context.Reporter, projectMap, browserConnector, _context.EnvironmentOptions);
114+
var scopedCssFileHandler = new ScopedCssFileHandler(_context.Reporter, projectMap, browserConnector, _context.Options, _context.EnvironmentOptions);
115115
var projectLauncher = new ProjectLauncher(_context, projectMap, browserConnector, compilationHandler, iteration);
116116
evaluationResult.ItemExclusions.Report(_context.Reporter);
117117

@@ -620,6 +620,11 @@ private bool AcceptChange(ChangedPath change)
620620
{
621621
var (path, kind) = change;
622622

623+
if (Path.GetExtension(path) == ".binlog")
624+
{
625+
return false;
626+
}
627+
623628
if (PathUtilities.GetContainingDirectories(path).FirstOrDefault(IsHiddenDirectory) is { } containingHiddenDir)
624629
{
625630
_context.Reporter.Report(MessageDescriptor.IgnoringChangeInHiddenDirectory, containingHiddenDir, kind, path);
@@ -765,6 +770,7 @@ private async ValueTask<EvaluationResult> EvaluateRootProjectAsync(bool restore,
765770
_context.RootProjectOptions.ProjectPath,
766771
_context.RootProjectOptions.BuildArguments,
767772
_context.Reporter,
773+
_context.Options,
768774
_context.EnvironmentOptions,
769775
restore,
770776
cancellationToken);
@@ -788,10 +794,6 @@ await FileWatcher.WaitForFileChangeAsync(
788794
{
789795
var buildOutput = new List<OutputLine>();
790796

791-
string[] binLogArguments = _context.EnvironmentOptions.GetTestBinLogPath(projectPath, "Build") is { } binLogPath
792-
? [$"-bl:{binLogPath}"]
793-
: [];
794-
795797
var processSpec = new ProcessSpec
796798
{
797799
Executable = _context.EnvironmentOptions.MuxerPath,
@@ -805,7 +807,7 @@ await FileWatcher.WaitForFileChangeAsync(
805807
}
806808
},
807809
// pass user-specified build arguments last to override defaults:
808-
Arguments = ["build", projectPath, "-consoleLoggerParameters:NoSummary;Verbosity=minimal", .. binLogArguments, .. buildArguments],
810+
Arguments = ["build", projectPath, "-consoleLoggerParameters:NoSummary;Verbosity=minimal", .. buildArguments]
809811
};
810812

811813
_context.Reporter.Output($"Building {projectPath} ...");

src/BuiltInTools/dotnet-watch/HotReload/ScopedCssFileHandler.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
namespace Microsoft.DotNet.Watch
88
{
9-
internal sealed class ScopedCssFileHandler(IReporter reporter, ProjectNodeMap projectMap, BrowserConnector browserConnector, EnvironmentOptions environmentOptions)
9+
internal sealed class ScopedCssFileHandler(IReporter reporter, ProjectNodeMap projectMap, BrowserConnector browserConnector, GlobalOptions options, EnvironmentOptions environmentOptions)
1010
{
1111
private const string BuildTargetName = TargetNames.GenerateComputedBuildStaticWebAssets;
1212

@@ -53,7 +53,7 @@ public async ValueTask HandleFileChangesAsync(IReadOnlyList<ChangedFile> files,
5353
return;
5454
}
5555

56-
var buildReporter = new BuildReporter(reporter, environmentOptions);
56+
var buildReporter = new BuildReporter(reporter, options, environmentOptions);
5757

5858
var buildTasks = projectsToRefresh.Select(projectNode => Task.Run(() =>
5959
{

src/BuiltInTools/dotnet-watch/Program.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ private async Task<int> ListFilesAsync(ProcessRunner processRunner, Cancellation
245245
rootProjectOptions.ProjectPath,
246246
rootProjectOptions.BuildArguments,
247247
processRunner,
248-
new BuildReporter(reporter, environmentOptions));
248+
new BuildReporter(reporter, options.GlobalOptions, environmentOptions));
249249

250250
if (await fileSetFactory.TryCreateAsync(requireProjectGraph: null, cancellationToken) is not { } evaluationResult)
251251
{

src/BuiltInTools/dotnet-watch/Properties/launchSettings.json

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,15 @@
22
"profiles": {
33
"dotnet-watch": {
44
"commandName": "Project",
5-
"commandLineArgs": "--verbose /bl:DotnetRun.binlog",
5+
"commandLineArgs": "--verbose -bl",
66
"workingDirectory": "E:\\sdk\\artifacts\\tmp\\Debug\\Aspire_BuildE---04F22750\\WatchAspire.AppHost",
77
"environmentVariables": {
88
"DOTNET_WATCH_DEBUG_SDK_DIRECTORY": "$(RepoRoot)artifacts\\bin\\redist\\$(Configuration)\\dotnet\\sdk\\$(Version)",
99
"DCP_IDE_REQUEST_TIMEOUT_SECONDS": "100000",
1010
"DCP_IDE_NOTIFICATION_TIMEOUT_SECONDS": "100000",
1111
"DCP_IDE_NOTIFICATION_KEEPALIVE_SECONDS": "100000",
1212
"DOTNET_WATCH_PROCESS_CLEANUP_TIMEOUT_MS": "100000",
13-
"ASPIRE_ALLOW_UNSECURED_TRANSPORT": "1",
14-
"__DOTNET_WATCH_TEST_FLAGS": "",
15-
"__DOTNET_WATCH_TEST_OUTPUT_DIR": "$(RepoRoot)\\artifacts\\tmp\\Debug"
13+
"ASPIRE_ALLOW_UNSECURED_TRANSPORT": "1"
1614
}
1715
}
1816
}

src/BuiltInTools/dotnet-watch/Watch/BuildEvaluator.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ protected virtual MSBuildFileSetFactory CreateMSBuildFileSetFactory()
3939
_context.RootProjectOptions.ProjectPath,
4040
_context.RootProjectOptions.BuildArguments,
4141
_context.ProcessRunner,
42-
new BuildReporter(_context.Reporter, _context.EnvironmentOptions));
42+
new BuildReporter(_context.Reporter, _context.Options, _context.EnvironmentOptions));
4343

4444
public IReadOnlyList<string> GetProcessArguments(int iteration)
4545
{

0 commit comments

Comments
 (0)