Skip to content
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

Jump to bottom

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

Merged
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 2 commits
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
5 changes: 4 additions & 1 deletion ...ons.FileProviders.Abstractions/ref/Microsoft.Extensions.FileProviders.Abstractions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netstandard2.0;net461</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;net461</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
<Compile Include="Microsoft.Extensions.FileProviders.Abstractions.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Primitives\ref\Microsoft.Extensions.Primitives.csproj" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == '$(NetCoreAppCurrent)'">
<Reference Include="System.Runtime" />
jozkee marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>
</Project>
7 changes: 6 additions & 1 deletion ...ons.FileProviders.Abstractions/src/Microsoft.Extensions.FileProviders.Abstractions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<RootNamespace>Microsoft.Extensions.FileProviders</RootNamespace>
<TargetFrameworks>netstandard2.0;net461</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;net461</TargetFrameworks>
<EnableDefaultItems>true</EnableDefaultItems>
<PackageDescription>Abstractions of files and directories.

Expand All @@ -21,4 +21,9 @@ Microsoft.Extensions.FileProviders.IFileProvider</PackageDescription>
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Primitives\src\Microsoft.Extensions.Primitives.csproj" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
<Reference Include="System.Runtime" />
<Reference Include="System.Linq" />
<Reference Include="System.Linq.Expressions" />
</ItemGroup>
</Project>
1 change: 1 addition & 0 deletions src/libraries/Microsoft.Extensions.FileProviders.Physical/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
<Import Project="..\Directory.Build.props" />
<PropertyGroup>
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<UnsupportedOSPlatforms>browser</UnsupportedOSPlatforms>
Copy link
Member

Choose a reason for hiding this comment

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

How is this unsupported on browser? Is the whole assembly not supported on browser?

Copy link
Member Author

Choose a reason for hiding this comment

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

this project references System.IO.FileSystem.Watcher and that is not supported in browser:

runtime/src/libraries/System.IO.FileSystem.Watcher/Directory.Build.props

Line 6 in 7aaffcb

<UnsupportedOSPlatforms>browser</UnsupportedOSPlatforms>

Copy link
Member

Choose a reason for hiding this comment

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

I see, so then should we add a runtime assembly for Browser that throws PNSE?

cc: @marek-safar @steveisok

Copy link
Member

Choose a reason for hiding this comment

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

/cc @lewing

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, so then should we add a runtime assembly for Browser that throws PNSE?

Yes which is what this setting does

Copy link
Member

@safern safern 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.

This setting doesn't generate a PNSE automatically, this setting is just to add an assembly attribute for the platform analyzer. Unless something changed since last time I did it, in order to generate PNSE we need to add a -Browser tfm and then set the flag to generate a pnse when TargetsBrowser == true. So then it seems like we should do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching this, I've addressed @safern's comment in 75fcf96, please let me know if something else (related to mark the project as unsupported in Browser) needs to be done.

</PropertyGroup>
</Project>
5 changes: 4 additions & 1 deletion ....Extensions.FileProviders.Physical/ref/Microsoft.Extensions.FileProviders.Physical.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netstandard2.0;net461</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;net461</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
<Compile Include="Microsoft.Extensions.FileProviders.Physical.cs" />
Expand All @@ -10,4 +10,7 @@
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.FileSystemGlobbing\ref\Microsoft.Extensions.FileSystemGlobbing.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Primitives\ref\Microsoft.Extensions.Primitives.csproj" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
<Reference Include="System.IO.FileSystem.Watcher" />
</ItemGroup>
</Project>
24 changes: 24 additions & 0 deletions ...ibraries/Microsoft.Extensions.FileProviders.Physical/src/Internal/FileSystemInfoHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using System.IO;

namespace Microsoft.Extensions.FileProviders.Physical
Expand All @@ -27,5 +28,28 @@ public static bool IsExcluded(FileSystemInfo fileSystemInfo, ExclusionFilters fi

return false;
}

public static FileInfo ResolveFileLinkTarget(string filePath)
#if NETCOREAPP
=> ResolveFileLinkTarget(new FileInfo(filePath));
#else
=> null;
#endif

