Ensure all evaluation data are in binlogs from file-based apps (#49841)

Author: jjonescz

src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs

@@ -201,7 +201,6 @@ public override int Execute()
         }
 
         Dictionary<string, string?> savedEnvironmentVariables = [];
-        ProjectCollection? projectCollection = null;
         try
         {
             // Set environment variables.
@@ -212,17 +211,20 @@ public override int Execute()
             }
 
             // Set up MSBuild.
-            ReadOnlySpan<ILogger> binaryLoggers = binaryLogger is null ? [] : [binaryLogger];
-            projectCollection = new ProjectCollection(
+            ReadOnlySpan<ILogger> binaryLoggers = binaryLogger is null ? [] : [binaryLogger.Value];
+            IEnumerable<ILogger> 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,
             };
 
+            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.
@@ -234,8 +236,6 @@ public override int Execute()
                     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)
                 {
@@ -250,12 +250,6 @@ public override int Execute()
                     CreateProjectInstance(projectCollection),
                     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)
                 {
@@ -282,7 +276,7 @@ public override int Execute()
                 Environment.SetEnvironmentVariable(key, value);
             }
 
-            binaryLogger?.Shutdown();
+            binaryLogger?.Value.ReallyShutdown();
             consoleLogger.Shutdown();
         }
 
@@ -310,7 +304,7 @@ static Action<IDictionary<string, string>> AddRestoreGlobalProperties(ReadOnlyDi
             };
         }
 
-        static ILogger? GetBinaryLogger(IReadOnlyList<string>? args)
+        static Lazy<FacadeLogger>? GetBinaryLogger(IReadOnlyList<string>? args)
         {
             if (args is null) return null;
             // Like in MSBuild, only the last binary logger is used.
@@ -319,12 +313,17 @@ static Action<IDictionary<string, string>> AddRestoreGlobalProperties(ReadOnlyDi
                 var arg = args[i];
                 if (LoggerUtility.IsBinLogArgument(arg))
                 {
-                    return 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",
-                    };
+                        var logger = new BinaryLogger
+                        {
+                            Parameters = arg.IndexOf(':') is >= 0 and var index
+                                ? arg[(index + 1)..]
+                                : "msbuild.binlog",
+                        };
+                        return LoggerUtility.CreateFacadeLogger([logger]);
+                    });
                 }
             }
 

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<BinaryLogger> binaryLoggers)
+    public static FacadeLogger CreateFacadeLogger(List<BinaryLogger> binaryLoggers)
     {
         var dispatcher = new PersistentDispatcher(binaryLoggers);
         return new FacadeLogger(dispatcher);

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);
     }
 
     /// <summary>