Skip to content

Make polling use the symbolic link target's LastWriteTime #55664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jozkee merged 18 commits into from
Jul 22, 2021
Merged

Make polling use the symbolic link target's LastWriteTime #55664

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f515710
Relax LinkTarget so it always returns null when steps on an error
jozkee Jul 14, 2021
08233ba
Make polling use the symbolic link target's LastWriteTime
jozkee Jul 14, 2021
d780995
Fix for the case where the link can change its target
jozkee Jul 15, 2021
ac4a845
Add more test cases and exclude them from non-netcoreapp tfms
jozkee Jul 15, 2021
b4895ad
Fix project references in ref projects
jozkee Jul 15, 2021
ebb0326
Do not use UnsupportedOSPlatforms on test project in order to fix CI …
jozkee Jul 16, 2021
1164e33
Do not return link's LastWriteTime when target not exists
jozkee Jul 19, 2021
98b737a
Address feedback on tests and improve them to cover more scenarios.
jozkee Jul 19, 2021
75fcf96
Make the project unsupported in browser.
jozkee Jul 19, 2021
144335a
Fix duplicate reference to PlatformAttributes with IncludePlatformAtt…
jozkee Jul 19, 2021
9c50a3a
Disable default items for browser
jozkee Jul 20, 2021
b2f9bad
Undo unrelated changes to Strings.resx
jozkee Jul 20, 2021
d8d143a
Replace Thread.Sleep with Task.Delay, add assertion messages to try t…
jozkee Jul 20, 2021
c7e28d4
Replace HasChanged for RegisterChangeCallback in tests
jozkee Jul 21, 2021
5b85631
Add messages to asserts to attempt to debug CI issues
jozkee Jul 21, 2021
5c27cae
Add date format to assertion messages.
jozkee Jul 21, 2021
19dd9c3
Increase delay between writes to one second since OSX doesn't report …
jozkee Jul 21, 2021
8c1b3a2
Merge branch 'main' of https://github.com/dotnet/runtime into symlink…
jozkee Jul 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Replace Thread.Sleep with Task.Delay, add assertion messages to try t… 
…o debug CI failures and increase delay between writes
  • Loading branch information
