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

Remove validation to stat call for symlinks since is a breaking change #57551

Merged
merged 3 commits into from
Aug 17, 2021

Conversation

jozkee
Copy link
Member

@jozkee jozkee commented Aug 17, 2021

Subsequently, remove the symlink cache logic as is no longer needed.
Fixes #57221

cc @danmoseley

@jozkee jozkee added this to the 6.0.0 milestone Aug 17, 2021
@jozkee jozkee self-assigned this Aug 17, 2021
@ghost
Copy link

ghost commented Aug 17, 2021

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Subsequently, remove the symlink cache logic as is no longer needed.
Fixes #57221

cc @danmoseley

Author: Jozkee
Assignees: Jozkee
Labels:

area-System.IO

Milestone: 6.0.0

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for investigating this.

@@ -52,72 +52,6 @@ protected override void AssertLinkExists(FileSystemInfo link)
}
}

[Theory]
Copy link
Member Author

Choose a reason for hiding this comment

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

These tests were validating that we throw when a self-referencing symlink is found in the recursion, while that is true for windows, that wasn't the case for Unix in 5.0, and it was going to be a breaking change caused by the same validation that I'm removing, so I updated the tests to ensure what is expected matches with 5.0

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

LGTM with the latest test updates.

@jozkee
Copy link
Member Author

jozkee commented Aug 17, 2021

CI issues are nuget erros and test failure is #57452.
Merging now to include it in RC1.

@jozkee jozkee merged commit f7ba49d into dotnet:main Aug 17, 2021
@jozkee jozkee deleted the remove-symlink-stat-validation branch August 17, 2021 18:51
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Microsoft.Extensions.FileProviders.Physical.Tests crashing on CI in FileSystemInfo API
2 participants