Skip to content

Commit

Permalink
Fix various codebase rots (stale CI, new Rust lints, broken MSRV chec…
Browse files Browse the repository at this point in the history
…ks by transitive dependency upgrades) (#164)

* game_activity/ffi: Drop cfg for inexistant `target_arch = "armv7"`

[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

* Fix `unexpected-cfgs` by adding `api-level-30` feature and removing `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.

* native_activity: Fix clippy lints around `NativeActivityGlue` not `SendSync` and unwritten `redraw_needed` field

* CI: Remove deprecated/unmaintained `actions-rs` toolchain setup

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.

* Bump dependency patch-versions to fix `-Zminimal-versions` and MSRV check

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.

* Elide redundant `impl` block lifetimes following stricter Rust 1.83 lint

Rust now points out that `impl<'a> (Trait for) Struct<'a>` is
superfluous whenever `'a` is not used anywhere else in the `impl` block.
  • Loading branch information
MarijnS95 authored Jan 27, 2025
1 parent 0d29930 commit fe171bc
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 49 deletions.
21 changes: 14 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ jobs:
steps:
- uses: actions/checkout@v4

# Downgrade all dependencies to their minimum version, both to ensure our
# minimum version bounds are correct and buildable, as well as to satisfy
# our MSRV check when arbitrary dependencies bump their MSRV beyond our
# MSRV in a patch-release.
# This implies that downstream consumers can only rely on our MSRV when
# downgrading various (transitive) dependencies.
- uses: hecrj/setup-rust-action@v2
with:
rust-version: nightly
if: ${{ matrix.rust-version != 'stable' }}
- name: Downgrade dependencies
run: cargo +nightly generate-lockfile -Zminimal-versions
if: ${{ matrix.rust-version != 'stable' }}

- uses: hecrj/setup-rust-action@v2
with:
rust-version: ${{ matrix.rust-version }}
Expand Down Expand Up @@ -87,13 +101,6 @@ jobs:
steps:
- uses: actions/checkout@v4

- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
override: true
components: rustfmt

- name: Format
run: cargo fmt --all -- --check
working-directory: android-activity
Expand Down
7 changes: 4 additions & 3 deletions android-activity/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ rust-version = "1.69.0"
default = []
game-activity = []
native-activity = []
api-level-30 = ["ndk/api-level-30"]

[dependencies]
log = "0.4"
Expand All @@ -35,15 +36,15 @@ cesu8 = "1"
jni = "0.21"
ndk-sys = "0.6.0"
ndk = { version = "0.9.0", default-features = false }
ndk-context = "0.1"
ndk-context = "0.1.1"
android-properties = "0.2"
num_enum = "0.7"
bitflags = "2.0"
libc = "0.2"
libc = "0.2.139"
thiserror = "1"

[build-dependencies]
cc = { version = "1.0", features = ["parallel"] }
cc = { version = "1.0.42", features = ["parallel"] }

[package.metadata.docs.rs]
targets = [
Expand Down
13 changes: 6 additions & 7 deletions android-activity/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ use ndk::configuration::{
ScreenSize, Touchscreen, UiModeNight, UiModeType,
};

/// A (cheaply clonable) reference to this application's [`ndk::configuration::Configuration`]
/// A runtime-replacable reference to [`ndk::configuration::Configuration`].
///
/// This provides a thread-safe way to access the latest configuration state for
/// an application without deeply copying the large [`ndk::configuration::Configuration`] struct.
/// # Warning
///
/// If the application is notified of configuration changes then those changes
/// will become visible via pre-existing configuration references.
/// 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.
#[derive(Clone)]
pub struct ConfigurationRef {
config: Arc<RwLock<Configuration>>,
Expand All @@ -28,8 +29,6 @@ impl PartialEq for ConfigurationRef {
}
}
impl Eq for ConfigurationRef {}
unsafe impl Send for ConfigurationRef {}
unsafe impl Sync for ConfigurationRef {}

impl fmt::Debug for ConfigurationRef {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand Down
11 changes: 4 additions & 7 deletions android-activity/src/game_activity/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,14 @@ use jni_sys::*;
use libc::{pthread_cond_t, pthread_mutex_t, pthread_t};
use ndk_sys::{AAssetManager, AConfiguration, ALooper, ALooper_callbackFunc, ANativeWindow, ARect};

#[cfg(all(
any(target_os = "android", feature = "test"),
any(target_arch = "arm", target_arch = "armv7")
))]
#[cfg(all(any(target_os = "android"), target_arch = "arm"))]
include!("ffi_arm.rs");

#[cfg(all(any(target_os = "android", feature = "test"), target_arch = "aarch64"))]
#[cfg(all(any(target_os = "android"), target_arch = "aarch64"))]
include!("ffi_aarch64.rs");

#[cfg(all(any(target_os = "android", feature = "test"), target_arch = "x86"))]
#[cfg(all(any(target_os = "android"), target_arch = "x86"))]
include!("ffi_i686.rs");

#[cfg(all(any(target_os = "android", feature = "test"), target_arch = "x86_64"))]
#[cfg(all(any(target_os = "android"), target_arch = "x86_64"))]
include!("ffi_x86_64.rs");
6 changes: 3 additions & 3 deletions android-activity/src/game_activity/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ pub(crate) struct PointerImpl<'a> {
index: usize,
}

