Release 0.5.1

[rib]

Cc: @Vrixyz, @MeoMix, @MarijnS95

[MarijnS95]

This is a breaking change fwiw... But we might just let it slide?

[MarijnS95]

As long as we add something along the lines of Warning: if you are depending on the `rwh_06` feature from the `ndk`, enable it via a manual dependency of sorts.

[rib]

Yeah I had overlooked that we expose the NativeWindow type.

I'm not 100% sure atm. It's a bit of a fuzzy area.

In this guide for Rust semver it would generally consider changing a feature for a dependency a minor change if that doesn't introduce a breaking change: https://doc.rust-lang.org/cargo/reference/semver.html#cargo-change-dep-feature

The question would be whether to consider the NativeWindow API change to be a breaking change for the android-activity API - and I'm not really sure that it does.

In practice, probably all that matters is whether Winit, or other crates that use raw-window-handle API are ensuring they enable whatever version they need explicitly without somehow being dependent on the default features that android-activity might enable.

In winit it looks like rwh_06 is also a default that explicitly enables the corresponding ndk feature so I don't think anything based on Winit will be dependent on whatever default android-activity enables.

So I think it might be safe to not consider this a breaking change?

[MarijnS95]

if that doesn't introduce a breaking change

It does, because an expected trait on a public type is no longer implemented, causing code depending on this to fail to compile.

In practice, probably all that matters is whether Winit, or other crates that use raw-window-handle API are ensuring they enable whatever version they need explicitly without somehow being dependent on the default features that android-activity might enable.

Winit may use this (and turns on the appropriate features), but it's more likely that a user is also utilizing it and not one of our plumbing crates.

---

We should always consider this a breaking change. The question is, are we okay with cheating and pretending it is safe for a patch release?

[MarijnS95]

It's a grey area because:

1. Probably barely anyone is using this raw;
2. It's easy to turn on the feature after all (but it's still a compilation-breaking change).

[MeoMix]

I'm admittedly quite biased because I want this shipped, so just acknowledging that upfront.

I agree with @MarijnS95 that this is a breaking change, but that it still falls into a gray area because of the relatively minor consequences. This isn't asking those affected, if there are any, to commit significant effort to accommodate us. It's asking them to update their project with an explicit feature flag. We should strive to make this need well documented so that, if someone's build process breaks, that the time loss is minimal and the rationale behind the change is clear.

IMO, all software development processes exist to serve us and to make life easier on us. In scenarios where we feel beholden to the process, but our intuition hints that the process might be limiting us, we should favor our intuition rather than becoming encumbered.

If I were depending on this project, and this update broke my project, but I were able to quickly go to the changelog and find exactly what happened with supporting discussion, I would not significantly fault anyone here. There would be a minor annoyance that would then pass quickly as I discover the fix is trivial.

That said, it's not trust in my project that's on the line here and without stake in the game I don't know how much weight my opinion should hold.

[rib]

Yep; I also see semver as a tool, and there can be cases where for all intents and purposes a change that is technically a semver break isn't actually going to be an API break in practice - and this seems to be one of those cases.

Playing devils advocate briefly though - I have to admit I also don't really see that this is that important of a change for justifying potential breakage (even though breakages seems fairly unlikely) and maybe Bevy shouldn't see it as a blocker that for an interim period it may pull in two versions of raw-window-handle while it's stuck using 0.5.

The ideal fix at some point will be that the ndk crate won't pull in any raw-window-handle crate by default - and by the same logic used here, couldn't we also just remove the default from the ndk crate which would be more beneficial I'd guess?

To some extent android-activity is essentially just one arbitrary crate that happens to depend on the ndk crate with default features, and instead of having a whack 'a' mole situation where you'd need to stop all crates depending on the ndk from using default features if you're trying to avoid raw-window-handles 0.6 then we could instead address this in the ndk crate.

[MarijnS95]

@rib I've already confirmed that this will be gone from the next breaking ndk release; but I find it more likely that those with raw use of the ndk crate rely on its raw-window-handle integration than those depending on it only indirectly via the android-activity crate.

[rib]

As long as we add something along the lines of Warning: if you are depending on the `rwh_06` feature from the `ndk`, enable it via a manual dependency of sorts.

yeah, I've put in a big blurb explaining that technically the change is breaking and said to enable rwh_06 explicitly in the unlikely case that something was depending on it being enable by default by the android-activity crate's ndk dependency.

[rib]

@rib I've already confirmed that this will be gone from the next breaking ndk release; but I find it more likely that those with raw use of the ndk crate rely on its raw-window-handle integration than those depending on it only indirectly via the android-activity crate.

Right, I'm wondering, out-loud, though if we could also justify removing the default for the ndk crate in a patch release too if we also figure that it's unlikely that anyone depends on it currently.