feat(wasm): Change matrix-sdk-ffi to rely on features over platform targets

[zzorba]

The system of platform targets was already quite messy, and becoming even worse as we start preparing for Wasm support. Switch to features instead to make this easier to work with.

The idea is that whether than a large number of platform specific checks in the toml file, these should be cfg features that can be enabled/disabled on the various platforms as needed. The xtask build system for the matrix-sdk-ffi bindings has been updated to use this to maintain existing behavior.

The new features are:

• sentry: whether sentry is included in setup, the rust sentry library does not work on wasm platforms.
• rustls-tls: whether rustls-tls should be used for network, this was previously implemented with target based conditionals.
• native-tls: whether native-tls should be used for network, this was previously implemented with target based conditionals.
• js: whether existing configs meant to run in a wasm environment targeting javascript are needed.

In the future, we will add the following additional features:

• react-native: used to ensure the uniffi exposed interfaces are the same across platforms. This will be used more in future PRs, and is meant to address a feature requirement of react-native (where native ios+android and web bindings must share a public interface, a limitation of react-native + typescript).
• sqlite: Will be used to control if sqlite session store is included and exposed.
• indexeddb: Will be used to control if indexeddb session store is included and exposed.

• Public API changes documented in changelogs (optional)

Signed-off-by: Daniel Salinas

[jplatte]

It would be nice to split the addition of the react-native feature into its own PR (because that's a new feature, everything else is refactors). Also the store backends maybe? Then this could already be reviewed and merged ahead of the remaining changes.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.12%. Comparing base (cc7f624) to head (4b83233).
Report is 19 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5211      +/-   ##
==========================================
- Coverage   90.13%   90.12%   -0.01%     
==========================================
  Files         334      334              
  Lines      104717   104717              
  Branches   104717   104717              
==========================================
- Hits        94389    94381       -8     
- Misses       6275     6282       +7     
- Partials     4053     4054       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zzorba]

The complement-crypto system just needs to have --feature native-tls added to it, but I'm not sure how that is handled. I opened a PR on that branch, but it will fail until this PR is landed: matrix-org/complement-crypto#189

[zzorba]

@Hywan this contains the recommendations we discussed today, along with its complement-crypto PR in matrix-org/complement-crypto#189

[poljar]

The complement-crypto system just needs to have --feature native-tls added to it, but I'm not sure how that is handled. I opened a PR on that branch, but it will fail until this PR is landed: matrix-org/complement-crypto#189

If you name the branch in complement crypto the same as the one here, complement crypto should™ use the branch for the CI here. Though perhaps that doesn't work for branches on forks.

[zzorba]

Worth a shot, opened up matrix-org/complement-crypto#190 on complement-crypto with the same branch name, though it won't run without someone granting it permission.

[poljar]

I left a couple of nits, can you please document the feature flags in the readme and explain why we're using feature flags instead of configuring things for users on a per-platform basis.

[poljar]

Suggested change

	# Use native-tls system, necessary on ios and wasm platforms.
	# Use the TLS implementation provided by the host system, necessary on iOS and Wasm platforms.

[poljar]

Suggested change

	# Enable sentry, not compatible with wasm platforms.
	# Enable sentry, not compatible with Wasm platforms.

[poljar]

There's a missing category for this entry and a missing newline and the entry doesn't quite explain what's going on. It's also a breaking feature.

Could we reword this somehow to explain that we're switching from platform specific configuration to feature specific configuration, explaining the rationale here would be helpful to other consumers as well.

Suggested change

	- Adjust features in the `matrix-sdk-ffi` crate to expose more platform-specific knobs
	- Adjust features in the `matrix-sdk-ffi` crate to expose more platform-specific knobs

[zzorba]

Thank you for the feedback @poljar, I expanded the Changelog with a motivation and backwards compatibility instructions.

I added a similar entry to the README file explaining the current features.

[poljar]

I left a small nit about whitespace in the changelog file, also please resolve the conflicts. After that we can merge this.

[zzorba]

@poljar merge conflicts addressed and last changes updated.

I believe complement-crypto should work with the corresponding PR, the code-coverage break seems to be intermittent?

[zzorba]

@poljar / @Hywan I've removed the "lib" change and placed it in its own PR. Can this be landed now?

[jplatte]

I don't think this can merge before CI is green, but looks like that's being worked on separately. Changes LGTM.

[zzorba]

Besides the break, the complement crypto won't go green without the corresponding pr, so one will have to break in order to land this

[zzorba]

I rebased to try to make the CI happy.