Fix CloudSync on Windows, enable by default

[KiralyCraft]

Fix Windows CloudSync and enable by default

Related Issues

#6875

Related Pull Requests

#15548

Reviewers

@warmenhoven

Pending to test

• Linux/Android/iOS initial sync + Windows MSVC builds (2019, 2022, UWP) target
• Linux/Android/iOS initial sync + Windows i686 (MXE (MinGW)) target
• Linux/Android/iOS initial sync + Windows x64 (MXE (MinGW)) target
• Windows initial sync + Linux/iOS/Android build

[hizzlekizzle]

Hey, just checking in on this. Is there anything you need from our end?

[KiralyCraft]

Hi, thanks for checking in. I didn't get to finishing the tests yet, I got stuck at trying to enable OpenSSL for Windows UWP builds. I ran out of time, and put the project on hold for the time being. They have a guide for compilation here: https://github.com/openssl/openssl/blob/master/NOTES-WINDOWS.md

I remember getting stuck at the end, something failed to link (or compile) within OpenSSL itself. Then, there's the matter of how to integrate that into the Visual Studio project in such a way that the "steps" of getting it compiled can be defined and built by the retroarch project itself.

In order to enable SSL for UWP, one would need a way of automating the build of OpenSSL while building retroarch. Windows is not particularly my cup of tea, so this is a bit slower. Although, enabling UWP SSL would also bring Cloud Sync to Xbox platforms, if I'm not mistaken. Maybe, if anyone has an Xbox with UWP, would you mind testing it there as-is, without SSL?

[hizzlekizzle]

Ah, okay, I don't have any xbox myself, but I'll see if I can get some eyes on this. Thanks for the update :)

[LibretroAdmin]

What if we excluded this for now for UWP builds so that we can at least merge this in for the non-UWP versions? Would that be a suitable compromise?

Also, I take it the current implementation and how this works is:

• The feature is compiled in, but the setting defaults to off and you have to manually enable it, right?

That is the way I'd like to see it implemented at least.

[KiralyCraft]

That could work, it seems that non-UWP builds work normally. In order to avoid confusion, I would've suggested that all enabling for a platform to be done at once. However, it seems that time is tight, and this feature works quite well as-is.

However, there's still one more thing we could do: Enable this for UWP as well (and Android), but define a flag which, if missing, changes the text associated with Cloud Sync to mention that it's experimental and unencrypted.

This would be needed anyway, since there are some platforms (such as the PS2 and Wii) where the "weight" of an SSL library would probably outweigh the benefits it brings.

The feature is compiled in, but the setting defaults to off and you have to manually enable it, right?

Yes, you have to manually enable and configure it with credentials and a WebDav server, from the "Saving" menu. The reason for concern about SSL is particularly this logging in, which over unencrypted connections makes me feel uneasy.

However, if the user is willing to take the risk, or is providing encryption some other way for the time being (like I am on Android, with a VPN), we might as well let them enable it.

I was thinking of:

• Using HAVE_SSL to change the label of this feature, to mention it's unencrypted
• Defining HAVE_CLOUDSYNC_TESTED to hide the fact that the feature is experimental

For example, a build where only HAVE_CLOUDSYNC is enabled would show the feature like Cloud Sync (Unencrypted) (Experimental), whereas an SSL-enabled but untested build would say Cloud Sync (Experimental).

We could probably drop this "tested" flag at some point. Given that this feature actually modifies user data, I would suggest to take this precaution, in order to avoid chaos and data corruption. Would that be alright?

[KiralyCraft]

I haven't forgotten about this, but I didn't really have time for it. It's still on the list, but unforeseen events forced a slight detour from this project. Just checking in :)

[kbridgford]

Just got webdav / cloud sync set up for IOS. Would love if windows cloud sync worked as well!

[LibretroAdmin]

Hi, can the merge conflicts be resolved?

[KiralyCraft]

Still alive! Didn't yet get time to patch this, although some of the conflicts seem problematic indeed. The "read" which reverted my change of "stub_read" will definitely cause an issue with gzguts.h, which causes some issues on some of the Windows builds (although I'm not sure which one it was). I'm not sure about the "delete" which was changed to "free", but the read will definitely cause trouble. This is why one of the commits changed it to stub_, but I don't have the problematic file on hand right now to confirm the situation for "free" as well.

[KiralyCraft]

So it seems that the underlying problems about SSL in UWP builds keeps this PR from merging. Therefore, I've come to the conclusion that Cloud Sync itself is more important than having SSL for it, so I'm thinking about splitting this feature in multiple PRs. One would be this one, which enables proper Cloud Sync for non-UWP Windows builds and SSL-less Cloud Sync for UWP builds, and there would be another which would enable HAVE_SSL on UWP. Currently, it seems that my latest commit broke "ReleaseAngle" UWP builds (though I'm not sure why, it doesn't seem related). I'll look into it, but aside from this I'd consider this ready.

This is also what keeps Android Cloud Sync from being a thing, so I'll take the same approach there.

EDIT: Fixed UWP builds (without SSL), this is now ready!

[KiralyCraft]

@warmenhoven There's also this to check out, I edited the original post but I'm not sure if you got notified that way

[warmenhoven]

Good catch here.

[KiralyCraft]

I wonder, is there anything preventing this PR from merging? I see that I've committed some of the files chaotically. Would creating another PR with better descriptions help?