Revert "[VMR] Codeflow 14f83a3-184c6c7"

This reverts commit 28a5753.

Author: jjonescz

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

@@ -63,6 +63,18 @@ internal sealed class VirtualProjectBuildingCommand : CommandBase
         "MSBuild.rsp",
     ];
 
+    /// <remarks>
+    /// Kept in sync with the default <c>dotnet new console</c> project file (enforced by <c>DotnetProjectAddTests.SameAsTemplate</c>).
+    /// </remarks>
+    private static readonly FrozenDictionary<string, string> s_defaultProperties = FrozenDictionary.Create<string, string>(StringComparer.OrdinalIgnoreCase,
+    [
+        new("OutputType", "Exe"),
+        new("TargetFramework", "net10.0"),
+        new("ImplicitUsings", "enable"),
+        new("Nullable", "enable"),
+        new("PublishAot", "true"),
+    ]);
+
     internal static readonly string TargetOverrides = """
           <!--
             Override targets which don't work with project files that are not present on disk.
@@ -201,7 +213,6 @@ public override int Execute()
         }
 
         Dictionary<string, string?> savedEnvironmentVariables = [];
-        ProjectCollection? projectCollection = null;
         try
         {
             // Set environment variables.
@@ -212,17 +223,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 +248,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 +262,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 +288,7 @@ public override int Execute()
                 Environment.SetEnvironmentVariable(key, value);
             }
 
-            binaryLogger?.Shutdown();
+            binaryLogger?.Value.ReallyShutdown();
             consoleLogger.Shutdown();
         }
 
@@ -310,7 +316,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 +325,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]);
+                    });
                 }
             }
 
@@ -720,34 +731,33 @@ public static void WriteProjectFile(
             writer.WriteLine();
         }
 
-        // Kept in sync with the default `dotnet new console` project file (enforced by `DotnetProjectAddTests.SameAsTemplate`).
-        writer.WriteLine($"""
-              <PropertyGroup>
-                <OutputType>Exe</OutputType>
-                <TargetFramework>net10.0</TargetFramework>
-                <ImplicitUsings>enable</ImplicitUsings>
-                <Nullable>enable</Nullable>
-                <PublishAot>true</PublishAot>
-              </PropertyGroup>
-            """);
-
-        if (isVirtualProject)
+        // Write default and custom properties.
         {
             writer.WriteLine("""
-
                   <PropertyGroup>
-                    <EnableDefaultCompileItems>false</EnableDefaultCompileItems>
-                  </PropertyGroup>
                 """);
-        }
 
-        if (propertyDirectives.Any())
-        {
-            writer.WriteLine("""
+            // First write the default properties except those specified by the user.
+            var customPropertyNames = propertyDirectives.Select(d => d.Name).ToHashSet(StringComparer.OrdinalIgnoreCase);
+            foreach (var (name, value) in s_defaultProperties)
+            {
+                if (!customPropertyNames.Contains(name))
+                {
+                    writer.WriteLine($"""
+                            <{name}>{EscapeValue(value)}</{name}>
+                        """);
+                }
+            }
 
-                  <PropertyGroup>
-                """);
+            // Write virtual-only properties.
+            if (isVirtualProject)
+            {
+                writer.WriteLine("""
+                        <EnableDefaultCompileItems>false</EnableDefaultCompileItems>
+                    """);
+            }
 
+            // Write custom properties.
             foreach (var property in propertyDirectives)
             {
                 writer.WriteLine($"""
@@ -757,24 +767,23 @@ public static void WriteProjectFile(
                 processedDirectives++;
             }
 
-            writer.WriteLine("  </PropertyGroup>");
-        }
+            // Write virtual-only properties which cannot be overridden.
+            if (isVirtualProject)
+            {
+                writer.WriteLine("""
+                        <Features>$(Features);FileBasedProgram</Features>
+                    """);
+            }
 
-        if (isVirtualProject)
-        {
-            // After `#:property` directives so they don't override this.
             writer.WriteLine("""
-
-                  <PropertyGroup>
-                    <Features>$(Features);FileBasedProgram</Features>
                   </PropertyGroup>
+
                 """);
         }
 
         if (packageDirectives.Any())
         {
             writer.WriteLine("""
-
                   <ItemGroup>
                 """);
 
@@ -796,13 +805,15 @@ public static void WriteProjectFile(
                 processedDirectives++;
             }
 
-            writer.WriteLine("  </ItemGroup>");
+            writer.WriteLine("""
+                  </ItemGroup>
+
+                """);
         }
 
         if (projectDirectives.Any())
         {
             writer.WriteLine("""
-
                   <ItemGroup>
                 """);
 
@@ -815,7 +826,10 @@ public static void WriteProjectFile(
                 processedDirectives++;
             }
 
-            writer.WriteLine("  </ItemGroup>");
+            writer.WriteLine("""
+                  </ItemGroup>
+
+                """);
         }
 
         Debug.Assert(processedDirectives + directives.OfType<CSharpDirective.Shebang>().Count() == directives.Length);
@@ -825,7 +839,6 @@ public static void WriteProjectFile(
             Debug.Assert(targetFilePath is not null);
 
             writer.WriteLine($"""
-
                   <ItemGroup>
                     <Compile Include="{EscapeValue(targetFilePath)}" />
                   </ItemGroup>
@@ -836,12 +849,12 @@ public static void WriteProjectFile(
             {
                 var targetDirectory = Path.GetDirectoryName(targetFilePath) ?? "";
                 writer.WriteLine($"""
-                  <ItemGroup>
-                    <RuntimeHostConfigurationOption Include="EntryPointFilePath" Value="{EscapeValue(targetFilePath)}" />
-                    <RuntimeHostConfigurationOption Include="EntryPointFileDirectoryPath" Value="{EscapeValue(targetDirectory)}" />
-                  </ItemGroup>
+                      <ItemGroup>
+                        <RuntimeHostConfigurationOption Include="EntryPointFilePath" Value="{EscapeValue(targetFilePath)}" />
+                        <RuntimeHostConfigurationOption Include="EntryPointFileDirectoryPath" Value="{EscapeValue(targetDirectory)}" />
+                      </ItemGroup>
 
-                """);
+                    """);
             }
 
             foreach (var sdk in sdkDirectives)
@@ -857,12 +870,14 @@ public static void WriteProjectFile(
                     """);
             }
 
-            writer.WriteLine();
-            writer.WriteLine(TargetOverrides);
+            writer.WriteLine($"""
+
+                {TargetOverrides}
+
+                """);
         }
 
         writer.WriteLine("""
-
             </Project>
             """);
 

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);