Make allowlisted late file-access warnings configurable

README.md

@@ -69,6 +69,7 @@ These settings are common across all plugins, although different implementations
 | `$(MSBuildCacheAsyncCacheMaterialization)` | `bool` | true | Whether files are materialized on disk asynchronously from the cache as opposed to delaying the completion signal to MSBuild until publishing is complete. Note: Materialization will be awaited when a depending project requires the files and before the overall build completes. |
 | `$(MSBuildCacheAllowFileAccessAfterProjectFinishProcessPatterns)` | `Glob[]` | `\**\vctip.exe` | Processes to allow file accesses after the project which launched it completes, ie accesses by a detached process. Note: these accesses will not be considered for caching. |
 | `$(MSBuildCacheAllowFileAccessAfterProjectFinishFilePatterns)` | `Glob[]` | | Files to allow to be accessed by a process launched by a project after the project completes, ie accesses by a detached process. Note: these accesses will not be considered for caching. |
+| `$(MSBuildCacheLogAllowFileAccessAfterProjectFinishMatchesAsMessages)` | `bool` | false | Whether late file accesses matching `$(MSBuildCacheAllowFileAccessAfterProjectFinishProcessPatterns)` or `$(MSBuildCacheAllowFileAccessAfterProjectFinishFilePatterns)` are logged as messages instead of warnings. This can keep these diagnostics from being promoted by `/warnaserror`; other warnings remain unaffected. |
 | `$(MSBuildCacheAllowProcessCloseAfterProjectFinishProcessPatterns)` | `Glob[]` | `\**\mspdbsrv.exe` | Processes to allow to exit after the project which launched it completes, ie detached processes. |
 | `$(MSBuildCacheGlobalPropertiesToIgnore)` | `string[]` | `CurrentSolutionConfigurationContents; ShouldUnsetParentConfigurationAndPlatform; BuildingInsideVisualStudio; BuildingSolutionFile; SolutionDir; SolutionExt; SolutionFileName; SolutionName; SolutionPath; _MSDeployUserAgent`, as well as all proeprties related to plugin settings | The list of global properties to exclude from consideration by the cache |
 | `$(MSBuildCacheGetResultsForUnqueriedDependencies)` | `bool` | false | Whether to try and query the cache for dependencies if they have not previously been requested. This option can help in cases where the build isn't done in graph order, or if some projects are skipped. |

src/Common.Tests/FileAccess/FileAccessRepositoryTests.cs