public static FileInfo ResolveFileLinkTarget(FileInfo fileInfo)
{
#if NETCOREAPP
if (fileInfo.Exists && fileInfo.LinkTarget != null)
{
FileSystemInfo targetInfo = fileInfo.ResolveLinkTarget(returnFinalTarget: true);
Copy link
Member

Choose a reason for hiding this comment

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

@jozkee have you considered keeping all these libs .NET Standard 2.0 and referencing the code that defines ResolveLinkTarget (and other things if needed)? We are already doing something like this for Microsoft.IO.Redist which targets net471. I am just wondering how much effort it would require to keep it NS2.0.

@jeffhandley @ericstj how long do we plan to keep targeting netstandard2.0 and net461 for the Microsoft.Extensions* libs?

Copy link
Member

Choose a reason for hiding this comment

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

@jeffhandley Jeff Handley FTE @ericstj Eric St. John FTE how long do we plan to keep targeting netstandard2.0 and net461 for the Microsoft.Extensions* libs?

At the moment we have no plans for dropping netstandard2.0 (and therefor net461) from the extensions. Perhaps it's something that could be considered in the future if this limitation hinders development considerably. They were intentionally omitted from aspnet/Announcements#324. cc @davidfowl @DamianEdwards @maryamariyan

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we're keeping ns2.0 and net461 until further notice.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can do a different (and more efficient) approach to get the target's Last write time, see @ericstj's suggestion #55664 (comment).
But (not) saving a couple of allocations is not a big issue considering that the performance penalty is relatively low compared to other pieces of the code that allocate much more.

Copy link
Member

Choose a reason for hiding this comment

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

We can do a different (and more efficient) approach to get the target's Last write time

@jozkee I wanted to know whether it would be possible to make this fix work for all TFMs (6 + NS2 + net461), because currently, it's going to work only for .NET 6.

Copy link
Member Author

@jozkee jozkee Jul 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It is, but we can defer fixing this on other TFMs if no one is asking for it.
Do you think we can take a dependency on MS.IO.Redist here to utilize the same set of Symbolic Link APIs in order to fix this in ns2.0?
cc @jeffhandley.

Copy link
Member

Choose a reason for hiding this comment

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

I think to fix on older TFMs it needs more than just MS.IO.Redist. I believe the implementation of the IO API you are using here (or would add) would depend on the system-native PALs which we don't consider part of the public API. Having a package depend on those is fragile at the least, and might not even work if the versions of those don't have the exports you need on older frameworks. Adding private copies of the system-native PALs (similar to what System.IO.Ports does) is expensive too. Something to consider around OOB'ing our IO API.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh, good info @ericstj. So far, we don't have any requests to have this fix apply to previous versions; I'm OK with this being net6.0+ until such requests come in.

if (targetInfo.Exists)
{
return (FileInfo)targetInfo;
}
}
#endif

return null;
}
}
}
15 changes: 13 additions & 2 deletions ....Extensions.FileProviders.Physical/src/Microsoft.Extensions.FileProviders.Physical.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<RootNamespace>Microsoft.Extensions.FileProviders</RootNamespace>
<TargetFrameworks>netstandard2.0;net461</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;net461</TargetFrameworks>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<EnableDefaultItems>true</EnableDefaultItems>
<PackageDescription>File provider for physical files for Microsoft.Extensions.FileProviders.</PackageDescription>
Expand All @@ -26,4 +26,15 @@
<PackageReference Include="System.Security.Cryptography.Algorithms" Version="$(SystemSecurityCryptographyAlgorithmsVersion)" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
<Reference Include="System.Collections.Concurrent" />
<Reference Include="System.ComponentModel.Primitives" />
<Reference Include="System.IO.FileSystem.Watcher" />
<Reference Include="System.Linq" />
<Reference Include="System.Linq.Expressions" />
<Reference Include="System.Runtime" />
<Reference Include="System.Security.Cryptography.Algorithms" />
<Reference Include="System.Security.Cryptography.Primitives" />
<Reference Include="System.Threading" />
</ItemGroup>
</Project>
2 changes: 1 addition & 1 deletion src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PollingFileChangeToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class PollingFileChangeToken : IPollingChangeToken
/// <param name="fileInfo">The <see cref="System.IO.FileInfo"/> to poll</param>
public PollingFileChangeToken(FileInfo fileInfo)
{
_fileInfo = fileInfo;
_fileInfo = FileSystemInfoHelper.ResolveFileLinkTarget(fileInfo) ?? fileInfo;
Copy link
Member

Choose a reason for hiding this comment

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

Will this raise new exceptions if the target of the file is not reachable? Perhaps we can add a test case for that: watching a normal file, watching a non-existent file, watching a symlinked file, watching a symlink with non-existent target.

Copy link
Member Author

Choose a reason for hiding this comment

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

It shouldn't, Exists and LinkTarget != null should guard for that.

runtime/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/Internal/FileSystemInfoHelper.cs

Lines 42 to 49 in 08233ba

if (fileInfo.Exists && fileInfo.LinkTarget != null)
{
FileSystemInfo targetInfo = fileInfo.ResolveLinkTarget(returnFinalTarget: true);
if (targetInfo.Exists)
{
return (FileInfo)targetInfo;
}
}

However since these APIs are not atomic, it could be that the link gets deleted between LinkTarget and ResolveLinkTarget. I don't know if there's an atomic way to do this.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's worse than just not being atomic, since you capture the FileInfo for the lifetime of the token, which might be long.

Here's another scenario. Suppose that the symlink initially points at one target, but then is changed to point at a new target and that new target has a new modified time? I think that's actually the scenario in this issue. If you captured the FileInfo from the initial target you'd miss that change since you'd be checking the original target.

Copy link
Member

Choose a reason for hiding this comment

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

What happens if the symlinks chain goes through an area that you can't read?

touch bar
mkdir foo
cd foo
ln -s ../bar .
cd ..
ln -s baz foo/bar
sha256sum bar baz
chmod a-x foo
sha256sum bar baz

In our world, baz.LinkTarget should resolve (to foo/bar). Does foo/bar then report Exists is false since it can't be read (the directory is no longer allowed to be traversed into)?

Copy link
Member Author

@jozkee jozkee Jul 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

In our world, baz.LinkTarget should resolve (to foo/bar). Does foo/bar then report Exists is false since it can't be read (the directory is no longer allowed to be traversed into)?

@bartonjs Yes, that's what I would expect. Therefore this should print false, unless that the FileInfo .ctor does some validation on inaccessible paths:

var targetInfo = File.ResolveLinkTarget("baz", returnFinaltarget: true);
Console.Write(targetInfo.Exists);

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for addressing the link target feedback. Please consider adding test cases for the deleted link target cases, as they seem unique to this addition as it has to handle non-existent link targets, and link targets that get deleted / added.

Copy link
Member Author

@jozkee jozkee Jul 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If the link target does not exists, we are returning the LastWriteTimeUtc from the link itself but I think that doesn't tell us anything, I will change it to return DateTime.MinValue instead (same thing we do when the file does not exists).
Let me know if you think that could be problematic.

I also added the requested tests: 98b737a.

_previousWriteTimeUtc = GetLastWriteTimeUtc();
}

