Merge pull request #149 from rust-mobile/release-0.5.1

Release 0.5.1

.github/workflows/ci.yml

@@ -17,13 +17,13 @@ jobs:
       fail-fast: false
       matrix:
         # See top README for MSRV policy
-        rust_version: [1.68.0, stable]
+        rust-version: [1.68.0, stable]
     steps:
       - uses: actions/checkout@v4
 
       - uses: hecrj/setup-rust-action@v2
         with:
-          rust-version: ${{ matrix.rust_version }}
+          rust-version: ${{ matrix.rust-version }}
 
       - name: Install Rust targets
         run: >
@@ -57,7 +57,7 @@ jobs:
           build --features native-activity
 
       - name: Build agdk-mainloop example
-        if: matrix.rust_version == 'stable'
+        if: matrix.rust-version == 'stable'
         working-directory: examples/agdk-mainloop
         run: >
           cargo ndk
@@ -68,7 +68,7 @@ jobs:
           -o app/src/main/jniLibs/ -- build
 
       - name: Build na-mainloop example
-        if: matrix.rust_version == 'stable'
+        if: matrix.rust-version == 'stable'
         working-directory: examples/na-mainloop
         run: >
           cargo ndk

README.md

@@ -3,7 +3,7 @@
 [![ci](https://github.com/rust-mobile/android-activity/actions/workflows/ci.yml/badge.svg)](https://github.com/rust-mobile/android-activity/actions/workflows/ci.yml)
 [![crates.io](https://img.shields.io/crates/v/android-activity.svg)](https://crates.io/crates/android-activity)
 [![Docs](https://docs.rs/android-activity/badge.svg)](https://docs.rs/android-activity)
-[![MSRV](https://img.shields.io/badge/rustc-1.64.0+-ab6000.svg)](https://blog.rust-lang.org/2022/09/22/Rust-1.64.0.html)
+[![MSRV](https://img.shields.io/badge/rustc-1.68.0+-ab6000.svg)](https://blog.rust-lang.org/2023/03/09/Rust-1.68.0.html)
 
 ## Overview
 
@@ -36,8 +36,8 @@ Cargo.toml
 ```toml
 [dependencies]
 log = "0.4"
-android_logger = "0.11"
-android-activity = { version = "0.4", features = [ "native-activity" ] }
+android_logger = "0.13"
+android-activity = { version = "0.5", features = [ "native-activity" ] }
 
 [lib]
 crate_type = ["cdylib"]
@@ -126,8 +126,8 @@ Middleware libraries can instead look at using the [ndk-context](https://crates.
 The steps to switch a simple standalone application over from `ndk-glue` to `android-activity` (still based on `NativeActivity`) should be:
 
 1. Remove `ndk-glue` from your Cargo.toml
-2. Add a dependency on `android-activity`, like `android-activity = { version="0.4", features = [ "native-activity" ] }`
-3. Optionally add a dependency on `android_logger = "0.11.0"`
+2. Add a dependency on `android-activity`, like `android-activity = { version="0.5", features = [ "native-activity" ] }`
+3. Optionally add a dependency on `android_logger = "0.13.0"`
 4. Update the `main` entry point to look like this:
 
 ```rust
@@ -157,8 +157,8 @@ Prior to working on android-activity, the existing glue crates available for bui
 ## MSRV
 
 We aim to (at least) support stable releases of Rust from the last three months. Rust has a 6 week release cycle which means we will support the last three stable releases.
-For example, when Rust 1.69 is released we would limit our `rust_version` to 1.67.
+For example, when Rust 1.69 is released we would limit our `rust-version` to 1.67.
 
-We will only bump the `rust_version` at the point where we either depend on a new features or a dependency has increased its MSRV, and we won't be greedy. In other words we will only set the MSRV to the lowest version that's _needed_.
+We will only bump the `rust-version` at the point where we either depend on a new features or a dependency has increased its MSRV, and we won't be greedy. In other words we will only set the MSRV to the lowest version that's _needed_.
 
-MSRV updates are not considered to be inherently semver breaking (unless a new feature is exposed in the public API) and so a `rust_version` change may happen in patch releases.
+MSRV updates are not considered to be inherently semver breaking (unless a new feature is exposed in the public API) and so a `rust-version` change may happen in patch releases.

android-activity/CHANGELOG.md

@@ -6,6 +6,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.5.1] - 2023-12-20
+
+### Changed
+- Avoids depending on default features for `ndk` crate to avoid pulling in any `raw-window-handle` dependencies ([#142](https://github.com/rust-mobile/android-activity/pull/142))
+
+    **Note:** Technically, this could be observed as a breaking change in case you
+    were depending on the `rwh_06` feature that was enabled by default in the
+    `ndk` crate. This could be observed via the `NativeWindow` type (exposed via
+    `AndroidApp::native_window()`) no longer implementing `rwh_06::HasWindowHandle`.
+
+    In the unlikely case that you were depending on the `ndk`'s `rwh_06` API
+    being enabled by default via `android-activity`'s `ndk` dependency, your crate
+    should explicitly enable the `rwh_06` feature for the `ndk` crate.
+
+    As far as could be seen though, it's not expected that anything was
+    depending on this (e.g. anything based on Winit enables the `ndk` feature
+    based on an equivalent `winit` feature).
+
+    The benefit of the change is that it can help avoid a redundant
+    `raw-window-handle 0.6` dependency in projects that still need to use older
+    (non-default) `raw-window-handle` versions. (Though note that this may be
+    awkward to achieve in practice since other crates that depend on the `ndk`
+    are still likely to use default features and also pull in
+    `raw-window-handles 0.6`)
+
+- The IO thread now gets named `stdio-to-logcat` and main thread is named `android_main` ([#145](https://github.com/rust-mobile/android-activity/pull/145))
+- Improved IO error handling in `stdio-to-logcat` IO loop. ([#133](https://github.com/rust-mobile/android-activity/pull/133))
+
 ## [0.5.0] - 2023-10-16
 ### Added
 - Added `MotionEvent::action_button()` exposing the button associated with button press/release actions ()

android-activity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "android-activity"
-version = "0.5.0"
+version = "0.5.1"
 edition = "2021"
 keywords = ["android", "ndk"]
 readme = "../README.md"
@@ -34,7 +34,7 @@ jni-sys = "0.3"
 cesu8 = "1"
 jni = "0.21"
 ndk-sys = "0.5.0"
-ndk = "0.8.0"
+ndk = { version = "0.8.0", default-features = false }
 ndk-context = "0.1"
 android-properties = "0.2"
 num_enum = "0.7"

android-activity/src/game_activity/mod.rs

@@ -1,21 +1,17 @@
 #![cfg(feature = "game-activity")]
 
 use std::collections::HashMap;
-use std::ffi::{CStr, CString};
-use std::fs::File;
-use std::io::{BufRead, BufReader};
 use std::marker::PhantomData;
 use std::ops::Deref;
-use std::os::unix::prelude::*;
 use std::panic::catch_unwind;
+use std::ptr;
 use std::ptr::NonNull;
 use std::sync::Weak;
 use std::sync::{Arc, Mutex, RwLock};
 use std::time::Duration;
-use std::{ptr, thread};
 
 use libc::c_void;
-use log::{error, trace, Level};
+use log::{error, trace};
 
 use jni_sys::*;
 
@@ -29,9 +25,9 @@ use ndk::native_window::NativeWindow;
 use crate::error::InternalResult;
 use crate::input::{Axis, KeyCharacterMap, KeyCharacterMapBinding};
 use crate::jni_utils::{self, CloneJavaVM};
-use crate::util::{abort_on_panic, android_log, log_panic};
+use crate::util::{abort_on_panic, forward_stdio_to_logcat, log_panic, try_get_path_from_ptr};
 use crate::{
-    util, AndroidApp, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, WindowManagerFlags,
+    AndroidApp, ConfigurationRef, InputStatus, MainEvent, PollEvent, Rect, WindowManagerFlags,
 };
 
 mod ffi;
@@ -617,21 +613,21 @@ impl AndroidAppInner {
     pub fn internal_data_path(&self) -> Option<std::path::PathBuf> {
         unsafe {
             let app_ptr = self.native_app.as_ptr();
-            util::try_get_path_from_ptr((*(*app_ptr).activity).internalDataPath)
+            try_get_path_from_ptr((*(*app_ptr).activity).internalDataPath)
         }
     }
 
     pub fn external_data_path(&self) -> Option<std::path::PathBuf> {
         unsafe {
             let app_ptr = self.native_app.as_ptr();
-            util::try_get_path_from_ptr((*(*app_ptr).activity).externalDataPath)
+            try_get_path_from_ptr((*(*app_ptr).activity).externalDataPath)
         }
     }
 
     pub fn obb_path(&self) -> Option<std::path::PathBuf> {
         unsafe {
             let app_ptr = self.native_app.as_ptr();
-            util::try_get_path_from_ptr((*(*app_ptr).activity).obbPath)
+            try_get_path_from_ptr((*(*app_ptr).activity).obbPath)
         }
     }
 }
@@ -768,18 +764,16 @@ impl<'a> From<Arc<InputReceiver>> for InputIteratorInner<'a> {
         let buffered = unsafe {
             let app_ptr = receiver.native_app.as_ptr();
             let input_buffer = ffi::android_app_swap_input_buffers(app_ptr);
-            if input_buffer.is_null() {
-                None
-            } else {
-                let buffer = InputBuffer::from_ptr(NonNull::new_unchecked(input_buffer));
+            NonNull::new(input_buffer).map(|input_buffer| {
+                let buffer = InputBuffer::from_ptr(input_buffer);
                 let keys_iter = KeyEventsLendingIterator::new(&buffer);
                 let motion_iter = MotionEventsLendingIterator::new(&buffer);
-                Some(BufferedEvents::<'a> {
+                BufferedEvents::<'a> {
                     buffer,
                     keys_iter,
                     motion_iter,
-                })
-            }
+                }
+            })
         };
 
         let native_app = receiver.native_app.clone();
@@ -913,33 +907,7 @@ extern "Rust" {
 #[no_mangle]
 pub unsafe extern "C" fn _rust_glue_entry(native_app: *mut ffi::android_app) {
     abort_on_panic(|| {
-        // Maybe make this stdout/stderr redirection an optional / opt-in feature?...
-
-        let file = {
-            let mut logpipe: [RawFd; 2] = Default::default();
-            libc::pipe2(logpipe.as_mut_ptr(), libc::O_CLOEXEC);
-            libc::dup2(logpipe[1], libc::STDOUT_FILENO);
-            libc::dup2(logpipe[1], libc::STDERR_FILENO);
-            libc::close(logpipe[1]);
-
-            File::from_raw_fd(logpipe[0])
-        };
-
-        thread::spawn(move || {
-            let tag = CStr::from_bytes_with_nul(b"RustStdoutStderr\0").unwrap();
-            let mut reader = BufReader::new(file);
-            let mut buffer = String::new();
-            loop {
-                buffer.clear();
-                if let Ok(len) = reader.read_line(&mut buffer) {
-                    if len == 0 {
-                        break;
-                    } else if let Ok(msg) = CString::new(buffer.clone()) {
-                        android_log(Level::Info, tag, &msg);
-                    }
-                }
-            }
-        });
+        let _join_log_forwarder = forward_stdio_to_logcat();
 
         let jvm = unsafe {
             let jvm = (*(*native_app).activity).vm;
@@ -955,6 +923,11 @@ pub unsafe extern "C" fn _rust_glue_entry(native_app: *mut ffi::android_app) {
         };
 
         unsafe {
+            // Name thread - this needs to happen here after attaching to a JVM thread,
+            // since that changes the thread name to something like "Thread-2".
+            let thread_name = std::ffi::CStr::from_bytes_with_nul(b"android_main\0").unwrap();
+            libc::pthread_setname_np(libc::pthread_self(), thread_name.as_ptr());
+
             let app = AndroidApp::from_ptr(NonNull::new(native_app).unwrap(), jvm.clone());
 
             // We want to specifically catch any panic from the application's android_main

android-activity/src/lib.rs

@@ -112,6 +112,8 @@
 //! [`GameActivity`]: https://developer.android.com/games/agdk/integrate-game-activity
 //! [Looper]: https://developer.android.com/reference/android/os/Looper
 
+#![deny(clippy::manual_let_else)]
+
 use std::hash::Hash;
 use std::sync::Arc;
 use std::sync::RwLock;

android-activity/src/native_activity/glue.rs

@@ -3,23 +3,17 @@
 //! synchronization between the two threads.
 
 use std::{
-    ffi::{CStr, CString},
-    fs::File,
-    io::{BufRead, BufReader},
     ops::Deref,
-    os::unix::prelude::{FromRawFd, RawFd},
     panic::catch_unwind,
     ptr::{self, NonNull},
     sync::{Arc, Condvar, Mutex, Weak},
 };
 
-use log::Level;
 use ndk::{configuration::Configuration, input_queue::InputQueue, native_window::NativeWindow};
 
 use crate::{
     jni_utils::CloneJavaVM,
-    util::android_log,
-    util::{abort_on_panic, log_panic},
+    util::{abort_on_panic, forward_stdio_to_logcat, log_panic},
     ConfigurationRef,
 };
 
@@ -834,32 +828,7 @@ extern "C" fn ANativeActivity_onCreate(
     saved_state_size: libc::size_t,
 ) {
     abort_on_panic(|| {
-        // Maybe make this stdout/stderr redirection an optional / opt-in feature?...
-        let file = unsafe {
-            let mut logpipe: [RawFd; 2] = Default::default();
-            libc::pipe2(logpipe.as_mut_ptr(), libc::O_CLOEXEC);
-            libc::dup2(logpipe[1], libc::STDOUT_FILENO);
-            libc::dup2(logpipe[1], libc::STDERR_FILENO);
-            libc::close(logpipe[1]);
-
-            File::from_raw_fd(logpipe[0])
-        };
-
-        std::thread::spawn(move || {
-            let tag = CStr::from_bytes_with_nul(b"RustStdoutStderr\0").unwrap();
-            let mut reader = BufReader::new(file);
-            let mut buffer = String::new();
-            loop {
-                buffer.clear();
-                if let Ok(len) = reader.read_line(&mut buffer) {
-                    if len == 0 {
-                        break;
-                    } else if let Ok(msg) = CString::new(buffer.clone()) {
-                        android_log(Level::Info, tag, &msg);
-                    }
-                }
-            }
-        });
+        let _join_log_forwarder = forward_stdio_to_logcat();
 
         log::trace!(
             "Creating: {:p}, saved_state = {:p}, save_state_size = {}",
@@ -899,6 +868,11 @@ extern "C" fn ANativeActivity_onCreate(
             rust_glue.notify_main_thread_running();
 
             unsafe {
+                // Name thread - this needs to happen here after attaching to a JVM thread,
+                // since that changes the thread name to something like "Thread-2".
+                let thread_name = std::ffi::CStr::from_bytes_with_nul(b"android_main\0").unwrap();
+                libc::pthread_setname_np(libc::pthread_self(), thread_name.as_ptr());
+
                 // We want to specifically catch any panic from the application's android_main
                 // so we can finish + destroy the Activity gracefully via the JVM
                 catch_unwind(|| {

android-activity/src/native_activity/mod.rs

@@ -485,11 +485,7 @@ impl<'a> InputIteratorInner<'a> {
     where
         F: FnOnce(&input::InputEvent) -> InputStatus,
     {
-        // XXX: would use `let Some(queue) = &self.receiver.queue else { return
-        // false; }` but we're stuck supporting Rust 1.64 for Winit currently
-        let queue = if let Some(queue) = &self.receiver.queue {
-            queue
-        } else {
+        let Some(queue) = &self.receiver.queue else {
             log::trace!("no queue available for events");
             return false;
         };

android-activity/src/util.rs

@@ -1,7 +1,12 @@
-use log::Level;
+use log::{error, Level};
 use std::{
     ffi::{CStr, CString},
-    os::raw::c_char,
+    fs::File,
+    io::{BufRead as _, BufReader, Result},
+    os::{
+        fd::{FromRawFd as _, RawFd},
+        raw::c_char,
+    },
 };
 
 pub fn try_get_path_from_ptr(path: *const c_char) -> Option<std::path::PathBuf> {
@@ -31,6 +36,44 @@ pub(crate) fn android_log(level: Level, tag: &CStr, msg: &CStr) {
     }
 }
 
+pub(crate) fn forward_stdio_to_logcat() -> std::thread::JoinHandle<Result<()>> {
+    // XXX: make this stdout/stderr redirection an optional / opt-in feature?...
+
+    let file = unsafe {
+        let mut logpipe: [RawFd; 2] = Default::default();
+        libc::pipe2(logpipe.as_mut_ptr(), libc::O_CLOEXEC);
+        libc::dup2(logpipe[1], libc::STDOUT_FILENO);
+        libc::dup2(logpipe[1], libc::STDERR_FILENO);
+        libc::close(logpipe[1]);
+
+        File::from_raw_fd(logpipe[0])
+    };
+
+    std::thread::Builder::new()
+        .name("stdio-to-logcat".to_string())
+        .spawn(move || -> Result<()> {
+            let tag = CStr::from_bytes_with_nul(b"RustStdoutStderr\0").unwrap();
+            let mut reader = BufReader::new(file);
+            let mut buffer = String::new();
+            loop {
+                buffer.clear();
+                let len = match reader.read_line(&mut buffer) {
+                    Ok(len) => len,
+                    Err(e) => {
+                        error!("Logcat forwarder failed to read stdin/stderr: {e:?}");
+                        break Err(e);
+                    }
+                };
+                if len == 0 {
+                    break Ok(());
+                } else if let Ok(msg) = CString::new(buffer.clone()) {
+                    android_log(Level::Info, tag, &msg);
+                }
+            }
+        })
+        .expect("Failed to start stdout/stderr to logcat forwarder thread")
+}
+
 pub(crate) fn log_panic(panic: Box<dyn std::any::Any + Send>) {
     let rust_panic = unsafe { CStr::from_bytes_with_nul_unchecked(b"RustPanic\0") };