Android: Remove external storage permission

[cwyc]

Description

The android client requests blanket access to internal storage, even though it only needs to access the self-contained /sdcard/RetroArch folder.
By getting rid of this permission, we can improve sandboxing and show respect to the user's privacy. Also, this issue is what's holding up play store updates.

We do this by moving the RetroArch folder to app-specific storage, /sdcard/Android/data/com.retroarch.aarch64/files/RetroArch. A DocumentsProvider is added to allow the user to easily access it. And a migration flow is added to prompt existing users to copy their RetroArch folder to the new location.

Consequences are that users will have to copy their content into app-specific storage before they play them, and that some automated backup workflows will be disrupted.

It wasn't practical to accomplish this by leaving the folder in place and accessing it through DocumentsContract, since the files couldn't be accessed through the POSIX apis that the rest of the codebase relies on, and because it has a performance penalty.

Related Issues

This issue is discussed here: #12181

Related Pull Requests

[Any other PRs from related repositories that might be needed for this pull request to work]

Reviewers

[If possible @mention all the people that should review your pull request]

[hizzlekizzle]

@LibretroAdmin

[hizzlekizzle]

if this will allow us to update the Play Store version, this is very exciting stuff! We very much appreciate the contribution!

[warmenhoven]

I'm not very familiar with Android at all, but fairly familiar with iOS. I took a look at this PR and it seems reasonable and straightforward to me, but I did have a couple concerns.

@cwyc , could you please describe in more detail how the migration affects:

1. paths in retroarch.cfg and playlists
2. roms that may not exist in the retroarch directory?

For the paths in the .cfg and .lpl files, on iOS if a file exists in the sandbox, the filename is prefixed/truncated to indicate it should be searched for in the sandbox. Does the same thing happen on Android? If so I'd expect it's probably not an issue except for any the files the user has created themselves outside the sandbox, which is basically the same question as 2.

[cwyc]

@warmenhoven
Thank you for pointing this out, I hadn't considered path references in config files.

Yes, retroarch.cfg and playlists do carry absolute paths. We will need to update them when we copy content over. This is complicated by the fact that the app gives users the option to individually relocate the content directories, so they might need to be selected and copied in separate operations.

As for roms outside of the RetroArch folder, they won't be copied over, and playlists referring to them will be broken. But for roms that are copied, we can update the paths in content*_history.lpl and other playlist files we find on a best-effort basis.

The current migration flow assumes all this exists in ~/RetroArch.
Instead, perhaps it needs to begin by searching config and playlist files for external references, giving users the option to select and copy each to standard locations in app-specific storage, and updating the paths. It will also have to be recursive, as files in the content directories might reference others. ROMs can go in a RetroArch/roms folder.

Still, we might not be able to catch and update all the references, and some people's setups might get messed up. I'd be open to other approaches.

[warmenhoven]

@cwyc
Ok so in my opinion:

• getting it on the play store again is a big important thing,
• upgrades/migrations are always an place where people are expecting weirdness/brokenness,
• and people who are putting retroarch-related stuff outside the retroarch folder are already more likely to be equipped to be able to fix the brokenness manually;
• so my opinion is that worst case this is fine to merge as is. Still, there should be best effort made to do something better if at all easy/possible. If not, at least you tried, and people will get over it.

Given that, I think it's safest not to try to get too smart/tricky with it. Update the paths in the .cfg and .lpl files that you are already directly affecting (the most common cases), and everything else the user is going to have to sort out for themselves.

[hizzlekizzle]

I don't know if there's an easy way to give people an option to migrate or not. If it were me as an end-user, I'd rather just start over fresh if weird breakage were a possibility.

We already have the welcome message that shows on first launch, so we could duplicate that function to make another message that shows only when the other message is disabled (i.e., already been seen) and refers them to a docs page that lays all this out in more detail.

I'm just wary, in general, of automagic stuff.

But whatever, I'm fine with it either way. Any breakage/discomfort is going to be transitory anyway, and, as warmenhoven stated, just getting the Play Store version updated is worth pretty much any amount of temporary inconvenience.

[WargusUser]

If it will make only possible to play ROMS only from folder “/sdcard/Android/data/com.retroarch.aarch64/files/RetroArch” than no, just delete google play support or make sure that APK version from site sill have external storage permission support. I anyway use APK from site because google play version is limited. Google don’t care about emulation scene anyway and you should not care about their paranoid security stuff that mostly makes no sense.
Also deleting retroarch will delete also your saves and savestates.
So, if you implement google play support, make sure it’s still possible to use retroarch in convenient way for APK from site. I will not use google play version anyway

[i30817]

RetroArch, either the site build or the playstore build is essentially useless for big collection of roms until it supports reading roms or making the playlists read roms from SAF (storage access framework).

