-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Change IsolatedStorageFile path for mobile #83380
Change IsolatedStorageFile path for mobile #83380
Conversation
Tagging subscribers to this area: @dotnet/area-system-io Issue DetailsIn case of mobile platforms return GetDataDirectory. Fixes #83231
|
Tagging subscribers to 'arch-android': @steveisok, @akoeplinger Issue DetailsIn case of mobile platforms return GetDataDirectory. Fixes #83231
|
Tagging subscribers to 'os-ios': @steveisok, @akoeplinger Issue DetailsIn case of mobile platforms return GetDataDirectory. Fixes #83231
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
/azp run runtime-android |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-ioslike |
Azure Pipelines successfully started running 1 pipeline(s). |
@akoeplinger could you please review last changes? |
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.
Looks good to me apart from one case on Android
{ | ||
specialFolder = | ||
IsMachine(scope) ? Environment.SpecialFolder.CommonApplicationData : | ||
IsRoaming(scope) ? Environment.SpecialFolder.LocalApplicationData: |
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.
we need to change this case on Android since we had the breaking change of Environment.SpecialFolder.LocalApplicationData
between Xamarin and dotnet:
Xamarin | /data/user/0/com.companyname.testandroidxamarin/files/.local/share |
dotnet | /data/user/0/com.companyname.TestMauiNet7/files |
You can use hardcoded SpecialFolder.UserProfile
+ ".local/share" for that when OperatingSystem.IsAndroid()
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 general I'd recommend creating a sample app that uses all possible combinations for IsolatedStorageFile scopes and then comparing each computed path between Xamarin and dotnet to make sure they match
// In legacy Xamarin for Roaming Scope we were using Environment.SpecialFolder.LocalApplicationData | ||
// In .Net 7 for Roaming Scope we are using Environment.SpecialFolder.ApplicationData | ||
// e.g. .Net 7 path = /data/user/0/{packageName}/files/.isolated-storage/{hash}/{hash}/AppFiles/ | ||
// e.g. Xamarin path = /data/user/0/{packageName}/files/.config/.isolated-storage" |
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.
these example paths are from Android, I'd also add iOS examples to make it clearer
internal static string GetDataDirectory(IsolatedStorageScope scope) | ||
{ | ||
// This is the relevant special folder for the given scope plus IsolatedStorageDirectoryName. | ||
// It is meant to replicate the behavior of the VM ComIsolatedStorage::GetRootDir(). |
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.
this comment isn't meaningful for us, I'd remove it.
/azp run runtime-ioslike |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-android |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-ioslike |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-android |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-android |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-ioslike |
Azure Pipelines successfully started running 1 pipeline(s). |
Failures are not related. |
@akoeplinger @mkhamoyan should we try to backport this to 7? |
Fix
GetRandomDirectory
for mobile, if there is no existing random directory create the path without hashes.In legacy
Xamarin
we didn't have a random directory inside of the isolated storage root for each app, we tried to preserve that in #75541 but the fix wasn't complete enough and we still created random directories when not using the Roaming scope.Since we shipped that behavior as part of
.NET 7
we can't change this now or upgraded apps wouldn't find their files anymore. We need to look for an existing random directory first before using the plain root directory.Example for
Android
:Before this change path is
/data/user/0/{packageName}/files/.isolated-storage/{hash}/{hash}/AppFiles/
After the change path will be
/data/user/0/{packageName}/files/.config/.isolated-storage/
Example for
iOS
:Before this change path is
/Users/userName/{packageName}/Documents/.isolated-storage/{hash}/{hash}/AppFiles/
After the change path will be
/Users/userName/{packageName}/Documents/.config/.isolated-storage/
Fixes #83231, #83384