Expand Down
3 changes: 2 additions & 1 deletion src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PollingWildCardChangeToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ private bool CalculateChanges()
/// <returns>The <see cref="DateTime"/> that the file was last modified.</returns>
protected virtual DateTime GetLastWriteUtc(string path)
{
return File.GetLastWriteTimeUtc(Path.Combine(_directoryInfo.FullName, path));
string filePath = Path.Combine(_directoryInfo.FullName, path);
return FileSystemInfoHelper.ResolveFileLinkTarget(filePath)?.LastWriteTimeUtc ?? File.GetLastWriteTimeUtc(filePath);
ericstj marked this conversation as resolved.
Show resolved Hide resolved
}

private static bool ArrayEquals(byte[] previousHash, byte[] currentHash)
Expand Down
62 changes: 62 additions & 0 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 @@ -1512,6 +1512,68 @@ public void UsePollingFileWatcher_FileWatcherNotNull_ReturnsFalse()
}
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged(bool useWildcard)
{
// Arrange
using var root = new DisposableFileSystem();
string fileName = Path.GetRandomFileName();
string filePath = Path.Combine(root.RootPath, fileName);
File.WriteAllText(filePath, "v1.1");

using var provider = new PhysicalFileProvider(root.RootPath) { UsePollingFileWatcher = true, UseActivePolling = true };
IChangeToken token = provider.Watch(useWildcard ? "*" : fileName);
Assert.False(token.HasChanged);

// Act
File.WriteAllText(filePath, "v1.2");
Thread.Sleep(GetTokenPollingInterval(token));

// Assert
jozkee marked this conversation as resolved.
Show resolved Hide resolved
Assert.True(token.HasChanged);
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public void UsePollingFileWatcher_UseActivePolling_HasChanged_SymbolicLink(bool useWildcard)
{
// Arrange
using var rootOfFile = new DisposableFileSystem();
string filePath = Path.Combine(rootOfFile.RootPath, Path.GetRandomFileName());
File.WriteAllText(filePath, "v1.1");

using var rootOfLink = new DisposableFileSystem();
string linkName = Path.GetRandomFileName();
string linkPath = Path.Combine(rootOfLink.RootPath, linkName);
File.CreateSymbolicLink(linkPath, filePath);

using var provider = new PhysicalFileProvider(rootOfLink.RootPath) { UsePollingFileWatcher = true, UseActivePolling = true };
IChangeToken token = provider.Watch(useWildcard ? "*" : linkName);
Assert.False(token.HasChanged);

// Act
File.WriteAllText(filePath, "v1.2");
Thread.Sleep(GetTokenPollingInterval(token));

// Assert
Assert.True(token.HasChanged);
}

private int GetTokenPollingInterval(IChangeToken changeToken)
{
TimeSpan pollingInterval = (changeToken as CompositeChangeToken).ChangeTokens[1] switch
{
PollingWildCardChangeToken wildcardChangeToken => wildcardChangeToken.PollingInterval,
PollingFileChangeToken => PollingFileChangeToken.PollingInterval,
_ => throw new InvalidOperationException()
};

return (int)pollingInterval.TotalMilliseconds;
}

[Fact]
public void CreateFileWatcher_CreatesWatcherWithPollingAndActiveFlags()
{
Expand Down
6 changes: 5 additions & 1 deletion ...icrosoft.Extensions.FileSystemGlobbing/ref/Microsoft.Extensions.FileSystemGlobbing.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netstandard2.0;net461</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;net461</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
<Compile Include="Microsoft.Extensions.FileSystemGlobbing.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
<Reference Include="System.Collections" />
jozkee marked this conversation as resolved.
Show resolved Hide resolved
<Reference Include="System.Runtime" />
</ItemGroup>
</Project>
11 changes: 8 additions & 3 deletions ...icrosoft.Extensions.FileSystemGlobbing/src/Microsoft.Extensions.FileSystemGlobbing.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>netstandard2.0;net461</TargetFrameworks>
<TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;net461</TargetFrameworks>
<EnableDefaultItems>true</EnableDefaultItems>
<PackageDescription>File system globbing to find files matching a specified pattern.</PackageDescription>
</PropertyGroup>
Expand All @@ -10,5 +10,10 @@
<Compile Include="$(CoreLibSharedDir)\System\Numerics\Hashing\HashHelpers.cs"
Link="System\Numerics\Hashing\HashHelpers.cs" />
</ItemGroup>


<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
<Reference Include="System.Collections" />
<Reference Include="System.Linq" />
<Reference Include="System.Runtime" />
</ItemGroup>
</Project>
22 changes: 14 additions & 8 deletions src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,34 +427,33 @@ internal static void CreateSymbolicLink(string path, string pathToTarget, bool i
{
string? targetPath = returnFinalTarget ?
GetFinalLinkTarget(linkPath, isDirectory) :
GetImmediateLinkTarget(linkPath, isDirectory, throwOnUnreachable: true, returnFullPath: true);
GetImmediateLinkTarget(linkPath, isDirectory, throwOnError: true, returnFullPath: true);

return targetPath == null ? null :
isDirectory ? new DirectoryInfo(targetPath) : new FileInfo(targetPath);
}

internal static string? GetLinkTarget(string linkPath, bool isDirectory)
=> GetImmediateLinkTarget(linkPath, isDirectory, throwOnUnreachable: false, returnFullPath: false);
=> GetImmediateLinkTarget(linkPath, isDirectory, throwOnError: false, returnFullPath: false);

/// <summary>
/// Gets reparse point information associated to <paramref name="linkPath"/>.
/// </summary>
/// <returns>The immediate link target, absolute or relative or null if the file is not a supported link.</returns>
internal static unsafe string? GetImmediateLinkTarget(string linkPath, bool isDirectory, bool throwOnUnreachable, bool returnFullPath)
internal static unsafe string? GetImmediateLinkTarget(string linkPath, bool isDirectory, bool throwOnError, bool returnFullPath)
{
using SafeFileHandle handle = OpenSafeFileHandle(linkPath,
Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS |
Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT);

if (handle.IsInvalid)
{
int error = Marshal.GetLastWin32Error();

if (!throwOnUnreachable && IsPathUnreachableError(error))
if (!throwOnError)
{
return null;
}

int error = Marshal.GetLastWin32Error();
// File not found doesn't make much sense coming from a directory.
if (isDirectory && error == Interop.Errors.ERROR_FILE_NOT_FOUND)
{
Expand All @@ -479,6 +478,11 @@ internal static void CreateSymbolicLink(string path, string pathToTarget, bool i

if (!success)
{
if (!throwOnError)
{
return null;
}

int error = Marshal.GetLastWin32Error();
// The file or directory is not a reparse point.
if (error == Interop.Errors.ERROR_NOT_A_REPARSE_POINT)
Expand Down Expand Up @@ -600,13 +604,15 @@ uint GetFinalPathNameByHandle(SafeFileHandle handle, char[] buffer)
{
// Since all these paths will be passed to CreateFile, which takes a string anyway, it is pointless to use span.
// I am not sure if it's possible to change CreateFile's param to ROS<char> and avoid all these allocations.
string? current = GetImmediateLinkTarget(linkPath, isDirectory, throwOnUnreachable: false, returnFullPath: true);

// We don't throw on error since we already did all the proper validations before.
string? current = GetImmediateLinkTarget(linkPath, isDirectory, throwOnError: false, returnFullPath: true);
string? prev = null;

while (current != null)
{
prev = current;
current = GetImmediateLinkTarget(current, isDirectory, throwOnUnreachable: false, returnFullPath: true);
current = GetImmediateLinkTarget(current, isDirectory, throwOnError: false, returnFullPath: true);
}

return prev;
Expand Down