Skip to content

Commit

Permalink
Fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
tmat committed Dec 4, 2024
1 parent 18e3fd5 commit c489432
Show file tree
Hide file tree
Showing 11 changed files with 225 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ private async ValueTask DisplayResultsAsync(WatchHotReloadService.Updates update
switch (updates.Status)
{
case ModuleUpdateStatus.None:
_reporter.Output("No hot reload changes to apply.");
_reporter.Report(MessageDescriptor.NoHotReloadChangesToApply);
break;

case ModuleUpdateStatus.Ready:
Expand Down
65 changes: 40 additions & 25 deletions src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke
var scopedCssFileHandler = new ScopedCssFileHandler(Context.Reporter, projectMap, browserConnector);
var projectLauncher = new ProjectLauncher(Context, projectMap, browserConnector, compilationHandler, iteration);
var outputDirectories = GetProjectOutputDirectories(evaluationResult.ProjectGraph);
var changeFilter = new Predicate<ChangedPath>(change => AcceptChange(change, evaluationResult, outputDirectories));

var rootProjectNode = evaluationResult.ProjectGraph.GraphRoots.Single();

Expand Down Expand Up @@ -167,7 +168,6 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke
return;
}

var buildCompletionTime = DateTime.UtcNow;
await compilationHandler.Workspace.UpdateProjectConeAsync(RootFileSetFactory.RootProjectFile, iterationCancellationToken);

// Solution must be initialized after we load the solution but before we start watching for file changes to avoid race condition
Expand All @@ -187,7 +187,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke

void FileChangedCallback(ChangedPath change)
{
if (AcceptChange(change, evaluationResult, outputDirectories, buildCompletionTime))
if (changeFilter(change))
{
Context.Reporter.Verbose($"File change: {change.Kind} '{change.Path}'.");
ImmutableInterlocked.Update(ref changedFilesAccumulator, changedPaths => changedPaths.Add(change));
Expand Down Expand Up @@ -328,7 +328,7 @@ void FileChangedCallback(ChangedPath change)
iterationCancellationToken.ThrowIfCancellationRequested();

// pause accumulating file changes during build:
fileWatcher.OnFileChange -= fileChangedCallback;
fileWatcher.SuppressEvents = true;
try
{
var buildResults = await Task.WhenAll(
Expand All @@ -341,19 +341,17 @@ void FileChangedCallback(ChangedPath change)
}
finally
{
fileWatcher.OnFileChange += fileChangedCallback;
fileWatcher.SuppressEvents = false;
}

iterationCancellationToken.ThrowIfCancellationRequested();

_ = await fileWatcher.WaitForFileChangeAsync(
changeFilter,
startedWatching: () => Context.Reporter.Report(MessageDescriptor.FixBuildError),
shutdownCancellationToken);
}

// Update build completion time, so that file changes caused by the rebuild do not affect our file watcher:
buildCompletionTime = DateTime.UtcNow;

// Changes made since last snapshot of the accumulator shouldn't be included in next Hot Reload update.
// Apply them to the workspace.
_ = await CaptureChangedFilesSnapshot(projectsToRebuild);
Expand Down Expand Up @@ -413,7 +411,7 @@ async Task<ImmutableList<ChangedFile>> CaptureChangedFilesSnapshot(ImmutableDict

if (hasAddedFile)
{
Context.Reporter.Verbose("File addition triggered re-evaluation.");
Context.Reporter.Report(MessageDescriptor.FileAdditionTriggeredReEvaluation);

evaluationResult = await EvaluateRootProjectAsync(iterationCancellationToken);

Expand Down Expand Up @@ -559,28 +557,23 @@ private async ValueTask WaitForFileChangeBeforeRestarting(FileWatcher fileWatche
fileWatcher.WatchContainingDirectories([RootFileSetFactory.RootProjectFile]);

_ = await fileWatcher.WaitForFileChangeAsync(
acceptChange: change => AcceptChange(change),
startedWatching: () => Context.Reporter.Report(MessageDescriptor.WaitingForFileChangeBeforeRestarting),
cancellationToken);
}
}

private bool AcceptChange(ChangedPath change, EvaluationResult evaluationResult, IReadOnlySet<string> outputDirectories, DateTime buildCompletionTime)
private bool AcceptChange(ChangedPath change, EvaluationResult evaluationResult, IReadOnlySet<string> outputDirectories)
{
var (path, kind) = change;

// only handle file changes:
if (Directory.Exists(path))
{
return false;
}

// Handler changes to files that are known to be project build inputs from its evaluation.
if (evaluationResult.Files.ContainsKey(path))
{
return true;
}

// Ignore other chnages output and intermediate output directories.
// Ignore other changes to output and intermediate output directories.
//
// Unsupported scenario:
// - msbuild target adds source files to intermediate output directory and Compile items
Expand All @@ -590,21 +583,36 @@ private bool AcceptChange(ChangedPath change, EvaluationResult evaluationResult,
// since the changes to additional file will trigger workspace update, which will trigger the source generator.
if (PathUtilities.ContainsPath(outputDirectories, path))
{
Context.Reporter.Verbose($"Ignoring change in output directory: {kind} '{path}'.");
Context.Reporter.Report(MessageDescriptor.IgnoringChangeInOutputDirectory, kind, path);
return false;
}

// Visual Studio is frequently creating new files in the .vs directory next to the solution.
// These would trigger undesirable reevaluations.
if (PathUtilities.HasDirectoryInPath(path, ".vs", PathUtilities.OSSpecificPathComparer))
return AcceptChange(change);
}

private bool AcceptChange(ChangedPath change)
{
var (path, kind) = change;

// only handle file changes:
if (Directory.Exists(path))
{
return false;
}

if (PathUtilities.GetContainingDirectories(path).FirstOrDefault(IsHiddenPath) is { } containingHiddenDir)
{
Context.Reporter.Verbose($"Ignoring change in .vs directory: {kind} '{path}'.");
Context.Reporter.Report(MessageDescriptor.IgnoringChangeInHiddenDirectory, containingHiddenDir, kind, path);
return false;
}

return true;
}

private static bool IsHiddenPath(string path)
// Note: the device root directory on Windows has hidden attribute:
=> File.GetAttributes(path).HasFlag(FileAttributes.Hidden) && Path.GetDirectoryName(path) != null;

internal static string FormatTimestamp(DateTime time)
=> time.ToString("HH:mm:ss.fffffff");

Expand All @@ -614,8 +622,15 @@ private static IReadOnlySet<string> GetProjectOutputDirectories(ProjectGraph pro

foreach (var projectNode in projectGraph.ProjectNodes)
{
projectOutputDirectories.Add(Path.TrimEndingDirectorySeparator(projectNode.GetOutputDirectory()));
projectOutputDirectories.Add(Path.TrimEndingDirectorySeparator(projectNode.GetIntermediateOutputDirectory()));
if (projectNode.GetOutputDirectory() is { } outputDir)
{
projectOutputDirectories.Add(Path.TrimEndingDirectorySeparator(outputDir));
}

if (projectNode.GetIntermediateOutputDirectory() is { } intermediateDir)
{
projectOutputDirectories.Add(Path.TrimEndingDirectorySeparator(intermediateDir));
}
}

return projectOutputDirectories;
Expand Down Expand Up @@ -643,14 +658,14 @@ internal static IEnumerable<ChangedPath> NormalizePathChanges(IEnumerable<Change

if (item.Kind == ChangeKind.Add)
{
// eliminate delete-add:
// eliminate delete-(update)*-add:
if (lastDelete.HasValue)
{
lastDelete = null;
lastAdd = null;
lastUpdate ??= item with { Kind = ChangeKind.Update };
}
else
else if (!lastUpdate.HasValue) // ignore add after update, but only if it is not preceded by delete
{
lastAdd = item;
}
Expand Down
21 changes: 10 additions & 11 deletions src/BuiltInTools/dotnet-watch/Internal/FileWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ internal sealed class FileWatcher(IReporter reporter) : IDisposable
private bool _disposed;
public event Action<ChangedPath>? OnFileChange;

public bool SuppressEvents { get; set; }

public void Dispose()
{
if (_disposed)
Expand Down Expand Up @@ -80,7 +82,10 @@ private void WatcherErrorHandler(object? sender, Exception error)

private void WatcherChangedHandler(object? sender, ChangedPath change)
{
OnFileChange?.Invoke(change);
if (!SuppressEvents)
{
OnFileChange?.Invoke(change);
}
}

private void DisposeWatcher(string directory)
Expand All @@ -98,30 +103,24 @@ private void DisposeWatcher(string directory)
private static string EnsureTrailingSlash(string path)
=> (path is [.., var last] && last != Path.DirectorySeparatorChar) ? path + Path.DirectorySeparatorChar : path;

public Task<ChangedPath?> WaitForFileChangeAsync(Action? startedWatching, CancellationToken cancellationToken)
=> WaitForFileChangeAsync(
changeFilter: _ => true,
startedWatching,
cancellationToken);

public async Task<ChangedFile?> WaitForFileChangeAsync(IReadOnlyDictionary<string, FileItem> fileSet, Action? startedWatching, CancellationToken cancellationToken)
{
var changedPath = await WaitForFileChangeAsync(
changeFilter: change => fileSet.ContainsKey(change.Path),
acceptChange: change => fileSet.ContainsKey(change.Path),
startedWatching,
cancellationToken);

return changedPath.HasValue ? new ChangedFile(fileSet[changedPath.Value.Path], changedPath.Value.Kind) : null;
}

public async Task<ChangedPath?> WaitForFileChangeAsync(Func<ChangedPath, bool> changeFilter, Action? startedWatching, CancellationToken cancellationToken)
public async Task<ChangedPath?> WaitForFileChangeAsync(Predicate<ChangedPath> acceptChange, Action? startedWatching, CancellationToken cancellationToken)
{
var fileChangedSource = new TaskCompletionSource<ChangedPath?>(TaskCreationOptions.RunContinuationsAsynchronously);
cancellationToken.Register(() => fileChangedSource.TrySetResult(null));

void FileChangedCallback(ChangedPath change)
{
if (changeFilter(change))
if (acceptChange(change))
{
fileChangedSource.TrySetResult(change);
}
Expand Down Expand Up @@ -150,7 +149,7 @@ public static async ValueTask WaitForFileChangeAsync(string filePath, IReporter
watcher.WatchDirectories([Path.GetDirectoryName(filePath)!]);

var fileChange = await watcher.WaitForFileChangeAsync(
changeFilter: change => change.Path == filePath,
acceptChange: change => change.Path == filePath,
startedWatching,
cancellationToken);

Expand Down
4 changes: 4 additions & 0 deletions src/BuiltInTools/dotnet-watch/Internal/IReporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ public bool TryGetMessage(string? prefix, object?[] args, [NotNullWhen(true)] ou
public static readonly MessageDescriptor ApplyUpdate_FileContentDoesNotMatchBuiltSource = new("{0} Expected if a source file is updated that is linked to project whose build is not up-to-date.", "⌚", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor ConfiguredToLaunchBrowser = new("dotnet-watch is configured to launch a browser on ASP.NET Core application startup.", "⌚", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor ConfiguredToUseBrowserRefresh = new("Configuring the app to use browser-refresh middleware", "⌚", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor IgnoringChangeInHiddenDirectory = new("Ignoring change in hidden directory '{0}': {1} '{2}'", "⌚", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor IgnoringChangeInOutputDirectory = new("Ignoring change in output directory: {0} '{1}'", "⌚", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor FileAdditionTriggeredReEvaluation = new("File addition triggered re-evaluation.", "⌚", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor NoHotReloadChangesToApply = new ("No hot reload changes to apply.", "⌚", MessageSeverity.Output, s_id++);
}

internal interface IReporter
Expand Down
16 changes: 6 additions & 10 deletions src/BuiltInTools/dotnet-watch/Utilities/PathUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,19 @@ public static bool ContainsPath(IReadOnlySet<string> directories, string fullPat
fullPath = containingDir;
}
}
public static bool HasDirectoryInPath(string filePath, string directoryName, IEqualityComparer<string> comparer)

public static IEnumerable<string> GetContainingDirectories(string path)
{
while (true)
{
var containingDir = Path.GetDirectoryName(filePath);
var containingDir = Path.GetDirectoryName(path);
if (containingDir == null)
{
return false;
}

var dirName = Path.GetFileName(containingDir);
if (comparer.Equals(dirName, directoryName))
{
return true;
yield break;
}

filePath = containingDir;
yield return containingDir;
path = containingDir;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ public static bool IsNetCoreApp(this ProjectGraphNode projectNode)
public static bool IsNetCoreApp(this ProjectGraphNode projectNode, Version minVersion)
=> IsNetCoreApp(projectNode) && IsTargetFrameworkVersionOrNewer(projectNode, minVersion);

public static string GetOutputDirectory(this ProjectGraphNode projectNode)
=> Path.Combine(projectNode.ProjectInstance.Directory, projectNode.ProjectInstance.GetPropertyValue("TargetPath"));
public static string? GetOutputDirectory(this ProjectGraphNode projectNode)
=> projectNode.ProjectInstance.GetPropertyValue("TargetPath") is { Length: >0 } path ? Path.GetDirectoryName(Path.Combine(projectNode.ProjectInstance.Directory, path)) : null;

public static string GetIntermediateOutputDirectory(this ProjectGraphNode projectNode)
=> Path.Combine(projectNode.ProjectInstance.Directory, projectNode.ProjectInstance.GetPropertyValue("IntermediateOutputPath"));
public static string? GetIntermediateOutputDirectory(this ProjectGraphNode projectNode)
=> projectNode.ProjectInstance.GetPropertyValue("IntermediateOutputPath") is { Length: >0 } path ? Path.Combine(projectNode.ProjectInstance.Directory, path) : null;

public static IEnumerable<string> GetCapabilities(this ProjectGraphNode projectNode)
=> projectNode.ProjectInstance.GetItems("ProjectCapability").Select(item => item.EvaluatedInclude);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,8 @@
<Warning Text="The value of property is '$(TestProperty)'" />
</Target>

<ItemGroup>
<!-- add item -->
</ItemGroup>

</Project>
8 changes: 8 additions & 0 deletions test/dotnet-watch.Tests/Directory.Build.targets
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!-- Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the MIT license. See License.txt in the project root for full license information. -->
<Project>
<Import Project="..\Directory.Build.targets" />

<!-- Workaround for https://github.com/dotnet/msbuild/issues/9709 -->
<Target Name="IncrementalClean" />

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,16 @@ public class HotReloadDotNetWatcherTests

[InlineData(new[] { ChangeKind.Add, ChangeKind.Delete, ChangeKind.Add, ChangeKind.Delete }, new ChangeKind[] { })]
[InlineData(new[] { ChangeKind.Update, ChangeKind.Delete, ChangeKind.Add, ChangeKind.Update }, new[] { ChangeKind.Update })]
[InlineData(new[] { ChangeKind.Update, ChangeKind.Delete, ChangeKind.Update, ChangeKind.Add, ChangeKind.Update }, new[] { ChangeKind.Update })]
[InlineData(new[] { ChangeKind.Add, ChangeKind.Delete, ChangeKind.Delete }, new ChangeKind[] { })]
[InlineData(new[] { ChangeKind.Add, ChangeKind.Add, ChangeKind.Delete }, new ChangeKind[] { })]
[InlineData(new[] { ChangeKind.Add, ChangeKind.Update, ChangeKind.Delete }, new ChangeKind[] { })]
[InlineData(new[] { ChangeKind.Update, ChangeKind.Add, ChangeKind.Delete }, new[] { ChangeKind.Update })]

// The following cases should not occur in practice:
[InlineData(new[] { ChangeKind.Update, ChangeKind.Add }, new[] { ChangeKind.Add })]
// File.WriteAllText on macOS may produce Update + Add.
[InlineData(new[] { ChangeKind.Update, ChangeKind.Add }, new[] { ChangeKind.Update })]

// The following case should not occur in practice:
[InlineData(new[] { ChangeKind.Delete, ChangeKind.Update }, new[] { ChangeKind.Delete })]
internal void NormalizeFileChanges(ChangeKind[] changes, ChangeKind[] expected)
{
Expand Down
Loading

0 comments on commit c489432

Please sign in to comment.