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.

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

Add symlink support #85

Merged
merged 8 commits into from
Jun 1, 2024
Merged

Add symlink support #85

merged 8 commits into from
Jun 1, 2024

Conversation

GerardSmit
Copy link
Contributor

Fixes #74

This PR implements IFileSystem.CreateSymbolicLink(UPath path, UPath pathToTarget)

Linux

OS: Ubuntu 20.04 focal
Kernel: x86_64 Linux 5.10.102.1-microsoft-standard-WSL2

$ dotnet test -f net8.0 --filter Symlink
Passed!  - Failed:     0, Passed:     2, Skipped:     0, Total:     2, Duration: 4 ms - Zio.Tests.dll (net8.0)

After adding .NET 6.0 to the targets (to validate the .NET Standard 2.1 implementation):

$ dotnet test -f net6.0 --filter Symlink
Passed!  - Failed:     0, Passed:     2, Skipped:     0, Total:     2, Duration: 3 ms - Zio.Tests.dll (net6.0)

Windows

OS: Windows 11 23H2
Note: needs to be tested with administrator rights

$ dotnet test -f net8.0 --filter Symlink
Passed!  - Failed:     0, Passed:     2, Skipped:     0, Total:     2, Duration: 19 ms - Zio.Tests.dll (net8.0)

$ dotnet test -f net472 --filter Symlink
Passed!  - Failed:     0, Passed:     2, Skipped:     0, Total:     2, Duration: 25 ms - Zio.Tests.dll (net472)

@xoofx
Copy link
Owner

xoofx commented May 24, 2024

Looks great, thanks!

I'm thinking that if we want to add this, we might want also to add File.ResolveLinkTarget as part of this PR?

The API would be similar, but taking a UPath and maybe returning a UPath? (Not ideal because we would lose the information of whether this is a file or directory...)

@GerardSmit
Copy link
Contributor Author

I've added the API, can you validate if this is how you want the API?

interface IFileSystem
{
    /// <summary>
    /// Resolves the target of a symbolic link.
    /// </summary>
    /// <param name="linkPath">The path of the symbolic link to resolve.</param>
    UPath? ResolveLinkTarget(UPath linkPath);
}

I wasn't sure about the return type, since the following is possible:

  1. Nullable<UPath> / UPath? - the consumer knows that the result can be null (current implementation)
  2. UPath - the consumer has to check for UPath.IsEmpty / default.
  3. UPath with IOException if the path doesn't exists.
  4. Other, e.g. LinkResult? with: class LinkResult { UPath Path, bool IsDirectory }

Other than the return type: the only thing I'll have to validate if the MountFileSystem and SubFileSystem are returning the correct path.

@GerardSmit GerardSmit marked this pull request as draft May 24, 2024 12:14
@xoofx
Copy link
Owner

xoofx commented May 24, 2024

Maybe something like this instead?

interface IFileSystem
{
    /// <summary>
    /// Resolves the target of a symbolic link.
    /// </summary>
    /// <param name="linkPath">The path of the symbolic link to resolve.</param>
    /// <param name="resolvedPath">The path of the symbolic link resolved if true is returned.</param>
    bool TryResolveLinkTarget(UPath linkPath, out UPath resolvedPath);
}

@xoofx xoofx marked this pull request as ready for review June 1, 2024 21:11
@xoofx xoofx merged commit e243083 into xoofx:main Jun 1, 2024
1 check passed
@xoofx
Copy link
Owner

xoofx commented Jun 1, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Symlinks?
2 participants