@jozkee
jozkee committed Jul 20, 2021
commit d8d143a2e8f3859cb15867201b92dd9ae87d5638
17 changes: 8 additions & 9 deletions src/libraries/Microsoft.Extensions.FileProviders.Physical/tests/PhysicalFileProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1515,7 +1515,7 @@ public void UsePollingFileWatcher_FileWatcherNotNull_ReturnsFalse()
[Theory]
[InlineData(false)]
[InlineData(true)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged(bool useWildcard)
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged(bool useWildcard)
{
// Arrange
using var root = new DisposableFileSystem();
Expand All @@ -1528,19 +1528,18 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged(bool useWildcard)
Assert.False(token.HasChanged);

// Act
Thread.Sleep(100); // Wait a bit before writing again, see https://github.com/dotnet/runtime/issues/55951.
await Task.Delay(200); // Wait a bit before writing again, see https://github.com/dotnet/runtime/issues/55951.
File.WriteAllText(filePath, "v1.2");
int timeout = GetTokenPollingInterval(token);
Thread.Sleep(timeout);
await Task.Delay(GetTokenPollingInterval(token));

// Assert
Assert.True(token.HasChanged);
Assert.True(token.HasChanged, $"Current time: {DateTime.UtcNow:O} file LastWriteTime: {File.GetLastWriteTimeUtc(filePath):O}");
}

[Theory]
[InlineData(false)]
//[InlineData(false)]
[InlineData(true)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged_FileDeleted(bool useWildcard)
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_FileDeleted(bool useWildcard)
{
// Arrange
using var root = new DisposableFileSystem();
Expand All @@ -1555,10 +1554,10 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged_FileDeleted(bool u

// Act
File.Delete(filePath);
Thread.Sleep(GetTokenPollingInterval(token));
await Task.Delay(GetTokenPollingInterval(token));

// Assert
Assert.True(token.HasChanged);
Assert.True(token.HasChanged, $"Current time: {DateTime.UtcNow:O} file LastWriteTime: {File.GetLastWriteTimeUtc(filePath):O}");
}

private int GetTokenPollingInterval(IChangeToken changeToken)
Expand Down
29 changes: 14 additions & 15 deletions ...Microsoft.Extensions.FileProviders.Physical/tests/PhysicalFileProviderTests.netcoreapp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@

using System;
using System.IO;
using System.Threading;
using Microsoft.Extensions.FileProviders.Physical;
using System.Threading.Tasks;
using Microsoft.Extensions.Primitives;
using Xunit;

Expand All @@ -15,7 +14,7 @@ public partial class PhysicalFileProviderTests
[Theory]
[InlineData(false)]
[InlineData(true)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink(bool useWildcard)
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink(bool useWildcard)
{
// Arrange
using var rootOfFile = new DisposableFileSystem();
Expand All @@ -32,12 +31,12 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink(bool
Assert.False(token.HasChanged);

// Act
Thread.Sleep(100); // Wait a bit before writing again, see https://github.com/dotnet/runtime/issues/55951.
await Task.Delay(200); // Wait a bit before writing again, see https://github.com/dotnet/runtime/issues/55951.
File.WriteAllText(filePath, "v1.2");
Thread.Sleep(GetTokenPollingInterval(token));
await Task.Delay(GetTokenPollingInterval(token));

// Assert
Assert.True(token.HasChanged);
Assert.True(token.HasChanged, $"Current time: {DateTime.UtcNow:O} file LastWriteTime: {File.GetLastWriteTimeUtc(filePath):O}");
}

[Theory]
Expand All @@ -64,7 +63,7 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_Targe
[InlineData(false, true)]
[InlineData(true, false)]
[InlineData(true, true)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_TargetChanged(bool useWildcard, bool fromTargetNonExistent)
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_TargetChanged(bool useWildcard, bool linkWasBroken)
{
// Arrange
using var rootOfFile = new DisposableFileSystem();
Expand All @@ -73,9 +72,9 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_Targe
File.WriteAllText(file2Path, "v2.1");

string file1Path = Path.Combine(rootOfFile.RootPath, Path.GetRandomFileName());
if (!fromTargetNonExistent)
if (!linkWasBroken)
{
Thread.Sleep(100); // Wait a bit before writing again, see https://github.com/dotnet/runtime/issues/55951.
await Task.Delay(200); // Wait a bit before writing again, see https://github.com/dotnet/runtime/issues/55951.
File.WriteAllText(file1Path, "v1.1");
}

Expand All @@ -92,16 +91,16 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_Targe
// Act - Change link target to file 2.
File.Delete(linkPath);
Copy link
Member

@ericstj ericstj Jul 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to reset the target without deleting the link?

Copy link
Member Author

@jozkee jozkee Jul 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Through File.CreateSymbolicLink, it is not. It may be possible to modify the target of an existing link by using FSCTL_SET_REPARSE_POINT, see https://docs.microsoft.com/en-us/windows/win32/fileio/reparse-point-operations.

For Unix, I know that you can use the -f switch in ln -sf bar foo to force the link to be created even if it already exists, but I guess that's just deleting the old link.

Copy link
Member

@bartonjs bartonjs Jul 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ln.html says that -f in ln is functionally equivalent to "call unlink", which we call File.Delete.

File.CreateSymbolicLink(linkPath, file2Path);
Thread.Sleep(GetTokenPollingInterval(token));
await Task.Delay(GetTokenPollingInterval(token));

// Assert
Assert.True(token.HasChanged); // It should report the change regardless of the timestamp being older.
// Assert - It should report the change regardless of the timestamp being older.
Assert.True(token.HasChanged, $"Current time: {DateTime.UtcNow:O} file 1 LastWriteTime: {File.GetLastWriteTimeUtc(file1Path):O} file 2 LastWriteTime: {File.GetLastWriteTimeUtc(file2Path):O}");
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_TargetDeleted(bool useWildcard)
public async Task UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_TargetDeleted(bool useWildcard)
{
// Arrange
using var rootOfFile = new DisposableFileSystem();
Expand All @@ -121,10 +120,10 @@ public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink_Targe

// Act
File.Delete(linkPath);
Thread.Sleep(GetTokenPollingInterval(token));
await Task.Delay(GetTokenPollingInterval(token));

// Assert
Assert.True(token.HasChanged);
Assert.True(token.HasChanged, $"Current time: {DateTime.UtcNow:O} file LastWriteTime: {File.GetLastWriteTimeUtc(filePath):O}");
}
}
}