[iOS] Make Environment.GetFolderPathCore Xamarin.iOS compatible #41959
Conversation
EgorBo
requested review from
akoeplinger,
marek-safar,
steveisok and
vargaz
as code owners
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
| @@ -78,7 +78,7 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio | |||
| return CombineSearchPath(NSSearchPathDirectory.NSLibraryDirectory, "Favorites"); | |||
|
|
|||
| case SpecialFolder.ProgramFiles: | |||
| return Interop.Sys.SearchPath(NSSearchPathDirectory.NSApplicationDirectory); | |||
There was a problem hiding this comment.
Doea this API not give the correct result on iOS? Does Apple intend that code that wants to run on both should hard code it instead?
There was a problem hiding this comment.
@danmosemsft these are different folders as far as I understand, e.g. /Applications on iOS simulator returns actual /Applications from the host (I guess on a device it's a symlink). And our goal is to be compatible with whatever current Xamarin.IOS returns (based on mono/mono impl)
There was a problem hiding this comment.
Should we detect when we're on macOS and continue to use NSApplicationDirectory? I'm just having trouble understanding why Apple offers an API if the correct answer is always /Applications. Even if that's what Mono offers - we should try to do the right thing, and Mono compat on macOS (rather than iOS) perhaps is less important. If they move it in future, presumably NSApplicationDirectory protects us?
There was a problem hiding this comment.
I believe compatibility with existing code is quite important, which is why I believe we should continue returning the same values as mono does.
That being said, the /Applications directory is quite useless on Apple's mobile platforms, because apps don't have read access to it (presumably that's why NSApplicationDirectory returns something else, even though Apple has it documented to return /Applications). I'm not sure what the best option here is, but returning NSApplicationDirectory doesn't seem like a bad choice.
On macOS I'd be fine with using NSApplicationDirectory, if that returns /Applications today.
On a separate note, the values mono returns are different in some cases for tvOS: mono/mono@77fb5c9#diff-ba633813d1681ae2aabd0a6bbd741f87R21-R30, and it doesn't look like .NET does that.
There was a problem hiding this comment.
regarding the tvOS (we don't support it yet) - we have a note in the code.
So should a leave it as is or bring back NSApplicationDirectory?
There was a problem hiding this comment.
Yes, bring back NSApplicationDirectory for ProgramFiles.
There was a problem hiding this comment.
Probably makes sense to leave it until later and file an issue.
ghost
locked as resolved and limited conversation to collaborators
Fixes: #41383
XI current implementation: https:/mono/mono/blob/master/mcs/class/corlib/System/Environment.iOS.cs