impl<'a> PointerImpl<'a> {
impl PointerImpl<'_> {
#[inline]
pub fn pointer_index(&self) -> usize {
self.index
Expand Down Expand Up @@ -333,7 +333,7 @@ impl<'a> Iterator for PointersIterImpl<'a> {
}
}

impl<'a> ExactSizeIterator for PointersIterImpl<'a> {
impl ExactSizeIterator for PointersIterImpl<'_> {
fn len(&self) -> usize {
self.count - self.next_index
}
Expand Down Expand Up @@ -740,7 +740,7 @@ impl<'a> KeyEvent<'a> {
}
}

impl<'a> KeyEvent<'a> {
impl KeyEvent<'_> {
/// Flags associated with this [`KeyEvent`].
///
/// See [the NDK docs](https://developer.android.com/ndk/reference/group/input#akeyevent_getflags)
Expand Down
8 changes: 4 additions & 4 deletions android-activity/src/game_activity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl<'a> StateSaver<'a> {
pub struct StateLoader<'a> {
app: &'a AndroidAppInner,
}
impl<'a> StateLoader<'a> {
impl StateLoader<'_> {
pub fn load(&self) -> Option<Vec<u8>> {
unsafe {
let app_ptr = self.app.native_app.as_ptr();
Expand Down Expand Up @@ -722,7 +722,7 @@ impl<'a> InputBuffer<'a> {
}
}

impl<'a> Drop for InputBuffer<'a> {
impl Drop for InputBuffer<'_> {
fn drop(&mut self) {
unsafe {
ffi::android_app_clear_motion_events(self.ptr.as_ptr());
Expand Down Expand Up @@ -801,7 +801,7 @@ pub(crate) struct InputIteratorInner<'a> {
text_event_checked: bool,
}

impl<'a> InputIteratorInner<'a> {
impl InputIteratorInner<'_> {
pub(crate) fn next<F>(&mut self, callback: F) -> bool
where
F: FnOnce(&input::InputEvent) -> InputStatus,
Expand Down Expand Up @@ -943,7 +943,7 @@ pub unsafe extern "C" fn _rust_glue_entry(native_app: *mut ffi::android_app) {
// code to look up non-standard Java classes.
android_main(app);
})
.unwrap_or_else(|panic| log_panic(panic));
.unwrap_or_else(log_panic);

// Let JVM know that our Activity can be destroyed before detaching from the JVM
//
Expand Down
6 changes: 3 additions & 3 deletions android-activity/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ pub struct InputIterator<'a> {
pub(crate) inner: crate::activity_impl::InputIteratorInner<'a>,
}

impl<'a> InputIterator<'a> {
impl InputIterator<'_> {
/// Reads and handles the next input event by passing it to the given `callback`
///
/// `callback` should return [`InputStatus::Unhandled`] for any input events that aren't directly
Expand All @@ -932,7 +932,7 @@ pub struct Pointer<'a> {
pub(crate) inner: PointerImpl<'a>,
}

impl<'a> Pointer<'a> {
impl Pointer<'_> {
#[inline]
pub fn pointer_index(&self) -> usize {
self.inner.pointer_index()
Expand Down Expand Up @@ -1026,7 +1026,7 @@ impl<'a> Iterator for PointersIter<'a> {
}
}

impl<'a> ExactSizeIterator for PointersIter<'a> {
impl ExactSizeIterator for PointersIter<'_> {
fn len(&self) -> usize {
self.inner.len()
}
Expand Down
15 changes: 11 additions & 4 deletions android-activity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,9 @@ pub enum MainEvent<'a> {
/// input focus.
LostFocus,

/// Command from main thread: the current device configuration has changed.
/// You can get a copy of the latest [`ndk::configuration::Configuration`] by calling
/// [`AndroidApp::config()`]
/// Command from main thread: the current device configuration has changed. Any
/// reference gotten via [`AndroidApp::config()`] will automatically contain the latest
/// [`ndk::configuration::Configuration`].
#[non_exhaustive]
ConfigChanged {},

Expand Down Expand Up @@ -617,7 +617,14 @@ impl AndroidApp {
self.inner.read().unwrap().create_waker()
}

/// Returns a (cheaply clonable) reference to this application's [`ndk::configuration::Configuration`]
/// Returns a **reference** to this application's [`ndk::configuration::Configuration`].
///
/// # Warning
///
/// The value held by this reference **will change** with every [`MainEvent::ConfigChanged`]
/// event that is raised. You should **not** [`Clone`] this type to compare it against a
/// "new" [`AndroidApp::config()`] when that event is raised, since both point to the same
/// internal [`ndk::configuration::Configuration`] and will be identical.
pub fn config(&self) -> ConfigurationRef {
self.inner.read().unwrap().config()
}
Expand Down
8 changes: 4 additions & 4 deletions android-activity/src/native_activity/glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,14 @@ pub struct WaitableNativeActivityState {
pub cond: Condvar,
}

// SAFETY: ndk::NativeActivity is also SendSync.
unsafe impl Send for WaitableNativeActivityState {}
unsafe impl Sync for WaitableNativeActivityState {}

#[derive(Debug, Clone)]
pub struct NativeActivityGlue {
pub inner: Arc<WaitableNativeActivityState>,
}
unsafe impl Send for NativeActivityGlue {}
unsafe impl Sync for NativeActivityGlue {}

impl Deref for NativeActivityGlue {
type Target = WaitableNativeActivityState;
Expand Down Expand Up @@ -223,7 +225,6 @@ pub struct NativeActivityState {
/// Set as soon as the Java main thread notifies us of an
/// `onDestroyed` callback.
pub destroyed: bool,
pub redraw_needed: bool,
pub pending_input_queue: *mut ndk_sys::AInputQueue,
pub pending_window: Option<NativeWindow>,
}
Expand Down Expand Up @@ -370,7 +371,6 @@ impl WaitableNativeActivityState {
thread_state: NativeThreadState::Init,
app_has_saved_state: false,
destroyed: false,
redraw_needed: false,
pending_input_queue: ptr::null_mut(),
pending_window: None,
}),
Expand Down
8 changes: 4 additions & 4 deletions android-activity/src/native_activity/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub struct MotionEvent<'a> {
ndk_event: ndk::event::MotionEvent,
_lifetime: PhantomData<&'a ndk::event::MotionEvent>,
}
impl<'a> MotionEvent<'a> {
impl MotionEvent<'_> {
pub(crate) fn new(ndk_event: ndk::event::MotionEvent) -> Self {
Self {
ndk_event,
Expand Down Expand Up @@ -248,7 +248,7 @@ pub(crate) struct PointerImpl<'a> {
ndk_pointer: ndk::event::Pointer<'a>,
}

impl<'a> PointerImpl<'a> {
impl PointerImpl<'_> {
#[inline]
pub fn pointer_index(&self) -> usize {
self.ndk_pointer.pointer_index()
Expand Down Expand Up @@ -303,7 +303,7 @@ impl<'a> Iterator for PointersIterImpl<'a> {
}
}

impl<'a> ExactSizeIterator for PointersIterImpl<'a> {
impl ExactSizeIterator for PointersIterImpl<'_> {
fn len(&self) -> usize {
self.ndk_pointers_iter.len()
}
Expand All @@ -319,7 +319,7 @@ pub struct KeyEvent<'a> {
ndk_event: ndk::event::KeyEvent,
_lifetime: PhantomData<&'a ndk::event::KeyEvent>,
}
impl<'a> KeyEvent<'a> {
impl KeyEvent<'_> {
pub(crate) fn new(ndk_event: ndk::event::KeyEvent) -> Self {
Self {
ndk_event,
Expand Down
6 changes: 3 additions & 3 deletions android-activity/src/native_activity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl<'a> StateSaver<'a> {
pub struct StateLoader<'a> {
app: &'a AndroidAppInner,
}
impl<'a> StateLoader<'a> {
impl StateLoader<'_> {
/// Returns whatever state was saved during the last [MainEvent::SaveState] event or `None`
pub fn load(&self) -> Option<Vec<u8>> {
self.app.native_activity.saved_state()
Expand Down Expand Up @@ -465,7 +465,7 @@ pub(crate) struct InputReceiver {
queue: Option<InputQueue>,
}

impl<'a> From<Arc<InputReceiver>> for InputIteratorInner<'a> {
impl From<Arc<InputReceiver>> for InputIteratorInner<'_> {
fn from(receiver: Arc<InputReceiver>) -> Self {
Self {
receiver,
Expand All @@ -480,7 +480,7 @@ pub(crate) struct InputIteratorInner<'a> {
_lifetime: PhantomData<&'a ()>,
}

impl<'a> InputIteratorInner<'a> {
impl InputIteratorInner<'_> {
pub(crate) fn next<F>(&self, callback: F) -> bool
where
F: FnOnce(&input::InputEvent) -> InputStatus,
Expand Down

0 comments on commit fe171bc

Please sign in to comment.