Fix various codebase rots (stale CI, new Rust lints, broken MSRV checks by transitive dependency upgrades) #164
Conversation
[Rust 1.80 from July 25th 2024] points out that `armv7` is not a known, valid value for the `target_arch` cfg variable. This is confirmed by the docs not listing it either: https://doc.rust-lang.org/reference/conditional-compilation.html#target_arch Hence drop this entirely, and rely purely on `target_arch = "arm"`. [Rust 1.80 from July 25th 2024]: https://blog.rust-lang.org/2024/07/25/Rust-1.80.0.html
…test` Some code copied from the NDK carried over the respective `feature` `cfg` guards, without ever adding the feature to the `[features]` list in `Cargo.toml`. Now that Rust detects these mishaps, we can fix it by removing `test` (bindings don't seem to be run-tested) and reexpose `ConfigurationRef::screen_round()` which was behind a previously unsettable `feature = "api-level-30"`. Also remove `unsafe impl Send/Sync for ConfigurationRef` since the upstream `ndk` already declares `Configuration` to be `Send` and `Sync`, and `RwLock` and `Arc` carry that through.
|
@rib how do we want to deal with this for our MSRV: Shall we run a |
MarijnS95
changed the title
unexpected-cfgs lint detected since Rust 1.80
|
@notgull would love to have your review on this as well, now that the CI is made green again. |
|
Please rerun the CI, I don't think this fixes the "invalid configuration" errors I was getting. |
|
@notgull the CI has ran quite a few times on a personal branch before I force-pushed the final changes here, I didn't see any spurious "invalid configuration" errors on any run and don't see how those would randomly trigger on a re-run. Otherwise, please link your CI failure. There is only one run from your PR: https://github.com/rust-mobile/android-activity/actions/workflows/ci.yml?query=actor%3Anotgull And that run failed specifically because of the issues that are fixed in this PR: https://github.com/rust-mobile/android-activity/actions/runs/11771937579 |
…ndSync` and unwritten `redraw_needed` field
The `actions-rs` containers haven't been maintained and updated for years and don't need to: GitHub's actions environment already comes fully loaded with a complete `stable` Rust installation with the standard tools (in this case `rustfmt`). Remove the remaining toolchain setup (which was already replaced with `hecrj/setup-rust-action` elsewhere) to get rid of ancient Node 12 deprecation warnings.
…heck Use `-Zminimal-versions` in our MSRV check. This not only ensures our minimum version bounds are actually solid and tested (even if they may be a bit conservative at times, i.e. we could allow older versions except for the crates that are bumped in this patch which were explicitly build-tested), it also allows us to use this as a base for the MSRV test, and preempt us from failing it whenever a (transitive!) dependency bumps its MSRV beyond ours in a *semver-compatible* release.
| /// The value held by this reference **will change** with every [`super::MainEvent::ConfigChanged`] | ||
| /// event that is raised. You should **not** [`Clone`] this type to compare it against a | ||
| /// "new" [`super::AndroidApp::config()`] when that event is raised, since both point to the same | ||
| /// internal [`ndk::configuration::Configuration`] and will be identical. |
There was a problem hiding this comment.
@rib I don't have any alternative to propose to users to compare configurations besides calling the expensive fn copy() to make a deep-clone of ndk::configuration::Configuration which exactly defeats the purpose of initially making this "cheap to copy".
By having this an updatable RwLock rather than a simple Arc it is no longer possible to compare for configuration differences and prevent possibly expensive reconfiguration/updates in the users' loops. If this was "just an Arc" users could do exactly that while still "cloning" the configuration around cheaply.
In any case, every time the callback fires a new AConfiguration is allocated and filled anyway, rather than "reading into" the existing AConfiguration. But the docs at https://developer.android.com/ndk/reference/group/configuration#aconfiguration_fromassetmanager are incredibly vague by mentioning Create and return a new AConfiguration while it has to take an existing allocated (but possibly new/empty?) AConfiguration.
Rust 1.80 from July 25th 2024 points out that
armv7is not a known, valid value for thetarget_archcfg variable. This is confirmed by the docs not listing it either: https://doc.rust-lang.org/reference/conditional-compilation.html#target_arch.Hence drop this entirely, and rely purely on
target_arch = "arm".Furthermore it also points out that we don't have a
testandapi-level-30feature: remove the unusedtestfeature and reexposeConfigurationRef::screen_round()which was behind a previously unsettablefeature = "api-level-30".EDIT: PR updated to fix everything that was holding back our CI.