From 232708e39e8f21fbdcfb74d5300cb3675514c98c Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Tue, 13 Aug 2024 17:41:32 +0200 Subject: [PATCH 01/11] Test cleanup --- .../FileWatcher/PollingFileWatcher.cs | 15 +++--- test/dotnet-watch.Tests/FileWatcherTests.cs | 48 ++++++++++++++----- 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs index da7630b71eb8..53ed3c45125e 100644 --- a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs @@ -158,16 +158,13 @@ private void RecordChange(FileSystemInfo fileInfo, bool isNewFile) _changes.Add(fileInfo.FullName, isNewFile); - if (fileInfo.FullName != _watchedDirectory.FullName) + if (fileInfo is FileInfo { Directory: { } directory }) { - if (fileInfo is FileInfo { Directory: { } directory }) - { - RecordChange(directory, isNewFile: false); - } - else if (fileInfo is DirectoryInfo { Parent: { } parent }) - { - RecordChange(parent, isNewFile: false); - } + RecordChange(directory, isNewFile: false); + } + else if (fileInfo is DirectoryInfo { Parent: { } parent }) + { + RecordChange(parent, isNewFile: false); } } diff --git a/test/dotnet-watch.Tests/FileWatcherTests.cs b/test/dotnet-watch.Tests/FileWatcherTests.cs index 03afa158fb79..cad73dd420d8 100644 --- a/test/dotnet-watch.Tests/FileWatcherTests.cs +++ b/test/dotnet-watch.Tests/FileWatcherTests.cs @@ -1,24 +1,16 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Testing; using Microsoft.DotNet.Watcher.Internal; namespace Microsoft.DotNet.Watcher.Tools { - public class FileWatcherTests + public class FileWatcherTests(ITestOutputHelper output) { - public FileWatcherTests(ITestOutputHelper output) - { - _output = output; - _testAssetManager = new TestAssetsManager(output); - } - private readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(60); private readonly TimeSpan NegativeTimeout = TimeSpan.FromSeconds(5); - private readonly ITestOutputHelper _output; - private readonly TestAssetsManager _testAssetManager; + private readonly TestAssetsManager _testAssetManager = new TestAssetsManager(output); private async Task TestOperation( string dir, @@ -36,7 +28,7 @@ private async Task TestOperation( using var watcher = FileWatcherFactory.CreateWatcher(dir, usePolling); if (watcher is DotnetFileWatcher dotnetWatcher) { - dotnetWatcher.Logger = m => _output.WriteLine(m); + dotnetWatcher.Logger = m => output.WriteLine(m); } var changedEv = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -99,6 +91,38 @@ await TestOperation( () => File.WriteAllText(testFileFullPath, string.Empty)); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task NewFileInNewDirectory(bool usePolling) + { + var dir = _testAssetManager.CreateTestDirectory(identifier: usePolling.ToString()).Path; + + var newDir = Path.Combine(dir, "Dir"); + var newFile = Path.Combine(newDir, "foo"); + + await TestOperation( + dir, + expectedChanges: !RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && !usePolling + ? new[] + { + (newDir, true), + (newFile, false), + (newFile, true), + } + : new[] + { + (newDir, true), + (newFile, true), + }, + usePolling, + () => + { + Directory.CreateDirectory(newDir); + File.WriteAllText(newFile, string.Empty); + }); + } + [Theory] [InlineData(true)] [InlineData(false)] @@ -276,7 +300,7 @@ private async Task AssertFileChangeRaisesEvent(string directory, IFileSystemWatc var expectedPath = Path.Combine(directory, Path.GetRandomFileName()); EventHandler<(string, bool)> handler = (_, f) => { - _output.WriteLine("File changed: " + f); + output.WriteLine("File changed: " + f); try { if (string.Equals(f.Item1, expectedPath, StringComparison.OrdinalIgnoreCase)) From 466c4fd5b605cd1cbde0d76d7b626a5c002f8de6 Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Wed, 14 Aug 2024 10:20:51 +0200 Subject: [PATCH 02/11] ChangeKind --- .../dotnet-watch/DotNetWatcher.cs | 10 ++-- src/BuiltInTools/dotnet-watch/FileItem.cs | 6 ++- .../dotnet-watch/Filters/BuildEvaluator.cs | 4 +- .../HotReload/IncrementalMSBuildWorkspace.cs | 24 ++++++++- .../HotReload/ScopedCssFileHandler.cs | 5 +- .../HotReload/StaticFileHandler.cs | 5 +- .../dotnet-watch/HotReloadDotNetWatcher.cs | 47 +++++++++++------ .../dotnet-watch/Internal/FileWatcher.cs | 16 +++--- .../Internal/FileWatcher/ChangeKind.cs | 13 +++++ .../Internal/FileWatcher/DotnetFileWatcher.cs | 18 +++---- .../FileWatcher/IFileSystemWatcher.cs | 2 +- .../FileWatcher/PollingFileWatcher.cs | 22 ++++---- .../Internal/HotReloadFileSetWatcher.cs | 16 +++--- test/dotnet-watch.Tests/FileWatcherTests.cs | 50 +++++++++---------- .../MSBuildEvaluationFilterTest.cs | 9 ++-- 15 files changed, 152 insertions(+), 95 deletions(-) create mode 100644 src/BuiltInTools/dotnet-watch/Internal/FileWatcher/ChangeKind.cs diff --git a/src/BuiltInTools/dotnet-watch/DotNetWatcher.cs b/src/BuiltInTools/dotnet-watch/DotNetWatcher.cs index 785219c3879e..d2f57ce7eb34 100644 --- a/src/BuiltInTools/dotnet-watch/DotNetWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/DotNetWatcher.cs @@ -27,7 +27,7 @@ public override async Task WatchAsync(CancellationToken cancellationToken) var environmentBuilder = EnvironmentVariablesBuilder.FromCurrentEnvironment(); - FileItem? changedFile = null; + ChangedFile? changedFile = null; var buildEvaluator = new BuildEvaluator(Context, RootFileSetFactory); await using var browserConnector = new BrowserConnector(Context); @@ -86,7 +86,7 @@ public override async Task WatchAsync(CancellationToken cancellationToken) var processTask = ProcessRunner.RunAsync(processSpec, Context.Reporter, isUserApplication: true, processExitedSource: null, combinedCancellationSource.Token); - Task fileSetTask; + Task fileSetTask; Task finishedTask; while (true) @@ -94,9 +94,9 @@ public override async Task WatchAsync(CancellationToken cancellationToken) fileSetTask = fileSetWatcher.GetChangedFileAsync(startedWatching: null, combinedCancellationSource.Token); finishedTask = await Task.WhenAny(processTask, fileSetTask, cancelledTaskSource.Task); - if (staticFileHandler != null && finishedTask == fileSetTask && fileSetTask.Result is FileItem fileItem) + if (staticFileHandler != null && finishedTask == fileSetTask && fileSetTask.Result.HasValue) { - if (await staticFileHandler.HandleFileChangesAsync([fileItem], combinedCancellationSource.Token)) + if (await staticFileHandler.HandleFileChangesAsync([fileSetTask.Result.Value], combinedCancellationSource.Token)) { // We're able to handle the file change event without doing a full-rebuild. continue; @@ -131,7 +131,7 @@ public override async Task WatchAsync(CancellationToken cancellationToken) Debug.Assert(finishedTask == fileSetTask); changedFile = fileSetTask.Result; Debug.Assert(changedFile != null, "ChangedFile should only be null when cancelled"); - Context.Reporter.Output($"File changed: {changedFile.Value.FilePath}"); + Context.Reporter.Output($"File changed: {changedFile.Value.Item.FilePath}"); } } } diff --git a/src/BuiltInTools/dotnet-watch/FileItem.cs b/src/BuiltInTools/dotnet-watch/FileItem.cs index 71779898168a..7fc91cd6fa1c 100644 --- a/src/BuiltInTools/dotnet-watch/FileItem.cs +++ b/src/BuiltInTools/dotnet-watch/FileItem.cs @@ -1,9 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.DotNet.Watcher.Internal; + namespace Microsoft.DotNet.Watcher { - internal readonly struct FileItem + internal readonly record struct FileItem { public string FilePath { get; init; } @@ -14,7 +16,7 @@ internal readonly struct FileItem public string? StaticWebAssetPath { get; init; } - public bool IsNewFile { get; init; } + public ChangeKind Change { get; init; } public bool IsStaticFile => StaticWebAssetPath != null; } diff --git a/src/BuiltInTools/dotnet-watch/Filters/BuildEvaluator.cs b/src/BuiltInTools/dotnet-watch/Filters/BuildEvaluator.cs index ad7599378ee2..0025fbbbcbf5 100644 --- a/src/BuiltInTools/dotnet-watch/Filters/BuildEvaluator.cs +++ b/src/BuiltInTools/dotnet-watch/Filters/BuildEvaluator.cs @@ -46,7 +46,7 @@ public IReadOnlyList GetProcessArguments(int iteration) return [context.RootProjectOptions.Command, .. context.RootProjectOptions.CommandArguments]; } - public async ValueTask EvaluateAsync(FileItem? changedFile, CancellationToken cancellationToken) + public async ValueTask EvaluateAsync(ChangedFile? changedFile, CancellationToken cancellationToken) { if (context.EnvironmentOptions.SuppressMSBuildIncrementalism) { @@ -54,7 +54,7 @@ public async ValueTask EvaluateAsync(FileItem? changedFile, Ca return _evaluationResult = await CreateEvaluationResult(cancellationToken); } - if (_evaluationResult == null || RequiresMSBuildRevaluation(changedFile)) + if (_evaluationResult == null || RequiresMSBuildRevaluation(changedFile?.Item)) { RequiresRevaluation = true; } diff --git a/src/BuiltInTools/dotnet-watch/HotReload/IncrementalMSBuildWorkspace.cs b/src/BuiltInTools/dotnet-watch/HotReload/IncrementalMSBuildWorkspace.cs index 17e4c6d27cd8..6a04b8c93687 100644 --- a/src/BuiltInTools/dotnet-watch/HotReload/IncrementalMSBuildWorkspace.cs +++ b/src/BuiltInTools/dotnet-watch/HotReload/IncrementalMSBuildWorkspace.cs @@ -15,6 +15,7 @@ using Microsoft.CodeAnalysis.ExternalAccess.Watch.Api; using System.Collections.Immutable; using Microsoft.CodeAnalysis.Text; +using Microsoft.DotNet.Watcher.Internal; namespace Microsoft.DotNet.Watcher.Tools; @@ -106,13 +107,24 @@ ImmutableArray MapDocuments(ProjectId mappedProjectId, IReadOnlyLi }).ToImmutableArray(); } - public async ValueTask UpdateFileContentAsync(IEnumerable changedFiles, CancellationToken cancellationToken) + public async ValueTask UpdateFileContentAsync(IEnumerable changedFiles, CancellationToken cancellationToken) { var updatedSolution = CurrentSolution; - foreach (var changedFile in changedFiles) + var documentsToRemove = new List(); + + foreach (var (changedFile, change) in changedFiles) { + // when a file is added we reevaluate the project: + Debug.Assert(change != ChangeKind.Add); + var documentIds = updatedSolution.GetDocumentIdsWithFilePath(changedFile.FilePath); + if (change == ChangeKind.Delete) + { + documentsToRemove.AddRange(documentIds); + continue; + } + foreach (var documentId in documentIds) { var textDocument = updatedSolution.GetDocument(documentId) @@ -140,9 +152,17 @@ public async ValueTask UpdateFileContentAsync(IEnumerable changedFiles } } + updatedSolution = RemoveDocuments(updatedSolution, documentsToRemove); + await ReportSolutionFilesAsync(SetCurrentSolution(updatedSolution), cancellationToken); } + private static Solution RemoveDocuments(Solution solution, IEnumerable ids) + => solution + .RemoveDocuments(ids.Where(id => solution.GetDocument(id) != null).ToImmutableArray()) + .RemoveAdditionalDocuments(ids.Where(id => solution.GetAdditionalDocument(id) != null).ToImmutableArray()) + .RemoveAnalyzerConfigDocuments(ids.Where(id => solution.GetAnalyzerConfigDocument(id) != null).ToImmutableArray()); + private static async ValueTask GetSourceTextAsync(string filePath, CancellationToken cancellationToken) { var zeroLengthRetryPerformed = false; diff --git a/src/BuiltInTools/dotnet-watch/HotReload/ScopedCssFileHandler.cs b/src/BuiltInTools/dotnet-watch/HotReload/ScopedCssFileHandler.cs index dfe625075227..3a6961854987 100644 --- a/src/BuiltInTools/dotnet-watch/HotReload/ScopedCssFileHandler.cs +++ b/src/BuiltInTools/dotnet-watch/HotReload/ScopedCssFileHandler.cs @@ -5,6 +5,7 @@ using System.Collections; using System.Diagnostics; using Microsoft.Build.Graph; +using Microsoft.DotNet.Watcher.Internal; using Microsoft.Extensions.Tools.Internal; namespace Microsoft.DotNet.Watcher.Tools @@ -13,14 +14,14 @@ internal sealed class ScopedCssFileHandler(IReporter reporter, ProjectNodeMap pr { private const string BuildTargetName = "GenerateComputedBuildStaticWebAssets"; - public async ValueTask HandleFileChangesAsync(IReadOnlyList files, CancellationToken cancellationToken) + public async ValueTask HandleFileChangesAsync(IReadOnlyList files, CancellationToken cancellationToken) { var projectsToRefresh = new HashSet(); var hasApplicableFiles = false; for (int i = 0; i < files.Count; i++) { - var file = files[i]; + var file = files[i].Item; if (!file.FilePath.EndsWith(".razor.css", StringComparison.Ordinal) && !file.FilePath.EndsWith(".cshtml.css", StringComparison.Ordinal)) diff --git a/src/BuiltInTools/dotnet-watch/HotReload/StaticFileHandler.cs b/src/BuiltInTools/dotnet-watch/HotReload/StaticFileHandler.cs index f8d9b8bffd12..ce53bc8a7ce3 100644 --- a/src/BuiltInTools/dotnet-watch/HotReload/StaticFileHandler.cs +++ b/src/BuiltInTools/dotnet-watch/HotReload/StaticFileHandler.cs @@ -7,6 +7,7 @@ using System.Text.Json; using System.Text.Json.Serialization; using Microsoft.CodeAnalysis.StackTraceExplorer; +using Microsoft.DotNet.Watcher.Internal; using Microsoft.Extensions.Tools.Internal; namespace Microsoft.DotNet.Watcher.Tools @@ -18,13 +19,13 @@ internal sealed class StaticFileHandler(IReporter reporter, ProjectNodeMap proje DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }; - public async ValueTask HandleFileChangesAsync(IReadOnlyList files, CancellationToken cancellationToken) + public async ValueTask HandleFileChangesAsync(IReadOnlyList files, CancellationToken cancellationToken) { var allFilesHandled = true; var refreshRequests = new Dictionary>(); for (int i = 0; i < files.Count; i++) { - var file = files[i]; + var file = files[i].Item; if (file.StaticWebAssetPath is null) { diff --git a/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs b/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs index ee09b3c257df..ef77166f3c9f 100644 --- a/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs @@ -74,7 +74,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke HotReloadFileSetWatcher? fileSetWatcher = null; EvaluationResult? evaluationResult = null; RunningProject? rootRunningProject = null; - Task? fileSetWatcherTask = null; + Task? fileSetWatcherTask = null; try { @@ -178,7 +178,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke // When a new file is added we need to run design-time build to find out // what kind of the file it is and which project(s) does it belong to (can be linked, web asset, etc.). // We don't need to rebuild and restart the application though. - if (changedFiles.Any(f => f.IsNewFile)) + if (changedFiles.Any(f => f.Change is ChangeKind.Add)) { Context.Reporter.Verbose("File addition triggered re-evaluation."); @@ -195,9 +195,9 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke // update files in the change set with new evaluation info: for (int i = 0; i < changedFiles.Length; i++) { - if (evaluationResult.Files.TryGetValue(changedFiles[i].FilePath, out var evaluatedFile)) + if (evaluationResult.Files.TryGetValue(changedFiles[i].Item.FilePath, out var evaluatedFile)) { - changedFiles[i] = evaluatedFile; + changedFiles[i] = changedFiles[i] with { Item = evaluatedFile }; } } @@ -336,24 +336,43 @@ await Task.WhenAll( } } - private void ReportFileChanges(IReadOnlyList fileItems) + private void ReportFileChanges(IReadOnlyList changedFiles) { - Report(added: true); - Report(added: false); + Report(kind: ChangeKind.Add); + Report(kind: ChangeKind.Update); + Report(kind: ChangeKind.Delete); - void Report(bool added) + void Report(ChangeKind kind) { - var items = fileItems.Where(item => item.IsNewFile == added).ToArray(); + var items = changedFiles.Where(item => item.Change == kind).ToArray(); if (items is not []) { - Context.Reporter.Output(GetMessage(items, added)); + Context.Reporter.Output(GetMessage(items, kind)); } } - string GetMessage(IReadOnlyList items, bool added) - => items is [var item] - ? (added ? "File added: " : "File changed: ") + GetRelativeFilePath(item.FilePath) - : (added ? "Files added: " : "Files changed: ") + string.Join(", ", items.Select(f => GetRelativeFilePath(f.FilePath))); + string GetMessage(IReadOnlyList items, ChangeKind kind) + => items is [{Item: var item }] + ? GetSingularMessage(kind) + ": " + GetRelativeFilePath(item.FilePath) + : GetPlurarMessage(kind) + ": " + string.Join(", ", items.Select(f => GetRelativeFilePath(f.Item.FilePath))); + + static string GetSingularMessage(ChangeKind kind) + => kind switch + { + ChangeKind.Update => "File updated", + ChangeKind.Add => "File added", + ChangeKind.Delete => "File deleted", + _ => throw new InvalidOperationException() + }; + + static string GetPlurarMessage(ChangeKind kind) + => kind switch + { + ChangeKind.Update => "Files updated", + ChangeKind.Add => "Files added", + ChangeKind.Delete => "Files deleted", + _ => throw new InvalidOperationException() + }; } private async ValueTask EvaluateRootProjectAsync(CancellationToken cancellationToken) diff --git a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher.cs b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher.cs index b844f8da4fa8..3a339bf63fb1 100644 --- a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher.cs @@ -10,7 +10,7 @@ internal sealed class FileWatcher(IReadOnlyDictionary fileSet, private readonly Dictionary _watchers = []; private bool _disposed; - public event Action? OnFileChange; + public event Action? OnFileChange; public void Dispose() { @@ -73,9 +73,9 @@ private void WatcherErrorHandler(object? sender, Exception error) } } - private void WatcherChangedHandler(object? sender, (string changedPath, bool newFile) args) + private void WatcherChangedHandler(object? sender, (string changedPath, ChangeKind kind) args) { - OnFileChange?.Invoke(args.changedPath, args.newFile); + OnFileChange?.Invoke(args.changedPath, args.kind); } private void DisposeWatcher(string directory) @@ -101,22 +101,22 @@ private void EnsureNotDisposed() private static string EnsureTrailingSlash(string path) => (path is [.., var last] && last != Path.DirectorySeparatorChar) ? path + Path.DirectorySeparatorChar : path; - public async Task GetChangedFileAsync(Action? startedWatching, CancellationToken cancellationToken) + public async Task GetChangedFileAsync(Action? startedWatching, CancellationToken cancellationToken) { StartWatching(); - var fileChangedSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var fileChangedSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); cancellationToken.Register(() => fileChangedSource.TrySetResult(null)); - void FileChangedCallback(string path, bool newFile) + void FileChangedCallback(string path, ChangeKind kind) { if (fileSet.TryGetValue(path, out var fileItem)) { - fileChangedSource.TrySetResult(fileItem); + fileChangedSource.TrySetResult(new ChangedFile(fileItem, kind)); } } - FileItem? changedFile; + ChangedFile? changedFile; OnFileChange += FileChangedCallback; try diff --git a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/ChangeKind.cs b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/ChangeKind.cs new file mode 100644 index 000000000000..5fef3b698624 --- /dev/null +++ b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/ChangeKind.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.DotNet.Watcher.Internal; + +internal enum ChangeKind +{ + Update, + Add, + Delete +} + +internal readonly record struct ChangedFile(FileItem Item, ChangeKind Change); diff --git a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs index ca622206e933..57d3610adb43 100644 --- a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs @@ -33,7 +33,7 @@ internal DotnetFileWatcher(string watchedDirectory, Func? OnFileChange; + public event EventHandler<(string filePath, ChangeKind kind)>? OnFileChange; public event EventHandler? OnError; @@ -80,8 +80,8 @@ private void WatcherRenameHandler(object sender, RenamedEventArgs e) Logger?.Invoke("Rename"); - NotifyChange(e.OldFullPath, newFile: false); - NotifyChange(e.FullPath, newFile: true); + NotifyChange(e.OldFullPath, ChangeKind.Delete); + NotifyChange(e.FullPath, ChangeKind.Add); if (Directory.Exists(e.FullPath)) { @@ -89,8 +89,8 @@ private void WatcherRenameHandler(object sender, RenamedEventArgs e) { // Calculated previous path of this moved item. var oldLocation = Path.Combine(e.OldFullPath, newLocation.Substring(e.FullPath.Length + 1)); - NotifyChange(oldLocation, newFile: false); - NotifyChange(newLocation, newFile: true); + NotifyChange(oldLocation, ChangeKind.Delete); + NotifyChange(newLocation, ChangeKind.Add); } } } @@ -103,7 +103,7 @@ private void WatcherChangeHandler(object sender, FileSystemEventArgs e) } Logger?.Invoke("Change"); - NotifyChange(e.FullPath, newFile: false); + NotifyChange(e.FullPath, ChangeKind.Update); } private void WatcherAddedHandler(object sender, FileSystemEventArgs e) @@ -114,13 +114,13 @@ private void WatcherAddedHandler(object sender, FileSystemEventArgs e) } Logger?.Invoke("Added"); - NotifyChange(e.FullPath, newFile: true); + NotifyChange(e.FullPath, ChangeKind.Add); } - private void NotifyChange(string fullPath, bool newFile) + private void NotifyChange(string fullPath, ChangeKind kind) { // Only report file changes - OnFileChange?.Invoke(this, (fullPath, newFile)); + OnFileChange?.Invoke(this, (fullPath, kind)); } private void CreateFileSystemWatcher() diff --git a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/IFileSystemWatcher.cs b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/IFileSystemWatcher.cs index f75c7e3e1481..ebdef49913ff 100644 --- a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/IFileSystemWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/IFileSystemWatcher.cs @@ -5,7 +5,7 @@ namespace Microsoft.DotNet.Watcher.Internal { internal interface IFileSystemWatcher : IDisposable { - event EventHandler<(string filePath, bool newFile)> OnFileChange; + event EventHandler<(string filePath, ChangeKind kind)> OnFileChange; event EventHandler OnError; diff --git a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs index 53ed3c45125e..d54975f070af 100644 --- a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs @@ -15,7 +15,7 @@ internal class PollingFileWatcher : IFileSystemWatcher private Dictionary _knownEntities = new(); private Dictionary _tempDictionary = new(); - private Dictionary _changes = new(); + private Dictionary _changes = new(); private Thread _pollingThread; private bool _raiseEvents; @@ -40,7 +40,7 @@ public PollingFileWatcher(string watchedDirectory) _pollingThread.Start(); } - public event EventHandler<(string filePath, bool newFile)>? OnFileChange; + public event EventHandler<(string filePath, ChangeKind kind)>? OnFileChange; #pragma warning disable CS0067 // not used public event EventHandler? OnError; @@ -107,7 +107,7 @@ private void CheckForChangedFiles() if (!_knownEntities.ContainsKey(fullFilePath)) { // New file - RecordChange(f, isNewFile: true); + RecordChange(f, ChangeKind.Add); } else { @@ -118,7 +118,7 @@ private void CheckForChangedFiles() if (fileMeta.FileInfo.LastWriteTime != f.LastWriteTime) { // File changed - RecordChange(f, isNewFile: false); + RecordChange(f, ChangeKind.Update); } _knownEntities[fullFilePath] = new FileMeta(fileMeta.FileInfo, foundAgain: true); @@ -137,7 +137,7 @@ private void CheckForChangedFiles() if (!file.Value.FoundAgain) { // File deleted - RecordChange(file.Value.FileInfo, isNewFile: false); + RecordChange(file.Value.FileInfo, ChangeKind.Delete); } } @@ -148,7 +148,7 @@ private void CheckForChangedFiles() _tempDictionary.Clear(); } - private void RecordChange(FileSystemInfo fileInfo, bool isNewFile) + private void RecordChange(FileSystemInfo fileInfo, ChangeKind kind) { if (_changes.ContainsKey(fileInfo.FullName) || fileInfo.FullName.Equals(_watchedDirectory.FullName, StringComparison.Ordinal)) @@ -156,15 +156,15 @@ private void RecordChange(FileSystemInfo fileInfo, bool isNewFile) return; } - _changes.Add(fileInfo.FullName, isNewFile); + _changes.Add(fileInfo.FullName, kind); if (fileInfo is FileInfo { Directory: { } directory }) { - RecordChange(directory, isNewFile: false); + RecordChange(directory, ChangeKind.Update); } else if (fileInfo is DirectoryInfo { Parent: { } parent }) { - RecordChange(parent, isNewFile: false); + RecordChange(parent, ChangeKind.Update); } } @@ -199,14 +199,14 @@ private static void ForeachEntityInDirectory(DirectoryInfo dirInfo, Action _changedFiles = new(StringComparer.Ordinal); + private readonly ConcurrentDictionary _changedFiles = new(StringComparer.Ordinal); - private TaskCompletionSource? _tcs; + private TaskCompletionSource? _tcs; private bool _initialized; private bool _disposed; @@ -65,7 +65,7 @@ private void EnsureInitialized() continue; } - FileItem[] changedFiles; + ChangedFile[] changedFiles; lock (_changedFilesLock) { changedFiles = _changedFiles.Values.ToArray(); @@ -82,7 +82,7 @@ private void EnsureInitialized() }, default, TaskCreationOptions.LongRunning, TaskScheduler.Default); - void FileChangedCallback(string path, bool newFile) + void FileChangedCallback(string path, ChangeKind kind) { // only handle file changes: if (Directory.Exists(path)) @@ -109,24 +109,24 @@ void FileChangedCallback(string path, bool newFile) return; } - if (newFile) + if (kind == ChangeKind.Add) { lock (_changedFilesLock) { - _changedFiles.TryAdd(path, new FileItem { FilePath = path, IsNewFile = newFile }); + _changedFiles.TryAdd(path, new ChangedFile(new FileItem { FilePath = path }, kind)); } } else if (fileSet.TryGetValue(path, out var fileItem)) { lock (_changedFilesLock) { - _changedFiles.TryAdd(path, fileItem); + _changedFiles.TryAdd(path, new ChangedFile(fileItem, kind)); } } } } - public Task GetChangedFilesAsync(CancellationToken cancellationToken, bool forceWaitForNewUpdate = false) + public Task GetChangedFilesAsync(CancellationToken cancellationToken, bool forceWaitForNewUpdate = false) { EnsureInitialized(); diff --git a/test/dotnet-watch.Tests/FileWatcherTests.cs b/test/dotnet-watch.Tests/FileWatcherTests.cs index cad73dd420d8..58dc1f643226 100644 --- a/test/dotnet-watch.Tests/FileWatcherTests.cs +++ b/test/dotnet-watch.Tests/FileWatcherTests.cs @@ -14,7 +14,7 @@ public class FileWatcherTests(ITestOutputHelper output) private async Task TestOperation( string dir, - (string, bool)[] expectedChanges, + (string path, ChangeKind kind)[] expectedChanges, bool usePolling, Action operation) { @@ -32,11 +32,11 @@ private async Task TestOperation( } var changedEv = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var filesChanged = new HashSet<(string, bool)>(); + var filesChanged = new HashSet<(string path, ChangeKind kind)>(); var testFileFullPath = Path.Combine(dir, "foo"); - EventHandler<(string path, bool newFile)> handler = null; + EventHandler<(string path, ChangeKind kind)> handler = null; handler = (_, f) => { filesChanged.Add(f); @@ -80,12 +80,12 @@ await TestOperation( expectedChanges: !RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && !usePolling ? new[] { - (testFileFullPath, false), - (testFileFullPath, true), + (testFileFullPath, ChangeKind.Update), + (testFileFullPath, ChangeKind.Add), } : new[] { - (testFileFullPath, true), + (testFileFullPath, ChangeKind.Add), }, usePolling, () => File.WriteAllText(testFileFullPath, string.Empty)); @@ -106,14 +106,14 @@ await TestOperation( expectedChanges: !RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && !usePolling ? new[] { - (newDir, true), - (newFile, false), - (newFile, true), + (newDir, ChangeKind.Add), + (newFile, ChangeKind.Update), + (newFile, ChangeKind.Add), } : new[] { - (newDir, true), - (newFile, true), + (newDir, ChangeKind.Add), + (newFile, ChangeKind.Add), }, usePolling, () => @@ -135,7 +135,7 @@ public async Task ChangeFile(bool usePolling) await TestOperation( dir, - expectedChanges: [(testFileFullPath, false)], + expectedChanges: [(testFileFullPath, ChangeKind.Update)], usePolling, () => File.WriteAllText(testFileFullPath, string.Empty)); } @@ -156,14 +156,14 @@ await TestOperation( expectedChanges: RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && !usePolling ? new[] { - (srcFile, false), - (srcFile, true), - (dstFile, true), + (srcFile, ChangeKind.Update), + (srcFile, ChangeKind.Delete), + (dstFile, ChangeKind.Add), } : new[] { - (srcFile, false), - (dstFile, true), + (srcFile, ChangeKind.Delete), + (dstFile, ChangeKind.Add), }, usePolling, () => File.Move(srcFile, dstFile)); @@ -184,8 +184,8 @@ public async Task FileInSubdirectory() await TestOperation( dir, expectedChanges: [ - (subdir, false), - (testFileFullPath, false) + (subdir, ChangeKind.Update), + (testFileFullPath, ChangeKind.Update) ], usePolling: true, () => File.WriteAllText(testFileFullPath, string.Empty)); @@ -270,7 +270,7 @@ public async Task MultipleFiles(bool usePolling) await TestOperation( dir, - expectedChanges: [(testFileFullPath, false)], + expectedChanges: [(testFileFullPath, ChangeKind.Update)], usePolling: true, () => File.WriteAllText(testFileFullPath, string.Empty)); } @@ -298,7 +298,7 @@ private async Task AssertFileChangeRaisesEvent(string directory, IFileSystemWatc { var changedEv = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var expectedPath = Path.Combine(directory, Path.GetRandomFileName()); - EventHandler<(string, bool)> handler = (_, f) => + EventHandler<(string, ChangeKind)> handler = (_, f) => { output.WriteLine("File changed: " + f); try @@ -356,10 +356,10 @@ public async Task DeleteSubfolder(bool usePolling) await TestOperation( dir, expectedChanges: [ - (subdir, false), - (f1, false), - (f2, false), - (f3, false), + (subdir, ChangeKind.Delete), + (f1, ChangeKind.Delete), + (f2, ChangeKind.Delete), + (f3, ChangeKind.Delete), ], usePolling: true, () => Directory.Delete(subdir, recursive: true)); diff --git a/test/dotnet-watch.Tests/MSBuildEvaluationFilterTest.cs b/test/dotnet-watch.Tests/MSBuildEvaluationFilterTest.cs index 0a64ca3f0c33..6b0aa27c851a 100644 --- a/test/dotnet-watch.Tests/MSBuildEvaluationFilterTest.cs +++ b/test/dotnet-watch.Tests/MSBuildEvaluationFilterTest.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.DotNet.Watcher.Internal; using Microsoft.Extensions.Tools.Internal; namespace Microsoft.DotNet.Watcher.Tools @@ -27,7 +28,7 @@ public async Task ProcessAsync_EvaluatesFileSetIfProjFileChanges() evaluator.RequiresRevaluation = false; - await evaluator.EvaluateAsync(changedFile: new() { FilePath = "Test.csproj" }, CancellationToken.None); + await evaluator.EvaluateAsync(changedFile: new(new() { FilePath = "Test.csproj" }, ChangeKind.Update), CancellationToken.None); Assert.True(evaluator.RequiresRevaluation); } @@ -51,7 +52,7 @@ public async Task ProcessAsync_DoesNotEvaluateFileSetIfNonProjFileChanges() evaluator.RequiresRevaluation = false; - await evaluator.EvaluateAsync(changedFile: new() { FilePath = "Controller.cs" }, CancellationToken.None); + await evaluator.EvaluateAsync(changedFile: new(new() { FilePath = "Controller.cs" }, ChangeKind.Update), CancellationToken.None); Assert.False(evaluator.RequiresRevaluation); Assert.Equal(1, counter); @@ -77,7 +78,7 @@ public async Task ProcessAsync_EvaluateFileSetOnEveryChangeIfOptimizationIsSuppr evaluator.RequiresRevaluation = false; - await evaluator.EvaluateAsync(changedFile: new() { FilePath = "Controller.cs" }, CancellationToken.None); + await evaluator.EvaluateAsync(changedFile: new(new() { FilePath = "Controller.cs" }, ChangeKind.Update), CancellationToken.None); Assert.True(evaluator.RequiresRevaluation); Assert.Equal(2, counter); @@ -118,7 +119,7 @@ public async Task ProcessAsync_SetsEvaluationRequired_IfMSBuildFileChanges_ButIs evaluator.RequiresRevaluation = false; evaluator.Timestamps["Proj.csproj"] = new DateTime(1007); - await evaluator.EvaluateAsync(new() { FilePath = "Controller.cs" }, CancellationToken.None); + await evaluator.EvaluateAsync(new(new() { FilePath = "Controller.cs" }, ChangeKind.Update), CancellationToken.None); Assert.True(evaluator.RequiresRevaluation); } From 178a36ee291b68cb3b73f42975984f14ad1a6422 Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Sun, 8 Sep 2024 13:24:02 -0700 Subject: [PATCH 03/11] Fix watcher --- .../Internal/HotReloadFileSetWatcher.cs | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/src/BuiltInTools/dotnet-watch/Internal/HotReloadFileSetWatcher.cs b/src/BuiltInTools/dotnet-watch/Internal/HotReloadFileSetWatcher.cs index 714a1e8538a0..76692d5381eb 100644 --- a/src/BuiltInTools/dotnet-watch/Internal/HotReloadFileSetWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/Internal/HotReloadFileSetWatcher.cs @@ -3,6 +3,7 @@ using System.Collections.Concurrent; +using System.Diagnostics; using Microsoft.Extensions.Tools.Internal; namespace Microsoft.DotNet.Watcher.Internal @@ -10,6 +11,7 @@ namespace Microsoft.DotNet.Watcher.Internal internal sealed class HotReloadFileSetWatcher(IReadOnlyDictionary fileSet, DateTime buildCompletionTime, IReporter reporter) : IDisposable { private static readonly TimeSpan s_debounceInterval = TimeSpan.FromMilliseconds(50); + private static readonly DateTime s_fileNotExistFileTime = DateTime.FromFileTime(0); private readonly FileWatcher _fileWatcher = new(fileSet, reporter); private readonly object _changedFilesLock = new(); @@ -90,23 +92,46 @@ void FileChangedCallback(string path, ChangeKind kind) return; } - try + if (kind != ChangeKind.Delete) { - // TODO: Deleted files will be ignored https://github.com/dotnet/sdk/issues/42535 - - // Do not report changes to files that happened during build: - var creationTime = File.GetCreationTimeUtc(path); - var writeTime = File.GetLastWriteTimeUtc(path); - if (creationTime.Ticks < buildCompletionTime.Ticks && writeTime.Ticks < buildCompletionTime.Ticks) + try + { + // Do not report changes to files that happened during build: + var creationTime = File.GetCreationTimeUtc(path); + var writeTime = File.GetLastWriteTimeUtc(path); + + if (creationTime == s_fileNotExistFileTime || writeTime == s_fileNotExistFileTime) + { + // file might have been deleted since we received the event + kind = ChangeKind.Delete; + } + else if (creationTime.Ticks < buildCompletionTime.Ticks && writeTime.Ticks < buildCompletionTime.Ticks) + { + reporter.Verbose( + $"Ignoring file change during build: {kind} '{path}' " + + $"(created {FormatTimestamp(creationTime)} and written {FormatTimestamp(writeTime)} before {FormatTimestamp(buildCompletionTime)})."); + + return; + } + else if (writeTime > creationTime) + { + reporter.Verbose($"File change: {kind} '{path}' (written {FormatTimestamp(writeTime)} after {FormatTimestamp(buildCompletionTime)})."); + } + else + { + reporter.Verbose($"File change: {kind} '{path}' (created {FormatTimestamp(creationTime)} after {FormatTimestamp(buildCompletionTime)})."); + } + } + catch (Exception e) { - reporter.Verbose($"Ignoring file updated during build: '{path}' ({FormatTimestamp(creationTime)},{FormatTimestamp(writeTime)} < {FormatTimestamp(buildCompletionTime)})."); + reporter.Verbose($"Ignoring file '{path}' due to access error: {e.Message}."); return; } } - catch (Exception e) + + if (kind == ChangeKind.Delete) { - reporter.Verbose($"Ignoring file '{path}' due to access error: {e.Message}."); - return; + reporter.Verbose($"File '{path}' deleted after {FormatTimestamp(buildCompletionTime)}."); } if (kind == ChangeKind.Add) @@ -142,6 +167,6 @@ void FileChangedCallback(string path, ChangeKind kind) } internal static string FormatTimestamp(DateTime time) - => time.ToString("yyyy-MM-dd HH:mm:ss.fffffff"); + => time.ToString("HH:mm:ss.fffffff"); } } From faed6f2f743b1c6de4dddcd58c50feb4d8556ac7 Mon Sep 17 00:00:00 2001 From: tmat Date: Mon, 9 Sep 2024 16:50:18 -0700 Subject: [PATCH 04/11] Log file watcher handler invocations --- test/dotnet-watch.Tests/FileWatcherTests.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/dotnet-watch.Tests/FileWatcherTests.cs b/test/dotnet-watch.Tests/FileWatcherTests.cs index 58dc1f643226..dbb674b32568 100644 --- a/test/dotnet-watch.Tests/FileWatcherTests.cs +++ b/test/dotnet-watch.Tests/FileWatcherTests.cs @@ -39,7 +39,14 @@ private async Task TestOperation( EventHandler<(string path, ChangeKind kind)> handler = null; handler = (_, f) => { - filesChanged.Add(f); + if (filesChanged.Add(f)) + { + output.WriteLine($"Observed new {f.kind}: '{f.path}' ({filesChanged.Count} out of {expectedChanges.Length})"); + } + else + { + output.WriteLine($"Already seen {f.kind}: '{f.path}'"); + } if (filesChanged.Count == expectedChanges.Length) { From 2ad1cb1ace2e596f1bf25a82668b24aa9ced77fc Mon Sep 17 00:00:00 2001 From: tmat Date: Tue, 10 Sep 2024 08:06:32 -0700 Subject: [PATCH 05/11] Fix dir handling in polling watcher --- .../Internal/FileWatcher/PollingFileWatcher.cs | 7 ++++--- test/dotnet-watch.Tests/FileWatcherTests.cs | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs index d54975f070af..2df2004ee84b 100644 --- a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/PollingFileWatcher.cs @@ -106,7 +106,7 @@ private void CheckForChangedFiles() if (!_knownEntities.ContainsKey(fullFilePath)) { - // New file + // New file or directory RecordChange(f, ChangeKind.Add); } else @@ -115,7 +115,8 @@ private void CheckForChangedFiles() try { - if (fileMeta.FileInfo.LastWriteTime != f.LastWriteTime) + if (!fileMeta.FileInfo.Attributes.HasFlag(FileAttributes.Directory) && + fileMeta.FileInfo.LastWriteTime != f.LastWriteTime) { // File changed RecordChange(f, ChangeKind.Update); @@ -136,7 +137,7 @@ private void CheckForChangedFiles() { if (!file.Value.FoundAgain) { - // File deleted + // File or directory deleted RecordChange(file.Value.FileInfo, ChangeKind.Delete); } } diff --git a/test/dotnet-watch.Tests/FileWatcherTests.cs b/test/dotnet-watch.Tests/FileWatcherTests.cs index dbb674b32568..3590ec72f26e 100644 --- a/test/dotnet-watch.Tests/FileWatcherTests.cs +++ b/test/dotnet-watch.Tests/FileWatcherTests.cs @@ -120,7 +120,6 @@ await TestOperation( : new[] { (newDir, ChangeKind.Add), - (newFile, ChangeKind.Add), }, usePolling, () => @@ -362,13 +361,14 @@ public async Task DeleteSubfolder(bool usePolling) await TestOperation( dir, - expectedChanges: [ + expectedChanges: + [ (subdir, ChangeKind.Delete), (f1, ChangeKind.Delete), (f2, ChangeKind.Delete), (f3, ChangeKind.Delete), ], - usePolling: true, + usePolling, () => Directory.Delete(subdir, recursive: true)); } } From e21388554f143431d9837947a2060c9705600b31 Mon Sep 17 00:00:00 2001 From: tmat Date: Tue, 10 Sep 2024 09:03:45 -0700 Subject: [PATCH 06/11] Tests --- .../HotReload/ApplyDeltaTests.cs | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index 53089a584dba..fed879ede8b1 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -120,5 +120,112 @@ public async Task BlazorWasm() //UpdateSourceFile(Path.Combine(testAsset.Path, "Pages", "Index.razor"), newSource); //await App.AssertOutputLineStartsWith(MessageDescriptor.HotReloadSucceeded); } + + [Theory] + [InlineData(true, Skip = "https://github.com/dotnet/sdk/issues/43320")] + [InlineData(false)] + public async Task RenameSourceFile(bool useMove) + { + Logger.WriteLine("RenameSourceFile started"); + + var testAsset = TestAssets.CopyTestAsset("WatchAppWithProjectDeps") + .WithSource(); + + var dependencyDir = Path.Combine(testAsset.Path, "Dependency"); + var oldFilePath = Path.Combine(dependencyDir, "Foo.cs"); + var newFilePath = Path.Combine(dependencyDir, "Renamed.cs"); + + var source = """ + using System; + using System.IO; + using System.Runtime.CompilerServices; + + public class Lib + { + public static void Print() => PrintFileName(); + + public static void PrintFileName([CallerFilePathAttribute] string filePath = null) + { + Console.WriteLine($"> {Path.GetFileName(filePath)}"); + } + } + """; + + File.WriteAllText(oldFilePath, source); + + App.Start(testAsset, [], "AppWithDeps"); + + await App.AssertWaitingForChanges(); + + // rename the file: + if (useMove) + { + File.Move(oldFilePath, newFilePath); + } + else + { + File.Delete(oldFilePath); + File.WriteAllText(newFilePath, source); + } + + Logger.WriteLine($"Renamed '{oldFilePath}' to '{newFilePath}'."); + + await App.AssertOutputLineStartsWith("> Renamed.cs"); + } + + [Theory] + [InlineData(true, Skip = "https://github.com/dotnet/sdk/issues/43320")] + [InlineData(false)] + public async Task RenameDirectory(bool useMove) + { + Logger.WriteLine("RenameSourceFile started"); + + var testAsset = TestAssets.CopyTestAsset("WatchAppWithProjectDeps") + .WithSource(); + + var dependencyDir = Path.Combine(testAsset.Path, "Dependency"); + var oldSubdir = Path.Combine(dependencyDir, "Subdir"); + var newSubdir = Path.Combine(dependencyDir, "NewSubdir"); + + var source = """ + using System; + using System.IO; + using System.Runtime.CompilerServices; + + public class Lib + { + public static void Print() => PrintDirectoryName(); + + public static void PrintDirectoryName([CallerFilePathAttribute] string filePath = null) + { + Console.WriteLine($"> {Path.GetFileName(Path.GetDirectoryName(filePath))}"); + } + } + """; + + File.Delete(Path.Combine(dependencyDir, "Foo.cs")); + Directory.CreateDirectory(oldSubdir); + File.WriteAllText(Path.Combine(oldSubdir, "Foo.cs"), source); + + App.Start(testAsset, [], "AppWithDeps"); + + await App.AssertWaitingForChanges(); + + // rename the file: + if (useMove) + { + Directory.Move(oldSubdir, newSubdir); + } + else + { + Directory.Delete(oldSubdir, recursive: true); + Directory.CreateDirectory(newSubdir); + File.WriteAllText(Path.Combine(newSubdir, "Foo.cs"), source); + } + + Logger.WriteLine($"Renamed '{oldSubdir}' to '{newSubdir}'."); + + await App.AssertOutputLineStartsWith("> NewSubdir"); + } } } From 85de2aebd666324fd0868cfbe88e4e55a976cf30 Mon Sep 17 00:00:00 2001 From: tmat Date: Tue, 10 Sep 2024 09:04:11 -0700 Subject: [PATCH 07/11] Comment --- test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index fed879ede8b1..810eeb98521e 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -211,7 +211,7 @@ public static void PrintDirectoryName([CallerFilePathAttribute] string filePath await App.AssertWaitingForChanges(); - // rename the file: + // rename the directory: if (useMove) { Directory.Move(oldSubdir, newSubdir); From d5fde372097b975b9119fbed9f8a7495ff58dea9 Mon Sep 17 00:00:00 2001 From: tmat Date: Tue, 10 Sep 2024 09:52:54 -0700 Subject: [PATCH 08/11] Use case-insensitive path comparison on Windows --- src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs b/src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs index 0941b5b89621..86dc964538cc 100644 --- a/src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs +++ b/src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs @@ -26,7 +26,8 @@ public static void Initialize() // When launching the application process dotnet-watch sets Hot Reload environment variables via CLI environment directives (dotnet [env:X=Y] run). // Currently, the CLI parser sets the env variables to the dotnet.exe process itself, rather then to the target process. // This may cause the dotnet.exe process to connect to the named pipe and break it for the target process. - if (Path.ChangeExtension(processPath, ".exe") != Path.ChangeExtension(s_targetProcessPath, ".exe")) + if (string.Equals(Path.ChangeExtension(processPath, ".exe"), Path.ChangeExtension(s_targetProcessPath, ".exe"), + Path.PathSeparator == '\\' ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal)) { Log($"Ignoring process '{processPath}', expecting '{s_targetProcessPath}'"); return; From ecb3eeafee88f50de80d0be4e21b2a70cbe17e13 Mon Sep 17 00:00:00 2001 From: tmat Date: Tue, 10 Sep 2024 12:01:09 -0700 Subject: [PATCH 09/11] Fix delete handler --- .../Internal/FileWatcher/DotnetFileWatcher.cs | 15 ++++++- test/dotnet-watch.Tests/FileWatcherTests.cs | 42 ++++++++++--------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs index 57d3610adb43..cee2aee99df0 100644 --- a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs @@ -95,6 +95,17 @@ private void WatcherRenameHandler(object sender, RenamedEventArgs e) } } + private void WatcherDeletedHandler(object sender, FileSystemEventArgs e) + { + if (_disposed) + { + return; + } + + Logger?.Invoke("Deleted"); + NotifyChange(e.FullPath, ChangeKind.Delete); + } + private void WatcherChangeHandler(object sender, FileSystemEventArgs e) { if (_disposed) @@ -140,7 +151,7 @@ private void CreateFileSystemWatcher() _fileSystemWatcher.IncludeSubdirectories = true; _fileSystemWatcher.Created += WatcherAddedHandler; - _fileSystemWatcher.Deleted += WatcherChangeHandler; + _fileSystemWatcher.Deleted += WatcherDeletedHandler; _fileSystemWatcher.Changed += WatcherChangeHandler; _fileSystemWatcher.Renamed += WatcherRenameHandler; _fileSystemWatcher.Error += WatcherErrorHandler; @@ -156,7 +167,7 @@ private void DisposeInnerWatcher() _fileSystemWatcher.EnableRaisingEvents = false; _fileSystemWatcher.Created -= WatcherAddedHandler; - _fileSystemWatcher.Deleted -= WatcherChangeHandler; + _fileSystemWatcher.Deleted -= WatcherDeletedHandler; _fileSystemWatcher.Changed -= WatcherChangeHandler; _fileSystemWatcher.Renamed -= WatcherRenameHandler; _fileSystemWatcher.Error -= WatcherErrorHandler; diff --git a/test/dotnet-watch.Tests/FileWatcherTests.cs b/test/dotnet-watch.Tests/FileWatcherTests.cs index 3590ec72f26e..30a0509242eb 100644 --- a/test/dotnet-watch.Tests/FileWatcherTests.cs +++ b/test/dotnet-watch.Tests/FileWatcherTests.cs @@ -18,12 +18,10 @@ private async Task TestOperation( bool usePolling, Action operation) { - // On Unix the native file watcher may surface events from - // the recent past. Delay to avoid those. - // On Unix the file write time is in 1s increments; - // if we don't wait, there's a chance that the polling - // watcher will not detect the change - await Task.Delay(1250); + // Used to filter all events prior to this write. + // On Unix the native file watcher may surface events from the recent past. + var barrierFileName = ".filewatcher"; + var barrier = File.Create(Path.Combine(dir, barrierFileName)); using var watcher = FileWatcherFactory.CreateWatcher(dir, usePolling); if (watcher is DotnetFileWatcher dotnetWatcher) @@ -34,11 +32,16 @@ private async Task TestOperation( var changedEv = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var filesChanged = new HashSet<(string path, ChangeKind kind)>(); - var testFileFullPath = Path.Combine(dir, "foo"); - EventHandler<(string path, ChangeKind kind)> handler = null; handler = (_, f) => { + if (Path.GetFileName(f.path) == barrierFileName) + { + filesChanged.Clear(); + output.WriteLine("Observed barrier"); + return; + } + if (filesChanged.Add(f)) { output.WriteLine($"Observed new {f.kind}: '{f.path}' ({filesChanged.Count} out of {expectedChanges.Length})"); @@ -159,18 +162,11 @@ public async Task MoveFile(bool usePolling) await TestOperation( dir, - expectedChanges: RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && !usePolling - ? new[] - { - (srcFile, ChangeKind.Update), - (srcFile, ChangeKind.Delete), - (dstFile, ChangeKind.Add), - } - : new[] - { + expectedChanges: + [ (srcFile, ChangeKind.Delete), (dstFile, ChangeKind.Add), - }, + ], usePolling, () => File.Move(srcFile, dstFile)); @@ -361,12 +357,20 @@ public async Task DeleteSubfolder(bool usePolling) await TestOperation( dir, - expectedChanges: + expectedChanges: usePolling ? [ (subdir, ChangeKind.Delete), (f1, ChangeKind.Delete), (f2, ChangeKind.Delete), (f3, ChangeKind.Delete), + ] + : + [ + (subdir, ChangeKind.Update), + (subdir, ChangeKind.Delete), + (f1, ChangeKind.Delete), + (f2, ChangeKind.Delete), + (f3, ChangeKind.Delete), ], usePolling, () => Directory.Delete(subdir, recursive: true)); From 7ecbe10a86da53c1cd96d7d6eb58c464e05cf872 Mon Sep 17 00:00:00 2001 From: tmat Date: Tue, 10 Sep 2024 12:45:35 -0700 Subject: [PATCH 10/11] Enable tests --- .../dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs | 7 +++---- .../HotReload/CompilationHandlerTests.cs | 2 +- .../HotReload/RuntimeProcessLauncherTests.cs | 2 +- test/dotnet-watch.Tests/Watch/GlobbingAppTests.cs | 13 ++++++------- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs index 810eeb98521e..254962001015 100644 --- a/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs +++ b/test/dotnet-watch.Tests/HotReload/ApplyDeltaTests.cs @@ -7,7 +7,7 @@ namespace Microsoft.DotNet.Watcher.Tests { public class ApplyDeltaTests(ITestOutputHelper logger) : DotNetWatchTestBase(logger) { - [Fact(Skip = "https://github.com/dotnet/sdk/issues/42850")] + [Fact] public async Task AddSourceFile() { Logger.WriteLine("AddSourceFile started"); @@ -42,7 +42,7 @@ public static void Print() await App.AssertOutputLineStartsWith("Changed!"); } - [Fact(Skip = "https://github.com/dotnet/sdk/issues/42850")] + [Fact] public async Task ChangeFileInDependency() { var testAsset = TestAssets.CopyTestAsset("WatchAppWithProjectDeps") @@ -67,8 +67,7 @@ public static void Print() await App.AssertOutputLineStartsWith("Changed!"); } - // Test is timing out on .NET Framework: https://github.com/dotnet/sdk/issues/41669 - [CoreMSBuildOnlyFact(Skip = "https://github.com/dotnet/sdk/issues/42850")] + [Fact] public async Task HandleTypeLoadFailure() { var testAsset = TestAssets.CopyTestAsset("WatchAppTypeLoadFailure") diff --git a/test/dotnet-watch.Tests/HotReload/CompilationHandlerTests.cs b/test/dotnet-watch.Tests/HotReload/CompilationHandlerTests.cs index fd7ad999968a..3187af62bf10 100644 --- a/test/dotnet-watch.Tests/HotReload/CompilationHandlerTests.cs +++ b/test/dotnet-watch.Tests/HotReload/CompilationHandlerTests.cs @@ -8,7 +8,7 @@ namespace Microsoft.DotNet.Watcher.Tests; public class CompilationHandlerTests(ITestOutputHelper logger) : DotNetWatchTestBase(logger) { - [Fact(Skip = "https://github.com/dotnet/sdk/issues/42850")] + [Fact] public async Task ReferenceOutputAssembly_False() { var testAsset = TestAssets.CopyTestAsset("WatchAppMultiProc") diff --git a/test/dotnet-watch.Tests/HotReload/RuntimeProcessLauncherTests.cs b/test/dotnet-watch.Tests/HotReload/RuntimeProcessLauncherTests.cs index 6c6c7a8c0e3f..f56d7bcc42eb 100644 --- a/test/dotnet-watch.Tests/HotReload/RuntimeProcessLauncherTests.cs +++ b/test/dotnet-watch.Tests/HotReload/RuntimeProcessLauncherTests.cs @@ -225,7 +225,7 @@ async Task MakeRudeEditChange() } } - [Theory(Skip = "https://github.com/dotnet/sdk/issues/42850")] + [Theory] [CombinatorialData] public async Task UpdateAppliedToNewProcesses(bool sharedOutput) { diff --git a/test/dotnet-watch.Tests/Watch/GlobbingAppTests.cs b/test/dotnet-watch.Tests/Watch/GlobbingAppTests.cs index 6b44db35472f..a51ebb729933 100644 --- a/test/dotnet-watch.Tests/Watch/GlobbingAppTests.cs +++ b/test/dotnet-watch.Tests/Watch/GlobbingAppTests.cs @@ -14,9 +14,8 @@ public GlobbingAppTests(ITestOutputHelper logger) { } - [ConditionalTheory(Skip = "https://github.com/dotnet/sdk/issues/42921")] - [InlineData(true)] - [InlineData(false)] + [Theory] + [CombinatorialData] public async Task ChangeCompiledFile(bool usePollingWatcher) { var testAsset = TestAssets.CopyTestAsset(AppName, identifier: usePollingWatcher.ToString()) @@ -36,7 +35,7 @@ public async Task ChangeCompiledFile(bool usePollingWatcher) await AssertCompiledAppDefinedTypes(expected: 2); } - [Fact(Skip = "https://github.com/dotnet/sdk/issues/42921")] + [Fact] public async Task DeleteCompiledFile() { var testAsset = TestAssets.CopyTestAsset(AppName) @@ -53,7 +52,7 @@ public async Task DeleteCompiledFile() await AssertCompiledAppDefinedTypes(expected: 1); } - [Fact(Skip = "https://github.com/dotnet/sdk/issues/42921")] + [Fact] public async Task DeleteSourceFolder() { var testAsset = TestAssets.CopyTestAsset(AppName) @@ -70,7 +69,7 @@ public async Task DeleteSourceFolder() await AssertCompiledAppDefinedTypes(expected: 1); } - [Fact(Skip = "https://github.com/dotnet/sdk/issues/42921")] + [Fact] public async Task RenameCompiledFile() { var testAsset = TestAssets.CopyTestAsset(AppName) @@ -87,7 +86,7 @@ public async Task RenameCompiledFile() await App.AssertStarted(); } - [Fact(Skip = "https://github.com/dotnet/sdk/issues/42921")] + [Fact] public async Task ChangeExcludedFile() { var testAsset = TestAssets.CopyTestAsset(AppName) From 4021056cfc29240d1992237d6034a1cb95f09c73 Mon Sep 17 00:00:00 2001 From: tmat Date: Tue, 10 Sep 2024 13:43:38 -0700 Subject: [PATCH 11/11] Fix --- .../Internal/FileWatcher/DotnetFileWatcher.cs | 8 ++-- test/dotnet-watch.Tests/FileWatcherTests.cs | 41 +++++++++++-------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs index cee2aee99df0..7040fe1a0763 100644 --- a/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/Internal/FileWatcher/DotnetFileWatcher.cs @@ -78,7 +78,7 @@ private void WatcherRenameHandler(object sender, RenamedEventArgs e) return; } - Logger?.Invoke("Rename"); + Logger?.Invoke($"Renamed '{e.OldFullPath}' to '{e.FullPath}'."); NotifyChange(e.OldFullPath, ChangeKind.Delete); NotifyChange(e.FullPath, ChangeKind.Add); @@ -102,7 +102,7 @@ private void WatcherDeletedHandler(object sender, FileSystemEventArgs e) return; } - Logger?.Invoke("Deleted"); + Logger?.Invoke($"Deleted '{e.FullPath}'."); NotifyChange(e.FullPath, ChangeKind.Delete); } @@ -113,7 +113,7 @@ private void WatcherChangeHandler(object sender, FileSystemEventArgs e) return; } - Logger?.Invoke("Change"); + Logger?.Invoke($"Updated '{e.FullPath}'."); NotifyChange(e.FullPath, ChangeKind.Update); } @@ -124,7 +124,7 @@ private void WatcherAddedHandler(object sender, FileSystemEventArgs e) return; } - Logger?.Invoke("Added"); + Logger?.Invoke($"Added '{e.FullPath}'."); NotifyChange(e.FullPath, ChangeKind.Add); } diff --git a/test/dotnet-watch.Tests/FileWatcherTests.cs b/test/dotnet-watch.Tests/FileWatcherTests.cs index 30a0509242eb..3a4c7b4d4108 100644 --- a/test/dotnet-watch.Tests/FileWatcherTests.cs +++ b/test/dotnet-watch.Tests/FileWatcherTests.cs @@ -18,11 +18,6 @@ private async Task TestOperation( bool usePolling, Action operation) { - // Used to filter all events prior to this write. - // On Unix the native file watcher may surface events from the recent past. - var barrierFileName = ".filewatcher"; - var barrier = File.Create(Path.Combine(dir, barrierFileName)); - using var watcher = FileWatcherFactory.CreateWatcher(dir, usePolling); if (watcher is DotnetFileWatcher dotnetWatcher) { @@ -35,13 +30,6 @@ private async Task TestOperation( EventHandler<(string path, ChangeKind kind)> handler = null; handler = (_, f) => { - if (Path.GetFileName(f.path) == barrierFileName) - { - filesChanged.Clear(); - output.WriteLine("Observed barrier"); - return; - } - if (filesChanged.Add(f)) { output.WriteLine($"Observed new {f.kind}: '{f.path}' ({filesChanged.Count} out of {expectedChanges.Length})"); @@ -150,8 +138,7 @@ await TestOperation( } [Theory] - [InlineData(true)] - [InlineData(false)] + [CombinatorialData] public async Task MoveFile(bool usePolling) { var dir = _testAssetManager.CreateTestDirectory(identifier: usePolling.ToString()).Path; @@ -162,7 +149,15 @@ public async Task MoveFile(bool usePolling) await TestOperation( dir, - expectedChanges: + expectedChanges: !RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && !usePolling ? + [ + // On Linux/OSX events from before we started observing are reported as well. + (srcFile, ChangeKind.Update), + (srcFile, ChangeKind.Add), + (srcFile, ChangeKind.Delete), + (dstFile, ChangeKind.Add), + ] + : [ (srcFile, ChangeKind.Delete), (dstFile, ChangeKind.Add), @@ -364,12 +359,26 @@ await TestOperation( (f2, ChangeKind.Delete), (f3, ChangeKind.Delete), ] - : + : RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? + [ + (subdir, ChangeKind.Update), + (subdir, ChangeKind.Delete), + (f1, ChangeKind.Delete), + (f2, ChangeKind.Delete), + (f3, ChangeKind.Delete), + ] + : // On Linux/OSX events from before we started observing are reported as well. [ (subdir, ChangeKind.Update), (subdir, ChangeKind.Delete), + (f1, ChangeKind.Update), + (f1, ChangeKind.Add), (f1, ChangeKind.Delete), + (f2, ChangeKind.Update), + (f2, ChangeKind.Add), (f2, ChangeKind.Delete), + (f3, ChangeKind.Update), + (f3, ChangeKind.Add), (f3, ChangeKind.Delete), ], usePolling,