@@ -0,0 +1,132 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using Microsoft.Build.Execution;
+using Microsoft.Build.Experimental.FileAccess;
+using Microsoft.MSBuildCache.FileAccess;
+using Microsoft.MSBuildCache.Tests.Mocks;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Microsoft.MSBuildCache.Tests.FileAccess;
+
+[TestClass]
+public sealed class FileAccessRepositoryTests
+{
+#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Justification: Always set by MSTest.
+    public TestContext TestContext { get; set; }
+#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor.
+
+    [TestMethod]
+    [DataRow(false)]
+    [DataRow(true)]
+    public void FilePatternMatchUsesConfiguredLogLevel(bool logAsMessage)
+    {
+        string testDirectory = CreateTestDirectory();
+        string lateAccessPath = Path.Combine(testDirectory, "late-access.txt");
+        PluginSettings settings = CreateSettings(
+            testDirectory,
+            new Dictionary<string, string>
+            {
+                [nameof(PluginSettings.AllowFileAccessAfterProjectFinishFilePatterns)] = lateAccessPath,
+                [nameof(PluginSettings.LogAllowFileAccessAfterProjectFinishMatchesAsMessages)] = logAsMessage.ToString(),
+            });
+
+        MockPluginLogger logger = new();
+        using FileAccessRepository repository = new(logger, settings);
+        NodeContext nodeContext = CreateNodeContext(testDirectory);
+
+        _ = repository.FinishProject(nodeContext);
+        repository.AddFileAccess(nodeContext, CreateFileAccess(lateAccessPath));
+
+        Assert.HasCount(1, logger.LogEntries);
+        Assert.AreEqual(logAsMessage ? PluginLogLevel.Message : PluginLogLevel.Warning, logger.LogEntries[0].LogLevel);
+    }
+
+    [TestMethod]
+    [DataRow(false)]
+    [DataRow(true)]
+    public void ProcessPatternMatchUsesConfiguredLogLevel(bool logAsMessage)
+    {
+        string testDirectory = CreateTestDirectory();
+        string processPath = Path.Combine(testDirectory, "detached.exe");
+        PluginSettings settings = CreateSettings(
+            testDirectory,
+            new Dictionary<string, string>
+            {
+                [nameof(PluginSettings.AllowFileAccessAfterProjectFinishProcessPatterns)] = processPath,
+                [nameof(PluginSettings.LogAllowFileAccessAfterProjectFinishMatchesAsMessages)] = logAsMessage.ToString(),
+            });
+
+        MockPluginLogger logger = new();
+        using FileAccessRepository repository = new(logger, settings);
+        NodeContext nodeContext = CreateNodeContext(testDirectory);
+
+        repository.AddFileAccess(nodeContext, CreateProcessAccess(processPath));
+        _ = repository.FinishProject(nodeContext);
+        repository.AddFileAccess(nodeContext, CreateFileAccess(Path.Combine(testDirectory, "other.txt")));
+
+        Assert.HasCount(1, logger.LogEntries);
+        Assert.AreEqual(logAsMessage ? PluginLogLevel.Message : PluginLogLevel.Warning, logger.LogEntries[0].LogLevel);
+    }
+
+    private string CreateTestDirectory()
+    {
+        string testDirectory = Path.Combine(
+            TestContext.TestResultsDirectory!,
+            nameof(FileAccessRepositoryTests),
+            Guid.NewGuid().ToString("N"));
+        Directory.CreateDirectory(testDirectory);
+        return testDirectory;
+    }
+
+    private static PluginSettings CreateSettings(string repoRoot, Dictionary<string, string> settings)
+        => PluginSettings.Create<PluginSettings>(settings, NullPluginLogger.Instance, repoRoot);
+
+    private static NodeContext CreateNodeContext(string testDirectory)
+    {
+        string projectPath = Path.Combine(testDirectory, "p.proj");
+        File.WriteAllText(projectPath, "<Project />");
+        ProjectInstance projectInstance = new(projectPath);
+
+        return new NodeContext(
+            testDirectory,
+            projectInstance,
+            Array.Empty<NodeContext>(),
+            "p",
+            new Dictionary<string, string>(),
+            Array.Empty<string>(),
+            referenceAssemblyRelativePath: null,
+            new HashSet<string>(StringComparer.OrdinalIgnoreCase));
+    }
+
+    private static FileAccessData CreateFileAccess(string path)
+        => new(
+            ReportedFileOperation.CreateFile,
+            RequestedAccess.Read,
+            processId: 123,
+            id: 0,
+            correlationId: 0,
+            error: 1,
+            DesiredAccess.GENERIC_READ,
+            FlagsAndAttributes.FILE_ATTRIBUTE_NORMAL,
+            path,
+            processArgs: null,
+            isAnAugmentedFileAccess: false);
+
+    private static FileAccessData CreateProcessAccess(string path)
+        => new(
+            ReportedFileOperation.Process,
+            RequestedAccess.None,
+            processId: 123,
+            id: 0,
+            correlationId: 0,
+            error: 0,
+            desiredAccess: 0,
+            flagsAndAttributes: 0,
+            path,
+            processArgs: null,
+            isAnAugmentedFileAccess: false);
+}

src/Common.Tests/PluginSettingsTests.cs

@@ -146,6 +146,12 @@ public void AllowFileAccessAfterProjectFinishFilePatternsSetting(GlobTestCase te
             testCase,
             pluginSettings => pluginSettings.AllowFileAccessAfterProjectFinishFilePatterns);
 
+    [TestMethod]
+    public void LogAllowFileAccessAfterProjectFinishMatchesAsMessagesSetting()
+        => TestBoolSetting(
+            nameof(PluginSettings.LogAllowFileAccessAfterProjectFinishMatchesAsMessages),
+            pluginSettings => pluginSettings.LogAllowFileAccessAfterProjectFinishMatchesAsMessages);
+
     [TestMethod]
     [DynamicData(nameof(GlobTestCases), DynamicDataDisplayName = nameof(GetTestCaseDisplayName))]
     public void AllowProcessCloseAfterProjectFinishProcessPatternsSetting(GlobTestCase testCase)

src/Common.Tests/WarningPolicyTests.cs

