Make file-based caching opt out work for the basic up-to-date check too (#50927)

Author: jjonescz

documentation/general/dotnet-run-file.md

@@ -289,16 +289,16 @@ The build is performed using MSBuild APIs on in-memory project files.
 If an up-to-date check detects that inputs didn't change in subsequent `dotnet run file.cs` invocations,
 building is skipped (as if `--no-build` option has been passed).
 The up-to-date check is not 100% precise (e.g., files imported through an implicit build file are not considered).
-It is possible to enforce a full build using `--no-cache` flag or `dotnet build file.cs`.
+It is possible to enforce a full build using `--no-cache` flag or `dotnet build file.cs`
+(for a more permanent opt-out, there is MSBuild property `FileBasedProgramCanSkipMSBuild=false`).
 Environment variable [`DOTNET_CLI_CONTEXT_VERBOSE=true`][verbose-env] can be used to get more details about caching decisions made by `dotnet run file.cs`.
 
 There are multiple optimization levels - skipping build altogether, running just the C# compiler, or running full MSBuild.
 We always need to re-run MSBuild if implicit build files like `Directory.Build.props` change but
 from `.cs` files, the only relevant MSBuild inputs are the `#:` directives,
 hence we can first check the `.cs` file timestamps and for those that have changed, compare the sets of `#:` directives.
 If only `.cs` files change, it is enough to invoke `csc.exe` (directly or via a build server)
-re-using command-line arguments that the last MSBuild invocation passed to the compiler
-(you can opt out of this via an MSBuild property `FileBasedProgramCanSkipMSBuild=false`).
+re-using command-line arguments that the last MSBuild invocation passed to the compiler.
 If no inputs change, it is enough to start the target executable without invoking the build at all.
 
 ## Alternatives and future work

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

@@ -356,9 +356,16 @@ public override int Execute()
                     // Cache run info (to avoid re-evaluating the project instance).
                     cache.CurrentEntry.Run = RunProperties.FromProject(buildRequest.ProjectInstance);
 
-                    TryCacheCscArguments(cache, buildResult, buildRequest.ProjectInstance);
+                    if (!MSBuildUtilities.ConvertStringToBool(buildRequest.ProjectInstance.GetPropertyValue(FileBasedProgramCanSkipMSBuild), defaultValue: true))
+                    {
+                        Reporter.Verbose.WriteLine($"Not saving cache because there is an opt-out via MSBuild property {FileBasedProgramCanSkipMSBuild}.");
+                    }
+                    else
+                    {
+                        CacheCscArguments(cache, buildResult);
 
-                    MarkBuildSuccess(cache);
+                        MarkBuildSuccess(cache);
+                    }
                 }
 
                 projectInstance = buildRequest.ProjectInstance;
@@ -445,14 +452,8 @@ static Action<IDictionary<string, string>> AddRestoreGlobalProperties(ReadOnlyDi
             return null;
         }
 
-        void TryCacheCscArguments(CacheInfo cache, BuildResult result, ProjectInstance projectInstance)
+        void CacheCscArguments(CacheInfo cache, BuildResult result)
         {
-            if (!MSBuildUtilities.ConvertStringToBool(projectInstance.GetPropertyValue(FileBasedProgramCanSkipMSBuild), defaultValue: true))
-            {
-                Reporter.Verbose.WriteLine($"Not saving CSC arguments because there is an opt-out via MSBuild property {FileBasedProgramCanSkipMSBuild}.");
-                return;
-            }
-
             // We cannot reuse CSC arguments from previous run and skip MSBuild if there are project references
             // because we cannot easily detect whether any referenced projects have changed.
             if (Directives.Any(static d => d is CSharpDirective.Project))

test/dotnet.Tests/CommandTests/Run/RunFileTests.cs

@@ -2787,24 +2787,97 @@ public void UpToDate_InvalidOptions()
     /// <summary>
     /// Up-to-date checks and optimizations currently don't support other included files.
     /// </summary>
-    [Fact]
-    public void UpToDate_DefaultItems()
+    [Theory, CombinatorialData] // https://github.com/dotnet/sdk/issues/50912
+    public void UpToDate_DefaultItems(bool optOut)
     {
         var testInstance = _testAssetsManager.CreateTestDirectory();
-        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), s_programReadingEmbeddedResource);
+        var code = $"""
+            {(optOut ? "#:property FileBasedProgramCanSkipMSBuild=false" : "")}
+            {s_programReadingEmbeddedResource}
+            """;
+        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), code);
         File.WriteAllText(Path.Join(testInstance.Path, "Resources.resx"), s_resx);
 
         Build(testInstance, BuildLevel.All, expectedOutput: "[MyString, TestValue]");
 
         // Update the RESX file.
         File.WriteAllText(Path.Join(testInstance.Path, "Resources.resx"), s_resx.Replace("TestValue", "UpdatedValue"));
 
