-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add symbolic link APIs #54253
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
Merged
Merged
Add symbolic link APIs #54253
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit
Hold shift + click to select a range
b63c7b7
ref declarations and src empty definitions
carlossanlop b223304
Implementation for Unix symlinks APIs and cross-platform unit tests.
carlossanlop 2e822fd
Add windows implementation of [sym]link APIs
jozkee 32e196c
Use ValueStringBuilder in ResolveLinkTarget
carlossanlop 8ed2772
Build failure in FileSystemWatcher csproj due to missing new interop …
carlossanlop abbecf0
Fix build failure. Address documentation suggestions.
fc49855
Add missing csproj references to System.Net.Ping.Functional.Tests (pr…
carlossanlop 86cbd19
Move CanCreateSymbolicLinks from FileSystemWatcher one class above, s…
480d3df
Fix some Microsoft.IO.Redist CI failures.
carlossanlop a6cd9da
Fix CI build failures from Net5Compat.Tests
carlossanlop 6952eb7
Clean CanCreateSymbolicLinks for readability. Remove incomplete code.
carlossanlop 797ace5
Support long paths in GetFinalPathNameByHandle
jozkee 8639e3b
Use MS.IO.Redist friendly APIs for marshalling
jozkee 6bb1aa9
Add IsBrowser check to CanCreateSymbolicLinks
carlossanlop 88bcadb
Add more tests and fix some issues in windows
jozkee 8701118
Add path to IO_InconsistentLinkType exception msg
carlossanlop d10e00c
Fix failure in test creating inconsistent file/dir type.
carlossanlop fa5ff40
Fix net48 error
jozkee 6788db7
Address CreateSymbolicLink initial check on Unix to verify pathToTarg…
45392b4
Use RemoteExecutor for a test related to change the current working dir
jozkee 38b6e88
Add missing argument to exception message in MS.IO.Redist
a70cdef
nit: Clean comment on CreateSymbolicLink p/invoke.
cbcf7af
Fix CI bug in Windows Nano where %TEMP% points to "C:\Windows\TEMP" b…
carlossanlop c43664a
Use ALLOW_UNPRIVILEGED_CREATE only on windows versions >= 10.0.14972
jozkee 9460264
Do not use IsWindowsVersionAtLeast since isn't avail in ns2.0
jozkee bbaa35a
Use PrintName (Dos) instead of SubstituteName (NT)
jozkee 7e2eaa9
Fix issues related to server share paths
jozkee 0d4250b
Fix CI issues
jozkee 1a05a56
Address suggestions
jozkee d656aed
Address suggestions
jozkee adcca16
Address suggestions about using PathInternal.IsExtended
jozkee 35165b0
Add more scenarios for "file system entry type is inconsistent with t…
jozkee 9d2694b
Remove duplicated validation checks
jozkee c79df26
Address suggestions for Unix
jozkee fc4241d
ifdef the list of paths used for theories
jozkee 4881f2b
Add tests to verify the limit of followed links
jozkee b377cb0
Fix bug related to follow links limit in Unix
jozkee 496487f
Trim extended prefix when the passed-in path is not extended
jozkee beaf701
Remove helper AssertFullNameEquals since is no longer needed and fix …
jozkee a5986fb
Add a check for versions < win10 build 14972 to fix silent error
jozkee b7ec269
Add Windows limit for ResolveLinkTarget to remarks
jozkee 7f37a86
Remove pipe test
jozkee 3e36f72
Remove lstat from ResolveLinkTarget
jozkee 6f5b99a
Avoid testing the reparse point/soft link limit very precisely
jozkee 2d09f7e
Fix LinkTarget invalidation logic
jozkee 4b5e133
Make returnFinalTarget no longer optional
jozkee 82dc88a
Merge branch 'main' of https://github.com/dotnet/runtime into symlinks
jozkee 48e1af1
Try to fix failing tests found in CI
jozkee 0495f59
Try smaller chains of links
jozkee 40cb737
Avoid using GetTestFilePath in order to correctly compare against the…
jozkee 995ead5
Use chain length of 20 instead of 30, which fails in Win 1809
jozkee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next
Next commit
ref declarations and src empty definitions
- Loading branch information
commit b63c7b7bde76dd5abd245a083fe83a8bd521a57b
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #24271 there was general consensus an
IsLinkproperty makes sense, though it was deferred to later.Thinking out-loud: if the property exists, would it be taken into account by this method?
e.g. when
!IsLinkthrowInvalidOperationExceptionorreturn nullimmediately?Adding this later would be a breaking change, and won't happen.
This was deferred because the semantics of
IsLinkare ambiguous. Maybe as part of this implementation it became more clear, and we can returnfalsefor anything whereLinkTargetis expected to returnnull?cc @mklement0 @adamsitnik
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, @tmds; #53577 is the follow-up issue about
IsLink; to quote my comment from #53577 (comment):We need to get clarity on what
.IsLinkreturningtrueis meant to signal:(a) Does it indicate whether the file-system itself considers something a link, as signaled on NTFS via the name-surrogate bit in the reparse-tag value?
(b) Or does it indicate that .NET not only knows that it is a link, but also knows how to explicitly resolve the link to its target? That is, can members such as
.LinkTargetor.ResolveLinkTarget()predictably be called?And later my thought was:
I'm now leaning toward (a):
.IsLinkreport everything that the file system thinks is a link, even if .NET doesn't understand the particular NTFS link type and cannot query its target,.LinkTargetand.ResolveLinkTarget()simply returnnullfor unknown link types; again, this NTFS-specific scenario could then be handled via the reparse-point API (assuming the caller understands the link format).Only if (b) is implemented can the property be used to short-circuit.
And based on the implementation here it sounds non-links / unknown link types should return
nullrather than throw.But ultimately I'm not sure whether (a) or (b) is the way to go (and perhaps the problem will rarely, if ever, arise).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
IsLinkwill need to be consistent withLinkTargeti.e:IsLink == (LinkTarget != null)if we apply your other suggestion #54253 (comment) (do not query LinkTarget multiple times even if null was returned), then I think that would be enough to achieve consistency.After that, adding
IsLinkin another release should no longer be a breaking change. Does that sounds OK to you folks?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really there is two common scenario: (1) manage link entity itself, (2) resolving link implicitly or explicitly in a code. Each of these scenarios requires a different approach. They are mutually exclusive. The first scenario requires a separate API like
public class SymbolicLinkorpublic class MountPoint. Second scenario requires injection in existing API/behavior that we do in the PR.Seeing how slowly this PR is progressing, I would put it off until the next version altogether. Much more important is the behavior of the existing API. For example, we already account for reparse points (RPs) in Directory.Remove. We need to implement the same in enumeration API. Then maybe we can create some internal API for RPs. This would be a great basis for further research. Looking specifically at behavior, I believe we probably won't need a generic API for links at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iSazonov I'm also leaning towards a.
When
IsLinkreturnsfalseI'd expectLinkTargetto throwInvalidOperationException. orreturn nullwithout querying the target.In any case, when
IsLinkreturnstrue,LinkTargetcan always still returnnullbecause the underlying file may have changed and no longer be a link. The user must handle this case.