From a13354b2410320cb06228ef42c86e6a9995525ab Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 17 Jul 2025 16:15:00 +0200 Subject: [PATCH 1/3] Ensure all evaluation data are in binlogs from file-based apps --- .../Run/VirtualProjectBuildingCommand.cs | 25 ++++++++----------- .../CommandTests/Run/RunFileTests.cs | 4 +-- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs b/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs index bf80f0b8c9b6..732f553cc3ef 100644 --- a/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs +++ b/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs @@ -180,7 +180,6 @@ public override int Execute() } Dictionary savedEnvironmentVariables = []; - ProjectCollection? projectCollection = null; try { // Set environment variables. @@ -192,7 +191,7 @@ public override int Execute() // Set up MSBuild. ReadOnlySpan binaryLoggers = binaryLogger is null ? [] : [binaryLogger]; - projectCollection = new ProjectCollection( + var projectCollection = new ProjectCollection( MSBuildArgs.GlobalProperties, [.. binaryLoggers, consoleLogger], ToolsetDefinitionLocations.Default); @@ -202,19 +201,23 @@ public override int Execute() LogTaskInputs = binaryLoggers.Length != 0, }; + // Projects need to be created before BeginBuild is called, otherwise binlog doesn't contain evaluation data. + var restoreProject = NoRestore ? null : CreateProjectInstance(projectCollection, addGlobalProperties: AddRestoreGlobalProperties(MSBuildArgs.RestoreGlobalProperties)); + var buildProject = NoBuild ? null : CreateProjectInstance(projectCollection); + + BuildManager.DefaultBuildManager.BeginBuild(parameters); + // Do a restore first (equivalent to MSBuild's "implicit restore", i.e., `/restore`). // See https://github.com/dotnet/msbuild/blob/a1c2e7402ef0abe36bf493e395b04dd2cb1b3540/src/MSBuild/XMake.cs#L1838 // and https://github.com/dotnet/msbuild/issues/11519. - if (!NoRestore) + if (restoreProject != null) { var restoreRequest = new BuildRequestData( - CreateProjectInstance(projectCollection, addGlobalProperties: AddRestoreGlobalProperties(MSBuildArgs.RestoreGlobalProperties)), + restoreProject, targetsToBuild: ["Restore"], hostServices: null, BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports | BuildRequestDataFlags.FailOnUnresolvedSdk); - BuildManager.DefaultBuildManager.BeginBuild(parameters); - var restoreResult = BuildManager.DefaultBuildManager.BuildRequest(restoreRequest); if (restoreResult.OverallResult != BuildResultCode.Success) { @@ -223,18 +226,12 @@ public override int Execute() } // Then do a build. - if (!NoBuild) + if (buildProject != null) { var buildRequest = new BuildRequestData( - CreateProjectInstance(projectCollection), + buildProject, targetsToBuild: MSBuildArgs.RequestedTargets ?? ["Build"]); - // For some reason we need to BeginBuild after creating BuildRequestData otherwise the binlog doesn't contain Evaluation. - if (NoRestore) - { - BuildManager.DefaultBuildManager.BeginBuild(parameters); - } - var buildResult = BuildManager.DefaultBuildManager.BuildRequest(buildRequest); if (buildResult.OverallResult != BuildResultCode.Success) { diff --git a/test/dotnet.Tests/CommandTests/Run/RunFileTests.cs b/test/dotnet.Tests/CommandTests/Run/RunFileTests.cs index 98b62c75dd06..2738e9d7f5f6 100644 --- a/test/dotnet.Tests/CommandTests/Run/RunFileTests.cs +++ b/test/dotnet.Tests/CommandTests/Run/RunFileTests.cs @@ -939,8 +939,8 @@ public void BinaryLog_EvaluationData() new FileInfo(binaryLogPath).Should().Exist(); var records = BinaryLog.ReadRecords(binaryLogPath).ToList(); - records.Any(static r => r.Args is ProjectEvaluationStartedEventArgs).Should().BeTrue(); - records.Any(static r => r.Args is ProjectEvaluationFinishedEventArgs).Should().BeTrue(); + records.Count(static r => r.Args is ProjectEvaluationStartedEventArgs).Should().Be(2); + records.Count(static r => r.Args is ProjectEvaluationFinishedEventArgs).Should().Be(2); } /// From 7e2d2f763938220488d0e3980f6eddba9a774576 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Fri, 18 Jul 2025 11:21:35 +0200 Subject: [PATCH 2/3] Use FacadeLogger --- .../Run/VirtualProjectBuildingCommand.cs | 24 +++++++++---------- src/Cli/dotnet/LoggerUtility.cs | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs b/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs index 732f553cc3ef..b805a8441487 100644 --- a/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs +++ b/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs @@ -191,29 +191,26 @@ public override int Execute() // Set up MSBuild. ReadOnlySpan binaryLoggers = binaryLogger is null ? [] : [binaryLogger]; + IEnumerable loggers = [.. binaryLoggers, consoleLogger]; var projectCollection = new ProjectCollection( MSBuildArgs.GlobalProperties, - [.. binaryLoggers, consoleLogger], + loggers, ToolsetDefinitionLocations.Default); var parameters = new BuildParameters(projectCollection) { - Loggers = projectCollection.Loggers, + Loggers = loggers, LogTaskInputs = binaryLoggers.Length != 0, }; - // Projects need to be created before BeginBuild is called, otherwise binlog doesn't contain evaluation data. - var restoreProject = NoRestore ? null : CreateProjectInstance(projectCollection, addGlobalProperties: AddRestoreGlobalProperties(MSBuildArgs.RestoreGlobalProperties)); - var buildProject = NoBuild ? null : CreateProjectInstance(projectCollection); - BuildManager.DefaultBuildManager.BeginBuild(parameters); // Do a restore first (equivalent to MSBuild's "implicit restore", i.e., `/restore`). // See https://github.com/dotnet/msbuild/blob/a1c2e7402ef0abe36bf493e395b04dd2cb1b3540/src/MSBuild/XMake.cs#L1838 // and https://github.com/dotnet/msbuild/issues/11519. - if (restoreProject != null) + if (!NoRestore) { var restoreRequest = new BuildRequestData( - restoreProject, + CreateProjectInstance(projectCollection, addGlobalProperties: AddRestoreGlobalProperties(MSBuildArgs.RestoreGlobalProperties)), targetsToBuild: ["Restore"], hostServices: null, BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports | BuildRequestDataFlags.FailOnUnresolvedSdk); @@ -226,10 +223,10 @@ public override int Execute() } // Then do a build. - if (buildProject != null) + if (!NoBuild) { var buildRequest = new BuildRequestData( - buildProject, + CreateProjectInstance(projectCollection), targetsToBuild: MSBuildArgs.RequestedTargets ?? ["Build"]); var buildResult = BuildManager.DefaultBuildManager.BuildRequest(buildRequest); @@ -258,7 +255,7 @@ public override int Execute() Environment.SetEnvironmentVariable(key, value); } - binaryLogger?.Shutdown(); + binaryLogger?.ReallyShutdown(); consoleLogger.Shutdown(); } @@ -286,7 +283,7 @@ static Action> AddRestoreGlobalProperties(ReadOnlyDi }; } - static ILogger? GetBinaryLogger(IReadOnlyList? args) + static FacadeLogger? GetBinaryLogger(IReadOnlyList? args) { if (args is null) return null; // Like in MSBuild, only the last binary logger is used. @@ -295,12 +292,13 @@ static Action> AddRestoreGlobalProperties(ReadOnlyDi var arg = args[i]; if (LoggerUtility.IsBinLogArgument(arg)) { - return new BinaryLogger + var logger = new BinaryLogger { Parameters = arg.IndexOf(':') is >= 0 and var index ? arg[(index + 1)..] : "msbuild.binlog", }; + return LoggerUtility.CreateFacadeLogger([logger]); } } diff --git a/src/Cli/dotnet/LoggerUtility.cs b/src/Cli/dotnet/LoggerUtility.cs index abbcfdcb277a..0210c47c02f5 100644 --- a/src/Cli/dotnet/LoggerUtility.cs +++ b/src/Cli/dotnet/LoggerUtility.cs @@ -48,14 +48,14 @@ internal static class LoggerUtility // We need a custom logger to handle this, because the MSBuild API for evaluation and execution calls logger Initialize and Shutdown methods, so will not allow us to do this. if (binaryLoggers.Count > 0) { - var fakeLogger = ConfigureDispatcher(binaryLoggers); + var fakeLogger = CreateFacadeLogger(binaryLoggers); return fakeLogger; } return null; } - private static FacadeLogger ConfigureDispatcher(List binaryLoggers) + public static FacadeLogger CreateFacadeLogger(List binaryLoggers) { var dispatcher = new PersistentDispatcher(binaryLoggers); return new FacadeLogger(dispatcher); From 3412209d7505ec9c506a108a0dcb7d2376cc640c Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Fri, 18 Jul 2025 12:57:12 +0200 Subject: [PATCH 3/3] Avoid unnecessary binlog file creations --- .../Run/VirtualProjectBuildingCommand.cs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs b/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs index b805a8441487..0743f221495b 100644 --- a/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs +++ b/src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs @@ -190,7 +190,7 @@ public override int Execute() } // Set up MSBuild. - ReadOnlySpan binaryLoggers = binaryLogger is null ? [] : [binaryLogger]; + ReadOnlySpan binaryLoggers = binaryLogger is null ? [] : [binaryLogger.Value]; IEnumerable loggers = [.. binaryLoggers, consoleLogger]; var projectCollection = new ProjectCollection( MSBuildArgs.GlobalProperties, @@ -255,7 +255,7 @@ public override int Execute() Environment.SetEnvironmentVariable(key, value); } - binaryLogger?.ReallyShutdown(); + binaryLogger?.Value.ReallyShutdown(); consoleLogger.Shutdown(); } @@ -283,7 +283,7 @@ static Action> AddRestoreGlobalProperties(ReadOnlyDi }; } - static FacadeLogger? GetBinaryLogger(IReadOnlyList? args) + static Lazy? GetBinaryLogger(IReadOnlyList? args) { if (args is null) return null; // Like in MSBuild, only the last binary logger is used. @@ -292,13 +292,17 @@ static Action> AddRestoreGlobalProperties(ReadOnlyDi var arg = args[i]; if (LoggerUtility.IsBinLogArgument(arg)) { - var logger = new BinaryLogger + // We don't want to create the binlog file until actually needed, hence we wrap this in a Lazy. + return new(() => { - Parameters = arg.IndexOf(':') is >= 0 and var index - ? arg[(index + 1)..] - : "msbuild.binlog", - }; - return LoggerUtility.CreateFacadeLogger([logger]); + var logger = new BinaryLogger + { + Parameters = arg.IndexOf(':') is >= 0 and var index + ? arg[(index + 1)..] + : "msbuild.binlog", + }; + return LoggerUtility.CreateFacadeLogger([logger]); + }); } }