@@ -0,0 +1,165 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+#if !NETFRAMEWORK
+using System.Diagnostics;
+using System.IO;
+using System.Security;
+#endif
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.Build.Execution;
+using Microsoft.Build.Experimental.ProjectCache;
+#if !NETFRAMEWORK
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+#endif
+
+namespace Microsoft.MSBuildCache.Tests;
+
+public sealed class WarningPolicyTestPlugin : ProjectCachePluginBase
+{
+    internal const string DiagnosticMessage = "Allowlisted late file access diagnostic";
+
+    public override Task BeginBuildAsync(CacheContext context, PluginLoggerBase logger, CancellationToken cancellationToken)
+    {
+        PluginSettings settings = PluginSettings.Create<PluginSettings>(context.PluginSettings, logger, Environment.CurrentDirectory);
+        logger.LogWarningOrMessage(DiagnosticMessage, settings.LogAllowFileAccessAfterProjectFinishMatchesAsMessages);
+        return Task.CompletedTask;
+    }
+
+    public override Task<CacheResult> GetCacheResultAsync(
+        BuildRequestData buildRequest,
+        PluginLoggerBase logger,
+        CancellationToken cancellationToken)
+        => Task.FromResult(CacheResult.IndicateNonCacheHit(CacheResultType.CacheNotApplicable));
+
+    public override Task EndBuildAsync(PluginLoggerBase logger, CancellationToken cancellationToken)
+        => Task.CompletedTask;
+}
+
+#if !NETFRAMEWORK
+[TestClass]
+public sealed class WarningPolicyTests
+{
+#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Justification: Always set by MSTest.
+    public TestContext TestContext { get; set; }
+#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor.
+
+    [TestMethod]
+    public async Task DefaultDiagnosticIsVisibleAsWarning()
+    {
+        BuildInvocationResult result = await RunBuildAsync(logAsMessage: false, warnAsError: false, emitUnrelatedWarning: false);
+
+        StringAssert.Contains(result.Output, "Build succeeded.", StringComparison.Ordinal);
+        StringAssert.Contains(result.Output, WarningPolicyTestPlugin.DiagnosticMessage, StringComparison.Ordinal);
+    }
+
+    [TestMethod]
+    public async Task WarnAsErrorEscalatesDefaultDiagnostic()
+    {
+        BuildInvocationResult result = await RunBuildAsync(logAsMessage: false, warnAsError: true, emitUnrelatedWarning: false);
+
+        StringAssert.Contains(result.Output, "Build FAILED.", StringComparison.Ordinal);
+        StringAssert.Contains(result.Output, WarningPolicyTestPlugin.DiagnosticMessage, StringComparison.Ordinal);
+    }
+
+    [TestMethod]
+    public async Task MessageSettingAvoidsWarnAsErrorEscalation()
+    {
+        BuildInvocationResult result = await RunBuildAsync(logAsMessage: true, warnAsError: true, emitUnrelatedWarning: false);
+
+        StringAssert.Contains(result.Output, "Build succeeded.", StringComparison.Ordinal);
+        StringAssert.Contains(result.Output, WarningPolicyTestPlugin.DiagnosticMessage, StringComparison.Ordinal);
+    }
+
+    [TestMethod]
+    public async Task MessageSettingDoesNotWeakenOtherWarnings()
+    {
+        BuildInvocationResult result = await RunBuildAsync(logAsMessage: true, warnAsError: true, emitUnrelatedWarning: true);
+
+        StringAssert.Contains(result.Output, "Build FAILED.", StringComparison.Ordinal);
+        StringAssert.Contains(result.Output, "Unrelated warning", StringComparison.Ordinal);
+    }
+
+    private async Task<BuildInvocationResult> RunBuildAsync(bool logAsMessage, bool warnAsError, bool emitUnrelatedWarning)
+    {
+        string testDirectory = Path.Combine(
+            TestContext.TestResultsDirectory!,
+            nameof(WarningPolicyTests),
+            TestContext.TestName!,
+            Guid.NewGuid().ToString("N"));
+        Directory.CreateDirectory(testDirectory);
+
+        string pluginPath = SecurityElement.Escape(typeof(WarningPolicyTestPlugin).Assembly.Location)!;
+        string commonTargetsPath = SecurityElement.Escape(FindCommonTargetsPath())!;
+        string projectPath = Path.Combine(testDirectory, "test.proj");
+        string projectContents =
+            $"""
+            <Project DefaultTargets="Build">
+              <PropertyGroup>
+                <MSBuildCacheAssembly>{pluginPath}</MSBuildCacheAssembly>
+                <MSBuildCacheLogAllowFileAccessAfterProjectFinishMatchesAsMessages>{logAsMessage}</MSBuildCacheLogAllowFileAccessAfterProjectFinishMatchesAsMessages>
+              </PropertyGroup>
+              <Import Project="{commonTargetsPath}" />
+              <Target Name="Build">
+                <Warning Condition="'$(EmitUnrelatedWarning)' == 'true'" Code="UNRELATED001" Text="Unrelated warning" />
+              </Target>
+            </Project>
+            """;
+        await File.WriteAllTextAsync(projectPath, projectContents);
+
+        ProcessStartInfo startInfo = new()
+        {
+            FileName = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH") ?? "dotnet",
+            WorkingDirectory = testDirectory,
+            RedirectStandardOutput = true,
+            RedirectStandardError = true,
+            UseShellExecute = false,
+            CreateNoWindow = true,
+        };
+        startInfo.ArgumentList.Add("msbuild");
+        startInfo.ArgumentList.Add(projectPath);
+        startInfo.ArgumentList.Add("-graphBuild");
+        startInfo.ArgumentList.Add("-maxCpuCount:1");
+        startInfo.ArgumentList.Add("-nodeReuse:false");
+        startInfo.ArgumentList.Add("-verbosity:normal");
+        if (warnAsError)
+        {
+            startInfo.ArgumentList.Add("-warnAsError");
+        }
+
+        if (emitUnrelatedWarning)
+        {
+            startInfo.ArgumentList.Add("-property:EmitUnrelatedWarning=true");
+        }
+
+        using Process process = Process.Start(startInfo)!;
+        Task<string> standardOutput = process.StandardOutput.ReadToEndAsync();
+        Task<string> standardError = process.StandardError.ReadToEndAsync();
+        await process.WaitForExitAsync();
+
+        string output = await standardOutput + await standardError;
+        return new BuildInvocationResult(output);
+    }
+
+    private static string FindCommonTargetsPath()
+    {
+        DirectoryInfo? directory = new FileInfo(typeof(WarningPolicyTestPlugin).Assembly.Location).Directory;
+        while (directory != null)
+        {
+            string candidatePath = Path.Combine(directory.FullName, "src", "Common", "build", "Microsoft.MSBuildCache.Common.targets");
+            if (File.Exists(candidatePath))
+            {
+                return candidatePath;
+            }
+
+            directory = directory.Parent;
+        }
+
+        throw new InvalidOperationException("Could not find Microsoft.MSBuildCache.Common.targets.");
+    }
+
+    private readonly record struct BuildInvocationResult(string Output);
+}
+#endif