The reason is that SAF middleware like RSAF is the only way to access localhost or remote NAS, like webdav, nfs, samba, etc. I can run (even if it's annoying to lose the nice playlist interface, because only dolphin actually bothers to iterate the dirs when looking for roms) remote roms in android PPSSPP, dolphin, etc using RSAF and a Linux webdav server. I can't in RA.

Ideally I'd be able to permit access to a webdav directory, with the usual portable playlist directory change and leave it at that, although I doubt it will ever happen.

[cwyc]

@hizzlekizzle I've added some code that just does a find-and-replace in .cfg and .lpl files, replacing the old default path with the new location. For good measure it makes a backup before it does so.
Agree with @i30817 integrating the file picker with SAF should be a priority, but I don't have experience with that codebase.

[i30817]

Playlist path auto changing has some danger because the manual playlists portable playlist relocation feature already autochanges paths in some circumstances. Please test what happens with portable playlists.

The trigger for the portable playlist change is explained in this post and the next one: #9163 (comment)

I think it's a bit of a obscure feature. Useful though.

About SAF, I don't really get the holdup. It's been like this for years and it's not like people are asking for implementations of what they want to use SAF for (webdav, samba). Other people did that, exactly so emulator devs won't have to care.

There are examples of c++ apps asking for a android SAF permission for a single dir and operating in the resulting path in scummvm (they scan the resulting path for games, recursively), so it's not even a question of only a single dir being available.

In fact, I wouldn't be surprised if the scummvm core allowed RSAF to access remotes from its internal gui, which would be extremely ironic. Since the core is just upstream now, plus a few tweaks and a gui to create a playlist.

I should try it out actually. Edit: didn't work unfortunately, the option that appears on the root ( as in filesystem root, not user root) in upstream that triggers the android filechooser doesn't appear.

[cwyc]

@i30817
I couldn't get the playlist relocation behaviour to work through the upgrade. At least with content_history.lpl, it looks in the old locations if we don't otherwise patch the paths. Any idea why?

But maybe we need to switch our focus to SAF in native code. I'll look into how scummvm does it. And if we achieve that, those who prefer it can leave the folder in sdcard.

[i30817]

It's possible that the portable playlist feature was left half baked when jdgleaver left, and content history never changes paths even if you change rgui_browser_directory and it's different from base_content_directory (that exists, ie it's not the first time you enable portable playlists with those files).

There is no reason for that be because base_content_directory actually exists in that playlist if you enable portable playlists, I have it.

It can also be that you have to restart RetroArch before these changes get reflected on the playlists, I kind of recall something like that. That is in itself a flaw, if so.

Edit: to get in the same page, I think RA portable playlists work by saving rgui_browser_directory in each playlist as base_content_directory if it doesn't exist yet, and every time a RetroArch instance starts up, and portable playlists are on and for each playlist base_content_directory that is different from rgui_browser_directory, replace the paths
parts with the old base_content_directory by the current gui_browser_directory and change the directory seperator of the rest of the path (if necessary), then replace base_content_directory by rgui_browser_directory again, then write out the playlist.

This sounds like something a first iteration would do when loading the playlists into memory the first time, instead of each time rgui_browser_directory changes.

I don't understand how that algorithm interacts with a forced path change in between enabling portable playlists and they getting written out if rgui_browser_directory changes. Maybe you only have to also delete base_content_directory from each playlist and force turn off portable playlist (to force the user to choose a base_content_path inside the RetroArch apk sandbox when and if they turn it on again).

(Edited out rant about useless dirs in the android RA filechooser start)

(Edited out rant about how the users will have to move the roms anyway so automagic is all useless really, and portable playlists can change the paths anyway - too cynical, and I think it's pretty clear that the average user has no clue how to use portable playlists 😂 if even devs don't know about it)

[i30817]

Edit: thinking some further, we obviously want a runtime mechanism like scummvm to ask android to ask the user for read\write permissions on the rom directory and copy, but not change playlists at all. Normally that path would be authorized during the scan, but on this transition or using portable playlists, it would be when changing rgui_browser_directory \ running the new version the first time.

RetroArch even has a very similar place to scummvm to put this in on ozone at least, where the file browser starts with memorized shortcut paths (scummvm allows adding authorized paths to its own shortcut paths by going up to the root and triggering <add directory> option, they just don't start already there but in the last dir used). Most of the paths already there are some variant of useless in android though (no permissions to read)

Every other path in the retroarch.cfg and playlists that might need to be changed should be user visible warnings at least, if those files are copied over (off the top of my head, the assets file, rgui_browser_directory, base_content_dir in playlists (may not correspond to the path a user added as the rom scan paths), may be forgetting some.

But honestly, changing locations of the base RetroArch dir is a minefield, it's the one thing that breaks a lot of ideas of using the same configs between installs or platforms. I do not recommend saving anything but the playlists, and even then you should probably warn about paths outside allowed dirs (but not change them IMO, except, see the next paragraph).

If at the same time, this allows RSAF to work, you get external SD card and webdav\NAS storage, and play store update, that's worth a few settings nuked, even portable playlists if necessary (base_content_directory in playlists might not be authorized, even if the rom dir was - imagine it's a parent of the authorized rom dir - I imagine this situation might confuse the update code, although rgui_broswer_directory would be null if retroarch.cfg was reset, so nothing might happen at first - even if you only copy the playlists during the transition as expect the user to authorize the rom dir for them to work, it might be worth it to remove base_content_directory, if it's not readable).

Edit: there is a catch 22 where to save the playlists into the new sandbox the new RetroArch has to ask permission to the user to find the
playlists in the expected place (outside the sandbox). You can't even check if the playlists exist in /storage/emulated/0/RetroArch/playlists without asking for the permission first I think, making the idea not very friendly unless newer versions stop trying eventually, and assume anyone that wanted to preserve playlists already did.

[hizzlekizzle]

@cwyc hey man, just so you don't think we've forgotten about this/you: we're hoping to get another stable release out very soon and then we'll be digging into this immediately after. We very much appreciate the work and are excited for the possibilities this opens up.