-        Build(testInstance, BuildLevel.None, expectedOutput: "[MyString, TestValue]"); // note: outdated output (build skipped)
+        Build(testInstance, optOut ? BuildLevel.All : BuildLevel.None, expectedOutput: optOut ? "[MyString, UpdatedValue]" : "[MyString, TestValue]"); // note: outdated output (build skipped)
+
+        // Update the C# file.
+        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), "//v2\n" + code);
+
+        Build(testInstance, optOut ? BuildLevel.All : BuildLevel.Csc, expectedOutput: optOut ? "[MyString, UpdatedValue]" : "[MyString, TestValue]"); // note: outdated output (only CSC used)
+
+        Build(testInstance, BuildLevel.All, ["--no-cache"], expectedOutput: "[MyString, UpdatedValue]");
+    }
+
+    /// <summary>
+    /// Combination of <see cref="UpToDate_DefaultItems"/> with <see cref="CscOnly"/> optimization.
+    /// </summary>
+    [Theory, CombinatorialData] // https://github.com/dotnet/sdk/issues/50912
+    public void UpToDate_DefaultItems_CscOnly(bool optOut)
+    {
+        var testInstance = _testAssetsManager.CreateTestDirectory(baseDirectory: OutOfTreeBaseDirectory);
+        var code = $"""
+            {(optOut ? "#:property FileBasedProgramCanSkipMSBuild=false" : "")}
+            {s_programReadingEmbeddedResource}
+            """;
+        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), code);
+        File.WriteAllText(Path.Join(testInstance.Path, "Resources.resx"), s_resx);
+
+        Build(testInstance, optOut ? BuildLevel.All : BuildLevel.Csc, expectedOutput: optOut ? "[MyString, TestValue]" : "Resource not found");
+
+        // Update the RESX file.
+        File.WriteAllText(Path.Join(testInstance.Path, "Resources.resx"), s_resx.Replace("TestValue", "UpdatedValue"));
+
+        Build(testInstance, optOut ? BuildLevel.All : BuildLevel.None, expectedOutput: optOut ? "[MyString, UpdatedValue]" : "Resource not found");
+
+        // Update the C# file.
+        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), "//v2\n" + code);
+
+        Build(testInstance, optOut ? BuildLevel.All : BuildLevel.Csc, expectedOutput: optOut ? "[MyString, UpdatedValue]" : "Resource not found");
+
+        Build(testInstance, BuildLevel.All, ["--no-cache"], expectedOutput: "[MyString, UpdatedValue]");
+
+        // Update the C# file.
+        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), "//v3\n" + code);
+
+        Build(testInstance, optOut ? BuildLevel.All : BuildLevel.Csc, expectedOutput: "[MyString, UpdatedValue]");
+    }
+
+    /// <summary>
+    /// Combination of <see cref="UpToDate_DefaultItems"/> with <see cref="CscOnly_AfterMSBuild"/> optimization.
+    /// </summary>
+    [Theory, CombinatorialData] // https://github.com/dotnet/sdk/issues/50912
+    public void UpToDate_DefaultItems_CscOnly_AfterMSBuild(bool optOut)
+    {
+        var testInstance = _testAssetsManager.CreateTestDirectory(baseDirectory: OutOfTreeBaseDirectory);
+        var code = $"""
+            #:property Configuration=Release
+            {(optOut ? "#:property FileBasedProgramCanSkipMSBuild=false" : "")}
+            {s_programReadingEmbeddedResource}
+            """;
+        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), code);
+        File.WriteAllText(Path.Join(testInstance.Path, "Resources.resx"), s_resx);
+
+        Build(testInstance, BuildLevel.All, expectedOutput: "[MyString, TestValue]");
+
+        // Update the C# file.
+        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), "//v2\n" + code);
+
+        Build(testInstance, optOut ? BuildLevel.All : BuildLevel.Csc, expectedOutput: optOut ? "[MyString, TestValue]" : "[MyString, TestValue]");
+
+        // Update the RESX file.
+        File.WriteAllText(Path.Join(testInstance.Path, "Resources.resx"), s_resx.Replace("TestValue", "UpdatedValue"));
+
+        Build(testInstance, optOut ? BuildLevel.All : BuildLevel.None, expectedOutput: optOut ? "[MyString, UpdatedValue]" : "[MyString, TestValue]");
 
         // Update the C# file.
-        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), "//v2\n" + s_programReadingEmbeddedResource);
+        File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), "//v3\n" + code);
 
-        Build(testInstance, BuildLevel.Csc, expectedOutput: "[MyString, TestValue]"); // note: outdated output (only CSC used)
+        Build(testInstance, optOut ? BuildLevel.All : BuildLevel.Csc, expectedOutput: optOut ? "[MyString, UpdatedValue]" : "[MyString, TestValue]");
 
         Build(testInstance, BuildLevel.All, ["--no-cache"], expectedOutput: "[MyString, UpdatedValue]");
     }