src/Common/FileAccess/FileAccessRepository.cs

@@ -126,15 +126,17 @@ public void AddFileAccess(FileAccessData fileAccessData)
 
                     if (processMatch != null)
                     {
-                        _logger.LogWarning(
+                        _logger.LogWarningOrMessage(
                             $"File access reported from process after the project finished, but process matched {nameof(_pluginSettings.AllowFileAccessAfterProjectFinishProcessPatterns)} `{processMatch}`. " +
-                            $"This may lead to incorrect caching. Node Id: {_nodeContext.Id}, Process Id: {fileAccessData.ProcessId} ProcessPath: `{processName}` File Path: `{path}`");
+                            $"This may lead to incorrect caching. Node Id: {_nodeContext.Id}, Process Id: {fileAccessData.ProcessId} ProcessPath: `{processName}` File Path: `{path}`",
+                            _pluginSettings.LogAllowFileAccessAfterProjectFinishMatchesAsMessages);
                     }
                     else if (fileMatch != null)
                     {
-                        _logger.LogWarning(
+                        _logger.LogWarningOrMessage(
                             $"File access reported from process after the project finished, but file path matched {nameof(_pluginSettings.AllowFileAccessAfterProjectFinishFilePatterns)} `{fileMatch}`. " +
-                            $"This may lead to incorrect caching. Node Id: {_nodeContext.Id}, Process Id: {fileAccessData.ProcessId} ProcessPath: `{processName}` File Path: `{path}`");
+                            $"This may lead to incorrect caching. Node Id: {_nodeContext.Id}, Process Id: {fileAccessData.ProcessId} ProcessPath: `{processName}` File Path: `{path}`",
+                            _pluginSettings.LogAllowFileAccessAfterProjectFinishMatchesAsMessages);
                     }
                     else
                     {

src/Common/PluginLoggerExtensions.cs

@@ -0,0 +1,22 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.Build.Experimental.ProjectCache;
+using Microsoft.Build.Framework;
+
+namespace Microsoft.MSBuildCache;
+
+internal static class PluginLoggerExtensions
+{
+    public static void LogWarningOrMessage(this PluginLoggerBase logger, string message, bool logAsMessage)
+    {
+        if (logAsMessage)
+        {
+            logger.LogMessage(message, MessageImportance.High);
+        }
+        else
+        {
+            logger.LogWarning(message);
+        }
+    }
+}

src/Common/PluginSettings.cs

@@ -97,6 +97,8 @@ public string LocalCacheRootPath
 
     public IReadOnlyCollection<Glob> AllowFileAccessAfterProjectFinishFilePatterns { get; init; } = Array.Empty<Glob>();
 
+    public bool LogAllowFileAccessAfterProjectFinishMatchesAsMessages { get; init; }
+
     public IReadOnlyCollection<Glob> AllowProcessCloseAfterProjectFinishProcessPatterns { get; init; } = Array.Empty<Glob>();
 
     public IReadOnlyList<string> GlobalPropertiesToIgnore { get; init; } = Array.Empty<string>();