PhysicalFileProvider: Use active polling instead of FileSystemWatcher on iOS/tvOS #58142
Conversation
|
Tagging subscribers to this area: @maryamariyan, @dotnet/area-extensions-filesystem Issue DetailsIn #57931 we found out that the FSEventStream APIs which are used to implement System.IO.FileSystemWatcher aren't allowed on the App Store. Mark System.IO.FileSystemWatcher as unsupported on iOS/tvOS and throw PNSE. Addresses #57931
|
… on iOS/tvOS In dotnet#57931 we found out that the FSEventStream APIs which are used to implement System.IO.FileSystemWatcher aren't allowed on the App Store. According to [Apple's docs](https://developer.apple.com/documentation/coreservices/1443980-fseventstreamcreate) these APIs are only supported on macOS and Mac Catalyst. Mark System.IO.FileSystemWatcher as unsupported on iOS/tvOS and throw PNSE. Switch PhysicalFileProvider to use active polling instead, like we do for Browser. Addresses dotnet#57931
akoeplinger
force-pushed
the
physicalfileprovider-polling
branch
from
742ac2a to
f2d56e3
Compare
| // For browser we will proactively fallback to polling since FileSystemWatcher is not supported. | ||
| if (OperatingSystem.IsBrowser()) | ||
| // For browser/iOS/tvOS we will proactively fallback to polling since FileSystemWatcher is not supported. | ||
| if (OperatingSystem.IsBrowser() || (OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst()) || OperatingSystem.IsTvOS()) |
There was a problem hiding this comment.
IMO, ideally we should have a runtime config like System.Globalization.Invariant corresponding to DOTNET_USE_POLLING_FILE_WATCHER environment variable; given how many different machine setups in practice are requiring fallback to polling even on Linux/macOS (see commit history / issues redirecting from aspnetcore when it broke on .NET 5 and still is broken since the fix was never backported..)
There was a problem hiding this comment.
Yeah, I agree. Something to keep in mind for .NET 7 but I'll keep this more focused change since we need to backport to .NET 6.
|
/backport to release/6.0-rc1 |
|
Started backporting to release/6.0-rc1: https://github.com/dotnet/runtime/actions/runs/1169962239 |
| [UnsupportedOSPlatform("browser")] | ||
| [UnsupportedOSPlatform("ios")] | ||
| [UnsupportedOSPlatform("tvos")] | ||
| [SupportedOSPlatform("maccatalyst")] |
There was a problem hiding this comment.
@buyaa-n - what does it say about other platforms (windows, linux, etc) when you have some attributes as UnsupportedOSPlatform and some attributes SupportedOSPlatform? What does this combination mean for windows?
There was a problem hiding this comment.
I talked to @buyaa-n and there's apparently a special case in the analyzer where this pattern causes it to be marked unsupported on iOS and supported on MacCatalyst and everywhere else.
There was a problem hiding this comment.
unsupported on iOS and supported on MacCatalyst and everywhere else.
Wouldn't it be the case if we e.g. delete [SupportedOSPlatform("maccatalyst")] line? Unsupported on ios, tvos and browser and supported everywhere else (including maccatalyst)
Or does [UnsupportedOSPlatform("ios")] somehow imply; unsupported on ios && maccatalyst?
There was a problem hiding this comment.
Thanks @akoeplinger. I found the PR that implemented this: dotnet/roslyn-analyzers#5266. It has a lot of good information on this specific situation.
There was a problem hiding this comment.
Or does
[UnsupportedOSPlatform("ios")]somehow imply; unsupported on ios && maccatalyst?
Yes [UnsupportedOSPlatform("ios")] imply unsupported on ios && maccatalyst (with latest change made in 6.0), to remove macatalyst unsupport from there need to add [SupportedOSPlatform("maccatalyst")]
There was a problem hiding this comment.
My concern was that adding [SupportedOSPlatform("maccatalyst")] didn't imply that it wasn't supported on other platforms, like windows and linux.
There was a problem hiding this comment.
My concern was that adding
[SupportedOSPlatform("maccatalyst")]didn't imply that it wasn't supported on other platforms, like windows and linux.
Originally it would imply what you said, but with the latest update for macatalyst-ios relation we special case related platforms scenario and this [SupportedOSPlatform("maccatalyst")] would only remove maccatalyst part from ios-maccatalyst relation
ghost
locked as resolved and limited conversation to collaborators
7 participants
In #57931 we found out that the FSEventStream APIs which are used to implement System.IO.FileSystem.Watcher aren't allowed on the App Store.
According to Apple's docs these APIs are only supported on macOS and Mac Catalyst.
Mark System.IO.FileSystem.Watcher as unsupported on iOS/tvOS and throw PNSE.
Switch PhysicalFileProvider to use active polling instead, like we do for Browser.
Addresses #57931