[3.14] gh-137242: Allow Android testbed to take all Python command-line options (GH-138805) (#139637)

Co-authored-by: Malcolm Smith <smith@chaquo.com>

Android/testbed/app/src/androidTest/java/org/python/testbed/PythonSuite.kt

@@ -20,7 +20,7 @@ class PythonSuite {
             val status = PythonTestRunner(
                 InstrumentationRegistry.getInstrumentation().targetContext
             ).run(
-                InstrumentationRegistry.getArguments()
+                InstrumentationRegistry.getArguments().getString("pythonArgs")!!,
             )
             assertEquals(0, status)
         } finally {

Android/testbed/app/src/main/c/main_activity.c

@@ -3,6 +3,7 @@
 #include <jni.h>
 #include <pthread.h>
 #include <Python.h>
+#include <signal.h>
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
@@ -15,6 +16,13 @@ static void throw_runtime_exception(JNIEnv *env, const char *message) {
         message);
 }
 
+static void throw_errno(JNIEnv *env, const char *error_prefix) {
+    char error_message[1024];
+    snprintf(error_message, sizeof(error_message),
+             "%s: %s", error_prefix, strerror(errno));
+    throw_runtime_exception(env, error_message);
+}
+
 
 // --- Stdio redirection ------------------------------------------------------
 
@@ -95,10 +103,7 @@ JNIEXPORT void JNICALL Java_org_python_testbed_PythonTestRunner_redirectStdioToL
     for (StreamInfo *si = STREAMS; si->file; si++) {
         char *error_prefix;
         if ((error_prefix = redirect_stream(si))) {
-            char error_message[1024];
-            snprintf(error_message, sizeof(error_message),
-                     "%s: %s", error_prefix, strerror(errno));
-            throw_runtime_exception(env, error_message);
+            throw_errno(env, error_prefix);
             return;
         }
     }
@@ -107,13 +112,38 @@ JNIEXPORT void JNICALL Java_org_python_testbed_PythonTestRunner_redirectStdioToL
 
 // --- Python initialization ---------------------------------------------------
 
-static PyStatus set_config_string(
-    JNIEnv *env, PyConfig *config, wchar_t **config_str, jstring value
-) {
-    const char *value_utf8 = (*env)->GetStringUTFChars(env, value, NULL);
-    PyStatus status = PyConfig_SetBytesString(config, config_str, value_utf8);
-    (*env)->ReleaseStringUTFChars(env, value, value_utf8);
-    return status;
+static char *init_signals() {
+    // Some tests use SIGUSR1, but that's blocked by default in an Android app in
+    // order to make it available to `sigwait` in the Signal Catcher thread.
+    // (https://cs.android.com/android/platform/superproject/+/android14-qpr3-release:art/runtime/signal_catcher.cc).
+    // That thread's functionality is only useful for debugging the JVM, so disabling
+    // it should not weaken the tests.
+    //
+    // There's no safe way of stopping the thread completely (#123982), but simply
+    // unblocking SIGUSR1 is enough to fix most tests.
+    //
+    // However, in tests that generate multiple different signals in quick
+    // succession, it's possible for SIGUSR1 to arrive while the main thread is busy
+    // running the C-level handler for a different signal. In that case, the SIGUSR1
+    // may be sent to the Signal Catcher thread instead, which will generate a log
+    // message containing the text "reacting to signal".
+    //
+    // Such tests may need to be changed in one of the following ways:
+    //   * Use a signal other than SIGUSR1 (e.g. test_stress_delivery_simultaneous in
+    //     test_signal.py).
+    //   * Send the signal to a specific thread rather than the whole process (e.g.
+    //     test_signals in test_threadsignals.py.
+    sigset_t set;
+    if (sigemptyset(&set)) {
+        return "sigemptyset";
+    }
+    if (sigaddset(&set, SIGUSR1)) {
+        return "sigaddset";
+    }
+    if ((errno = pthread_sigmask(SIG_UNBLOCK, &set, NULL))) {
+        return "pthread_sigmask";
+    }
+    return NULL;
 }
 
 static void throw_status(JNIEnv *env, PyStatus status) {
@@ -121,27 +151,47 @@ static void throw_status(JNIEnv *env, PyStatus status) {
 }
 
 JNIEXPORT int JNICALL Java_org_python_testbed_PythonTestRunner_runPython(
-    JNIEnv *env, jobject obj, jstring home, jstring runModule
+    JNIEnv *env, jobject obj, jstring home, jarray args
 ) {
+    const char *home_utf8 = (*env)->GetStringUTFChars(env, home, NULL);
+    char cwd[PATH_MAX];
+    snprintf(cwd, sizeof(cwd), "%s/%s", home_utf8, "cwd");
+    if (chdir(cwd)) {
+        throw_errno(env, "chdir");
+        return 1;
+    }
+
+    char *error_prefix;
+    if ((error_prefix = init_signals())) {
+        throw_errno(env, error_prefix);
+        return 1;
+    }
+
     PyConfig config;
     PyStatus status;
-    PyConfig_InitIsolatedConfig(&config);
+    PyConfig_InitPythonConfig(&config);
 
-    status = set_config_string(env, &config, &config.home, home);
-    if (PyStatus_Exception(status)) {
+    jsize argc = (*env)->GetArrayLength(env, args);
+    const char *argv[argc + 1];
+    for (int i = 0; i < argc; i++) {
+        jobject arg = (*env)->GetObjectArrayElement(env, args, i);
+        argv[i] = (*env)->GetStringUTFChars(env, arg, NULL);
+    }
+    argv[argc] = NULL;
+
+    // PyConfig_SetBytesArgv "must be called before other methods, since the
+    // preinitialization configuration depends on command line arguments"
+    if (PyStatus_Exception(status = PyConfig_SetBytesArgv(&config, argc, (char**)argv))) {
         throw_status(env, status);
         return 1;
     }
 
-    status = set_config_string(env, &config, &config.run_module, runModule);
+    status = PyConfig_SetBytesString(&config, &config.home, home_utf8);
     if (PyStatus_Exception(status)) {
         throw_status(env, status);
         return 1;
     }
 
-    // Some tests generate SIGPIPE and SIGXFSZ, which should be ignored.
-    config.install_signal_handlers = 1;
-
     status = Py_InitializeFromConfig(&config);
     if (PyStatus_Exception(status)) {
         throw_status(env, status);

Android/testbed/app/src/main/java/org/python/testbed/MainActivity.kt

@@ -5,6 +5,7 @@ import android.os.*
 import android.system.Os
 import android.widget.TextView
 import androidx.appcompat.app.*
+import org.json.JSONArray
 import java.io.*
 
 
@@ -15,30 +16,25 @@ class MainActivity : AppCompatActivity() {
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         setContentView(R.layout.activity_main)
-        val status = PythonTestRunner(this).run("-m", "test", "-W -uall")
+        val status = PythonTestRunner(this).run("""["-m", "test", "-W", "-uall"]""")
         findViewById<TextView>(R.id.tvHello).text = "Exit status $status"
     }
 }
 
 
 class PythonTestRunner(val context: Context) {
-    fun run(instrumentationArgs: Bundle) = run(
-        instrumentationArgs.getString("pythonMode")!!,
-        instrumentationArgs.getString("pythonModule")!!,
-        instrumentationArgs.getString("pythonArgs") ?: "",
-    )
-
     /** Run Python.
      *
-     * @param mode Either "-c" or "-m".
-     * @param module Python statements for "-c" mode, or a module name for
-     *     "-m" mode.
-     * @param args Arguments to add to sys.argv. Will be parsed by `shlex.split`.
+     * @param args Python command-line, encoded as JSON.
      * @return The Python exit status: zero on success, nonzero on failure. */
-    fun run(mode: String, module: String, args: String) : Int {
-        Os.setenv("PYTHON_MODE", mode, true)
-        Os.setenv("PYTHON_MODULE", module, true)
-        Os.setenv("PYTHON_ARGS", args, true)
+    fun run(args: String) : Int {
+        // We leave argument 0 as an empty string, which is a placeholder for the
+        // executable name in embedded mode.
+        val argsJsonArray = JSONArray(args)
+        val argsStringArray = Array<String>(argsJsonArray.length() + 1) { it -> ""}
+        for (i in 0..<argsJsonArray.length()) {
+            argsStringArray[i + 1] = argsJsonArray.getString(i)
+        }
 
         // Python needs this variable to help it find the temporary directory,
         // but Android only sets it on API level 33 and later.
@@ -47,10 +43,7 @@ class PythonTestRunner(val context: Context) {
         val pythonHome = extractAssets()
         System.loadLibrary("main_activity")
         redirectStdioToLogcat()
-
-        // The main module is in src/main/python. We don't simply call it
-        // "main", as that could clash with third-party test code.
-        return runPython(pythonHome.toString(), "android_testbed_main")
+        return runPython(pythonHome.toString(), argsStringArray)
     }
 
     private fun extractAssets() : File {
@@ -59,6 +52,13 @@ class PythonTestRunner(val context: Context) {
             throw RuntimeException("Failed to delete $pythonHome")
         }
         extractAssetDir("python", context.filesDir)
+
+        // Empty directories are lost in the asset packing/unpacking process.
+        val cwd = File(pythonHome, "cwd")
+        if (!cwd.exists()) {
+            cwd.mkdir()
+        }
+
         return pythonHome
     }
 
@@ -88,5 +88,5 @@ class PythonTestRunner(val context: Context) {
     }
 
     private external fun redirectStdioToLogcat()
-    private external fun runPython(home: String, runModule: String) : Int
+    private external fun runPython(home: String, args: Array<String>) : Int
 }

Android/testbed/app/src/main/python/android_testbed_main.py

@@ -1,48 +0,0 @@
-import os
-import runpy
-import shlex
-import signal
-import sys
-
-# Some tests use SIGUSR1, but that's blocked by default in an Android app in
-# order to make it available to `sigwait` in the Signal Catcher thread.
-# (https://cs.android.com/android/platform/superproject/+/android14-qpr3-release:art/runtime/signal_catcher.cc).
-# That thread's functionality is only useful for debugging the JVM, so disabling
-# it should not weaken the tests.
-#
-# There's no safe way of stopping the thread completely (#123982), but simply
-# unblocking SIGUSR1 is enough to fix most tests.
-#
-# However, in tests that generate multiple different signals in quick
-# succession, it's possible for SIGUSR1 to arrive while the main thread is busy
-# running the C-level handler for a different signal. In that case, the SIGUSR1
-# may be sent to the Signal Catcher thread instead, which will generate a log
-# message containing the text "reacting to signal".
-#
-# Such tests may need to be changed in one of the following ways:
-#   * Use a signal other than SIGUSR1 (e.g. test_stress_delivery_simultaneous in
-#     test_signal.py).
-#   * Send the signal to a specific thread rather than the whole process (e.g.
-#     test_signals in test_threadsignals.py.
-signal.pthread_sigmask(signal.SIG_UNBLOCK, [signal.SIGUSR1])
-
-mode = os.environ["PYTHON_MODE"]
-module = os.environ["PYTHON_MODULE"]
-sys.argv[1:] = shlex.split(os.environ["PYTHON_ARGS"])
-
-cwd = f"{sys.prefix}/cwd"
-if not os.path.exists(cwd):
-    # Empty directories are lost in the asset packing/unpacking process.
-    os.mkdir(cwd)
-os.chdir(cwd)
-
-if mode == "-c":
-    # In -c mode, sys.path starts with an empty string, which means whatever the current
-    # working directory is at the moment of each import.
-    sys.path.insert(0, "")
-    exec(module, {})
-elif mode == "-m":
-    sys.path.insert(0, os.getcwd())
-    runpy.run_module(module, run_name="__main__", alter_sys=True)
-else:
-    raise ValueError(f"unknown mode: {mode}")